diff --git a/adopt-project.py b/adopt-project.py index 169ad39f..556dbc53 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -822,6 +822,16 @@ def _pin_findings(plan: "Plan", tree: list, # `validate-pins.py` finds a source it reads that cannot answer. `check` and # `execute` ask this through `_entry_findings`, the one list. # +# NOR IS SUCH A PIN'S COMMIT ASKED, and the NOTE says so (#912). What `plan` +# asks of a commit (#651), whether it is one or an annotated tag's id, `check` +# asks of the clone it recomputes the digest in (`commit_in_clone`). Where +# that clone is gone or partial, or `gh api` read the digest, it was asked +# of nothing, and the NOTE said only that the digest was not rechecked, so a +# tag's id edited into the plan passed `check` and `execute` wrote it under +# `revision_kind: commit`. The forge's `gh api graphql`, which `plan` asks +# where it has no clone, is not asked here, for the reason above, and a +# partial clone's `git` would fetch an id it lacks over the network. +# # A DIRECTORY THAT IS NO CLONE'S TOP LEVEL is asked about FIRST (#800), before # a partial clone's NOTE: a folder inside a clone, or a directory that is no # clone at all, is `plan-pin-source-not-a-clone`, as `plan` refuses it @@ -968,7 +978,7 @@ def _pin_digest_findings(plan: "Plan") -> tuple[list[str], list[str]]: a digest the clone `plan` read it from recomputes otherwise at the plan's commit, or cannot recompute there; otherwise one NOTE for `check` to print, that the digest recomputes there, or that it was not - rechecked here and why.""" + rechecked here, nor the commit asked what it names (#912), and why.""" findings: list[str] = [] notes: list[str] = [] for said, _, values in _read_pins(plan)[0]: @@ -978,9 +988,12 @@ def _pin_digest_findings(plan: "Plan") -> tuple[list[str], list[str]]: if not (found or why): found = _recomputed_pin_finding(said, values) if why: - notes.append(f"NOTE {said}: digest NOT rechecked here: {why}. The " - "root's own `validate-pins.py` recomputes it " - "wherever it can read the product.") + notes.append(f"NOTE {said}: digest NOT rechecked here, nor its " + f"`commit:` asked whether it names a commit: {why}. " + "The root's own `validate-pins.py` recomputes the " + "digest wherever it can read the product; `plan` " + "asks what the commit names where it reads the " + "digest.") elif found: findings.append(found) else: @@ -15219,6 +15232,12 @@ def _neutral_product_pins(pins) -> str: from `plan` as well as from a plan edited by hand: `plan` records it quoted, and `check` passes it. The list is a block placeholder, so `render` writes it as it stands, and nothing else quotes it. + + AND A NAME A STANDARD YAML READER TAKES FOR NO TEXT IS DOUBLE-QUOTED + HERE, though `y` writes it bare in the plan (#912): `yes`, `on`, `.5` + and `.inf` among them, which the project's reader reads back as typed + and YAML 1.1 or 1.2 reads as a bool or a float. The plan is read by + this tool alone; the manifest by any tool. """ if not pins: return "[]" diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 1bcdec3a..4448cd07 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -1008,6 +1008,13 @@ def peeled_target(submodule: Path, repository: str, target: str) -> str: a question asked before it finds nothing and lets the tag's id through to the reachability check. An id the leg does not hold at all is not asked here: it is the reachability verdict's, `bump-leg-commit-not-on-remote`. + + NOR IS A TAG THE LEG HOLDS WITHOUT WHAT IT NAMES (#912): its peel cannot + be read there (git: "missing object referenced by" the tag), so the leg + cannot say what the tag names, and its id goes on, as typed, to the + same verdict as an id the leg does not hold. It was refused as "an + annotated tag that names no commit", which no tag of a commit is, and + which sent the person looking at a tag that was never wrong. """ kind = object_kind(submodule, target) if kind is None or kind == "commit": @@ -1021,6 +1028,8 @@ def peeled_target(submodule: Path, repository: str, target: str) -> str: encoding="utf-8", errors="replace", check=False) peeled = proc.stdout.strip().lower() if proc.returncode == 0 else "" names = object_kind(submodule, peeled) if peeled else None + if names is None: + return target if names == "commit": return peeled if kind != "tag": diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 81fd2ad6..7b19de1a 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -520,12 +520,35 @@ def neutral_pin_path(product: str) -> str: #: `Icon\n` bare, over two lines of the plan (#199). PLAIN_SCALAR_RE = re.compile(r"^[A-Za-z_.][A-Za-z0-9_./@:+-]*\Z") +#: WHAT A STANDARD YAML READER TAKES FOR NO TEXT (#912): of the plain scalars +#: `PLAIN_SCALAR_RE` takes, which begin with a letter, `_` or `.`, the ones +#: YAML 1.1 or YAML 1.2's core schema resolves to a bool, a null or a float. +#: The bools are YAML 1.1's (yaml.org/type/bool.html): `y`, `n`, `yes`, `no`, +#: `on` and `off`, which 1.2 dropped, and `true` and `false`, which both +#: keep, each in its lower, capitalised and upper case. The null is `null`, +#: in both and in the same three cases. The floats begin with a point: +#: `.inf` and `.nan` in their three cases, and a point before digits, as +#: YAML 1.1's pattern is written (`\.[0-9.]*`, so `.` and `..` too), and as +#: its own example `685.230_15e+03` and its readers read it, with `_` among +#: the digits (PyYAML takes `.5_` for 0.5, ruamel.yaml `._` for a float), and +#: YAML 1.2's unsigned exponent, `.5e3`. Nothing else either resolves to no +#: text is a plain scalar `PLAIN_SCALAR_RE` takes: `~` and the empty null, +#: YAML 1.1's `<<` and `=`, and an int, a timestamp or a float that begins +#: with a digit or a sign. +YAML_NOT_TEXT_RE = re.compile( + r"^(?:[yYnN]|yes|Yes|YES|no|No|NO|on|On|ON|off|Off|OFF" + r"|true|True|TRUE|false|False|FALSE|null|Null|NULL" + r"|\.(?:inf|Inf|INF|nan|NaN|NAN)" + r"|\.[0-9.]*(?:[eE][-+][0-9]+)?" + r"|\.[0-9_]+(?:[eE][-+]?[0-9]+)?)\Z") + def neutral_pin_entry(product: str) -> str: """One `neutral_product_pins:` entry, as `scaffold-project.py` and `adopt-project.py execute` both write it: `product` bare where it is a plain scalar (`PLAIN_SCALAR_RE`) that the project's reader reads back as - `product`, and `double_quoted` (#284) where it is not. + `product` and that YAML 1.1 and 1.2 read as text (`YAML_NOT_TEXT_RE`), + and `double_quoted` (#284) where it is not. ONE SPELLING, WHICHEVER TOOL MADE THE ROOT (#924). The scaffold wrote a product bare wherever the reader read it back, and adopt wrote it as `y` @@ -536,9 +559,17 @@ def neutral_pin_entry(product: str) -> str: scaffold's `0x1F` for 31 and its `1_000` for 1000. Both tools call this one function now, and it is adopt's rule: a name that begins with a digit or `-` is double-quoted, as `plan` quotes it in `product:`, and - `openGlass`, `007` and `yes` are written as both tools wrote them. A - bare `yes`, `on`, `.5` or `.inf` stays bare: the project's reader reads - it back as typed, though PyYAML does not (#457's L2-14, #912). + `openGlass` and `007` are written as both tools wrote them. + + A NAME ONLY A STANDARD READER MISREADS IS DOUBLE-QUOTED TOO (#912, + #457's L2-14). `yes`, `no`, `on`, `off`, `.5`, `.inf` and `.nan` were + written bare, because the project's reader reads each back as typed, + and PyYAML's `safe_load` read them as True, False, 0.5, inf and nan: no + gate of the root failed, but any other tool reading the manifest read + a pin on a bool or a number. So a name YAML 1.1 or YAML 1.2's core + schema resolves to a bool, a null or a float (`YAML_NOT_TEXT_RE`) is + double-quoted whatever the project's reader makes of it, and every + other name is written as it was: `openGlass` byte for byte. THE READER IS ASKED, as `family.py` asks it of a member's id (#259), rather than a list of the words it takes for something else: in the two @@ -547,16 +578,17 @@ def neutral_pin_entry(product: str) -> str: ` - - a` is a list of its own, where `id: - a` is the text (#715; `id: -` is refused since #393). A spelling the reader refuses is read back as nothing, so it is double-quoted too. A name it reads back bare, - `openGlass` and every other plain name, is the line it always was; - `007`, `2026`, `1.5`, `true` and `null` are double-quoted, and a - double-quoted name always reads back. + `openGlass` and every other plain name a standard reader reads as text + too, is the line it always was; `007`, `2026`, `1.5`, `true` and `null` + are double-quoted, and a double-quoted name always reads back. """ - if PLAIN_SCALAR_RE.match(product) and all( - _reads_back(written, read) for written, read in ( + if (PLAIN_SCALAR_RE.match(product) + and not YAML_NOT_TEXT_RE.match(product) + and all(_reads_back(written, read) for written, read in ( (f"key: {product}", {"key": product}), (f"- {product}", [product]), (f"neutral_product_pins:\n - {product}", - {"neutral_product_pins": [product]}))): + {"neutral_product_pins": [product]})))): return product return double_quoted(product) @@ -3893,7 +3925,15 @@ def commit_in_clone(clone: Path, oid: str) -> tuple[str, str]: names no commit, `why` as `not_a_commit` says it. And `(oid, "")` where the clone cannot say, no object with that id or no clone there, which the digest's reader then names in its own words - (`pin-source-unreadable`).""" + (`pin-source-unreadable`). + + A TAG WHOSE TARGET THE CLONE LACKS IS ONE IT CANNOT SAY (#912). Its peel + cannot be read there (git: "missing object referenced by" the tag), so + nothing in the clone says what the tag names, and it is `(oid, "")`: + the digest's reader then finds no tree at the id and says to point + `--pin-source` at a clone that has the commit, `git fetch --all` first, + as it said before #651. It was refused as "an annotated tag that names + no commit", which no tag of a commit is.""" kind = _git_answer(clone, "cat-file", "-t", oid) if kind in (None, COMMIT_KIND): return oid, "" @@ -3903,6 +3943,8 @@ def commit_in_clone(clone: Path, oid: str) -> tuple[str, str]: f"{oid}^{{}}") or "").lower() names = (_git_answer(clone, "cat-file", "-t", peeled) if peeled else None) + if names is None: + return oid, "" if names == COMMIT_KIND: return peeled, "" return oid, not_a_commit(kind, names) diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index c68675c8..86c12462 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -523,8 +523,12 @@ def test_a_pinned_product_passes_the_adopted_roots_own_gate(adopted_as_text): NUMBERS = ("007", "2026", "1.5", "-1", "-0.5", "20261011") KEYWORDS = ("true", "True", "TRUE", "false", "False", "FALSE", "null", "Null", "NULL") -PLAIN = ("openGlass", "open-glass", "open_glass", "openGlass2", ".5", "x.y", - "_x", "Openglass") +PLAIN = ("openGlass", "open-glass", "open_glass", "openGlass2", "x.y", "_x", + "Openglass") +#: Text to the reader bare, and quoted all the same (#912): YAML 1.1 or +#: 1.2's core schema reads each as a float or a bool, as PyYAML's +#: `safe_load` reads the manifest. +STOCK_MISREAD = (".5", ".inf", "yes", "off") #: Text to the reader bare, and quoted all the same, as `plan` quotes them in #: `product:`: `y` writes bare only a name that begins with a letter, `_` or #: `.`, and a YAML reader other than the project's takes `0x1F` and `1e3` @@ -548,6 +552,8 @@ def entries_read_back(entries: str) -> object: *(pytest.param(name, id=f"keyword-{name}") for name in KEYWORDS), *(pytest.param(name, id=f"plain-{name}") for name in PLAIN), *(pytest.param(name, id=f"quoted-by-y-{name}") for name in QUOTED_BY_Y), + *(pytest.param(name, id=f"stock-misread-{name}") + for name in STOCK_MISREAD), ]) def test_each_entry_reads_back_as_the_product(adopter, product): """Every product reads back as its text. A name `y` writes bare and the @@ -557,7 +563,9 @@ def test_each_entry_reads_back_as_the_product(adopter, product): reader takes for a bool or None (#181). It reaches `execute` only from a plan edited by hand, because `plan` writes `product: true` bare and `check` then refuses it, but the manifest reads back as the plan does, - edited or not.""" + edited or not. A name `y` writes bare that YAML 1.1 or 1.2 reads as no + text, `.5` or `yes`, is double-quoted in the manifest though not in the + plan (#912).""" entries = adopter._neutral_product_pins({product}) assert entries_read_back(entries) == [product] if product in PLAIN: @@ -569,7 +577,7 @@ def test_each_entry_reads_back_as_the_product(adopter, product): def test_every_entry_together_reads_back_in_order(adopter): """All of them in one list, as a plan with many pins writes it: one line each, sorted, and each read back as its text.""" - every = {*NUMBERS, *KEYWORDS, *PLAIN, *QUOTED_BY_Y} + every = {*NUMBERS, *KEYWORDS, *PLAIN, *QUOTED_BY_Y, *STOCK_MISREAD} entries = adopter._neutral_product_pins(every) assert len(entries.splitlines()) == len(every) + 1 assert entries_read_back(entries) == sorted(every) @@ -1406,10 +1414,13 @@ def test_a_plan_whose_pin_execute_cannot_write_is_found_and_refused( PIN_SOURCE_UNREADABLE = "plan-pin-source-unreadable" #: How `check` names the one pin these plans hold. ENTRY = "`pins:` entry 1 ('openGlass')" -#: Where every NOTE about a pin `check` did not recheck says it is. -NOT_RECHECKED = f"NOTE {ENTRY}: digest NOT rechecked here: " -GATE_RECOMPUTES = ("The root's own `validate-pins.py` recomputes it wherever " - "it can read the product.") +#: Where every NOTE about a pin `check` did not recheck says it is, and +#: that it did not ask what its commit names either (#912). +NOT_RECHECKED = (f"NOTE {ENTRY}: digest NOT rechecked here, nor its " + "`commit:` asked whether it names a commit: ") +GATE_RECOMPUTES = ("The root's own `validate-pins.py` recomputes the digest " + "wherever it can read the product; `plan` asks what the " + "commit names where it reads the digest.") #: A commit the stand-in does not hold. NOT_IN_THE_CLONE = "c0ffee" * 6 + "c0ff" #: How `plan-pin-source-unreadable` says both causes and each way out. diff --git a/tests/test_bump_leg_dry_run_truth.py b/tests/test_bump_leg_dry_run_truth.py index 2f4184c2..ff553421 100644 --- a/tests/test_bump_leg_dry_run_truth.py +++ b/tests/test_bump_leg_dry_run_truth.py @@ -30,7 +30,9 @@ `--local-remote-dir` bare repository and from the leg's own `origin` alike (both are run below). A tag whose commit is on no branch is NOT brought, and neither is its commit: it is `bump-leg-commit-not-on-remote` in both runs, and -one test holds that. The peel is therefore asked AFTER the fetch. +one test holds that. The peel is therefore asked AFTER the fetch. A tag the +leg holds WITHOUT its commit cannot be peeled there, and gets the same verdict +(#912), not `bump-leg-target-not-a-commit`; one test holds that too. Every case here runs the dry run AND the real run over the same root and asserts that they give one answer. The leg's remote is a PRIVATE bare copy of @@ -43,6 +45,7 @@ from __future__ import annotations import re +import subprocess import sys from pathlib import Path @@ -393,6 +396,57 @@ def test_a_tag_the_fetch_never_brings_is_not_on_the_remote_in_both_runs( assert "no branch containing" in stderr +@pytest.fixture(scope="module") +def unpushed_tag(leg_remote, tmp_path_factory) -> dict: + """An annotated tag of a commit pushed NOWHERE, and the tag's own bytes: + a tag the remote does not advertise, so no fetch brings it or its + commit. (A tag the remote does advertise is fetched with its commit + once its object is in the leg: git 2.43 follows a tag whose object it + holds, measured for #912.)""" + work = private_work(leg_remote, tmp_path_factory, "unpushed-tag") + git("checkout", "-q", "--detach", cwd=work) + (work / "UNPUSHED.md").write_text("# pushed nowhere\n") + git("add", "--", "UNPUSHED.md", cwd=work) + git("commit", "-qm", "A commit pushed nowhere", cwd=work) + commit = git("rev-parse", "HEAD", cwd=work).stdout.strip() + tag = tagged(work, "unpushed-w912", commit) + raw = subprocess.run(["git", "cat-file", "tag", tag], cwd=work, + capture_output=True, check=True).stdout + return {"commit": commit, "tag": tag, "raw": raw} + + +@pytest.mark.parametrize("via", VIA) +def test_a_tag_the_leg_holds_without_its_commit_is_not_on_the_remote( + project, leg_remote, unpushed_tag, via): + """#912. The tag object is in the leg's own store and the commit it names + is not, so its peel cannot be read there (git: "missing object + referenced by" the tag). On main 6c861635 `peeled_target` refused it + `bump-leg-target-not-a-commit`, "an annotated tag that names no + commit", which no tag of a commit is. The leg cannot say what such a tag + names, so its id goes on as typed to the reachability verdict, as an id + the leg does not hold does: `bump-leg-commit-not-on-remote`, from both + runs, with nothing written.""" + tag, commit = unpushed_tag["tag"], unpushed_tag["commit"] + made = subprocess.run(["git", "hash-object", "-t", "tag", "-w", + "--stdin"], cwd=project / "spec", + input=unpushed_tag["raw"], capture_output=True, + check=True) + assert made.stdout.decode().strip() == tag + assert holds(project, tag), "premise: the leg holds the tag" + assert not holds(project, commit), "and not its commit" + branched(project) + run = runner(project, leg_remote, via) + before = snapshot(project) + + stderr = assert_one_refusal(project, run, tag, + "bump-leg-commit-not-on-remote", before) + + assert f"has no branch containing {tag}" in stderr, stderr + assert "bump-leg-target-not-a-commit" not in stderr + assert "annotated tag" not in stderr + assert not holds(project, commit), "the fetch did not bring it" + + # --- the branch a tag's id names, on the tracking branch -------------------- #: The branch a `git switch -c` line creates, wherever the line sits. diff --git a/tests/test_neutral_pin_entry_one_spelling.py b/tests/test_neutral_pin_entry_one_spelling.py index f1fefef8..bd82cacd 100644 --- a/tests/test_neutral_pin_entry_one_spelling.py +++ b/tests/test_neutral_pin_entry_one_spelling.py @@ -24,6 +24,15 @@ the manifest template's own line, and the lines are compared with each other and with adopt's spelling. No network, no git and no GitHub: the two tools are loaded as modules, and nothing is run. + +#912 (#457's L2-14): the one spelling still wrote `yes`, `no`, `on`, `off`, +`.5`, `.inf` and `.nan` bare, because the project's reader reads each back +as typed, and PyYAML's `safe_load` read them as True, False, 0.5, inf and +nan. Measured on main 6c861635 over every name `--pin` admits of up to three +characters: 132 of those written bare PyYAML read as no text, and 132 +ruamel.yaml did (YAML 1.2). A name YAML 1.1 or 1.2's core schema resolves to +a bool, a null or a float is double-quoted now, whatever the project's +reader makes of it, and every other is written as it was. """ from __future__ import annotations @@ -38,6 +47,7 @@ from conftest import ADOPT, REPO, SCAFFOLD sys.path.insert(0, str(REPO / "scripts")) +import shape_materialize # noqa: E402 from repo_shape import YamlError, double_quoted, parse_yaml # noqa: E402 from shape_materialize import neutral_pin_entry, render # noqa: E402 @@ -45,6 +55,10 @@ import yaml as pyyaml # the oracle only: nothing shipped imports it except ImportError: # pragma: no cover - depends on the machine pyyaml = None +try: # a YAML 1.2 reader, and a 1.1 one: the oracle only, as PyYAML is + from ruamel.yaml import YAML as RuamelYAML +except ImportError: # pragma: no cover - depends on the machine + RuamelYAML = None NEEDS_PYYAML = pytest.mark.skipif( pyyaml is None, @@ -53,7 +67,8 @@ "tests beside this one") #: The issue's names, each with the line both tools now write for it: -#: adopt's spelling, double-quoted where `y` quotes the plan's `product:`. +#: adopt's spelling, double-quoted where `y` quotes the plan's `product:`, +#: and where a standard YAML reader takes the name for no text (#912). SPELLED = { "0x1F": '"0x1F"', "1_000": '"1_000"', @@ -62,8 +77,26 @@ "-x": '"-x"', "openGlass": "openGlass", "007": '"007"', - "yes": "yes", + "yes": '"yes"', } +#: Every word YAML 1.1 (yaml.org/type/bool.html, null.html) or YAML 1.2's +#: core schema resolves to a bool or a null, as each spec lists them. +YAML_WORDS = ("y", "Y", "yes", "Yes", "YES", "n", "N", "no", "No", "NO", + "true", "True", "TRUE", "false", "False", "FALSE", + "on", "On", "ON", "off", "Off", "OFF", "null", "Null", "NULL") +#: Floats `--pin` admits, so beginning with a point: YAML 1.1's `.inf` and +#: `.nan`; its pattern as written (`\.[0-9.]*`, `.`, `..` and `.5.5` +#: among it, with a signed exponent); and as its example `685.230_15e+03`, +#: PyYAML and ruamel.yaml read it, with `_` among the digits, and YAML 1.2's +#: unsigned exponent. +YAML_FLOATS = (".inf", ".Inf", ".INF", ".nan", ".NaN", ".NAN", + ".", "..", ".5.5", ".e+5", ".5", ".05", ".5e+3", ".5E-3", + "._", ".5_", "._5", ".5_5", ".5e3", ".5E3") +#: Names that only look like one of those, which every reader reads as +#: text: each is written bare, as on main. +LOOK_ALIKES = ("openGlass", "openYes", "yEs", "nO", "oN", "Nan", "inf", "_", + "y_", "no_", "NULL_", "True1", ".x", ".5x", "x.5", ".e5", + "._.", ".nan_", ".infinity", "..x") #: Text `y`'s rule is asked of: every text of one or two characters from #: the characters YAML gives a meaning to, beside letters and digits. YAML_ALPHABET = "aZ09_.-:@/+ #'\"&*!?[{%|>," @@ -144,26 +177,112 @@ def adopts_old_entry(adopter, text: str) -> str: return double_quoted(text) +#: The names #912 moved among those: bare in adopt's own spelling, and +#: double-quoted now, because a standard YAML reader takes each for no text. +MOVED_BY_912 = (".", "..", ".0", ".9", "._", "yes") + + def test_an_entry_is_bare_exactly_where_y_writes_the_plan_bare(adopter): """The shared function is adopt's rule, moved: for every short text YAML gives a meaning to, and the issue's names, it writes what adopt's own function wrote, which is `y`'s spelling of the plan's `product:` - wherever the entry reads back. A change to `y`'s rule that left the - entry's behind, or the other way round, fails here.""" + wherever the entry reads back, but for a name a standard YAML reader + takes for no text, which #912 double-quotes; and those are the only + ones that differ. A change to `y`'s rule that left the entry's behind, + or the other way round, fails here.""" differ = {text: (neutral_pin_entry(text), adopts_old_entry(adopter, text)) for text in (*TEXTS, *SPELLED) if neutral_pin_entry(text) != adopts_old_entry(adopter, text)} - assert differ == {} + assert differ == {text: (double_quoted(text), text) + for text in MOVED_BY_912} @NEEDS_PYYAML -def test_a_stock_reader_reads_a_name_that_opens_with_a_digit_as_text( - scaffolder): +def test_a_stock_reader_reads_the_issues_names_as_text(scaffolder): """The point of #924: PyYAML reads the scaffold's `0x1F` and `1_000` as - the text typed, as it reads adopt's. A bare `yes` it still reads as - True, from both tools alike: the project's own reader reads it back, - and whether to quote it is #457's L2-14, on #912.""" - names = [name for name in sorted(SPELLED) if not name[0].isalpha()] + the text typed, as it reads adopt's; and since #912 the bare `yes` it + read as True from both tools alike, the project's own reader reading it + back, is double-quoted, and read as `yes`.""" + names = sorted(SPELLED) entries = scaffolder._neutral_product_pins(names) assert pyyaml.safe_load(rendered(entries))["neutral_product_pins"] \ == names + + +# --- a name a standard YAML reader takes for no text (#912) ------------------ + +@pytest.mark.parametrize("name", [*YAML_WORDS, *YAML_FLOATS]) +def test_a_name_yaml_takes_for_no_text_is_double_quoted(name): + """Each is written double-quoted, and the project's own reader reads it + back as the name, through the manifest template's own line.""" + entry = neutral_pin_entry(name) + assert entry == double_quoted(name) + assert parse_yaml(rendered(f"\n - {entry}"))["neutral_product_pins"] \ + == [name] + + +@pytest.mark.parametrize("name", [*YAML_WORDS, *YAML_FLOATS]) +def test_the_rule_quotes_it_whatever_the_projects_reader_reads(monkeypatch, + name): + """The rule is the YAML specs', not the project's reader's: with that + reader made to read every spelling back, each is double-quoted all the + same. The project's reader quotes `true` and `null` itself (#181), so + this is what holds the rule to them.""" + monkeypatch.setattr(shape_materialize, "_reads_back", lambda *_: True) + assert neutral_pin_entry(name) == double_quoted(name) + + +@pytest.mark.parametrize("name", LOOK_ALIKES) +def test_a_name_that_only_looks_like_one_is_written_bare(name): + """`openGlass` byte for byte, and each name that only looks like a + bool, a null or a float: bare, as on main, and read back as the + name.""" + assert neutral_pin_entry(name) == name + assert parse_yaml(rendered(f"\n - {name}"))["neutral_product_pins"] \ + == [name] + + +#: The characters `PIN_ARG_RE` admits in a product's name. +PIN_ALPHABET = ("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" + "0123456789_.-") +#: Every name `--pin` admits of one or two characters, every one of three +#: that begins with a point, and the words and look-alikes above. +PIN_NAMES = tuple(dict.fromkeys(( + *("".join(chars) for size in (1, 2) + for chars in itertools.product(PIN_ALPHABET, repeat=size)), + *("." + "".join(chars) + for chars in itertools.product(PIN_ALPHABET, repeat=2)), + *YAML_WORDS, *LOOK_ALIKES))) + + +def stock_readers() -> dict: + """The standard readers installed here, by name: PyYAML's `safe_load` + (YAML 1.1), and ruamel.yaml's safe loader as YAML 1.2 and as 1.1.""" + readers = {} + if pyyaml is not None: + readers["PyYAML"] = pyyaml.safe_load + if RuamelYAML is not None: + for version in ((1, 2), (1, 1)): + loader = RuamelYAML(typ="safe", pure=True) + loader.version = version + readers[f"ruamel.yaml {version}"] = loader.load + return readers + + +@pytest.mark.skipif(not stock_readers(), + reason="neither PyYAML nor ruamel.yaml is installed " + "here; the rows above hold the rule without them") +def test_every_stock_reader_reads_every_entry_as_the_name(scaffolder): + """The issue's Goal: any YAML reader reads a pinned product back as + text. Every name in `PIN_NAMES`, in one list through the scaffold, is + read by each standard reader here as the name; on main 6c861635 + PyYAML read `yes`, `on` and `.5` as True, True and 0.5, and ruamel.yaml + failed on `._`.""" + names = list(PIN_NAMES) + text = rendered(scaffolder._neutral_product_pins(names)) + for reader, load in stock_readers().items(): + read = load(text)["neutral_product_pins"] + assert len(read) == len(names), reader + misread = {name: got for name, got in zip(names, read) + if got != name} + assert misread == {}, reader diff --git a/tests/test_pin_annotated_tag.py b/tests/test_pin_annotated_tag.py index d6ae7449..795b50e0 100644 --- a/tests/test_pin_annotated_tag.py +++ b/tests/test_pin_annotated_tag.py @@ -24,11 +24,21 @@ NO NETWORK AND NO GITHUB: the product is a local stand-in, and the forge is the stub `gh` of `test_adopt_pins.py`, which answers from a file. + +#912 holds two questions this file asks. A tag the `--pin-source` clone +holds without the commit it names cannot be peeled there, and was refused +"an annotated tag that names no commit"; the clone cannot say what it +names, so it is the clone that cannot answer, `pin-source-unreadable`, as +before #651. And where `check` asks the pin nothing, because the clone +`plan` read it from is gone or partial or `gh api` read it, its NOTE said +only that the digest was not rechecked, so a recorded tag's id passed as a +commit; the NOTE now says its commit was not asked either. """ from __future__ import annotations import shutil +import subprocess from pathlib import Path from types import SimpleNamespace @@ -37,12 +47,13 @@ from conftest import (SCAFFOLD, WINDOWS_SKIP, git, planned, refused_before_any_leg, run_script) from test_adopt_pins import ( # noqa: F401 - ENTRY, ORG, PIN_FILE, PLANNED, PROJECT, adopter, asked_what, check_plan, - forge_objects, gh_calls, gh_env, make_product, plan_with, read_from, + ENTRY, GATE_RECOMPUTES, NOT_RECHECKED, ORG, PIN_FILE, PLANNED, PROJECT, + adopter, asked_what, check_plan, forge_objects, gh_calls, gh_env, + make_product, pin_entry, pin_notes, plan_with, read_from, shape_materialize, tree_listing, with_pins, ) from test_scaffold_pin_and_reuse import assert_pin_digest_recomputed -from repo_shape import load_yaml, tree_digest +from repo_shape import double_quoted, load_yaml, tree_digest NOT_A_COMMIT = "pin-not-a-commit" PIN_SOURCE, DRY_RUN = "--pin-source", "--dry-run" @@ -305,3 +316,161 @@ def test_check_finds_a_recorded_id_that_is_no_commit( refusal = refused_before_any_leg(adopter, monkeypatch, plan, tmp_path) assert refusal.code == PLAN_NOT_A_COMMIT assert gh_calls(gh_env) == "" + + +# --- a tag whose commit the clone lacks (#912) ------------------------------- +# +# `commit_in_clone` peels an annotated tag with `rev-parse ^{}`, and in a +# clone that holds the tag but not the commit it names, git answers "missing +# object referenced by" the tag, and the peel nothing. On main 6c861635 the +# id was then refused "an annotated tag that names no commit", by `plan`, the +# scaffold and `check` alike, which no tag of a commit is. Nothing in that +# clone says what the tag names, so it is asked no further: the digest's +# reader finds no tree at the id and refuses the clone as one that cannot +# answer, saying to fetch the commit into it, as it did before #651. + +#: The refusal of a clone that cannot answer for a pin, and of a plan's. +UNREADABLE = "pin-source-unreadable" +PLAN_UNREADABLE = "plan-pin-source-unreadable" + + +@pytest.fixture +def lacking(tmp_path, tagged) -> Path: + """A clone that holds the annotated tag `v1` and the tag of it, and not + the commit they name: the two tag objects alone, written into it as + `git hash-object` writes an object, from their bytes.""" + clone = tmp_path / "openGlass-lacking" + git("init", "-q", "-b", "main", str(clone), cwd=tmp_path) + for oid in (tagged.tag, tagged.tag_of_tag): + raw = subprocess.run(["git", "cat-file", "tag", oid], + cwd=tagged.path, capture_output=True, + check=True).stdout + made = subprocess.run(["git", "hash-object", "-t", "tag", "-w", + "--stdin"], cwd=clone, input=raw, + capture_output=True, check=True) + assert made.stdout.decode().strip() == oid + assert git("cat-file", "-e", tagged.commit, cwd=clone, + check=False).returncode != 0 + return clone + + +@pytest.mark.parametrize("kind", PEELED) +def test_a_clone_lacking_a_tags_commit_cannot_say_what_it_names( + tagged, lacking, kind): + """`commit_in_clone` answers such a tag as an id the clone cannot say + anything of: the id as given, with no reason to refuse it.""" + named = getattr(tagged, kind) + assert shape_materialize.commit_in_clone(lacking, named) == (named, "") + + +@pytest.mark.parametrize("kind", PEELED) +@pytest.mark.parametrize("tool", ["adopt-plan", "scaffold-dry-run"]) +def test_a_tag_whose_commit_the_clone_lacks_is_unreadable_there( + source_repo, tmp_path, tagged, lacking, gh_env, kind, tool): + """Both tools refuse it as a clone that cannot answer for the pin, with + the remediation that says to fetch the commit into it, and never as an + id that names no commit: before `plan` writes a plan or the scaffold + makes a remote, and with no `gh`.""" + named = getattr(tagged, kind) + source = (PIN_SOURCE, str(lacking)) + if tool == "adopt-plan": + made = tmp_path / "plan.yaml" + result = plan_with(source_repo, made, f"openGlass@{named}", + extra=source, env=gh_env) + else: + made = tmp_path / "remotes" + result = scaffold(tmp_path, named, *source, DRY_RUN, env=gh_env) + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr.startswith( + f"REFUSED {UNREADABLE}: {lacking.resolve()} could not answer for " + f"{REPOSITORY} @ {named}: "), result.stderr + assert "a clone of that repository that HAS the pinned commit (`git " \ + "fetch --all` first)" in result.stderr, result.stderr + assert "annotated tag" not in result.stdout + result.stderr + assert not made.exists() + assert gh_calls(gh_env) == "" + + +def test_check_finds_a_recorded_clone_lacking_a_tags_commit_unreadable( + adopter, monkeypatch, source_repo, tmp_path, tagged, lacking, + gh_env): + """`check` asks the clone `plan` read the digest from the same question, + so a plan that records such a tag's id finds that clone unable to + answer for it, as for a commit it lacks; `execute` refuses it before + any leg.""" + plan = planned(source_repo, tmp_path, PLANNED) + with_pins(plan, read_from(lacking, tagged.tag, + tree_digest(tagged.path, tagged.commit))) + checked = check_plan(plan, env=gh_env) + assert checked.returncode == 1, checked.stdout + checked.stderr + lines = [line for line in checked.stderr.splitlines() + if line.startswith("FINDING ")] + assert len(lines) == 1, checked.stderr + assert lines[0].startswith( + f"FINDING {PLAN_UNREADABLE}: {ENTRY} records `commit: {tagged.tag}`, " + "and the clone its `digest_source:` names cannot answer for it: " + f"{lacking} could not answer for {REPOSITORY} @ {tagged.tag}: "), \ + lines[0] + assert "annotated tag" not in checked.stdout + checked.stderr + refusal = refused_before_any_leg(adopter, monkeypatch, plan, tmp_path) + assert refusal.code == PLAN_UNREADABLE + assert gh_calls(gh_env) == "" + + +# --- a recorded id `check` asks nothing of (#912) ---------------------------- +# +# #457's L1-82, its second case. `check` asks a recorded `commit:` what it +# names only of the clone `plan` read the digest from, still at its path and +# no partial clone. Where it is gone or partial, or `gh api` read the digest, +# `check` asks the pin nothing, and reads no network to ask it, so on main +# 6c861635 a tag's id recorded there passed `check`, and `execute` wrote it, +# with a NOTE that said only that the digest was not rechecked. The NOTE now +# says the commit was not asked either, and why. + +#: Where `check` asks a recorded pin nothing. +NOT_ASKED = ["gone", "partial", "gh-api"] + + +def not_asked(where: str, base: Path, clone: Path) -> tuple[str, str]: + """`(the digest_source: a plan records, why check asks the pin nothing)` + for `where`: a clone no longer at its path under `base`, `clone` made a + partial clone by its config alone (nothing is fetched), or `gh api`.""" + if where == "gh-api": + return ("gh-api-tree-recursive", + "`plan` read it with `gh api`, and `check` reads no network") + if where == "gone": + gone = base / "openGlass-gone" + return (f"local-clone ({gone})", + f"the clone `plan` read it from is not at {gone}") + for setting in (("core.repositoryformatversion", "1"), + ("extensions.partialClone", "origin"), + ("remote.origin.promisor", "true")): + git("config", *setting, cwd=clone) + return (f"local-clone ({clone})", + f"the clone at {clone} is a partial clone (`git clone " + "--filter`), whose `git` fetches what it lacks over the network, " + "and `check` reads no network") + + +@pytest.mark.parametrize("where", NOT_ASKED) +def test_check_says_it_did_not_ask_a_recorded_id_it_cannot_ask( + source_repo, tmp_path, tagged, tagged_clone, gh_env, where): + """A tag's id recorded as the pin's `commit:`, with its commit's digest, + where `check` asks the pin nothing: it still passes and runs no `gh`, + and its one NOTE about the pin says that neither the digest nor what + the commit names was asked here, and why.""" + recorded, why = not_asked(where, tmp_path, tagged_clone) + plan = planned(source_repo, tmp_path, PLANNED) + with_pins(plan, "pins:\n" + pin_entry( + commit=tagged.tag, digest=tree_digest(tagged.path, tagged.commit), + source=double_quoted(recorded)[1:-1])) + checked = check_plan(plan, env=gh_env) + assert checked.returncode == 0, checked.stdout + checked.stderr + notes = pin_notes(checked) + assert notes == [f"{NOT_RECHECKED}{why}. {GATE_RECOMPUTES}"], \ + checked.stdout + assert (f"NOTE {ENTRY}: digest NOT rechecked here, nor its `commit:` " + f"asked whether it names a commit: {why}. ") in notes[0], notes + assert notes[0].endswith("; `plan` asks what the commit names where it " + "reads the digest."), notes + assert gh_calls(gh_env) == "" diff --git a/tests/test_pin_source_top_level.py b/tests/test_pin_source_top_level.py index 5608d92a..f718b0fc 100644 --- a/tests/test_pin_source_top_level.py +++ b/tests/test_pin_source_top_level.py @@ -311,8 +311,9 @@ def test_check_asks_a_recorded_top_level_as_it_always_did( with_pins(plan, read_from(path, stand_ins.commit, stand_ins.digest)) checked = check_plan(plan, env=gh_env) assert checked.returncode == 0, checked.stdout + checked.stderr - said = (f"digest NOT rechecked here: the clone at {path} is a partial " - "clone" if where == PARTIAL else + said = (f"digest NOT rechecked here, nor its `commit:` asked whether it " + f"names a commit: the clone at {path} is a partial clone" + if where == PARTIAL else f"digest recomputes at {stand_ins.commit[:12]} in the clone " f"`plan` read it from, {path}.") assert f"NOTE {ENTRY}: {said}" in checked.stdout, checked.stdout diff --git a/tests/test_scaffold_pin_and_reuse.py b/tests/test_scaffold_pin_and_reuse.py index cba0d6eb..18b20d49 100644 --- a/tests/test_scaffold_pin_and_reuse.py +++ b/tests/test_scaffold_pin_and_reuse.py @@ -763,10 +763,14 @@ def scaffolder(): } #: Names that begin with a letter, `_` or `.` and that it reads back bare as #: the same text, so each is written bare, as main wrote it: the plain -#: names, and those that only look like a number or a word to a reader -#: other than the project's. +#: names. READ_BARE = ("openGlass", "Openglass", "open-glass", "open_glass", - "openGlass2", "_x", "x.y", ".5", ".inf", "yes", "off") + "openGlass2", "_x", "x.y") +#: Names it reads back bare as the same text, but that only look like a +#: number or a word to a reader other than the project's: YAML 1.1 or 1.2's +#: core schema reads each as a float or a bool, as PyYAML's `safe_load` +#: does. They were written bare until #912; each is double-quoted now. +STOCK_MISREAD = (".5", ".inf", "yes", "off") #: Names it reads back bare as the same text, but that begin with a digit #: or `-`, so `adopt-project.py`'s `y` quotes them in a plan's `product:`. #: The scaffold wrote them bare until #924, and a standard YAML reader took @@ -795,13 +799,16 @@ def listed(entries: str) -> object: for kind, names in MISREAD_BARE.items() for name in names), *(pytest.param(name, id=f"bare-{name}") for name in READ_BARE), *(pytest.param(name, id=f"quoted-by-y-{name}") for name in QUOTED_BY_Y), + *(pytest.param(name, id=f"stock-misread-{name}") + for name in STOCK_MISREAD), ]) def test_each_entry_reads_back_as_the_product(scaffolder, product): """Every product reads back as its text. A name that begins with a letter, `_` or `.` and that the reader reads back bare is written bare, - so a root's `openGlass` line does not move by a byte; every other is - double-quoted, as `adopt-project.py` writes it (#924), which the reader - always reads back.""" + so a root's `openGlass` line does not move by a byte, unless YAML 1.1 + or 1.2 reads it as no text (#912); every other is double-quoted, as + `adopt-project.py` writes it (#924), which the reader always reads + back.""" entries = scaffolder._neutral_product_pins([product]) assert listed(entries) == [product] spelled = product if product in READ_BARE else double_quoted(product) @@ -812,7 +819,7 @@ def test_every_entry_together_reads_back_in_the_order_given(scaffolder): """All of them in one list, as many `--pin`s write it: one line each, in the order the pins were given, as the scaffold has always listed them.""" every = [*(name for names in MISREAD_BARE.values() for name in names), - *READ_BARE, *QUOTED_BY_Y] + *READ_BARE, *QUOTED_BY_Y, *STOCK_MISREAD] entries = scaffolder._neutral_product_pins(every) assert entries.count("\n") == len(every) assert listed(entries) == every