From 62f70dd7f34933d6b3e433a48269c7810c1653f0 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sun, 8 Nov 2026 12:23:24 +0000 Subject: [PATCH] Re-sync the assembly root's shape copies to openRepoShape v1.1.1 with update-shape.py apply, moving project.yaml's and contracts/shape-pin.yaml's shape pin from 190133d to 5017830 (part of opensoft/openRepoTools#186) openRepoShape's update-shape.py apply made this change, run from a checkout of openRepoShape v1.1.1 (tag v1.1.1, commit 50178305e4229275d18e71c2152403b4e9d3004e) as the v1.1.1 release notes prescribe: update-shape.py apply --root ROOT \ --at 50178305e4229275d18e71c2152403b4e9d3004e \ --yes --branch shape/update-5017830 What moved: - project.yaml's shape: block and contracts/shape-pin.yaml now record commit 50178305e4229275d18e71c2152403b4e9d3004e and tree_sha256 204d73aabf2379554600234d4b36dc08d3c3a1da422395dec44637b5ca7a5184, in place of v1.1.0's 190133d5e704e380c430f87ca9c4e31746de0012 and 7a66972c7de193c050f77f97bffa13d8543c28d74f61ea210644e65aa3f37912. The tool recomputed the digest under sorted-ls-tree-r-v1. - The six copies v1.1.1 changed now hold v1.1.1's bytes verbatim, and their sha256 rows in contracts/shape-pin.yaml are recomputed: scripts/validate-pins.py, scripts/validate-manifest.py, scripts/bootstrap.py and .github/workflows/validate.yml, from templates/assembly-root/, and scripts/repo_shape.py and scripts/validate-repository-naming.py, from scripts/. - The four copies v1.1.1 left alone keep their bytes and their rows: Makefile, shape/.gitattributes, AGENTS-shape.md and contracts/repository-naming.yaml. .gitattributes, README.md and both gitlinks are untouched. The tool committed the change as 84126f2de87d under its own identity. This commit keeps that commit's tree, 453f8ab9f51fc2bad395d19cc07ad02bf2e10b3f, and changes only the author and the message. As the tool's message says, the copies re-copied from the upstream are not hand-edited and the digests are recomputed, not adjusted. Inside its run the tool ran the root's own validate-pins.py and validate-manifest.py, now v1.1.1's, and both passed; so does the naming check. Brett Heap ruled v1.1.1 on 10-22, and he wants both of the triad's openRepoShape pins moved: this root's shape copies, here, and the code leg's mounted pin, in the code leg. This change is independent of that code-leg change and of the later root change that moves the code gitlink, and it touches no gitlink. Lane: openRepoTools-1 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/validate.yml | 166 +++++++- contracts/shape-pin.yaml | 16 +- project.yaml | 4 +- scripts/bootstrap.py | 465 +++++++++++++++++++--- scripts/repo_shape.py | 188 +++++++-- scripts/validate-manifest.py | 117 +++++- scripts/validate-pins.py | 541 ++++++++++++++++++++------ scripts/validate-repository-naming.py | 187 ++++++++- 8 files changed, 1404 insertions(+), 280 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 4641aa9..731464c 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -56,6 +56,32 @@ jobs: # failing) on any that don't, since those can only ever be reached by # `SHAPE_LEGS_TOKEN`. Runs only when the App secrets are present — its # only consumer is the mint step below, which is guarded the same way. + # + # Each url is read by its exact key: `--name-only` lists the keys, then + # `--get` asks for one, as git reads it. Splitting `--get-regexp`'s + # `key value` lines at the first space gave a section named `members/My + # IRRS` the url `IRRS.url `, which matched neither form and dropped + # that leg from the token (#741). A key holds no line break, so one per + # line is exact; the two steps below that print the urls read them the + # same way. A url holding a line break or a carriage return is skipped + # in all three: it could print a workflow command of its own, and git + # cannot clone it. And each repository is named once, however many + # sections or paths give it. + # + # And only what git reads (#786), by the rules the doctor and `siblings.py` + # read the same file by (#733): only a `submodule..url` key counts; + # a section whose name is empty or holds a `..` component is ignored, as is + # one with no `path`; a value that begins with `-` is ignored, that value + # alone, so an earlier one stands; nothing is read while `.gitmodules` is + # unmerged; and a file git refuses whole (one it cannot parse, a `path`, + # `url`, `ignore`, `update` or `branch` with no value, an `update` other + # than none, checkout, rebase or merge, a `shallow` that is no boolean, a + # `fetchRecurseSubmodules` that is neither a boolean nor on-demand) fails + # the step with its cause and the key or line, since git reads none of it + # and a repository named from it would be one git never fetches. The two + # steps below that print the urls read by the same key rules; the fetch + # step refuses a file git refuses, and the failing step is never reached + # with one. - name: leg repository names from .gitmodules id: leg-names if: env.SHAPE_LEGS_APP_SET == 'true' @@ -64,8 +90,52 @@ jobs: run: | set -e legs="" - if [ -f .gitmodules ]; then - while IFS= read -r url; do + if [ -f .gitmodules ] && [ -z "$(git ls-files -u -- .gitmodules)" ]; then + # git reads none of a file it refuses, and neither does this (#786). Whatever + # of the file is printed is workflow-command data: % and CR are escaped. + esc() { sed -e 's/%/%25/g' -e "s/$(printf '\r')/%0D/g"; } + if ! err=$(git config --file .gitmodules --list 2>&1 >/dev/null); then + err=$(printf '%s\n' "$err" | sed -n '$s/^fatal: //p' | esc) + printf '%s\n' "::error::.gitmodules holds a line git cannot parse${err:+ ($err)}, so git refuses the whole file: it reads none of it, and no repository is named from it. git submodule status names the line git stops at." + exit 1 + fi + cause() { printf '%s (%s)\n' "$1" "$(printf '%s' "$key" | esc)"; } + us=$(printf '\001') + refusal=$(git config -z --file .gitmodules --list | tr '\000\n' '\n\001' | while IFS= read -r rec; do + case $rec in *"$us"*) value=${rec#*"$us"}; bare= ;; *) value=; bare=y ;; esac + key=${rec%%"$us"*} + section=${key%.*} + case $section in submodule.?*) ;; *) continue ;; esac + case /${section#submodule.}/ in *[/\\]..[/\\]*) continue ;; esac + field=${key##*.} + case $field in + path|url|ignore|update|branch) + [ -z "$bare" ] || { case $field in ignore|update) a=an ;; *) a=a ;; esac; cause "$a $field key with no value"; break; } ;; + esac + case $field in + update) + case $value in none|checkout|rebase|merge) ;; *) cause "an update that is not none, checkout, rebase or merge"; break ;; esac ;; + shallow) + git -c "x.y=$value" config --type=bool --get x.y >/dev/null 2>&1 || { cause "a shallow that is not a boolean"; break; } ;; + fetchrecursesubmodules) + [ "$value" = on-demand ] || git -c "x.y=$value" config --type=bool --get x.y >/dev/null 2>&1 || { cause "a fetchRecurseSubmodules that is neither a boolean nor on-demand"; break; } ;; + esac + done || true) + if [ -n "$refusal" ]; then + printf '%s\n' "::error::.gitmodules holds $refusal, so git refuses the whole file: it reads none of it, and no repository is named from it. git submodule status names the key and line git stops at." + exit 1 + fi + while IFS= read -r key; do + case $key in submodule.?*.url) ;; *) continue ;; esac + name=${key#submodule.} + name=${name%.url} + case /$name/ in *[/\\]..[/\\]*) continue ;; esac + # A value beginning with - is ignored, that value alone: git keeps an earlier one. + url=$(git config --file .gitmodules --get -- "$key" '^([^-]|$)') || continue + git config --file .gitmodules --get -- "submodule.$name.path" '^([^-]|$)' >/dev/null || continue + # A url holding a line break could print a workflow command of its own. + case $url in *' + '*|*"$(printf '\r')"*) continue ;; esac [ -n "$url" ] || continue case "$url" in https://github.com/*) repo="${url#https://github.com/}" ;; @@ -76,6 +146,7 @@ jobs: owner="${repo%%/*}" name="${repo#*/}" if [ "$owner" = "$REPO_OWNER" ]; then + case ",$legs," in *",$name,"*) continue ;; esac if [ -z "$legs" ]; then legs="$name" else @@ -84,7 +155,7 @@ jobs: else echo "::warning::leg $owner/$name is owned by a different account ($owner) than this repository ($REPO_OWNER); a GitHub App installation token is per-owner, so it is excluded from the App-minted token here and can only be reached via SHAPE_LEGS_TOKEN." fi - done < <(git config --file .gitmodules --get-regexp '\.url$' | cut -d' ' -f2- || true) + done < <(git config --file .gitmodules --name-only --get-regexp '\.url$' || true) fi echo "legs=$legs" >> "$GITHUB_OUTPUT" echo "legs for the App-minted token: $legs" @@ -135,7 +206,9 @@ jobs: # carry: `https://github.com/...` and SSH `git@github.com:...`. With no # credential configured this falls back to a plain, unauthenticated # `git submodule update --init --recursive` — the same command that - # already ran here, unchanged, for a project whose legs are public. + # already ran here, unchanged, for a project whose legs are public. A fetch + # that fails over a `.gitmodules` git refuses is no such degrade: no + # credential cures it, so the step names the cause and fails (#786). - name: fetch the legs (submodules) id: submodules env: @@ -169,10 +242,60 @@ jobs: echo "legs_available=true" >> "$GITHUB_OUTPUT" else echo "legs_available=false" >> "$GITHUB_OUTPUT" + readable= + if [ -f .gitmodules ] && [ -z "$(git ls-files -u -- .gitmodules)" ]; then readable=y; fi + if [ -n "$readable" ]; then + # git reads none of a file it refuses, and neither does this (#786). Whatever + # of the file is printed is workflow-command data: % and CR are escaped. + esc() { sed -e 's/%/%25/g' -e "s/$(printf '\r')/%0D/g"; } + if ! err=$(git config --file .gitmodules --list 2>&1 >/dev/null); then + err=$(printf '%s\n' "$err" | sed -n '$s/^fatal: //p' | esc) + printf '%s\n' "::error::.gitmodules holds a line git cannot parse${err:+ ($err)}, so git refuses the whole file: it reads none of it, and no repository is named from it. git submodule status names the line git stops at." + exit 1 + fi + cause() { printf '%s (%s)\n' "$1" "$(printf '%s' "$key" | esc)"; } + us=$(printf '\001') + refusal=$(git config -z --file .gitmodules --list | tr '\000\n' '\n\001' | while IFS= read -r rec; do + case $rec in *"$us"*) value=${rec#*"$us"}; bare= ;; *) value=; bare=y ;; esac + key=${rec%%"$us"*} + section=${key%.*} + case $section in submodule.?*) ;; *) continue ;; esac + case /${section#submodule.}/ in *[/\\]..[/\\]*) continue ;; esac + field=${key##*.} + case $field in + path|url|ignore|update|branch) + [ -z "$bare" ] || { case $field in ignore|update) a=an ;; *) a=a ;; esac; cause "$a $field key with no value"; break; } ;; + esac + case $field in + update) + case $value in none|checkout|rebase|merge) ;; *) cause "an update that is not none, checkout, rebase or merge"; break ;; esac ;; + shallow) + git -c "x.y=$value" config --type=bool --get x.y >/dev/null 2>&1 || { cause "a shallow that is not a boolean"; break; } ;; + fetchrecursesubmodules) + [ "$value" = on-demand ] || git -c "x.y=$value" config --type=bool --get x.y >/dev/null 2>&1 || { cause "a fetchRecurseSubmodules that is neither a boolean nor on-demand"; break; } ;; + esac + done || true) + if [ -n "$refusal" ]; then + printf '%s\n' "::error::.gitmodules holds $refusal, so git refuses the whole file: it reads none of it, and no repository is named from it. git submodule status names the key and line git stops at." + exit 1 + fi + fi echo "::warning::submodule checkout failed (git exit $status) using credential source '$legs_credential'. If the legs are PRIVATE or internal sibling repositories, the default GITHUB_TOKEN cannot read them. Configure a GitHub App (SHAPE_LEGS_APP_ID + SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or set SHAPE_LEGS_TOKEN — a fine-grained PAT — with contents:read on the legs, as a repository or organisation secret. Leg URLs:" - git config --file .gitmodules --get-regexp '\.url$' | while read -r _ url; do - echo "::warning:: $url" - done + if [ -n "$readable" ]; then + git config --file .gitmodules --name-only --get-regexp '\.url$' | awk '!seen[$0]++' | while IFS= read -r key; do + case $key in submodule.?*.url) ;; *) continue ;; esac + name=${key#submodule.} + name=${name%.url} + case /$name/ in *[/\\]..[/\\]*) continue ;; esac + # A value beginning with - is ignored, that value alone: git keeps an earlier one. + url=$(git config --file .gitmodules --get -- "$key" '^([^-]|$)') || continue + git config --file .gitmodules --get -- "submodule.$name.path" '^([^-]|$)' >/dev/null || continue + # A url holding a line break could print a workflow command of its own. + case $url in *' + '*|*"$(printf '\r')"*) continue ;; esac + echo "::warning:: $url" + done + fi fi - uses: actions/setup-python@v5 @@ -209,15 +332,30 @@ jobs: # a credential that IS configured (App or PAT) that still could not # check out the legs (wrong scope, missing contents:read grant, an # incomplete App installation, a leg URL problem) is a finding a human - # must act on, so this fails the job — the only case that does. The - # root checkout above never depended on either credential, so a - # failure here can only mean the credential named below cannot read one - # of the LEG repositories. + # must act on, so this fails the job — the only case that does, but for a + # `.gitmodules` git refuses, which the fetch step above fails whatever the + # credential, since none cures it. The root checkout above never depended + # on either credential, so a failure here can only mean the credential + # named below cannot read one of the LEG repositories. - name: fail — a leg credential is configured but the legs still could not be fetched if: steps.submodules.outputs.legs_available != 'true' && (env.SHAPE_LEGS_TOKEN_SET == 'true' || env.SHAPE_LEGS_APP_SET == 'true') run: | echo "::error::A leg credential is configured (source: ${{ steps.submodules.outputs.legs_credential }}) but 'git submodule update --init --recursive' still failed. The root checkout does not use this token — it reads with the workflow's default token — so this means the '${{ steps.submodules.outputs.legs_credential }}' credential cannot read one or more of the following LEG repositories (wrong token/App scope, a missing contents:read grant, an incomplete App installation, or a leg URL problem):" - git config --file .gitmodules --get-regexp '\.url$' | while read -r _ url; do - echo "::error:: $url" - done + if [ -f .gitmodules ] && [ -z "$(git ls-files -u -- .gitmodules)" ]; then + # No refusal here: the fetch step above exits 1 on a file git refuses, so + # this step is never reached with one. + git config --file .gitmodules --name-only --get-regexp '\.url$' | awk '!seen[$0]++' | while IFS= read -r key; do + case $key in submodule.?*.url) ;; *) continue ;; esac + name=${key#submodule.} + name=${name%.url} + case /$name/ in *[/\\]..[/\\]*) continue ;; esac + # A value beginning with - is ignored, that value alone: git keeps an earlier one. + url=$(git config --file .gitmodules --get -- "$key" '^([^-]|$)') || continue + git config --file .gitmodules --get -- "submodule.$name.path" '^([^-]|$)' >/dev/null || continue + # A url holding a line break could print a workflow command of its own. + case $url in *' + '*|*"$(printf '\r')"*) continue ;; esac + echo "::error:: $url" + done + fi exit 1 diff --git a/contracts/shape-pin.yaml b/contracts/shape-pin.yaml index 6791108..f513251 100644 --- a/contracts/shape-pin.yaml +++ b/contracts/shape-pin.yaml @@ -27,13 +27,13 @@ pin_role: shape source_repository: opensoft/openRepoShape materialization: copied -commit: "190133d5e704e380c430f87ca9c4e31746de0012" +commit: "50178305e4229275d18e71c2152403b4e9d3004e" revision_kind: commit digest_algorithm: sha256 digest_definition: sorted-ls-tree-r-v1 digests: - tree_sha256: "7a66972c7de193c050f77f97bffa13d8543c28d74f61ea210644e65aa3f37912" + tree_sha256: "204d73aabf2379554600234d4b36dc08d3c3a1da422395dec44637b5ca7a5184" verify_pin: scripts/validate-pins.py @@ -41,22 +41,22 @@ verify_pin: scripts/validate-pins.py # as written by the scaffold. Written by `scaffold-project.py`; do not hand-edit. files: - path: scripts/validate-pins.py - sha256: "09820c5dd29355bdbbd3c82bb6197f31e7a0a234e8beaaa5940862d09c32597f" + sha256: "7acc22fc632a046aec692dd34550ab65fef6eaf4b89278f1bb5e6b73b860e2e7" - path: scripts/validate-manifest.py - sha256: "7f36200f55562ab3e9588c00aaaefb0763b74f979b07d4688b70be38b1236adc" + sha256: "bc60bd26fc6f3104bf499f80ba694afc102458e32ab57092d127aa59980dfdda" - path: scripts/bootstrap.py - sha256: "d2bd1cfc8515088980df66c8adda3fdaea908e7e2eba6f94062c1b9e65610995" + sha256: "89ff92e5e469003fb5a1f13a5adbe5be2f9c2913ad152eb3f49e8545f0a8c3cc" - path: Makefile sha256: "6d65dd06e12564a7c018a72a5a2e3c80b999d7026ce5643e77ca8dd6e9254dfa" - path: shape/.gitattributes sha256: "e2b88cf25928c5a58bb2acaae1779d934c262273db708d0aa6dfb01591b15baa" - path: .github/workflows/validate.yml - sha256: "fb8c0360863ad953af7825f3be01ed27f8cf1269e5cab19dc0d6c5423161b487" + sha256: "58726aabbec255f79ce629a5f60529efa28c64c5d0d24384a2f676ff70b06a92" - path: AGENTS-shape.md sha256: "84f8bf1afb83d7cd5bd556f81a3bfc01319f00ff87c33a95e98cf5d1fada5fee" - path: scripts/repo_shape.py - sha256: "be5358aad1384f723e321bb089e1c04bfb552157f922554e9f872fae9372a904" + sha256: "2ae28ea2af0fbf508cea28c1f8136afdf7737588de0130e510e1ad6ef14c108e" - path: scripts/validate-repository-naming.py - sha256: "527273cf0b8ada97166c7d60e2247c2a44e16983bf643843936ce92b19bb0f68" + sha256: "39a57203e2a142335e7b89ea2d69a9979092e70099e954d10e31f3fb9a7a8cec" - path: contracts/repository-naming.yaml sha256: "a4eebad21541cbfe9a3e94a0ee667a2d5a7866c3567eea2a000d8339f6f096a4" diff --git a/project.yaml b/project.yaml index 6e29b9e..e21a889 100644 --- a/project.yaml +++ b/project.yaml @@ -74,11 +74,11 @@ tracking_branch: "main" shape: repository: opensoft/openRepoShape revision_kind: commit - commit: "190133d5e704e380c430f87ca9c4e31746de0012" + commit: "50178305e4229275d18e71c2152403b4e9d3004e" digest_algorithm: sha256 digest_definition: sorted-ls-tree-r-v1 digests: - tree_sha256: "7a66972c7de193c050f77f97bffa13d8543c28d74f61ea210644e65aa3f37912" + tree_sha256: "204d73aabf2379554600234d4b36dc08d3c3a1da422395dec44637b5ca7a5184" # The neutral products this project declares a pin on, by name. OPTIONAL, and # empty in a fresh scaffold. It is what makes a `` name a diff --git a/scripts/bootstrap.py b/scripts/bootstrap.py index f92f804..f7531de 100755 --- a/scripts/bootstrap.py +++ b/scripts/bootstrap.py @@ -41,7 +41,13 @@ file is. A branch no command can be given (a NUL, or a lone surrogate this system cannot encode) is never tried: the leg is left as it is and the line says why (#455); step (b)'s manifest - validator names the key when the manifest holds it. Advancing a pin + validator names the key when the manifest holds it. Nor is a branch + holding a character this system's encoding cannot write (#710): where + git takes the name in UTF-8, its line says the name is good and to + run bootstrap under a UTF-8 locale, and where git refuses it there + too, the line gives git's words. Every name git prints back is read + as bytes and decoded so that it reaches git again as the bytes it + was (#710). Advancing a pin remains an explicit commit in this repository — see `README.md`, "The lockstep invariant". This is why `.gitmodules` carries no `branch=`: that would buy the same ergonomics by weakening the pin, and then "what @@ -61,9 +67,16 @@ wallet-carried authority is not misconfigured, and a bootstrap that failed there would have made the layout load-bearing again. - (d) EXITS 0 unless a validator failed, and 2 where step (a) or the - upstream notice cannot start git, refused `git-not-runnable` as the - validators refuse it (#694). + (d) EXITS 0 unless a validator failed, and says which way. A validator + that FOUND something, any exit but 2, ends the run `bootstrap FAILED: + (exit )`, exit 1. A validator that REFUSED, its exit 2, + could not check at all, and the run ends `bootstrap REFUSED: + (exit 2)`, exit 2 (#764). Where one validator found and another + refused, both lines are said, the finding first, and the run exits 2, + since a check that could not run outranks a finding. Step (a) and the + upstream notice refuse `git-not-runnable`, exit 2, where git cannot be + started, as the validators do (#694): the run stops there and says + neither line, though a validator may have found or refused by then. SCHEMA-NEUTRAL BY CONTRACT. A one-repository project runs the same command: step (a) is a no-op where there are no submodules, and steps (c) and (d) are @@ -75,6 +88,7 @@ from __future__ import annotations import argparse +import codecs import os import re import shlex @@ -122,6 +136,13 @@ ("CHERRY_PICK_HEAD", "a cherry-pick", "cherry-pick"), ("REVERT_HEAD", "a revert", "revert"), ) +#: The two `IN_PROGRESS` markers git writes as REFS (#764). On a leg whose +#: refs are in a reftable (`git init --ref-format=reftable`, git 2.45 and +#: later) neither is a file in the git dir while its operation is in +#: progress: each is in the ref store, which step (a) asks as well +#: (`_in_ref_store`). `MERGE_HEAD` is a file on both backends, as the two +#: rebases' directories and `sequencer/todo` are (measured with git 2.47.1). +IN_REF_STORE = ("CHERRY_PICK_HEAD", "REVERT_HEAD") #: What a cherry-pick or a revert of SEVERAL commits leaves while it is not #: done, asked after `IN_PROGRESS`: its stop committed by hand takes the #: head file away and leaves this, whose first word says which it is. @@ -131,6 +152,10 @@ "Remediation: put git on PATH and run this again. Step (a) reads each " "leg's pin and moves the leg through git, and the validators read the " "pins through git, so without it nothing here can be placed or checked.") +#: What `_argument_fault` calls a character this system's encoding cannot +#: write (#710): the one fault of a name that is good, so `_say_not_tried` +#: gives it a line of its own. +UNWRITABLE = "a character this system's encoding cannot write" REGISTER_RELPATH = Path("governance") / "review-authority" / "register.yaml" VALIDATORS = ( ("naming", ["scripts/validate-repository-naming.py", "--project", @@ -138,23 +163,114 @@ ("manifest", ["scripts/validate-manifest.py"]), ("pins", ["scripts/validate-pins.py"]), ) +#: The exit code a validator ends a REFUSAL with: it could not check, which +#: is not a finding (#764). Every other non-zero code is a failure. +VALIDATOR_REFUSED = 2 def _run(root: Path, args: list[str]) -> int: # Flush first: this process's own stdout is block-buffered when it is a # pipe, and a child writing straight to the fd would otherwise appear - # BEFORE the heading that introduces it. + # BEFORE the heading that introduces it. The child is told what that fd + # is (`inherited_child_environment`, #836). sys.stdout.flush() - proc = subprocess.run([sys.executable, *args], cwd=str(root), check=False) + proc = subprocess.run([sys.executable, *args], cwd=str(root), + env=inherited_child_environment(), check=False) sys.stdout.flush() return proc.returncode +#: The built-in handlers that never raise on a character an encoding cannot +#: hold (#836), so a validator is told the one this stdout has. `strict` and +#: `surrogateescape` raise on such a character, and are not among them. +#: `scripts/shape_materialize.py`'s own set, which a project does not carry. +HANDLERS_THAT_NEVER_RAISE = frozenset(( + "backslashreplace", "ignore", "namereplace", "replace", + "xmlcharrefreplace")) + + +def inherited_child_environment() -> dict[str, str]: + """The environment `_run` starts each validator in: this process's own, + with `PYTHONIOENCODING` naming the encoding of the stdout the validator + writes into (#836). + + WHAT WAS WRONG. A child takes its stdout's encoding and handler from its + OWN environment, and not from the `escape_unencodable_stdout` this + process ran in `main`. So on a stdout whose code page cannot hold the + project's name — `PYTHONIOENCODING=cp1252`, or the cp1252 with + `surrogateescape` Windows gives any stdout that is not the console — + this file printed `bootstrap: \\u65e5\\u672c`, and `validate-manifest.py` + then raised `UnicodeEncodeError` at `manifest ok: 日本`: `make + bootstrap` ended `bootstrap FAILED: manifest (exit 1)` over a manifest + with nothing wrong in it. It is the defect #254 fixed in `bump-leg.py` + and `update-shape.py apply`, which run the same two validators. + + THE RULE IS #254's, `scripts/shape_materialize.py`'s + `inherited_child_environment`: a child that writes into this stdout is + told the stdout's own encoding, with its handler where that one never + raises and `backslashreplace` where it does, which is what + `escape_unencodable_stdout` leaves it with. So a validator's lines and + this file's are written alike: escaped where the code page cannot hold a + character, and as they are where it can. A stdout with no usable + encoding leaves the environment as it is. + + A COPY, as the validators' own `_shown` is. A project does not carry + `shape_materialize.py`, and a root re-copies its pinned files one row at + a time, so this file may meet a `repo_shape.py` older than itself, and + imports nothing new from it. `tests/test_validator_child_encoding.py` + holds the copy to the library's answer. + """ + env = dict(os.environ) + try: + encoding = codecs.lookup(sys.stdout.encoding).name + except (AttributeError, LookupError, TypeError): + return env + errors = getattr(sys.stdout, "errors", None) + if errors not in HANDLERS_THAT_NEVER_RAISE: + errors = "backslashreplace" + env["PYTHONIOENCODING"] = f"{encoding}:{errors}" + return env + + def _try_git(args: list[str], cwd: Path) -> str | None: + """What `git ` printed in `cwd`, stripped, or `None` where git + exited non-zero. + + STEP (a)'S ONE DECODE RULE (#710). What git prints is read as BYTES and + decoded the way the name is encoded when git is handed it again. On + POSIX that is `os.fsdecode`, as `_head_of` and `_in_progress` read + theirs: the inverse of the `os.fsencode` that `subprocess` applies to + each argument, so a name git prints reaches git again as the same bytes + under any locale. On Windows `subprocess` encodes no argument: it hands + the command line to `CreateProcessW` as UTF-16, and git for Windows + reads it, and writes a ref name, in UTF-8. So the bytes are decoded as + UTF-8 there, as `git_out` decoded them, and not by `os.fsdecode`, which + `PYTHONLEGACYWINDOWSFSENCODING=1` makes `mbcs` with `replace`, losing + every character the code page lacks (RV824's N3). Through `git_out`'s + strict UTF-8, `_upstream_holding` ended `make bootstrap` in a + `UnicodeDecodeError` traceback over an upstream named with a byte that + is not UTF-8 (`refs/remotes/origin/ab`), and, under an `ascii` + encoding (`LC_ALL=C PYTHONUTF8=0`), in a `UnicodeEncodeError` over one + named in UTF-8, since the text it read could not be handed back to + `rev-parse`. + + `git_out` itself keeps its strict decode, as #710's ruling took Option + 2: the readers of `origin`'s URL in `shape-doctor.py` and in the + advisory it reads take its `UnicodeDecodeError` to mean the URL names + nothing (#385). + + The BYTES are stripped, of the ASCII whitespace git ends a line with, + and not the text: `str.strip` also takes away a U+00A0 or a U+3000 that + ends a name, so `refs/remotes/origin/a` was read as `.../a`, and + a branch with no commit of its own past the pin was not moved back. + """ try: - return git_out(args, cwd=cwd) + raw = git_out(args, cwd=cwd, binary=True).strip() except Refusal: return None + # #710's ruling, Option 2: the decode is step (a)'s own, here, and + # `repo_shape.git_out` keeps its strict UTF-8 for every other caller. + return raw.decode("utf-8") if os.name == "nt" else os.fsdecode(raw) @contextmanager @@ -183,6 +299,12 @@ def _git_runs(unread: str) -> Iterator[None]: f"({exc.strerror or exc})", GIT_NOT_RUNNABLE) from exc +#: What `_argument_fault` calls a character that does not print, or white +#: space (#715): git takes the name, and no person can tell it from the name +#: without it, so `_say_not_tried` gives it a line of its own. +UNSEEN = "a character that does not print or is white space" + + def _argument_fault(branch: str) -> str | None: """What stops `branch` being an argument to a command, or `None` (#455). @@ -199,8 +321,21 @@ def _argument_fault(branch: str) -> str | None: exception `_try_git` would then have to swallow: a `UnicodeDecodeError`, which `git_out` raises over output that is not UTF-8, is a `ValueError` too, and catching that there would read a leg whose HEAD is on such a - branch as detached. Only a NUL and a lone surrogate are looked for; a - character the system's encoding merely lacks is not this fix's. + branch as detached. + + A CHARACTER THIS SYSTEM'S ENCODING CANNOT WRITE IS A FAULT TOO (#710, + Brett Heap's ruling (a) on L4-124), named `UNWRITABLE`. Under + `LC_ALL=C PYTHONUTF8=0` the encoding is `ascii`, and + `tracking_branch: "feature/日本"`, which `validate-manifest.py` passes + and git takes under a UTF-8 locale, ended step (a) in a + `UnicodeEncodeError` traceback from `subprocess.py`. The leg is left as + it is, and `_say_not_tried` says why. Its line can call the name good, + so this fault is asked last, and a name that holds a surrogate at all + is named for the surrogate instead, even one that is one byte of a name + (RV824's S1): `validate-manifest.py` names every surrogate (#280), so + `"日\\udcff"` is not a good name, and #455's line is the true one. + Whether git takes the name is asked before a line calls it good + (`_git_refuses_in_utf8`). A surrogate that is ONE BYTE of a name is not refused. POSIX spells a name that is not UTF-8 with U+DC80 to U+DCFF and `os.fsencode` writes each back @@ -210,13 +345,38 @@ def _argument_fault(branch: str) -> str | None: is not UTF-8 for ANY of them, so a lone surrogate is never handed to git there either, where a `CreateProcess` would pass it on mangled. The answer is `os.fsencode`'s own, so it is the system's and not a list kept here. + + NOR IS A CHARACTER THAT DOES NOT PRINT, OR WHITE SPACE (#715), `UNSEEN`, + which `validate-manifest.py` refuses in a branch too (`siblings.py` asks + `isprintable()` alone, so a space it takes). + git takes `main` followed by U+00A0, and since #715 the reader reads that + no-break space as YAML does, where it used to strip it: so this step made + a second `main` beside the first in every leg and checked it out. Asked + once the name holds neither a NUL nor a lone surrogate, and never of a + surrogate, so the byte-carrying ones above still reach git. Asked + BEFORE `UNWRITABLE`, for that fault's reason: a name holding such a + character is not good under any encoding, so the line that calls a + name good is never said of it (`"日本 x"` under `LC_ALL=C` is named + for its space, which the root's own validator names too). """ for char in branch: if char == "\x00": return "U+0000, a NUL" if "\ud800" <= char <= "\udfff" and _fsencoded_length(char) != 1: return f"U+{ord(char):04X}, a lone surrogate" - return None + for char in branch: + if not "\ud800" <= char <= "\udfff" and ( + not char.isprintable() or char.isspace()): + return f"U+{ord(char):04X}, {UNSEEN}" + unwritable = next((char for char in branch + if _fsencoded_length(char) == 0), None) + if unwritable is None: + return None + surrogate = next((char for char in branch + if "\ud800" <= char <= "\udfff"), None) + if surrogate is not None: + return f"U+{ord(surrogate):04X}, a lone surrogate" + return f"U+{ord(unwritable):04X}, {UNWRITABLE}" def _fsencoded_length(char: str) -> int: @@ -227,18 +387,78 @@ def _fsencoded_length(char: str) -> int: return 0 -def _say_not_tried(where: str, named: str, fault: str) -> None: +def _say_not_tried(where: str, named: str, fault: str, + refused: list[str] | None = None) -> None: """A branch `_argument_fault` found: no command was run with it. The leg stays where it is, and the line says which of the three places the - person's value can have come from holds it (#455).""" - print(f" {where}: could not create branch {named}: it holds {fault}, " - "which no command can be given as an argument; the leg is left as " - "it is") + person's value can have come from holds it (#455). + + `UNSEEN` (#715) has its own words: git takes `main` followed by U+00A0, + so "which no command can be given" would be wrong of it, and the name + to give is the one without that character. + + A character this system's encoding cannot write (`UNWRITABLE`, #710) is + a branch step (a) DID NOT TRY, not one it could not create: the branch + can be there already, made by a run under UTF-8 (RV824's N2). Where git + takes the name in UTF-8 (`refused` is `None`), the line is the one Brett + Heap ruled on L4-124: the name is good, the encoding is named + (`sys.getfilesystemencoding()`, the one `os.fsencode` writes with), and + the two ways to run bootstrap under UTF-8 follow, a UTF-8 locale or + Python's UTF-8 mode. Where git refuses it in UTF-8 too, `refused` is + git's lines (`_git_refuses_in_utf8`): the name is not called good, git's + words are said, and #455's way out follows, since the name itself has + to change (RV824's S1).""" + if fault.endswith(UNSEEN): + print(f" {where}: could not create branch {named}: it holds " + f"{fault}, which no person can tell from the name without it; " + "the leg is left as it is") + print(" the branch is the leg's `branch:`, else `--branch`, " + "else `tracking_branch`. Give that one a name with no such " + "character and run bootstrap again.") + return + unwritable = fault.endswith(UNWRITABLE) + tried = "did not try" if unwritable else "could not create" + print(f" {where}: {tried} branch {named}: it holds {fault}, which no " + "command can be given as an argument; the leg is left as it is") + if unwritable and refused is None: + print(" the name is good, and this system's encoding, " + f"{sys.getfilesystemencoding()}, cannot write it: run bootstrap " + "again under `LC_ALL=C.UTF-8`, or with `PYTHONUTF8=1`.") + return + if unwritable: + print(" and git refuses it as a branch name in UTF-8 too:") + _say_lines(refused) print(" the branch is the leg's `branch:`, else `--branch`, else " "`tracking_branch`. Give that one a name git can create and run " "bootstrap again.") +def _git_refuses_in_utf8(root: Path, branch: str) -> list[str] | None: + """What git says of `branch` as a branch name in the bytes a run under + UTF-8 would hand it, or `None` when git takes it (#710, RV824's S1). + + A name `_argument_fault` faults as `UNWRITABLE` is not called good until + git has been asked: `feature/日本..x`, `日本.lock` and `日本 x` are names + this system's encoding cannot write AND names git refuses in any + encoding, and the way out the line gave ended in `fatal: ... is not a + valid branch name`. `git check-ref-format --branch` is git's own answer. + It is asked of `branch.encode("utf-8", "surrogateescape")`, handed to + `subprocess` through `os.fsdecode`, which POSIX's `os.fsencode` writes + back as exactly those bytes under any locale. On Windows no character + is `UNWRITABLE` (its `os.fsencode` writes UTF-8, or replaces), so this + is never asked there. + + ASKED IN THE ROOT, not in the leg (RV824's R1-N1). `--branch` makes git + set up the repository it runs in, so in a leg whose `.git` file names a + git dir that is gone it exits 128 (`not a git repository`), and a good + name read as one git refuses there, while the same name was good in the + next leg. The root is the repository this command was found in, and + the answer is about the name alone. + """ + utf8 = os.fsdecode(branch.encode("utf-8", "surrogateescape")) + return _git_says(["check-ref-format", "--branch", utf8], root) + + def _short(sha: str | None) -> str: return sha[:12] if sha else "?" @@ -301,7 +521,11 @@ def _upstream_holding(sub: Path, branch: str, local: str) -> str | None: remote-tracking ref is not there (never fetched, or pruned) and one whose tip is not the upstream's. The two are compared by object id. `for-each-ref` names the upstream whether or not its ref exists, and - takes the branch after `refs/heads/`, where git reads no option. + takes the branch after `refs/heads/`, where git reads no option. Its + name is read by `_try_git`'s rule, so on POSIX an upstream named with + a byte that is not UTF-8, or in UTF-8 under an `ascii` encoding, is the + same ref when `rev-parse` is handed it, and no longer a traceback + (#710). """ upstream = _try_git(["for-each-ref", "--format=%(upstream)", f"refs/heads/{branch}"], sub) @@ -465,9 +689,10 @@ def _on_a_branch(current: str | None) -> bool: return current is not None and current.startswith("refs/heads/") -def _in_progress(sub: Path) -> tuple[str, str, str] | None: +def _in_progress(sub: Path) -> tuple[tuple[str, str, str], str] | None: """The `IN_PROGRESS` row of the operation git has in progress in the - leg, or `None` (#704). + leg, with where its marker is (`git dir` or `ref store`), or `None` + (#704). Asked of the leg's own git dir, which `git rev-parse --git-dir` names (a submodule's is under the root's `.git/modules/`, a linked @@ -476,6 +701,12 @@ def _in_progress(sub: Path) -> tuple[str, str, str] | None: path, so no name of a directory can fail the read. A leg git names no git dir for has nothing to read here, and the move after this says what git says of it. + + A cherry-pick's or a revert's marker is asked of the leg's ref store + too (`IN_REF_STORE`, #764). On a reftable leg it is there and in no + file, so a leg stopped in either was moved, and the move ended it as + #704 measured on the files backend: `CHERRY_PICK_HEAD` or `REVERT_HEAD` + gone, and what it had staged carried onto the tracking branch. """ try: said = git_out(["rev-parse", "--git-dir"], cwd=sub, binary=True) @@ -484,8 +715,24 @@ def _in_progress(sub: Path) -> tuple[str, str, str] | None: gitdir = sub / os.fsdecode(said.rstrip(b"\r\n")) for row in IN_PROGRESS: if os.path.lexists(gitdir / row[0]): - return row - return _sequence_in_progress(gitdir) + return row, "git dir" + if row[0] in IN_REF_STORE and _in_ref_store(sub, row[0]): + return row, "ref store" + sequence = _sequence_in_progress(gitdir) + return None if sequence is None else (sequence, "git dir") + + +def _in_ref_store(sub: Path, name: str) -> bool: + """Whether the leg's ref store holds the ref called exactly `name` + (#764). + + `git show-ref --exists` (git 2.43 and later) takes the name as it is, + where `git rev-parse --verify CHERRY_PICK_HEAD` also resolves a branch + or a tag of that name, which no operation leaves. A git before 2.43 + refuses the option and is answered no: such a git has no reftable + either, so the file `_in_progress` asks first is the whole answer + there.""" + return _try_git(["show-ref", "--exists", name], sub) is not None def _sequence_in_progress(gitdir: Path) -> tuple[str, str, str] | None: @@ -511,11 +758,12 @@ def _sequence_in_progress(gitdir: Path) -> tuple[str, str, str] | None: return SEQUENCER_TODO, "a cherry-pick", "cherry-pick" -def _say_in_progress(label: str, path: str, - row: tuple[str, str, str]) -> None: +def _say_in_progress(label: str, path: str, row: tuple[str, str, str], + held: str) -> None: """A leg with an operation in progress (#704): what is in progress, - that nothing moved, and the two commands that end it, the leg's path - shell-quoted, then spelled, as `move_back_to_pin` prints its command. + where git holds its marker (`held`, #764), that nothing moved, and the + two commands that end it, the leg's path shell-quoted, then spelled, as + `move_back_to_pin` prints its command. No move is made from here. A checkout of the tracking branch, even from HEAD on it already or detached at the pin, ends a merge, a cherry-pick @@ -529,7 +777,7 @@ def _say_in_progress(label: str, path: str, marker, doing, verb = row command = f"git -C {spelled(shlex.quote(path))} {verb}" print(f" {label}: {doing} is in progress in the leg ({marker} is in its " - "git dir), so nothing moved") + f"{held}), so nothing moved") print(f" finish it (`{command} --continue`) or abort it " f"(`{command} --abort`), then run bootstrap again.") @@ -812,7 +1060,9 @@ def checkout_tracking_branch(root: Path, leg: dict, branch: str) -> None: return fault = _argument_fault(branch) if fault is not None: - _say_not_tried(where, named, fault) + _say_not_tried(where, named, fault, + _git_refuses_in_utf8(root, branch) + if fault.endswith(UNWRITABLE) else None) return pin = recorded_gitlink(root, path) local = _try_git(["rev-parse", "--verify", "--quiet", @@ -821,7 +1071,7 @@ def checkout_tracking_branch(root: Path, leg: dict, branch: str) -> None: f"refs/remotes/origin/{branch}"], sub) doing = _in_progress(sub) if doing is not None: - _say_in_progress(where, path, doing) + _say_in_progress(where, path, *doing) return if local is None: @@ -862,27 +1112,24 @@ def shape_upstream_notice(root: Path) -> None: an absent answer is not a finding, and it never fails the bootstrap. A clone named there that git cannot be STARTED to read is no absent answer but a machine with no git, refused `git-not-runnable` (#694). + + A READ THE SYSTEM REFUSES IS AN ABSENT ANSWER TOO (#764), and so is a + `contracts/shape-pin.yaml` that is not a mapping, which `validate-pins.py` + has refused by then: each ended `make bootstrap` in a traceback, exit 1. + The two reads of the answer itself are `_clone_tip` and `_cached_tip`. """ pin_path = root / "contracts" / "shape-pin.yaml" - if not pin_path.is_file(): - return try: - pin = load_yaml(pin_path) - except Refusal: + pin = load_yaml(pin_path) if pin_path.is_file() else None + except (OSError, Refusal): return - pinned = str((pin or {}).get("commit") or "").lower() + pinned = str((pin if isinstance(pin, dict) else {}).get("commit") + or "").lower() if not COMMIT_40.match(pinned): return - tip = None - upstream = os.environ.get(SHAPE_UPSTREAM_ENV) - if upstream and Path(upstream).is_dir(): - with _git_runs(f"the clone {SHAPE_UPSTREAM_ENV} names, " - f"{spelled(upstream)}, could not be read"): - tip = _try_git(["rev-parse", "HEAD"], Path(upstream)) + tip = _clone_tip(os.environ.get(SHAPE_UPSTREAM_ENV)) if tip is None: - cache = root / SHAPE_UPSTREAM_CACHE - if cache.is_file(): - tip = cache.read_text(encoding="utf-8").strip() + tip = _cached_tip(root) if not tip or not COMMIT_40.match(tip): return if tip.lower() == pinned: @@ -893,6 +1140,48 @@ def shape_upstream_notice(root: Path) -> None: f"is wrong until you have.") +def _clone_tip(upstream: str | None) -> str | None: + """`HEAD` of the clone `SHAPE_UPSTREAM_PATH` names, or `None` where no + clone there can be read (#764). + + A directory this process may not enter is no clone it can read. Python + 3.12 and 3.13 raise `PermissionError` from `Path.is_dir` for a path + under one, and `subprocess` raises it for a `cwd` it cannot enter, + naming that directory, which `_git_runs` passes on as not git's. Each + ended `make bootstrap` in a traceback, exit 1, where the notice is + silent over every other absent answer, and the cache is still asked. A + git that cannot be STARTED is still `git-not-runnable` (#694): that is + the `Refusal` `_git_runs` raises, which `except OSError` does not catch. + """ + if not upstream: + return None + try: + if not Path(upstream).is_dir(): + return None + with _git_runs(f"the clone {SHAPE_UPSTREAM_ENV} names, " + f"{spelled(upstream)}, could not be read"): + return _try_git(["rev-parse", "HEAD"], Path(upstream)) + except OSError: + return None + + +def _cached_tip(root: Path) -> str | None: + """What `.shape-upstream-tip` holds, less a byte-order mark it starts + with (#855), or `None` where it cannot be read: not there, a file this + process may not read, or bytes that are not UTF-8, none of which is a + commit (#764). The last two ended `make bootstrap` in a `PermissionError` + or a `UnicodeDecodeError` traceback.""" + cache = root / SHAPE_UPSTREAM_CACHE + try: + # `utf-8-sig` takes off the byte-order mark a Windows editor may + # start the file with (#855). `strip()` keeps U+FEFF, so a marked + # tip read as no commit and the notice said nothing. + return (cache.read_text(encoding="utf-8-sig").strip() + if cache.is_file() else None) + except (OSError, UnicodeDecodeError): + return None + + def _register_paths(root: Path, legs: list[dict]) -> list[Path]: candidates = [root / REGISTER_RELPATH] for leg in legs: @@ -1042,18 +1331,53 @@ def _load_root_and_manifest(args) -> tuple[Path, dict | None]: return root, manifest +def _legs_of(manifest: dict | None) -> tuple[list[dict], list[str]]: + """`(the legs, what names no leg)`: each entry of `project.yaml`'s + `legs:` that is a mapping, and for what is not, `validate-manifest.py`'s + finding in its words (#714, #832). + + Iterated as `legs or []`, `legs: 5` ended `make bootstrap` in a + `TypeError` before step (a) printed a line, and a scalar `openXdox` or a + mapping was walked one character, or one key, at a time, so step (a) + said "no submodule legs declared", as of a one-repository project. A + `legs:` that is not a list names no leg; a list's entry that is not a + mapping is no leg, and the mappings beside it are still placed, as + `validate-pins.py` checks the rest past such an entry. Absent or null + declares no leg and says nothing here: step (b)'s manifest validator + says `legs is missing or empty`. + """ + declared = (manifest or {}).get("legs") + if declared is None: + return [], [] + if not isinstance(declared, list): + return [], [f"manifest-legs: legs is {declared!r}, expected a list of " + "`role:`, `repository:` and `path:` mappings; no leg is " + "placed, and the validators below refuse until `legs:` " + "is a list"] + return ([leg for leg in declared if isinstance(leg, dict)], + [f"manifest-leg: a leg is not a mapping: {leg!r}; it is not " + "placed, and the validators below refuse until it is a mapping" + for leg in declared if not isinstance(leg, dict)]) + + def _checkout_all_tracking_branches(root: Path, sub_legs: list[dict], manifest: dict | None, - branch_arg: str | None) -> None: + branch_arg: str | None, + unread: list[str] = ()) -> None: """Step (a): place every submodule leg on its tracking branch. Split from `main` for #132. Git that cannot be started is `git-not-runnable`, naming the leg it met it at (#694): one fact about - the machine, so no leg after it is tried. + the machine, so no leg after it is tried. `unread` is what `_legs_of` + found naming no leg, each said first, and a step with nothing else to + place is not a one-repository project's no-op (#832). """ + for line in unread: + print(f" {line}") if not sub_legs: - print(" no submodule legs declared. A one-repository project runs " - "the same command and this step is a no-op.") + if not unread: + print(" no submodule legs declared. A one-repository project " + "runs the same command and this step is a no-op.") return branch = branch_arg or (manifest or {}).get("tracking_branch") or "main" for leg in sub_legs: @@ -1065,11 +1389,12 @@ def _checkout_all_tracking_branches(root: Path, sub_legs: list[dict], def _run_neutral_validators(root: Path, manifest: dict | None, - skip: bool) -> list[str]: + skip: bool) -> list[tuple[str, int]]: """Step (b): run the neutral validators, then the upstream notice. - Split from `main` for #132. Returns the labels of any validator that - failed, exactly as `main`'s local `failed` list used to accumulate them. + Split from `main` for #132. Returns the label and the exit code of each + validator that failed, in the order they ran: `_end_of_run` says which + were findings and which refusals (#764). """ if skip: print(" skipped (--skip-validators)") @@ -1087,11 +1412,41 @@ def _run_neutral_validators(root: Path, manifest: dict | None, print(f" --- {label} ---") code = _run(root, argv_) if code != 0: - failed.append(f"{label} (exit {code})") + failed.append((label, code)) shape_upstream_notice(root) return failed +def _end_of_run(failed: list[tuple[str, int]]) -> int: + """The last line or two of the run, and its exit code (#764). + + A validator's exit 2 is its REFUSAL, that it could not check, and is said + as one: `bootstrap REFUSED: (exit 2)`, exit 2, the code every + other refusal here ends with. Any other non-zero exit is a finding, as it + was: `bootstrap FAILED: (exit )`, exit 1. Where both + happened both lines are said, the findings first, and the run exits 2, + since a check that could not run outranks a finding. + """ + # Stdout first: piped, it is block-buffered and stderr is not, so a log of + # both would show the lines below before step (c)'s, as `main`'s + # `git-not-runnable` path takes care that it does not. + sys.stdout.flush() + found = [f"{label} (exit {code})" for label, code in failed + if code != VALIDATOR_REFUSED] + refused = [f"{label} (exit {code})" for label, code in failed + if code == VALIDATOR_REFUSED] + if found: + print("bootstrap FAILED: " + ", ".join(found), file=sys.stderr) + if refused: + print("bootstrap REFUSED: " + ", ".join(refused), file=sys.stderr) + if refused: + return 2 + if found: + return 1 + print("bootstrap ok") + return 0 + + def main(argv: list[str] | None = None) -> int: escape_unencodable_stdout() parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) @@ -1108,8 +1463,7 @@ def main(argv: list[str] | None = None) -> int: print(str(exc), file=sys.stderr) return 2 - legs = [leg for leg in ((manifest or {}).get("legs") or []) - if isinstance(leg, dict)] + legs, unread = _legs_of(manifest) sub_legs = [leg for leg in legs if leg.get("role") != "assembly"] name = (manifest or {}).get("name") or root.name @@ -1117,7 +1471,8 @@ def main(argv: list[str] | None = None) -> int: try: print("\n(a) legs on tracking branches, pins untouched") - _checkout_all_tracking_branches(root, sub_legs, manifest, args.branch) + _checkout_all_tracking_branches(root, sub_legs, manifest, args.branch, + unread) print("\n(b) neutral validators") failed = _run_neutral_validators(root, manifest, args.skip_validators) @@ -1133,11 +1488,7 @@ def main(argv: list[str] | None = None) -> int: read_authority(root, legs) print() - if failed: - print("bootstrap FAILED: " + ", ".join(failed), file=sys.stderr) - return 1 - print("bootstrap ok") - return 0 + return _end_of_run(failed) if __name__ == "__main__": diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index d689296..66b1b59 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -34,9 +34,11 @@ - `spelled` and `escape_unencodable_stdout` — how both roots' `bootstrap.py` print a person's text: what a terminal must not be handed raw is escaped, and a stdout that cannot encode it never raises (#391). - - `shell_quoted` and `shell_line` — how `scripts/family.py` and a holder's - `scripts/siblings.py` spell a path in a command a person pastes, so that - it reaches the command as ONE argument in `sh` or PowerShell (#486). + - `shell_quoted` and `shell_line` — how `scripts/family.py` spells a path + in a command a person pastes, so that it reaches the command as ONE + argument in `sh` or PowerShell (#486), and + `host_shell_dialect`, which of the two a line printed here is for: Git + Bash on Windows gets `sh`'s (#711). This file is COPIED into a scaffolded assembly root by `scaffold-project.py` and its sha256 is recorded in that project's `contracts/shape-pin.yaml`, so an @@ -207,10 +209,11 @@ def __init__(self, message: str, remediation: str = ""): # with the `-` and `+` chomping indicators accepted and `-` honoured). # # REFUSED (raises YamlError): tabs used for indentation, anchors and aliases -# (`&`, `*`), tags (`!`), merge keys (`<<`), explicit keys (`? `), a -# single-pair mapping in a flow sequence (`[a: b]`, #392), and more than one -# document. Each of those has a meaning this reader would have to invent, and -# an invented meaning in a validator is a wrong answer with a confident tone. +# (`&`, `*`), tags (`!`), merge keys (`<<`), explicit keys (`? `), an empty +# block key (`: a`, #715), a single-pair mapping in a flow sequence (`[a: b]`, +# #392), and more than one document. Each of those has a meaning this reader +# would have to invent, and an invented meaning in a validator is a wrong +# answer with a confident tone. # # A QUOTE OPENS A QUOTED SCALAR ONLY WHERE A SCALAR BEGINS, as in YAML; in # the middle of a plain scalar (`it's`) it is a character of it (#249). A @@ -231,7 +234,22 @@ def __init__(self, message: str, remediation: str = ""): # `@a: 1`, and `|` or `>` opening a plain scalar. See `_scalar`, # `_refuse_entry_indicator` and `_split_flow`. -_DASH_RE = re.compile(r"^-(\s+|$)") +#: YAML'S WHITE SPACE IS THE SPACE AND THE TAB (#715), and every strip of a +#: line below names it. `str.strip()` with no argument, like the regex `\s`, +#: also takes U+001F and sixteen Unicode spaces (U+00A0 NO-BREAK SPACE, +#: U+1680, U+2000 to U+200A, U+202F, U+205F and U+3000 IDEOGRAPHIC SPACE), +#: which YAML keeps as text. So `k: \u3000]` was read as `k: ]` and refused +#: quoting `']'`, a text the file does not hold, `k: [a,\u3000,b]` was +#: refused as an empty entry, and `k: \xa0` read as no value, where PyYAML +#: reads `'\u3000]'`, `['a', '\u3000', 'b']` and `'\xa0'`. U+001F, a control +#: character, is now text, as a raw ESC or NUL already is. The other +#: characters `str.strip()` takes are line breaks to `str.splitlines`, which +#: ends a line at them, so none of them reaches a strip. +_YAML_WHITE = " \t" + +#: A sequence entry's `-`, then YAML's white space (`_YAML_WHITE`, #715) or +#: the end of the line: `-\u3000a` is a plain scalar, and no entry. +_DASH_RE = re.compile(r"^-([ \t]+|$)") class _Rec: @@ -329,8 +347,8 @@ def _strip_comment(text: str) -> str: (`_unquoted`), at the start of the text or after a space or tab.""" for i, ch in _unquoted(text): if ch == "#" and (i == 0 or text[i - 1] in " \t"): - return text[:i].rstrip() - return text.rstrip() + return text[:i].rstrip(_YAML_WHITE) + return text.rstrip(_YAML_WHITE) def _split_key(text: str) -> tuple[str, str] | None: @@ -347,14 +365,15 @@ def _split_key(text: str) -> tuple[str, str] | None: bracket = False for i, ch in _unquoted(text): if ch in "[{": - if not text[:i].strip(): + if not text[:i].strip(_YAML_WHITE): return None # a flow collection cannot be a key in this subset bracket = True elif ch == ":" and (i + 1 == len(text) or text[i + 1] in " \t"): + key = text[:i].strip(_YAML_WHITE) if bracket: raise YamlError("a key holding `[` or `{` is not supported: " - f"{text[:i].strip()!r}") - return _unquote_key(text[:i].strip()), text[i + 1:].strip() + f"{key!r}") + return _unquote_key(key), text[i + 1:].strip(_YAML_WHITE) return None @@ -551,7 +570,7 @@ def _scalar(raw: str) -> Any: a scalar (`k: a]`, `- see [1]`, `k: a, b`), and every quoted scalar (`k: "]"`), read as before. """ - text = raw.strip() + text = raw.strip(_YAML_WHITE) if text == "" or text in ("null", "~", "Null", "NULL"): return None if text[0] in "\"'": @@ -613,9 +632,19 @@ def _refuse_entry_indicator(text: str, remedy: bool = True) -> None: indicator, as in YAML and #252: `[-]`, `[-a]`, `{-: 1}` and `{?a: b}` read as they did. A `]` or `}` is left to `_refuse_flow_indicator`, which names it in #301's words. + + A KEY THAT IS A LONE `?` IS REFUSED WITH THEM (#715): a `?` whose `:` + opens a value, with a space, a tab or nothing after it. `k: {?: 1}` read + the key `'?'`, where PyYAML reads an explicit key that has no key, + `{None: 1}`, while the lone `?` of `[?]`, `{?}` and `{a: ?}` was + refused as one; `[?: 1]` was refused as a single pair. Both are now + #252's refusal. `{?:a}` holds no `: `, and `[?:a]` is a `?` with text + after it, the text #252 keeps for `?a`; a block `?: 1` is the key `'?'` + to PyYAML too. """ first, after = text[:1], text[1:2] if (first in ":#@`%" or first in "-?" and after in (" ", "\t") + or text.startswith("?:") and text[2:3] in ("", " ", "\t") or text == "?"): refusal = _indicator_refusal(text) if not remedy: @@ -648,26 +677,44 @@ def _flow_value(text: str, item: bool = False) -> Any: cannot spell. So `pins: [openGlass: x]` is a typo, refused as one. `{a: b}` in a sequence is a flow mapping and reads, and `[a:b]` is the text `a:b`, as before. + + AN ITEM WITH A SECOND `:` OPENING A VALUE IS NO PAIR AT ALL (#715). + YAML ends the pair's plain value at that `:`, and PyYAML refuses + `[a: b: c]` as invalid, so the single-pair words named a mapping the + file does not hold. It is refused as invalid YAML. `[a:b: c]`, whose key + is `a:b`, and `[a: "b: c"]`, whose value is quoted, are single pairs and + keep those words. """ if text.startswith(("[", "{")): return _flow(text) _refuse_flow_indicator(text) if item: _refuse_entry_indicator(text) - if _split_key(text) is not None: + pair = _split_key(text) + if pair is not None: + # A QUOTED item skipped the test above, so its value is asked + # here, before it is split again (#715): `["a": b[: c]` names + # the `[` in #301's words, not a key holding it. A value that + # OPENS a flow collection is not a plain scalar: `["a": [b]]` + # is a single pair to YAML, and keeps the single-pair words. + if not pair[1].startswith(("[", "{")): + _refuse_flow_indicator(pair[1]) + if _split_key(pair[1]) is not None: + raise YamlError("a flow sequence entry with a second `:` " + f"opening a value is not valid YAML: {text!r}") raise YamlError("a single-pair mapping in a flow sequence is not " f"supported: {text!r}") return _scalar(text) def _flow(text: str) -> Any: - text = text.strip() + text = text.strip(_YAML_WHITE) if text.startswith("[") and text.endswith("]"): - body = text[1:-1].strip() + body = text[1:-1].strip(_YAML_WHITE) return [] if not body else [_flow_value(p, item=True) for p in _split_flow(body)] if text.startswith("{") and text.endswith("}"): - body = text[1:-1].strip() + body = text[1:-1].strip(_YAML_WHITE) out: dict[str, Any] = {} if body: for part in _split_flow(body): @@ -740,9 +787,9 @@ def _split_flow(body: str) -> list[str]: elif ch in "]}": depth -= 1 elif ch == "," and depth == 0: - parts.append(body[start:i].strip()) + parts.append(body[start:i].strip(_YAML_WHITE)) start = i + 1 - parts.append(body[start:].strip()) + parts.append(body[start:].strip(_YAML_WHITE)) if len(parts) > 1 and parts[-1] == "": parts.pop() if "" in parts: @@ -766,9 +813,9 @@ def _significant_text(raw: str, line: int) -> str | None: refused outright, and a blank line or a whole-line comment carries no record at all. """ - if "\t" in raw[: len(raw) - len(raw.lstrip())]: + if "\t" in raw[: len(raw) - len(raw.lstrip(_YAML_WHITE))]: raise YamlError(f"line {line}: tab used for indentation") - stripped = raw.strip() + stripped = raw.strip(_YAML_WHITE) if not stripped or stripped.startswith("#"): return None return stripped @@ -807,16 +854,25 @@ def _peel_dashes(recs: list[_Rec], indent: int, content: str, def _refuse_unsupported(content: str, line: int) -> None: - """The two constructs this reader will not guess at, refused by name. + """The three constructs this reader will not guess at, refused by name. Split out of `_tokenise` for #135. An explicit key and a merge key each have a meaning the reader would have to invent, and an invented meaning in a validator is a wrong answer with a confident tone. + + SO DOES AN EMPTY KEY (#715): a line, or a sequence item, that opens with + a `:` opening a value, `: a` or `- :`. PyYAML refuses it, and this + reader read the key `''`, which no reader gives: `- :` was + `[{'': None}]`. Its flow twin, `{: a}`, is refused by + `_refuse_entry_indicator` (#392). A quoted empty key, `"": a`, is the + key `''` to PyYAML too, and still reads. """ if content.startswith("? "): raise YamlError(f"line {line}: explicit keys are not supported") if content.startswith("<<"): raise YamlError(f"line {line}: merge keys are not supported") + if content.startswith(":") and content[1:2] in ("", " ", "\t"): + raise YamlError(f"line {line}: empty keys are not supported") def _opens_block_scalar(kv: tuple[str, str] | None) -> bool: @@ -838,13 +894,15 @@ def _block_scalar_body(raw_lines: list[str], i: int, Split out of `_tokenise` for #135. The body is every following line MORE INDENTED than the key, with the first such line's column taken as the block's own indentation and stripped from all of them. A blank line - belongs to the body whatever its column, because it has none. + belongs to the body whatever its column, because it has none. A line of + Unicode spaces at the block's own indentation or deeper is not blank + (#715): it is text, as YAML reads it. """ body: list[str] = [] block_indent = None while i < len(raw_lines): nxt = raw_lines[i] - if nxt.strip() == "": + if nxt.strip(_YAML_WHITE) == "": body.append("") i += 1 continue @@ -1036,9 +1094,34 @@ def _block_value(text: str, line: int) -> Any: raise +#: U+FEFF, the BYTE-ORDER MARK, which a Windows editor may start a file a +#: person saved with (#855): `parse_yaml` takes it off. +BYTE_ORDER_MARK = "\ufeff" + + def parse_yaml(text: str) -> Any: - """Parse the supported YAML subset. Raises `YamlError` on anything else.""" - recs = _tokenise(text) + """Parse the supported YAML subset. Raises `YamlError` on anything else. + + A BYTE-ORDER MARK AT THE START IS NO PART OF THE TEXT (#855), and every + one there is taken off. YAML 1.2.2 allows a mark at the start of a + document and none inside one (5.2; 9.1.1's `l-document-prefix` is a + mark, then comment lines). Kept, it was the first character of the + first key, so a `project.yaml` that says `schema_version: 1` was + `schema_version is None` to `validate-manifest.py`, and a file whose + first line is a comment was refused at line 1. + + HERE, NOT IN `load_yaml`'S DECODE, because one file read two ways is + worse than either answer. The readers of a pin at a commit + (`shape-doctor.py`'s `pin_at`, `update-shape.py`'s `_committed_pin`, + `adopt-project.py`'s `_pinned_shape_paths`) and `pin_values_in` hand + this the text of no file `load_yaml` read: with the mark taken off + there alone, `pin_at` read a pin every validator reads as broken, and + the doctor's NEXT put back the pin from before it. Every mark goes, as + `shape_advisory.py` takes them off, so the two agree. `load_yaml` still + decodes as `utf-8`, so the byte its refusal names is counted from the + file's first byte, the mark's three included. The tools write no mark. + """ + recs = _tokenise(text.lstrip(BYTE_ORDER_MARK)) if not recs: return None value, end = _parse_nodes(recs, 0, recs[0].indent) @@ -1055,11 +1138,20 @@ def load_yaml(path: Path) -> Any: and only then fails to decode it, so the `OSError` clause never sees it and, uncaught, it left every reader as a Python traceback (#192) rather than the `REFUSED yaml-unreadable` line a missing file already gets. + + AN `OSError` NAMES THE FILE ONCE (#712). `str()` of one is `[Errno 13] + Permission denied: ''`, so `: ` named the path twice, + the second time in Python's quoting. The line is `: cannot be read + ()`, the words `validate-pins.py` gives a workflow + file it cannot open (#359), and the refusal is still raised `from` the + `OSError`, which that validator reads to name the file relative to its + root. """ try: text = Path(path).read_text(encoding="utf-8") except OSError as exc: - raise Refusal("yaml-unreadable", f"{path}: {exc}") from exc + raise Refusal("yaml-unreadable", + f"{path}: cannot be read ({_os_reason(exc)})") from exc except UnicodeDecodeError as exc: raise Refusal( "yaml-unreadable", @@ -3341,10 +3433,44 @@ def spelled(value: object) -> str: POWERSHELL_SINGLE_QUOTES = frozenset("'‘’‚‛") +def host_shell_dialect(name: str | None = None, environ=None) -> str: + """Which shell a line printed on this host is quoted for, as the + `platform` `shell_quoted` takes: `"nt"` for PowerShell's dialect, and + `"posix"` for `sh`'s (#711). `name` and `environ` are `os.name` and + `os.environ` when None, so a test can ask about any host from any host. + + THE RULE. A Windows host (`os.name` is `nt`) is PowerShell's, unless its + environment has a non-empty `MSYSTEM`: then the tool was run from Git + Bash, or another MSYS2 shell, which reads a line as `sh` does, so the + line gets `sh`'s quoting. Every other host is `sh`'s. It is asked each + time a line is quoted, not once at import, so the answer is the + environment's the line is printed in. + + WHY `MSYSTEM`. Under Git Bash `os.name` is `nt`, as it is under + PowerShell, because the Python Git Bash runs is the same Windows Python, + so `os.name` cannot tell the two shells apart. Git Bash and every MSYS2 + shell set `MSYSTEM` (`MINGW64`, `UCRT64`, `CLANGARM64`, `MSYS`, ...) and + hand it to every program they start; PowerShell and cmd.exe do not set + it. An EMPTY `MSYSTEM` names no MSYS2 shell, so it is not Git Bash. + + WHAT IT DOES NOT COVER. A PowerShell or cmd.exe started from inside Git + Bash inherits `MSYSTEM`, so a tool run there is answered `sh`'s and + prints `sh`'s quoting into PowerShell: nothing the tool can read tells + that PowerShell from Git Bash, and this does not try. It answers the + quoting and the shell syntax around a word, and nothing about the + machine: `PYTHON` stays `python` under Git Bash, which runs the Windows + Python, and a path keeps its Windows spelling. + """ + name = os.name if name is None else name + environ = os.environ if environ is None else environ + return "nt" if name == "nt" and not environ.get("MSYSTEM") else "posix" + + def shell_quoted(value: object, platform: str | None = None) -> str: """`value` as ONE word of a command a person pastes into the shell of - `platform` (`os.name` when None): bare where that shell reads every - character of it as itself, else whole, in single quotes (#486). + `platform` (`host_shell_dialect()` when None, #711): bare where that + shell reads every character of it as itself, else whole, in single + quotes (#486). THE DOCTOR'S RULE, `shape-doctor.py`'s `quote_arg`, which has quoted every path its NEXT lines print since #101/#102: `sh` and PowerShell are @@ -3373,7 +3499,7 @@ def shell_quoted(value: object, platform: str | None = None) -> str: `quote_arg` still leaves it bare: a known gap, named where #486 lands. """ text = str(value) - windows = (os.name if platform is None else platform) == "nt" + windows = (host_shell_dialect() if platform is None else platform) == "nt" bare = SHELL_BARE_NT if windows else SHELL_BARE_POSIX if not text.startswith("=") and bare.fullmatch(text): return text diff --git a/scripts/validate-manifest.py b/scripts/validate-manifest.py index ab6344f..fe9cb2f 100755 --- a/scripts/validate-manifest.py +++ b/scripts/validate-manifest.py @@ -123,11 +123,35 @@ def _declared_pins(manifest: dict) -> set[str]: `1.5` ended the run in a `TypeError` before that finding could print, and a scalar `openXdox` was read as the pins `X`, `d`, `e`, `n`, `o`, `p`, `x` for every leg's classification. + + AN ENTRY THAT IS NOT A NAME DECLARES NOTHING (#714), and the others are + still declared, as `validate-pins.py` checks the rest past an entry it + calls malformed: `[5]` was the pin `'5'`. `_not_a_name` is the rule, + and `_neutral_product_pins_type_finding` names each such entry. """ declared = manifest.get("neutral_product_pins") if not isinstance(declared, list): return set() - return {str(pin) for pin in declared if pin} + return {pin for pin in declared if _not_a_name(pin) is None} + + +def _not_a_name(entry) -> str | None: + """Why a `neutral_product_pins:` entry or a `naming.referent_chain:` + link is not a name, or None where it is one (#714). + + The naming gate's `_not_a_name`, word for word, so every gate that reads + the two fields says one sentence. A name is text (`validate-pins.py`'s + `neutral-pin-entry-malformed` asks the same), it is not blank (a blank + link was always no link), and every character in it is printable + (`str.isprintable`). `[5]` was the pin `'5'`, and a link holding ESC a + name this validator classified with, `manifest ok`.""" + if not isinstance(entry, str): + return "it is not text" + if not entry.strip(): + return "it is blank" + if not entry.isprintable(): + return "it holds a character that is not printable" + return None def _recorded_chain(naming: dict) -> tuple: @@ -136,13 +160,19 @@ def _recorded_chain(naming: dict) -> tuple: A single string is read as a one-link chain rather than as characters, and anything else yields `()` — the caller reports the shape separately, so a malformed record never becomes a silently different question. + + A link that is not a name (`_not_a_name`) makes the record malformed + too, so it records no chain (#714): `[5]` was the link `'5'`, and + dropping the one link would be the different question. A blank string + is the empty record it always was. """ recorded = naming.get(CHAIN_RECORD_FIELD) if isinstance(recorded, str): - recorded = [recorded] - if not isinstance(recorded, list): + recorded = [recorded] if recorded.strip() else [] + if not isinstance(recorded, list) or any( + _not_a_name(link) is not None for link in recorded): return () - return tuple(str(link) for link in recorded if str(link).strip()) + return tuple(recorded) def _chain_and_links(naming, root: Path | None) -> tuple[tuple, dict]: @@ -223,14 +253,27 @@ def _library_takes_remedies() -> bool: def _naming_chain_field_finding(leg_role, naming: dict) -> list[str]: """Is `naming[CHAIN_RECORD_FIELD]`, if present, a string or a list? - Split from `_naming_findings` for #132. + Split from `_naming_findings` for #132. Each link that is not a name + (`_not_a_name`) is a finding too, with why (#714); a blank string is no + link and no finding. """ recorded = naming.get(CHAIN_RECORD_FIELD) - if recorded is not None and not isinstance(recorded, (str, list)): + if recorded is None: + return [] + if isinstance(recorded, str): + recorded = [recorded] if recorded.strip() else [] + if not isinstance(recorded, list): return [f"FINDING naming-referent-chain: leg {leg_role!r}: " f"{CHAIN_RECORD_FIELD} is {recorded!r}, expected a list of " "neutral-product names"] - return [] + out = [] + for link in recorded: + why = _not_a_name(link) + if why is not None: + out.append(f"FINDING naming-referent-chain: leg {leg_role!r}: " + f"{CHAIN_RECORD_FIELD} link {link!r} is not a " + f"neutral-product name: {why}") + return out def _naming_classification_findings(leg_role, name: str, naming: dict, @@ -596,22 +639,33 @@ def _branch_finding(code: str, what: str, value) -> list[str]: `subprocess.py` and, with that fixed, would only say it could not place the leg, while this file said `manifest ok`. So no tool named the key. Any surrogate is found, as `name` is (#280): no UTF-8 file or console can - hold one, so none is a name anybody wrote. That is the whole of the - question. Whether the value is a name git will create (`a..b`, `x.lock`) - is `git check-ref-format`'s to answer, as the scaffold asks it for - `--tracking-branch` (#283); and a value that is not text is left alone, - as `bootstrap.py` turns it into text. + hold one, so none is a name anybody wrote. Whether the value is a name git + will create (`a..b`, `x.lock`) is `git check-ref-format`'s to answer, as + the scaffold asks it for `--tracking-branch` (#283); and a value that is + not text is left alone, as `bootstrap.py` turns it into text. + + NOR A CHARACTER THAT DOES NOT PRINT, OR WHITE SPACE (#715); `siblings.py` + uses no branch that does not print, though it takes a space. git takes + `main` followed by U+00A0, and since #715 the reader reads that no-break + space as YAML does, where it used to strip it: so `bootstrap.py` made a + second `main` beside the first, and `bump-leg.py`'s guard of the + tracking branch let a commit be made on `main`. No person can tell such + a name from the one without it. """ if not isinstance(value, str): return [] - char = next((c for c in value if c == "\x00" or "\ud800" <= c <= "\udfff"), + char = next((c for c in value if not c.isprintable() or c.isspace()), None) if char is None: return [] - kind = ("a NUL: no argument, file name or ref name can hold one" - if char == "\x00" else - "a lone surrogate: no UTF-8 file or console can hold it, so no " - "branch name is spelled with one") + if char == "\x00": + kind = "a NUL: no argument, file name or ref name can hold one" + elif "\ud800" <= char <= "\udfff": + kind = ("a lone surrogate: no UTF-8 file or console can hold it, so " + "no branch name is spelled with one") + else: + kind = ("a character that does not print or is white space: a person " + "reading the name cannot tell it from the name without it") return [_finding( code, f"{what} is {value!r}, which holds U+{ord(char):04X}, {kind}. Name a " @@ -680,15 +734,26 @@ def _shape_findings(manifest: dict) -> list[str]: def _neutral_product_pins_type_finding(manifest: dict) -> list[str]: """`neutral_product_pins`, OPTIONAL. - Split from `_findings` for #132. + Split from `_findings` for #132. In a list, each entry that is not a + name (`_not_a_name`) is a finding, with why (#714). """ declared_pins = manifest.get("neutral_product_pins") - if declared_pins is not None and not isinstance(declared_pins, list): + if declared_pins is None: + return [] + if not isinstance(declared_pins, list): return [_finding( "manifest-neutral-product-pins", f"neutral_product_pins is {declared_pins!r}, expected a list of " "neutral product names")] - return [] + out = [] + for pin in declared_pins: + why = _not_a_name(pin) + if why is not None: + out.append(_finding( + "manifest-neutral-product-pins", + f"neutral_product_pins entry {pin!r} is not a neutral " + f"product name: {why}")) + return out def _roles_finding(legs: list) -> list[str]: @@ -980,7 +1045,17 @@ def _findings(manifest: dict, policy: NamingPolicy, root=None) -> list[str]: out.extend(_record_findings(manifest, ANSWER_ROWS)) legs = manifest.get("legs") - if not isinstance(legs, list) or not legs: + # A `legs:` that is not a list is said as what it is (#714): `5` was + # "missing or empty", which it is not. The naming gate and + # `validate-pins.py` say this sentence too. `0`, `false` and `''` are + # not a list either, as #443 reads `neutral_product_pins:`. + if legs is not None and not isinstance(legs, list): + out.append(_finding( + "manifest-legs", + f"legs is {legs!r}, expected a list of `role:`, `repository:` " + "and `path:` mappings")) + return out + if not legs: out.append(_finding("manifest-legs", "legs is missing or empty")) return out diff --git a/scripts/validate-pins.py b/scripts/validate-pins.py index 6324772..65607ce 100755 --- a/scripts/validate-pins.py +++ b/scripts/validate-pins.py @@ -45,10 +45,11 @@ recheckable neutral-product pin disagrees 1 a FINDING: drift a reviewer can act on 2 a REFUSAL: the question could not be asked — an uninitialized submodule, - an unresolvable commit, an unreadable pin or workflow file (one that is - not UTF-8, or that this user cannot open), or a `.github/workflows/` - directory this user cannot list or search (#434). An unanswerable pin - question is never an implicit pass. + an unresolvable commit, an unreadable pin, manifest, workflow file or + pinned shape copy (one that is not UTF-8, or that this user cannot + open or reach, #712), or a `.github/workflows/` directory this user + cannot list or search (#434). An unanswerable pin question is never an + implicit pass. No git to run is one too (`git-not-runnable`, #412), a git that is missing, will not run or cannot be executed: the gitlinks and the digests are read through git. @@ -166,10 +167,13 @@ def unused(self, text: str) -> None: class _Unreadable(NamedTuple): """One workflow file that could not be read (#359): the line that names it, - and whether it failed on its ENCODING (`True`) or on being opened.""" + whether it failed on its ENCODING (`True`) or on being opened, and, for + one that could not be opened, whether the operating system refused a + PERMISSION (#712): any other cause gets its own advice, not `chmod`.""" line: str encoding: bool + permission: bool = False def _read_workflow(root: Path, path: Path) -> tuple[str, _Unreadable | None]: @@ -183,17 +187,18 @@ def _read_workflow(root: Path, path: Path) -> tuple[str, _Unreadable | None]: it into git and into an editor that opens it from the root. A file this user cannot open (`OSError`) is the same line with the operating system's own reason, because it is the same unguarded read. + The name is spelled as `_named` spells it (#712): a newline in it began a + line of its own, and a bidi control reordered the line it was printed on. """ - rel = path.relative_to(root).as_posix() + rel = _named(root, path) try: return path.read_text(encoding="utf-8"), None except UnicodeDecodeError as exc: return "", _Unreadable( f"{rel}: not UTF-8 at byte {exc.start} ({exc.reason})", True) except OSError as exc: - return "", _Unreadable( - f"{rel}: cannot be read ({exc.strerror or type(exc).__name__})", - False) + return "", _Unreadable(_cannot_read(rel, exc), False, + isinstance(exc, PermissionError)) def _workflow_unreadable(files: list[_Unreadable]) -> Refusal: @@ -204,6 +209,12 @@ def _workflow_unreadable(files: list[_Unreadable]) -> Refusal: them all before it raises. The first line follows the id and the rest follow it, one file to a line, so `shape-doctor.py`, which quotes the first `REFUSED` line of this script's output, quotes a file's name. + + THE ADVICE FOLLOWS THE CAUSE (#712), as the directory's does (#434): + "restore read access" was said of every `cannot be read` file, and a + file whose `stat` or read failed with an I/O error or a name too long + is not put right by a mode. A permission keeps its sentence; any other + cause gets `OTHER_REASON`, the words `bump-leg.py` gives it (#383). """ advice = "" if any(one.encoding for one in files): @@ -213,12 +224,14 @@ def _workflow_unreadable(files: list[_Unreadable]) -> Refusal: "as UTF-8 (a UTF-16 file with a byte-order mark needs converting, " "not editing), or replace that byte with the character it was " "meant to be, and commit it. ") - if not all(one.encoding for one in files): + if any(one.permission for one in files): advice += ( "A `cannot be read` file is not an encoding failure: this user " "cannot open it, so restore read access to it, and search access " "to every directory above it inside the root (a file in a " "directory this user can list but not search cannot be opened). ") + if any(not one.encoding and not one.permission for one in files): + advice += OTHER_REASON return Refusal( "workflow-unreadable", "\n".join(one.line for one in files), @@ -229,7 +242,7 @@ def _workflow_unreadable(files: list[_Unreadable]) -> Refusal: "workflow it skipped could hide a `@` reference that is out of " "lockstep, which is what it exists to find, and a lossy read would " "call green a tree that the project's other readers refuse. " - + advice + "Then run the same command again.") + + advice + RUN_AGAIN) #: The `OSError`s that `Path.is_file()` and `Path.is_dir()` answered False to @@ -266,6 +279,139 @@ def _is_a(path: Path, kind: Callable[[int], bool]) -> bool: return kind(mode) +def _named(root: Path, path: Path) -> str: + """`path` as this validator names a file (#712): relative to `root`, POSIX + on every platform, as #633's `workflow-unreadable` lines are, and spelled + by `_shown`, so a name holding a newline, a bidi or C1 control, an ESC or + a byte that is not UTF-8 prints as `repr` spells it and stays on its + line. A path that is not under the root (a role or a product spelled as + an absolute path) is named whole, since `relative_to` would end the run + in a `ValueError`.""" + try: + return _shown(path.relative_to(root).as_posix()) + except ValueError: + return _shown(str(path)) + + +def _cannot_read(named: str, exc: OSError) -> str: + """`: cannot be read ()`: the line + for a file whose `stat` or open the operating system refused, in every + refusal that names one (#359, #434, #712).""" + return f"{named}: cannot be read ({exc.strerror or type(exc).__name__})" + + +#: What a file the operating system refused for ANOTHER reason than +#: permission is told (#712): an I/O error, a name longer than the +#: filesystem takes, a file replaced while this ran. No `chmod` puts one +#: right. `bump-leg.py`'s words for the same refusal (#383). +OTHER_REASON = ( + "Where the operating system gave another reason, put a regular file this " + "user can open back in its place, or take away one nobody wants, and make " + "sure nothing is changing the directory while this runs. ") + +#: The sentence every refusal of a file or directory this validator cannot +#: read ends on, said once (#712). +RUN_AGAIN = "Then run the same command again." + +#: What this validator reads, as `_unreadable_file`'s remedy says it. +FILES_READ = ("each leg's pin, the shape pin, each neutral-product pin, " + "project.yaml and every copy the shape pin lists") + + +def _unreadable_file(code: str, named: str, exc: OSError, + reads: str = FILES_READ) -> Refusal: + """The refusal for a pin, the manifest or a pinned shape copy that this + validator must read and the operating system will not let it (#712): + its `stat` refused (a `contracts/` this user cannot search) or its open + refused (a pin at mode 000). + + NAMED ONCE, RELATIVE TO THE ROOT. A pin this user could not open was + `load_yaml`'s line, `: [Errno 13] Permission denied: + ''`, under THE LOCKSTEP RULE's remedy, which moves a pin + and says nothing of access. This is `_cannot_read`'s line, with a remedy + of its own, so `main` adds no other. + + THE ADVICE FOLLOWS THE CAUSE, in `bump-leg.py`'s words for the same + file (#383): a permission (`PermissionError`, which is `EACCES` and + `EPERM`) is put right by the mode or the owner, and any other cause is + `OTHER_REASON`, which is not told to `chmod`. `reads` names what the + validator reads, a leg's checkout where that is what it cannot reach. + """ + advice = ( + "Make what this user may not open readable by this user: `chmod u+r` " + "on a file, `chmod u+rx` on a directory (and on every directory above " + "it inside the root), or fix who owns it. " + if isinstance(exc, PermissionError) else OTHER_REASON) + return Refusal( + code, _cannot_read(named, exc), + f"Remediation: this validator reads {reads}, and one it cannot read " + "is a question it cannot answer, never a pass. " + advice + RUN_AGAIN) + + +def _is_file(root: Path, path: Path, code: str = "yaml-unreadable", + named: str | None = None) -> bool: + """Whether `path` is a file, asked as `_is_a` asks it, or `code`'s + refusal naming it (`named`, else `_named`) where its `stat` was refused + (#712). The four `is_file()` calls on a pin and the manifest, and the + one on each shape copy, are this. + + `Path.is_file()` raised a refused `stat` out of the run on Python 3.9 to + 3.13, so a `contracts/` this user could not search ended in a + `PermissionError` traceback, exit 1; on 3.14 it answered False, and the + run refused `pin-missing`, a present file said not to exist, under the + lockstep remedy. A name the operating system cannot take at all (a NUL + in a role or a product) is no file, as `is_file()` answered it.""" + try: + return _is_a(path, stat.S_ISREG) + except OSError as exc: + raise _unreadable_file(code, named or _named(root, path), + exc) from exc + except ValueError: + return False + + +def _is_there(path: Path) -> bool: + """Whether anything is at `path` (a checkout's `.git`, a file or a + directory), asked as `_is_a` asks it, raising the `OSError` of a `stat` + the operating system refused (#712, after RV827's S1). + + NOT `Path.exists()`, which raised a refused `stat` out of the run up to + Python 3.13, so a leg checkout or a checkout beside the root that this + user cannot search ended in a `PermissionError` traceback, and which + answers False to it from 3.14, so the leg read as an uninitialized + submodule. A name the system cannot take at all is nothing there, as + `exists()` answered it.""" + try: + return _is_a(path, lambda _mode: True) + except ValueError: + return False + + +def _load(root: Path, path: Path): + """`load_yaml(path)`, with every refusal it raises naming the file as + `_named` does (#712). + + `load_yaml` names the path it was given, which is absolute here and in + the platform's own spelling, raw. A file this user cannot open is + `_unreadable_file`'s refusal, `yaml-unreadable`, the id it always had; + any other keeps its id, its words and its remedy, with the path at its + start named relative to the root. Asked of the refusal and not of new + words in `repo_shape`, so a root whose `scripts/repo_shape.py` is older + than this file names the file the same way (#370, #610): `load_yaml` + has raised its refusal `from` the `OSError` since it was written.""" + try: + return load_yaml(path) + except Refusal as exc: + if isinstance(exc.__cause__, OSError): + raise _unreadable_file("yaml-unreadable", _named(root, path), + exc.__cause__) from exc.__cause__ + absolute = f"{path}: " + if not exc.detail.startswith(absolute): + raise + raise Refusal(exc.code, _named(root, path) + ": " + + exc.detail[len(absolute):], exc.remediation) from exc + + def _workflows_unlistable(root: Path, workflows: Path, exc: OSError) -> Refusal: """`workflow-unreadable` for the DIRECTORY, named as one (#434). @@ -300,8 +446,7 @@ def _workflows_unlistable(root: Path, workflows: Path, "neither skips one nor reads one lossily: a workflow it skipped " "could hide a `@` reference that is out of lockstep, which is " "what it exists to find, and a directory it cannot list is one whose " - "workflows it cannot read. " + advice - + "Then run the same command again.") + "workflows it cannot read. " + advice + RUN_AGAIN) def _workflow_refs(root: Path) -> list[tuple[Path, str, str]]: @@ -345,10 +490,11 @@ def _workflow_refs(root: Path) -> list[tuple[Path, str, str]]: if not _is_a(path, stat.S_ISREG): continue except OSError as exc: - # The words `_read_workflow` gives a file it cannot open. + # The words `_read_workflow` gives a file it cannot open, the + # name spelled and the cause kept for the advice (#712). unreadable.append(_Unreadable( - f"{path.relative_to(root).as_posix()}: cannot be read " - f"({exc.strerror or type(exc).__name__})", False)) + _cannot_read(_named(root, path), exc), False, + isinstance(exc, PermissionError))) continue text, problem = _read_workflow(root, path) if problem is not None: @@ -368,23 +514,27 @@ def _load_leg_pin(root: Path, role, path: str, repository: str, Split from `_check_leg` for #132: everything that answers "does the pin file itself look right" belongs together. Returns the pin, its path - relative to `root`, and its `commit:` lowercased. + relative to `root` as `_named` spells it, and its `commit:` lowercased. + + A pin that cannot be reached or read is refused by name (#712): see + `_is_file` and `_load`. Every name is spelled, the leg's role and + repository in the lines below as the pin's path is. """ pin_path = root / "contracts" / f"{role}-pin.yaml" - if not pin_path.is_file(): + # POSIX, on every platform: a finding names a path in THIS repository, + # and `str()` of a relative Path on Windows spells it with backslashes — + # a path the reader cannot paste back into git. + rel = _named(root, pin_path) + if not _is_file(root, pin_path): raise Refusal( "pin-missing", - f"leg {role!r} declares {repository} at {path!r} but " - f"{pin_path.relative_to(root).as_posix()} does not exist", + f"leg {role!r} declares {_shown(repository)} at {path!r} but " + f"{rel} does not exist", ) - pin = load_yaml(pin_path) + pin = _load(root, pin_path) if not isinstance(pin, dict): - raise Refusal("pin-unreadable", f"{pin_path}: not a mapping") + raise Refusal("pin-unreadable", f"{rel}: not a mapping") - # POSIX, on every platform: a finding names a path in THIS repository, - # and `str()` of a relative Path on Windows spells it with backslashes — - # a path the reader cannot paste back into git. - rel = pin_path.relative_to(root).as_posix() if pin.get("kind") != "pinned_contract_manifest": report.finding("pin-wrong-kind", f"{rel}: kind is {pin.get('kind')!r}, expected " @@ -406,7 +556,8 @@ def _load_leg_pin(root: Path, role, path: str, repository: str, if pin.get("submodule_path") != path: report.finding("pin-path-mismatch", f"{rel}: submodule_path is {pin.get('submodule_path')!r} " - f"but project.yaml puts the {role} leg at {path!r}") + f"but project.yaml puts the {_shown(str(role))} leg at " + f"{path!r}") if pin.get("source_repository") != repository: report.finding("pin-repository-mismatch", f"{rel}: source_repository is " @@ -493,10 +644,22 @@ def _gitlink_repairs(root: Path, role, gitlink: str, commit: str) -> str: "commit and the second the pin's", gitlink, commit) +def _quoted(value) -> str: + """`value` as one word of a command this validator prints: `shlex.quote`d, + as every such word always was, unless a character in it is not printable, + and then as `_shown` spells it (#712). Quoted, an ESC or a line break in + a root's path or a leg's role reached the terminal as the control it is; + spelled, the word cannot be pasted as the value it names, and no quoting + could make it so.""" + text = str(value) + return shlex.quote(text) if text.isprintable() else _shown(text) + + def _bump_command(root: Path, role) -> str: - """`bump-leg.py` for this root's leg `role`, up to its `--to` (#478).""" - return (f"{PYTHON} scripts/bump-leg.py --root {shlex.quote(str(root))} " - f"--leg {shlex.quote(str(role))} --to") + """`bump-leg.py` for this root's leg `role`, up to its `--to` (#478), + each word as `_quoted` spells it (#712).""" + return (f"{PYTHON} scripts/bump-leg.py --root {_quoted(root)} " + f"--leg {_quoted(role)} --to") #: What a `bump-leg.py` line needs to commit, said once for every finding @@ -576,10 +739,11 @@ def _put_back(paths: list[str]) -> str: other leg's, which holds the next line back and asks for this step again. AN UNDO IS FOLLOWED BY `git submodule update` over the legs: it puts the gitlink back and leaves the leg checked out at the move, a - root `bump-leg.py` refuses as dirty.""" + root `bump-leg.py` refuses as dirty. Each path is spelled (#712).""" one = len(paths) == 1 - which, moved = (f"{paths[0]}'s", "it") if one else ("one leg's", "them") - legs = " ".join(shlex.quote(path) for path in paths) + which, moved = ((f"{_shown(paths[0])}'s", "it") if one + else ("one leg's", "them")) + legs = " ".join(_quoted(path) for path in paths) return (f"put {which} gitlink back at its pin (check that leg out at the " "pin's commit and commit its gitlink in this root, with every " "workflow reference to its repository at that commit too), or " @@ -593,15 +757,17 @@ def _held_then(lines: str, whom: str, back: Back | None, for `whom`. Never a line while `back`'s own digest stands, since two legs are then out of step with no gitlink off; and in the gitlink's own finding (`other_legs`), how its move is kept if it was meant: the - whole commit, which is the only `--to` `bump-leg.py` takes.""" + whole commit, which is the only `--to` `bump-leg.py` takes. `back`'s + path is spelled (#712); `whom` comes spelled.""" if back is not None and back.digest: - return (f"names no line while {back.path}'s own pin-digest-mismatch " - "stands.") + return (f"names no line while {_shown(back.path)}'s own " + "pin-digest-mismatch stands.") if back is not None and other_legs: after = ("after that line" if lines.endswith("line") else "after one of them") - return (f"names {lines} for {whom}, and if {back.path}'s move was " - f"meant, bumping {back.path} to {back.gitlink} {after} keeps " + named = _shown(back.path) + return (f"names {lines} for {whom}, and if {named}'s move was " + f"meant, bumping {named} to {back.gitlink} {after} keeps " "it.") return f"names {lines} for {whom}." @@ -627,20 +793,21 @@ def _gitlink_repairs_held_back(paths: list[str], others: list[str], by hand, for `back`. The gitlink's own finding is given `other_legs`, `{path: codes}`, and `lines` are then that leg's; the other leg's finding names its own `lines`, for itself by its path, since "that leg" - would read as `back`. `_held_then` says what follows. + would read as `back`. `_held_then` says what follows. Every path is + spelled (#712). """ what, first, whom = [], [], "that leg" if len(paths) > 1: - what.append(" and ".join(paths) + (" are both" if len(paths) == 2 - else " are all") + what.append(" and ".join(_shown(path) for path in paths) + + (" are both" if len(paths) == 2 else " are all") + " off their pins") whom = "the leg that is left" elif other_legs: - whom = " and ".join(other_legs) + whom = " and ".join(_shown(path) for path in other_legs) elif back is not None: - what.append(f"{back.path} is off its pin") - whom = paths[0] - what += [f"{path} is out of step too ({', '.join(codes)})" + what.append(f"{_shown(back.path)} is off its pin") + whom = _shown(paths[0]) + what += [f"{_shown(path)} is out of step too ({', '.join(codes)})" for path, codes in (other_legs or {}).items()] if others: what.append("this validator also reports " + ", ".join(others)) @@ -777,9 +944,10 @@ def _out_of_step(report: Report) -> tuple[list, list[str]]: def _curable_ending(root: Path, one: Curable, legs: list, untagged: list[str], off: dict, named) -> str: """What `_end_curable_findings` adds to the finding `one`, or `""`. - `off` is `{role: Back}` of each leg whose gitlink is off its pin.""" + `off` is `{role: Back}` of each leg whose gitlink is off its pin. The + leg's path is spelled (#712).""" if one.role in off: - return (f"Either bump-leg.py line under {one.path}'s " + return (f"Either bump-leg.py line under {_shown(one.path)}'s " "pin-gitlink-mismatch above clears this finding too." if one.role == named else "") # Every other leg out of step, as the `Back` of its gitlink off its pin, @@ -801,28 +969,31 @@ def _check_gitlink_matches_pin(root: Path, role, path: str, commit: str, rel: str, report: Report) -> str: """Fact 1 vs fact 2: the recorded gitlink against the pin file's `commit:`. Split from `_check_leg` for #132. Returns the gitlink so the - caller can check it again against fact 3, the workflow references. + caller can check it again against fact 3, the workflow references. The + role and the path are spelled (#712). """ - with _git_runs(f"the {role} leg's gitlink recorded at {path!r} could " - "not be read"): + role_named = _shown(str(role)) + with _git_runs(f"the {role_named} leg's gitlink recorded at {path!r} " + "could not be read"): gitlink = recorded_gitlink(root, path) if gitlink is None: raise Refusal( "pin-gitlink-absent", - f"no gitlink (mode 160000) recorded at {path!r}. The {role} leg is " - "declared in project.yaml but this repository does not record it " - "as a submodule.", + f"no gitlink (mode 160000) recorded at {path!r}. The {role_named} " + "leg is declared in project.yaml but this repository does not " + "record it as a submodule.", ) if gitlink != commit: report.finding( "pin-gitlink-mismatch", - f"{path}: gitlink {gitlink} != {rel} commit {commit}\n " + LOCKSTEP, + f"{_shown(path)}: gitlink {gitlink} != {rel} commit {commit}\n " + + LOCKSTEP, leg=role, ) report.mismatches.append((len(report.findings) - 1, role, path, gitlink, commit)) else: - report.note(f"{path}: gitlink == {rel} commit {commit[:12]}") + report.note(f"{_shown(path)}: gitlink == {rel} commit {commit[:12]}") return gitlink @@ -831,20 +1002,34 @@ def _check_pin_digest(root: Path, path: str, commit: str, pin: dict, rel: str, """The digest: split from `_check_leg` for #132. Recomputes the leg's tree sha256 at the pinned commit, because a gitlink and a pin file that agree on a commit may still disagree on that commit's bytes. A mismatch - is one `bump-leg.py` clears, since it recomputes the digest (#478). + is one `bump-leg.py` clears, since it recomputes the digest (#478). The + leg's path is spelled in every line (#712). + + A CHECKOUT THIS USER CANNOT SEARCH IS REFUSED BY NAME (#712, after + RV827's S1), `pin-submodule-unreadable`, naming its `.git`: the digest + cannot be recomputed, and the leg is not uninitialized, which is what + Python 3.14's `exists()` would have said of it. """ submodule = root / path - if not (submodule / ".git").exists(): + named = _shown(path) + try: + initialized = _is_there(submodule / ".git") + except OSError as exc: + raise _unreadable_file( + "pin-submodule-unreadable", _named(root, submodule / ".git"), exc, + "each leg's checkout, to recompute its pinned tree digest from " + "the leg's own objects") from exc + if not initialized: raise Refusal( "pin-submodule-uninitialized", - f"{path} is not an initialized submodule checkout, so the pinned " + f"{named} is not an initialized submodule checkout, so the pinned " "digest cannot be recomputed. Presence is not identity and an " "unreadable surface must fail rather than degrade.", ) # `_git_runs` OUTSIDE the `try`: the refusal it raises is a `Refusal`, and # inside, the `except Refusal` below would turn "no git to run" into # `pin-commit-unresolvable`, which says the commit is missing. - with _git_runs(f"{path}: the pinned commit {commit[:12]} could not be " + with _git_runs(f"{named}: the pinned commit {commit[:12]} could not be " "looked up in that submodule"): try: git_out(["rev-parse", "--verify", "--quiet", @@ -852,7 +1037,7 @@ def _check_pin_digest(root: Path, path: str, commit: str, pin: dict, rel: str, except Refusal as exc: raise Refusal( "pin-commit-unresolvable", - f"{path}: the pinned commit {commit} is not in that " + f"{named}: the pinned commit {commit} is not in that " f"submodule's object store ({exc.detail})", ) from exc recorded = (pin.get("digests") or {}).get("tree_sha256") @@ -867,7 +1052,7 @@ def _check_pin_digest(root: Path, path: str, commit: str, pin: dict, rel: str, "unstated is a number, not an identity.", ) else: - with _git_runs(f"{path}: the tree digest at {commit[:12]} could not " + with _git_runs(f"{named}: the tree digest at {commit[:12]} could not " "be recomputed"): actual = tree_digest(submodule, commit) if actual.lower() != recorded.lower(): @@ -882,7 +1067,8 @@ def _check_pin_digest(root: Path, path: str, commit: str, pin: dict, rel: str, report.curable.append(Curable(len(report.findings) - 1, role, path, commit, None)) else: - report.note(f"{path}: tree digest recomputes ({recorded[:12]}…)") + report.note(f"{named}: tree digest recomputes " + f"({recorded[:12]}…)") def _check_workflow_refs_for_leg(refs, repository: str, path: str, @@ -891,8 +1077,14 @@ def _check_workflow_refs_for_leg(refs, repository: str, path: str, """Fact 3: split from `_check_leg` for #132 — every workflow `@` reference naming this leg's repository must agree with the gitlink. A mismatch is one `bump-leg.py` clears, since it moves every one (#478). + + THE NOTE COUNTS THE REFERENCES THAT AGREE (#739). It counted every + reference naming the repository, so one off its gitlink was its finding + and, beside it, `1 workflow @ reference(s) agree with the gitlink`. + Only those at the gitlink are counted, `N of M` where some are not, and + no note is printed where none is. """ - seen = 0 + seen = agree = 0 for wf_path, wf_repo, wf_sha in refs: if wf_repo != repository: continue @@ -900,14 +1092,17 @@ def _check_workflow_refs_for_leg(refs, repository: str, path: str, if wf_sha != gitlink: report.finding( "pin-workflow-ref-mismatch", - f"{wf_path.name}: `{wf_repo}@{wf_sha}` != the gitlink " + f"{_shown(wf_path.name)}: `{wf_repo}@{wf_sha}` != the gitlink " f"{gitlink} recorded at {path!r}\n " + LOCKSTEP, leg=role, ) report.curable.append(Curable(len(report.findings) - 1, role, path, gitlink, wf_sha)) - if seen: - report.note(f"{repository}: {seen} workflow @ reference(s) " + else: + agree += 1 + if agree: + counted = str(agree) if agree == seen else f"{agree} of {seen}" + report.note(f"{repository}: {counted} workflow @ reference(s) " f"agree with the gitlink") @@ -929,14 +1124,19 @@ def _check_shape_pin_header(root: Path, manifest: dict, report: Report Split from `_check_shape_pin` for #132. Returns the pin and its path relative to `root`. + + Named relative to the root in every line, the two refusals included, + which named it absolute, and refused by name where it cannot be reached + or read (#712). The two commits it compares are spelled: neither is + asked to be 40 hex before the finding that names both. """ pin_path = root / "contracts" / "shape-pin.yaml" - if not pin_path.is_file(): - raise Refusal("shape-pin-missing", f"{pin_path} does not exist") - pin = load_yaml(pin_path) - rel = pin_path.relative_to(root).as_posix() + rel = _named(root, pin_path) + if not _is_file(root, pin_path): + raise Refusal("shape-pin-missing", f"{rel} does not exist") + pin = _load(root, pin_path) if not isinstance(pin, dict): - raise Refusal("shape-pin-unreadable", f"{pin_path}: not a mapping") + raise Refusal("shape-pin-unreadable", f"{rel}: not a mapping") if pin.get("revision_kind") != "commit": report.finding("pin-tag-only", f"{rel}: revision_kind is {pin.get('revision_kind')!r}") @@ -947,8 +1147,8 @@ def _check_shape_pin_header(root: Path, manifest: dict, report: Report declared = (manifest.get("shape") or {}).get("commit") if declared and str(declared).lower() != commit.lower(): report.finding("shape-pin-manifest-disagree", - f"{rel} commit {commit} != project.yaml shape.commit " - f"{declared}") + f"{rel} commit {_shown(commit)} != project.yaml " + f"shape.commit {_shown(str(declared))}") return pin, rel @@ -971,16 +1171,26 @@ def _check_shape_copy(root: Path, row, rel: str, report: Report) -> None: f"{path!r}, not the path of a file") return target = root / path - if not target.is_file(): + # A copy whose `stat` or read is refused is `shape-copy-unreadable`, a + # refusal naming it as the row does (#712). `is_file()` raised a refused + # `stat` out of the run up to Python 3.13, and called the copy "pinned + # but absent" on 3.14; a copy at mode 000 ended in `file_sha256`'s + # `PermissionError` on every Python. The row's path and digest are + # spelled, as the pin spells them. + named = _shown(path) + if not _is_file(root, target, "shape-copy-unreadable", named): report.finding("shape-copy-missing", - f"{rel}: {row.get('path')} is pinned but absent") + f"{rel}: {named} is pinned but absent") return - actual = file_sha256(target) + try: + actual = file_sha256(target) + except OSError as exc: + raise _unreadable_file("shape-copy-unreadable", named, exc) from exc if actual != str(row.get("sha256", "")).lower(): report.finding( "shape-copy-drift", - f"{row.get('path')}: sha256 {actual}\n" - f" {rel} records {row.get('sha256')}\n" + f"{named}: sha256 {actual}\n" + f" {rel} records {_shown(str(row.get('sha256')))}\n" " A shape file was edited in place. Either revert it, or " "carry the change upstream to openRepoShape and re-pin.", ) @@ -1040,9 +1250,9 @@ class PinSources: that a source keyed to a product `a` that nothing declares, which no pin ever looked up — the digest went to `gh api` and a `skip`, exit 0, with the skip line itself suggesting the `--pin-source` the person had just - passed. So the text before the first `=` is a PRODUCT only when it - casefolds to a declared pin's name, the same casefold the lookup below - uses, and any other value is a bare PATH, whole. A declared name wins + passed. So the text before the first `=` is a PRODUCT only when its + `pin_product_key` is a declared pin's, the key the lookup below uses, + and any other value is a bare PATH, whole. A declared name wins even when a directory is spelled like the whole value (`openGlass=x/openGlass` beside a directory `openGlass=x`): the reading is decided by the manifest and never by what happens to be on the disk, and @@ -1057,14 +1267,24 @@ class PinSources: A VALUE THAT NAMES NO PATH IS REFUSED (#442), `pin-source-no-path`: `openGlass=` with nothing after its `=`, or an empty value. Either was `Path("")`, which is `.`, so the directory this ran in was asked for the - pinned commit and the finding blamed `.`. `.` typed still means it. + pinned commit and the finding blamed `.`. `.` typed still means it. The + value and the product it names are spelled, as `_shown` spells them. + + KEYED BY `pin_product_key`, the pin file's own fold (#712, after #638). + `casefold()` took `openss=` for `openß`, two pin files, where the chain + readers' `LinkSources` read them as two: the two copied tools read + `[PRODUCT=]PATH` one way over ASCII alone. A pin's digest stopped a wrong + source from passing; the source was simply looked up under the other + spelling. """ def __init__(self, values: list[str], declared) -> None: #: The declared pins, spelled as `project.yaml` spells them. self.declared = [p for p in declared if isinstance(p, str) and p] - names = {product.casefold(): product for product in self.declared} - #: `{product casefolded: (typed, path)}`, the last value given wins. + names = {pin_product_key(product): product + for product in self.declared} + #: `{pin_product_key(product): (typed, path)}`, the last value given + #: wins. self.keyed: dict[str, tuple[str, Path]] = {} #: `(typed, path)` of the last bare value, if any. self.bare: tuple[str, Path] | None = None @@ -1077,18 +1297,19 @@ def __init__(self, values: list[str], declared) -> None: self.used: set[str] = set() for typed in values: key, sep, path = typed.partition("=") - product = names.get(key.casefold()) if sep else None + product = names.get(pin_product_key(key)) if sep else None self.reading[typed] = product if product is not None: - earlier = self.keyed.get(product.casefold()) - self.keyed[product.casefold()] = (typed, Path(path)) + earlier = self.keyed.get(pin_product_key(product)) + self.keyed[pin_product_key(product)] = (typed, Path(path)) else: earlier = self.bare self.bare = (typed, Path(typed)) self.replaced.pop(typed, None) if earlier is not None and earlier[0] != typed: self.replaced[earlier[0]] = typed - no_path = [f"`--pin-source {typed}` names no path for {product}" + no_path = [f"`--pin-source {_shown(typed)}` names no path for " + f"{_shown(product)}" if typed else "`--pin-source ''` names no path" for typed, product in self.reading.items() if not (typed.partition("=")[2] if product else typed)] @@ -1105,14 +1326,16 @@ def read_as_path(self, typed: str | None) -> str: or `""` — what a person who meant `PRODUCT=PATH` and mistyped the product needs told, since nothing else in the run names it. Nothing when no pin is declared: the note has already said so. The key is - spelled as `_shown` spells the value (#610).""" + spelled as `_shown` spells the value (#610), and so is each declared + pin, which is the manifest's text (#712).""" if typed is None or self.reading.get(typed) is not None \ or "=" not in typed or not self.declared: return "" key = typed.partition("=")[0] what = f"`{_shown(key)}` is not" if key else "nothing before its `=` is" + declared = ", ".join(_shown(product) for product in self.declared) return (f"It was read as a PATH: {what} a declared neutral-product " - f"pin (declared: {', '.join(self.declared)}).") + f"pin (declared: {declared}).") def resolve_neutral_pin_source(root: Path, product: str, repository: str, @@ -1123,8 +1346,9 @@ def resolve_neutral_pin_source(root: Path, product: str, repository: str, Tried in order, the most explicit ask first: - 1. `--pin-source =` (case-insensitive on the name, which - must be a declared pin's for the value to be read this way, #295) + 1. `--pin-source =` (case-insensitive on the name as + `pin_product_key` folds it, #712, and it must be a declared pin's + for the value to be read this way, #295) 2. `SHAPE_PIN_SOURCE_` in the environment 3. a checkout sitting BESIDE this assembly root, named for the pinned repository (`opensoft/openGlass` -> `../openGlass`) — the ordinary @@ -1143,19 +1367,30 @@ def resolve_neutral_pin_source(root: Path, product: str, repository: str, Returns `(path, how, typed)`, `typed` being the `--pin-source` value as given when one of those answered, so the caller can mark it used (#295). `(None, None, None)` when nothing above answers; the caller's next step - is `gh api`, and after that, a SKIP. + is `gh api`, and after that, a SKIP. `how` is printed, so the product + and the sibling's path in it are spelled (#712). + + A CHECKOUT BESIDE THE ROOT THAT THIS USER CANNOT SEARCH IS THE SOURCE + (#712, after RV827's S1): something is there, so it answers, and + `_digest_from_source` names why it cannot, a finding. `exists()` raised + its `PermissionError` out of the run up to Python 3.13, and from 3.14 + would have passed it over for `gh api`. """ - keyed = sources.keyed.get(product.casefold()) + keyed = sources.keyed.get(pin_product_key(product)) if keyed is not None: typed, path = keyed - return path, f"--pin-source {product}=...", typed + return path, f"--pin-source {_shown(product)}=...", typed env_name = pin_source_env_name(product) env_value = None if env_shared else os.environ.get(env_name) if env_value: return Path(env_value), f"${env_name}", None sibling = root.parent / repo_basename(repository) - if (sibling / ".git").exists(): - return sibling, f"sibling checkout ({sibling})", None + try: + beside = _is_there(sibling / ".git") + except OSError: + beside = True + if beside: + return sibling, f"sibling checkout ({_shown(str(sibling))})", None if sources.bare is not None: typed, path = sources.bare return path, "--pin-source", typed @@ -1206,6 +1441,12 @@ def _check_neutral_pin_entry_and_load(root: Path, product, report: Report the entry itself is malformed — already reported, and there is no pin file name to go read — so the caller moves on to the next entry exactly as the original loop's `continue` did. + + THE PIN'S PATH IS SPELLED (#712). The entry was `repr`'d and the path + built from it was not, so `["\\x1b[31mRED"]` put an ESC on stderr inside + `contracts/[31mred-pin.yaml`, and in the remedy's `--pin`. The pin + is named relative to the root in every line, and refused by name where + it cannot be reached or read. """ if not isinstance(product, str) or not product: report.finding( @@ -1215,21 +1456,21 @@ def _check_neutral_pin_entry_and_load(root: Path, product, report: Report ) return None pin_path = root / "contracts" / f"{product.lower()}-pin.yaml" - if not pin_path.is_file(): + rel = _named(root, pin_path) + if not _is_file(root, pin_path): raise Refusal( "neutral-pin-missing", - f"project.yaml declares a pin on {product!r} but " - f"{pin_path.relative_to(root).as_posix()} does not exist", + f"project.yaml declares a pin on {product!r} but {rel} does not " + "exist", "Remediation: `scaffold-project.py --pin " - f"{product}@` writes that file. A declaration in " + f"{_shown(product)}@` writes that file. A declaration in " "project.yaml with no pin file beside it is a claim with no " "referent — either restore the file or remove the " "declaration.", ) - pin = load_yaml(pin_path) - rel = pin_path.relative_to(root).as_posix() + pin = _load(root, pin_path) if not isinstance(pin, dict): - raise Refusal("neutral-pin-unreadable", f"{pin_path}: not a mapping") + raise Refusal("neutral-pin-unreadable", f"{rel}: not a mapping") return pin, rel @@ -1307,11 +1548,13 @@ def _recompute_neutral_pin_digest(root: Path, product: str, repository: str, Split from `_check_neutral_product_pins` for #132. Returns `(actual, how)`, or `None` when nothing could answer — a finding or a skip is already reported in that case, exactly as the original loop's - `continue` left it. + `continue` left it. The product and the pin's `source_repository` are + spelled wherever they are printed (#712). """ env_shared = pin_product_key(product) in shared source, how, typed = resolve_neutral_pin_source(root, product, repository, sources, env_shared) + named = _shown(repository) if source is not None: if typed is not None: sources.used.add(typed) @@ -1321,11 +1564,11 @@ def _recompute_neutral_pin_digest(root: Path, product: str, repository: str, report.finding( "neutral-pin-source-unreadable", f"{rel}: {_shown(str(source))} ({how}) could not answer for " - f"{repository} @ {commit}: {exc.detail}" + f"{named} @ {commit}: {exc.detail}" + "".join(f"\n {line}" for line in ( sources.read_as_path(typed), _also_a_directory(typed, sources), - f"Remediation: point it at a clone of {repository} that " + f"Remediation: point it at a clone of {named} that " "has that commit (`git fetch` in it first), or drop it " "so the next source in the order answers.") if line), ) @@ -1337,13 +1580,13 @@ def _recompute_neutral_pin_digest(root: Path, product: str, repository: str, # bare `--pin-source`, which answers for every pin that has no more # specific answer: the keyed form is the one that says which pin it # is for (#275). - offered = (f"A --pin-source {product}= or" if env_shared else - f"A --pin-source, {pin_source_env_name(product)}, or") + offered = (f"A --pin-source {_shown(product)}= or" if env_shared + else f"A --pin-source, {pin_source_env_name(product)}, or") report.skip( - f"{product}: pin commit {commit[:12]} digest NOT " + f"{_shown(product)}: pin commit {commit[:12]} digest NOT " f"rechecked — no local checkout ({exc.code}: " f"{exc.detail}). {offered} a checkout at " - f"../{repo_basename(repository)} beside " + f"{_shown('../' + repo_basename(repository))} beside " "this assembly root would verify it offline.", ) return None @@ -1384,7 +1627,7 @@ def _check_one_neutral_product_pin(root: Path, product, report: Report, "descent from has moved, or the pin was hand-edited.", ) else: - report.note(f"{product}: pin commit {commit[:12]} digest " + report.note(f"{_shown(product)}: pin commit {commit[:12]} digest " f"recomputes ({how})") @@ -1457,7 +1700,8 @@ def _check_neutral_product_pins(root: Path, manifest: dict, report: Report, which `or []` read as an empty list, are the finding too. """ products = manifest.get("neutral_product_pins") - if products is not None and not isinstance(products, list): + not_a_list = products is not None and not isinstance(products, list) + if not_a_list: report.finding( "manifest-neutral-product-pins", f"neutral_product_pins is {products!r}, expected a list of " @@ -1469,10 +1713,11 @@ def _check_neutral_product_pins(root: Path, manifest: dict, report: Report, shared = frozenset(pin_source_env_shared(products)) for product in products: _check_one_neutral_product_pin(root, product, report, sources, shared) - _note_unused_pin_sources(sources, report) + _note_unused_pin_sources(sources, report, not_a_list) -def _note_unused_pin_sources(sources: PinSources, report: Report) -> None: +def _note_unused_pin_sources(sources: PinSources, report: Report, + not_a_list: bool = False) -> None: """A note for each `--pin-source` value no declared pin read (#295), naming it as typed and why: a later value replaced it, its pin was never recomputed, nothing is declared, or every pin had a more specific @@ -1480,7 +1725,13 @@ def _note_unused_pin_sources(sources: PinSources, report: Report) -> None: that is the mistyped key's only trace in the run. "As typed" is `_shown`'s (#610): a value that is not printable, and the - later value that replaced it, are named as `repr` spells them. + later value that replaced it, are named as `repr` spells them, and so is + the declared product a keyed value names (#712). + + A FIELD THAT IS NOT A LIST IS NAMED (#712): `not_a_list` says that + `neutral_product_pins` was `manifest-neutral-product-pins`'s finding, + so it declares nothing, and the note points at that finding, where it + said only that nothing is declared and left the reader to find why. """ for typed, product in sources.reading.items(): if typed in sources.used: @@ -1489,7 +1740,12 @@ def _note_unused_pin_sources(sources: PinSources, report: Report) -> None: if later is not None: why = f"the later `--pin-source {_shown(later)}` replaced it" elif product is not None: - why = f"{product}'s digest was not recomputed from any source" + why = (f"{_shown(product)}'s digest was not recomputed from any " + "source") + elif not sources.declared and not_a_list: + why = ("project.yaml's neutral_product_pins is not a list, so it " + "declares no neutral-product pin (see the " + "manifest-neutral-product-pins finding)") elif not sources.declared: why = "project.yaml declares no neutral-product pin" else: @@ -1501,26 +1757,59 @@ def _note_unused_pin_sources(sources: PinSources, report: Report) -> None: sources.read_as_path(typed))))) -def _load_legs_to_check(root: Path) -> tuple[dict, list[dict]]: +def _load_legs_to_check(root: Path, report: Report + ) -> tuple[dict, list[dict]]: """The manifest and its non-assembly legs, or a Refusal naming why there is nothing here to check. Split from `main` for #132. + + `project.yaml` is named relative to the root, as every file this + validator names is, where three refusals named its absolute path raw, + and it is refused by name where it cannot be reached or read (#712). + + A `legs:` THAT IS NOT A LIST IS A FINDING, NOT THAT REFUSAL (#714), in + `validate-manifest.py`'s words for it (`manifest-legs`), and no leg is + checked. Iterated as `manifest.get("legs") or []`, `5` or `true` ended + this validator in a `TypeError`, and a scalar `openXdox` or a mapping + was walked one character, or one key, at a time, found no leg, and was + refused `manifest-no-legs` with the lockstep remedy, which says to run + `git submodule update`: nothing to do with the cause. The shape pin and + the neutral-product pins are still checked. A refusal one of them makes + is printed alone, as every refusal here is, so the finding is not + printed then. Absent, null or `[]` is still that refusal. + + AN ENTRY THAT IS NOT A MAPPING IS `validate-manifest.py`'S `manifest-leg` + FINDING, in its words (#714), as the naming gate says it too, and the + refusal is chosen only where no entry was one: `legs: [5]` was refused + `manifest-no-legs` with that same remedy, where the other two gates + found the entry. """ manifest_path = root / "project.yaml" - if not manifest_path.is_file(): + if not _is_file(root, manifest_path): raise Refusal( "manifest-missing", - f"{manifest_path} does not exist, so there is no declaration of " + "project.yaml does not exist, so there is no declaration of " "which legs to check. A one-repository project has no legs and " "does not run this validator.", ) - manifest = load_yaml(manifest_path) + manifest = _load(root, manifest_path) if not isinstance(manifest, dict): - raise Refusal("manifest-unreadable", f"{manifest_path}: not a mapping") - legs = [leg for leg in (manifest.get("legs") or []) + raise Refusal("manifest-unreadable", "project.yaml: not a mapping") + declared = manifest.get("legs") + if declared is not None and not isinstance(declared, list): + report.finding( + "manifest-legs", + f"legs is {declared!r}, expected a list of `role:`, " + "`repository:` and `path:` mappings") + return manifest, [] + not_mappings = [leg for leg in (declared or []) + if not isinstance(leg, dict)] + for leg in not_mappings: + report.finding("manifest-leg", f"a leg is not a mapping: {leg!r}") + legs = [leg for leg in (declared or []) if isinstance(leg, dict) and leg.get("role") != "assembly"] - if not legs: + if not legs and not not_mappings: raise Refusal("manifest-no-legs", - f"{manifest_path}: no non-assembly legs declared") + "project.yaml: no non-assembly legs declared") return manifest, legs @@ -1529,7 +1818,7 @@ def _run_all_checks(root: Path, report: Report, """Every check `main` runs inside its one `try`, in order. Split from `main` for #132 so that `try` wraps a single call. """ - manifest, legs = _load_legs_to_check(root) + manifest, legs = _load_legs_to_check(root, report) refs = _workflow_refs(root) for leg in legs: _check_leg(root, leg, report, refs) diff --git a/scripts/validate-repository-naming.py b/scripts/validate-repository-naming.py index fcd5a1c..b24b29c 100755 --- a/scripts/validate-repository-naming.py +++ b/scripts/validate-repository-naming.py @@ -86,7 +86,9 @@ the topic agreed with the classification) 1 a FINDING: at least one name matches no family, or a leg's declared role disagrees with the form of its name, or (#443) `neutral_product_pins:` - or a leg's `naming.referent_chain:` is not a list + or a leg's `naming.referent_chain:` is not a list, or (#714) `legs:` + is missing, empty or not a list, or an entry of the first two is not + a name 2 a REFUSAL: the question could not be asked at all — the policy file is missing, unparsable, or not a naming policy, or a `--link-source` names no path @@ -138,6 +140,26 @@ def _shown(path) -> str: return text if text.isprintable() else repr(text) +def _not_a_name(entry) -> str | None: + """Why a `neutral_product_pins:` entry or a `naming.referent_chain:` + link is not a name, or None where it is one (#714). + + `validate-manifest.py`'s `_not_a_name`, word for word, so every gate + that reads the two fields says one sentence. A name is text + (`validate-pins.py`'s `neutral-pin-entry-malformed` asks the same), it + is not blank (a blank link was always no link), and every character in + it is printable (`str.isprintable`, `_shown`'s rule). `[5]` was the pin + `'5'`, and a link holding ESC was a name this gate classified with, + exit 0, and printed raw in `--explain`'s CHAIN line.""" + if not isinstance(entry, str): + return "it is not text" + if not entry.strip(): + return "it is blank" + if not entry.isprintable(): + return "it holds a character that is not printable" + return None + + def _library_takes_remedies() -> bool: """Does this root's `scripts/repo_shape.py` take `remedies=`? The skew rule `validate-manifest.py` states (#661): an older library has @@ -685,11 +707,16 @@ def _pins_from_project(data: dict) -> set[str]: ended this gate, the first command of `make validate`, in a `TypeError`; a scalar `openXdox` was read as the pins `X`, `d`, `e`, `n`, `o`, `p`, `x`, and a mapping as its keys, and the names classified, exit 0. + + AN ENTRY THAT IS NOT A NAME DECLARES NOTHING (#714), and the others are + still declared, as `validate-pins.py` checks the rest past an entry it + calls malformed. `_not_a_name` is the rule, and `_pins_field_findings` + names each such entry. """ declared = data.get("neutral_product_pins") if not isinstance(declared, list): return set() - return {str(pin) for pin in declared if pin} + return {pin for pin in declared if _not_a_name(pin) is None} def _pins_field_findings(data: dict) -> list[str]: @@ -698,12 +725,22 @@ def _pins_field_findings(data: dict) -> list[str]: names the field where it used to end in a traceback, and every validator that reads the field says the same sentence. Absent or null declares no pin and is no finding; anything else that is not a list is one, `0`, - `false` and `''` included, as that validator says.""" + `false` and `''` included, as that validator says. In a list, each + entry that is not a name is one too, with why (#714).""" declared = data.get("neutral_product_pins") - if declared is None or isinstance(declared, list): + if declared is None: return [] - return [f"manifest-neutral-product-pins: neutral_product_pins is " - f"{declared!r}, expected a list of neutral product names"] + if not isinstance(declared, list): + return [f"manifest-neutral-product-pins: neutral_product_pins is " + f"{declared!r}, expected a list of neutral product names"] + findings = [] + for pin in declared: + why = _not_a_name(pin) + if why is not None: + findings.append( + f"manifest-neutral-product-pins: neutral_product_pins entry " + f"{pin!r} is not a neutral product name: {why}") + return findings def _chain_from_leg(leg: dict) -> tuple: @@ -718,32 +755,54 @@ def _chain_from_leg(leg: dict) -> tuple: no chain, which `_chain_field_findings` reports. This copy had the string rule and not the list rule, so `5`, `true` or `1.5` ended the gate in a `TypeError` and a mapping was read as a chain of its keys. + + A CHAIN WITH A LINK THAT IS NOT A NAME RECORDS NO CHAIN (#714), as one + that is not a list records none: `_not_a_name` is the rule, and + `_chain_field_findings` names the link. `[5]` was the link `'5'`, and + `"\\x1b[31mRED"` a link this gate classified with, exit 0. Dropping the + one link would ask about a chain nobody recorded. A blank string is the + empty record it always was. """ naming = leg.get("naming") if not isinstance(naming, dict): return () recorded = naming.get(CHAIN_RECORD_FIELD) if isinstance(recorded, str): - recorded = [recorded] - if not isinstance(recorded, list): + recorded = [recorded] if recorded.strip() else [] + if not isinstance(recorded, list) or any( + _not_a_name(link) is not None for link in recorded): return () - return tuple(str(link) for link in recorded if str(link).strip()) + return tuple(recorded) def _chain_field_findings(leg: dict) -> list[str]: """`validate-manifest.py`'s finding for a leg's `naming.referent_chain:` that is neither a string nor a list, in its words (#443): the leg named by its declared `role:`, as that validator names it. Absent or null - records no chain, and a string is one link, so neither is a finding.""" + records no chain, and a string is one link, so neither is a finding. + Each link that is not a name is one, with why (#714); a blank string is + no link and no finding.""" naming = leg.get("naming") if not isinstance(naming, dict): return [] recorded = naming.get(CHAIN_RECORD_FIELD) - if recorded is None or isinstance(recorded, (str, list)): + if recorded is None: return [] - return [f"naming-referent-chain: leg {leg.get('role')!r}: " - f"{CHAIN_RECORD_FIELD} is {recorded!r}, expected a list of " - "neutral-product names"] + if isinstance(recorded, str): + recorded = [recorded] if recorded.strip() else [] + if not isinstance(recorded, list): + return [f"naming-referent-chain: leg {leg.get('role')!r}: " + f"{CHAIN_RECORD_FIELD} is {recorded!r}, expected a list of " + "neutral-product names"] + findings = [] + for link in recorded: + why = _not_a_name(link) + if why is not None: + findings.append( + f"naming-referent-chain: leg {leg.get('role')!r}: " + f"{CHAIN_RECORD_FIELD} link {link!r} is not a neutral-product " + f"name: {why}") + return findings def _leg_target(policy: NamingPolicy, path: Path, leg, root: Path, @@ -756,9 +815,20 @@ def _leg_target(policy: NamingPolicy, path: Path, leg, root: Path, walks every leg in the manifest. `(None, [finding])` is a leg with no `repository:` to classify at all — the same case the loop's own `finding` list has always carried, now returned instead of appended in place. + + AN ENTRY THAT IS NOT A MAPPING IS `validate-manifest.py`'S `manifest-leg` + FINDING, in its words (#714), as `validate-pins.py` says it too: `5` + was "a leg has no `repository:`", which is not what is wrong with it. A + mapping with no `repository:` still says that, and names the manifest + relative to the project root, spelled by `_shown`: the `--project` path + was printed raw, as given, and the doctor gives it as its root joined + to `project.yaml`. """ - if not isinstance(leg, dict) or "repository" not in leg: - return None, [f"{path}: a leg has no `repository:`"] + if not isinstance(leg, dict): + return None, [f"manifest-leg: a leg is not a mapping: {leg!r}"] + if "repository" not in leg: + manifest = _shown(path.resolve().relative_to(root).as_posix()) + return None, [f"{manifest}: a leg has no `repository:`"] name = repo_basename(str(leg["repository"])) declared = leg.get("role") declared = str(declared) if declared else None @@ -806,11 +876,44 @@ def _project_manifest(path: Path) -> dict: return data +def _project_legs(data: dict) -> list: + """The manifest's `legs:` where it is a list, and `[]` where it is not + (#714): a value that is not a list names no leg, as + `validate-manifest.py` reads it, and `_legs_field_findings` says so. + Iterated as `data.get("legs") or []`, `5` or `true` ended the gate in a + `TypeError` before any name was classified, and a scalar `openXdox` or + a mapping was walked one character, or one key, at a time: no leg was a + target, and the gate read names from stdin instead, which waited on a + pipe left open.""" + legs = data.get("legs") + return legs if isinstance(legs, list) else [] + + +def _legs_field_findings(data: dict) -> list[str]: + """`validate-manifest.py`'s finding for a `legs:` that is not a list, or + that is absent, null or `[]`, in its words (#714); `validate-pins.py` + says the first too. + + A MANIFEST THAT NAMES NO LEG IS THAT FINDING, NOT `naming-no-target` + (lane 4's ruling on #714). The refusal said "Pass names, `--project + project.yaml`, or names on stdin" to a run that had passed + `--project project.yaml`, and under `--project` this gate no longer + reads stdin, so two of its three offers were no way out. The manifest + is what is wrong, and `validate-manifest.py` says so, exit 1.""" + legs = data.get("legs") + if isinstance(legs, list) and legs: + return [] + if legs is None or isinstance(legs, list): + return ["manifest-legs: legs is missing or empty"] + return [f"manifest-legs: legs is {legs!r}, expected a list of `role:`, " + "`repository:` and `path:` mappings"] + + def _project_leg_chains(data: dict) -> list[tuple[str, tuple]]: """`(name, the chain it records)` for each leg `_leg_target` makes a target of, and no other.""" return [(repo_basename(str(leg["repository"])), _chain_from_leg(leg)) - for leg in data.get("legs") or [] + for leg in _project_legs(data) if isinstance(leg, dict) and "repository" in leg] @@ -852,9 +955,11 @@ def _targets_from_project(policy: NamingPolicy, path: Path, data: dict, root = path.resolve().parent targets: list[Target] = [] # Pins that are not a list are said before any leg's finding, and every - # leg is classified as declaring none (#443). + # leg is classified as declaring none (#443). Legs that are not a list + # are said next, and name no leg (#714). findings: list[str] = _pins_field_findings(data) - for leg in data.get("legs") or []: + findings.extend(_legs_field_findings(data)) + for leg in _project_legs(data): target, leg_findings = _leg_target(policy, path, leg, root, pins, sources) if target is not None: @@ -872,11 +977,41 @@ def _targets_from_stdin(role: str | None, pins: set, chain: tuple, Split from `main` for #136: reading the stdin fallback is its own question, asked only once `main` has decided no other source answered. + + A BYTE THAT IS NOT UTF-8 IS READ AS THE NAME ARGV READS (#714). A + `strict` stdin, which `PYTHONIOENCODING=utf-8` gives (the doctor sets it + for its children), ended the run in a `UnicodeDecodeError` on + `Atl\\xffas`; read with `surrogateescape`, as `os.fsdecode` reads an + argument, it is `Atl\\udcffas` and reaches `naming-unclassified`. Any + other handler, one somebody chose, is left alone. """ + if getattr(sys.stdin, "errors", None) == "strict": + try: + sys.stdin.reconfigure(errors="surrogateescape") + except (AttributeError, ValueError, OSError): + pass return [Target(repo_basename(line.strip()), role, pins, chain, here) for line in sys.stdin if line.strip()] +def _escape_what_stdout_cannot_encode() -> None: + """A character stdout cannot encode is printed escaped (#714), as + `validate-pins.py`'s `_emit_report` prints it (#295): a `strict` or + `surrogateescape` stdout, the two that raise, becomes + `backslashreplace`, the handler stderr already has, and any other + handler is left alone. A name that is not UTF-8 (`Atl\\udcffas`, from + the byte `\\xff`) ended `--explain` and the plain listing in a + `UnicodeEncodeError` under `PYTHONIOENCODING=utf-8`, which the doctor + gives its children, where `--quiet` said `naming-unclassified`. This + script runs on its own too, so the doctor setting another handler for + its child would not be enough.""" + if getattr(sys.stdout, "errors", None) in ("strict", "surrogateescape"): + try: + sys.stdout.reconfigure(errors="backslashreplace") + except (AttributeError, ValueError, OSError): + pass + + def _print_classification(name: str, found: Classification | None) -> None: """The one plain-listing line `main` prints for a name, un-`--explain`d. @@ -1045,6 +1180,7 @@ def main(argv: list[str] | None = None) -> int: "of them won, and what the others are recorded as") parser.add_argument("--quiet", action="store_true") args = parser.parse_args(argv) + _escape_what_stdout_cannot_encode() try: policy = NamingPolicy.load(args.policy) @@ -1080,9 +1216,18 @@ def main(argv: list[str] | None = None) -> int: extra, findings = _targets_from_project(policy, args.project, data, sources) targets.extend(extra) - if not targets and not sys.stdin.isatty(): + # UNDER `--project`, STDIN IS NEVER READ (#714). The manifest is the + # question; a manifest that names no leg, or whose `legs:` is not a + # list, read names from a stdin that is not a terminal, and a pipe left + # open (a parent that holds it) made the gate wait forever. + # A finding with no name to classify is still said, exit 1, and under + # `--project` a run with no target always has one (`legs:` missing, + # empty or not a list, or a leg with no `repository:`): the + # `no names given` refusal is for a run without `--project` that found + # nothing to ask. + if not targets and data is None and not sys.stdin.isatty(): targets = _targets_from_stdin(args.role, cli_pins, cli_chain, here) - if not targets: + if not targets and not findings: print("REFUSED naming-no-target: no names given. Pass names, " "`--project project.yaml`, or names on stdin.", file=sys.stderr) return 2