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