diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index b4e8e2b4..226e7b0d 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -121,12 +121,12 @@ sys.path.insert(0, str(SHAPE_ROOT / "scripts")) from repo_shape import ( # noqa: E402 COMMIT_RE, SAFE_PATH_RE, Refusal, checked_value, git_out, load_yaml, - recorded_gitlink, repo_basename, tree_digest, + recorded_gitlink, repo_basename, spelled, tree_digest, ) from shape_materialize import ( # noqa: E402 CommandFailed, captured_child_environment, check_program, commit_trailers, - escape_what_stdout_cannot_encode, is_nested_key, rewrite_fault, run, - trailer_line, with_scalar, write_lf, + escape_what_stdout_cannot_encode, is_nested_key, pin_values_in, + rewrite_fault, run, trailer_line, with_scalar, write_lf, ) MANIFEST = "project.yaml" @@ -1067,11 +1067,35 @@ def commit_once(root: Path, message: str, paths: list[str], return git_out(["rev-parse", "HEAD"], cwd=root).lower() +#: Where git keeps a branch: the root's HEAD names `refs/heads/`. +HEADS = "refs/heads/" + + def current_branch(root: Path) -> str | None: - proc = subprocess.run(["git", "symbolic-ref", "--quiet", "--short", "HEAD"], + """The branch the root's HEAD is on, or None: detached, or a HEAD that + names a ref outside `refs/heads/`, which is no branch. + + THE FULL REF, NOT `--short` (#672). `symbolic-ref --short` gives the + shortest name that is not ambiguous, so beside a tag of the same name it + printed `heads/`. Measured with a tag `main` beside the tracking + branch `main`: the compare with `tracking_branch` read `heads/main` + against `main`, `bump-leg-on-tracking-branch` was not given, and the bump + committed on the default branch and printed a push to it. The name after + `refs/heads/` is the branch's own, whatever tags there are. + + ONLY THE NEWLINE IS TAKEN OFF. A ref name holds no control character, + so git's line ending is all there is to drop; `strip()` also took a + trailing U+00A0, U+2028 or U+0085, which a branch name may end in, and + read a root on `feat` as `feat`, a branch of its own that the + push line then pushed instead of the bump. + """ + proc = subprocess.run(["git", "symbolic-ref", "--quiet", "HEAD"], cwd=str(root), capture_output=True, text=True, encoding="utf-8", errors="replace", check=False) - return proc.stdout.strip() or None + ref = proc.stdout.rstrip("\n") + if not ref.startswith(HEADS): + return None + return ref[len(HEADS):] or None # --------------------------------------------------------------------------- @@ -1123,11 +1147,20 @@ def _ensure_root_clean(root: Path) -> None: dirt = git_out(["status", "--porcelain", "--untracked-files=no", "--ignore-submodules=untracked"], cwd=root, binary=True) if dirt: - shown = dirt.decode("utf-8", "backslashreplace").strip() + # EACH LINE `spelled` (#672), as `bootstrap.py` spells what it read + # from the disk: under `core.quotePath=false` git prints a name as + # the bytes it holds, so a C1 control or a bidi override in a file + # name reached the terminal raw. Split on git's own newlines and no + # others: `splitlines` also breaks at U+0085 and U+2028, which are + # `spelled`'s to show, and `strip()` took them off the last name and + # the leading space off the first line's ` M`, which then read as + # `M` (staged). git quotes a name with a control in it or a space at + # its end, so the last newline is all there is to take off. + shown = dirt.decode("utf-8", "backslashreplace").rstrip("\n") raise Refusal( "bump-leg-root-dirty", f"{root} has uncommitted changes:\n" - + "\n".join(f" {line}" for line in shown.splitlines()), + + "\n".join(f" {spelled(line)}" for line in shown.split("\n")), "Remediation: commit or stash them first. A lockstep commit that " "cannot be read as exactly one change is the thing the rule " "exists to keep readable.") @@ -1160,9 +1193,10 @@ def _switch_command(root: Path, role: str, commit: str) -> str: #: stands now; going back to the old branch is right only where it holds work #: not merged yet, and `git switch` keeps its commits. IN WORDS, NOT A SECOND #: COMMAND, as `family.py` and the holder's `validate-family.py` say it: no -#: line of ours changes, so none needs quoting, and the three tools follow one -#: rule. THE SAME SENTENCE as their `BRANCH_LEFT` (#607), word for word, with -#: "the project" where they say "the holder". +#: line of ours changes, so none needs quoting. FOUR COPIES OF ONE RULE: the +#: `BRANCH_LEFT` of `family.py`, of the holder's `validate-family.py` (#607) +#: and of `setup-project.py` (#647) is this sentence word for word, with +#: "the holder" where this one says "the project". BRANCH_LEFT = ("If `git switch -c` says that branch already exists, an " "earlier act left it: give the branch another name, which " "starts it where the project stands, or, only if that branch " @@ -1611,11 +1645,70 @@ def _apply_rewrite(root: Path, submodule: Path, commit: str, was: str, raise +def _moved_from(was: str, commit: str, pin_text: str) -> str: + """The commit a bump moves the leg FROM, as the report's `commit` line + and the commit message name it (#672): the gitlink's, or, where the + gitlink already records `commit`, the pin's own `commit:`. + + A person who moved the gitlink by hand and committed it left the pin + behind, and the bump is what brings the pin to it. `was` is the + gitlink's, so both lines read `X -> X` for a commit that moved the pin + from another one. The pin's value is read as the validators read it, + from the text `rewrite_pin` was given; one that does not read, or is + not a commit, leaves the gitlink's, and the `gitlink` line under it says + which one did not move. + """ + if was != commit: + return was + try: + pinned = pin_values_in(pin_text)[0].strip().lower() + except ValueError: + return was + return pinned if COMMIT_RE.match(pinned) else was + + +def _committed_paths(root: Path, head: str) -> str: + """The paths the commit `head` changes, as git records them, for the + `committed` line (#672), or a sentence saying they could not be read. + + FROM GIT, NOT FROM THE LIST STAGED. The leg's path is staged whether or + not its gitlink moves, so where it already recorded the new commit the + line named `spec` beside the pin while the commit held the pin alone. + `diff-tree` is plumbing: it reads no `diff.ignoreSubmodules` (measured, + `git show` under `=all` drops the leg), and `--ignore-submodules=none` + overrides a `submodule..ignore` (measured, which `diff-tree` does + honour). NUL-separated, so a name is the bytes git holds, and each is + `spelled`, as the dirty-root list is. + + AFTER THE COMMIT, NOTHING HERE MAY REFUSE: the bump has landed, and a + `git-failed` exit 2 would say it had not. + """ + try: + raw = git_out(["diff-tree", "--no-commit-id", "--name-only", "-r", + "-z", "--ignore-submodules=none", head], cwd=root, + binary=True) + except (Refusal, OSError) as exc: + why = " ".join((exc.detail if isinstance(exc, Refusal) + else str(exc)).split()) + return (f"its paths could not be read back ({why}); `git show " + f"--name-only --ignore-submodules=none {head[:12]}` lists " + "them") + return ", ".join(spelled(name.decode("utf-8", "backslashreplace")) + for name in raw.split(b"\0") if name) + + def cmd_bump(args) -> int: """Advance one leg. The ORDER is what makes `--dry-run` the real run's answer (#534, #438): everything that can REFUSE the move is asked before the dry-run return, so the dry run and the real run refuse alike and the - only thing the dry run skips is the writing. + only thing the dry run skips is the writing — and the project's own + validators, which judge the rewritten tree and so run only once it is + written. A pin they would refuse as it stands is previewed, exit 0, and + the real run ends `bump-leg-validators-red`, rolled back; the dry run's + closing paragraph says so (#672). Running them over the tree as it + stands would put `validate-pins.py`'s `gh api` call (#411) into a command + that promises to fetch into the leg and write nothing else, and would see + only what is red already. 1. the root: clean, on a branch, the leg initialized and recorded; 2. the fetch, and the commit `--to` names in the leg's objects (an @@ -1661,7 +1754,14 @@ def cmd_bump(args) -> int: pin_text = _leg_pin_text(root, pin_path) plans = plan_workflows(root, repository, commit) - print(f"commit {was[:12]} -> {commit[:12]}") + # `old`, NOT `was`, IS WHAT THE BUMP MOVES FROM (#672), on this line and + # in the commit message; `was` is the gitlink's, which the rollback + # restores. They differ only where the gitlink already records `commit`. + old = _moved_from(was, commit, pin_text) + print(f"commit {old[:12]} -> {commit[:12]}") + if was == commit: + print(f"gitlink {path} already records {commit[:12]}, so this " + "does not move it") print(f"tree {digest[:12]}… ({digest})") print(f"pin {pin_rel}") _print_workflow_plan(root, plans) @@ -1688,7 +1788,10 @@ def cmd_bump(args) -> int: print("\n--dry-run: nothing was changed. The leg's object store was " "fetched into, which is how the digest above was computed; the " "working tree, the pin, the workflows and the index are " - "untouched.") + "untouched. A dry run does not run the project's own " + f"validators ({' and '.join(VALIDATORS)}): the real run runs " + "them after the rewrite, and can still end " + "`bump-leg-validators-red`, with every byte rolled back.") if branch == project.tracking: # LAST, after everything a dry run on a branch prints, so that # line and the sentence under it are the whole of the difference @@ -1705,32 +1808,51 @@ def cmd_bump(args) -> int: staged: list[str] = [path, pin_rel] + [ wf_path.relative_to(root).as_posix() for wf_path, _, _ in plans] body = ( - f"{was[:12]} -> {commit[:12]}, tree {digest}.\n\n" + f"{old[:12]} -> {commit[:12]}, tree {digest}.\n\n" + + (f"The gitlink at {path} already recorded {commit[:12]}, so this " + "commit does not move it.\n\n" if was == commit else "") + (f"{sum(count for _, _, count in plans)} workflow `@` " f"reference(s) in {len(plans)} file(s) moved with it.\n\n" if plans else - "No workflow reference names this leg, so the gitlink and the pin " - "file are the whole of it.\n\n") - + f"The gitlink at {path}, `commit:` and `digests.tree_sha256` in " - f"{pin_rel}, and every workflow reference to {repository} move in " - "ONE commit because `scripts/validate-pins.py` refuses when they " - "disagree — a gitlink advanced alone is red on the next pull " - "request, and on somebody else's change.\n\nWritten by " + "No workflow reference names this leg, so " + + ("the pin file is" if was == commit + else "the gitlink and the pin file are") + + " the whole of it.\n\n") + + (f"`commit:` and `digests.tree_sha256` in {pin_rel}, and every " + f"workflow reference to {repository}, are brought to the gitlink " + f"at {path} in ONE commit because `scripts/validate-pins.py` " + "refuses when they disagree — a gitlink advanced alone, as this " + "one was, is red on the next pull request, and on somebody " + "else's change.\n\n" if was == commit else + f"The gitlink at {path}, `commit:` and `digests.tree_sha256` in " + f"{pin_rel}, and every workflow reference to {repository} move " + "in ONE commit because `scripts/validate-pins.py` refuses when " + "they disagree — a gitlink advanced alone is red on the next " + "pull request, and on somebody else's change.\n\n") + + "Written by " "openRepoShape's `scripts/bump-leg.py`; the digest is recomputed " "from the leg's own objects, not adjusted.\n") head = _apply_rewrite( root, submodule, commit, was, pin_path, new_pin, plans, staged, f"Bump {role} leg to {commit[:12]} in {project.display_name}\n\n" + body, args.trailer) - print(f"\n committed {head[:12]}: " + ", ".join(staged)) + print(f"\n committed {head[:12]}: {_committed_paths(root, head)}") print(f" {path}/ is left DETACHED at {commit[:12]}; `make bootstrap` in " "the root puts it on its tracking branch AT the new pin, or says " "why not") # QUOTED WHOLE (#532), the root and the branch: a root under `My # Projects/` pasted as two arguments, and the line is the one this run - # ends on, the one a person runs next. + # ends on, the one a person runs next. THE BRANCH BY ITS FULL REFNAME, + # ON BOTH SIDES (#672): a bare `` is a refspec, and git reads a + # leading `+` as FORCE, so a root on `+x` was told `push -u origin +x`, + # which pushed a local `x` over origin's, forced, and not `+x` at all. + # `refs/heads/:refs/heads/` is that branch and nothing + # else, beside a tag of its name too, and `-u` still records + # `origin/` as its upstream. It is the form #689 and #674 give + # `family.py`'s and `update-shape.py`'s push lines: one for the three. print("\nNEXT " + shlex.join(["git", "-C", str(root), "push", "-u", - "origin", branch])) + "origin", + f"{HEADS}{branch}:{HEADS}{branch}"])) print(f" then open a pull request against {project.tracking}; never " "push the bump to the default branch directly") return 0 diff --git a/tests/test_bump_leg.py b/tests/test_bump_leg.py index 031d7075..1ddf07cc 100644 --- a/tests/test_bump_leg.py +++ b/tests/test_bump_leg.py @@ -232,10 +232,14 @@ def test_the_message_and_the_next_step_are_a_pull_request(project, scaffolded, assert tree_digest(project / "spec", advanced) in body assert "validate-pins.py" in body - assert f"push -u origin {WORKING_BRANCH}" in result.stdout + # THE BRANCH BY ITS FULL REFNAME (#672): a bare name is a refspec. + pushed = f"refs/heads/{WORKING_BRANCH}" + assert f"push -u origin {pushed}:{pushed}" in result.stdout assert "pull request" in result.stdout - assert "push -u origin main" not in result.stdout, ( - "never a direct push to the default branch") + for default in ("main", "refs/heads/main"): + assert f"push -u origin {default}" not in result.stdout, ( + "never a direct push to the default branch") + assert f":{default}" not in result.stdout def test_bumping_to_the_commit_already_pinned_does_nothing(project, scaffolded): diff --git a/tests/test_bump_leg_branch_and_push_line.py b/tests/test_bump_leg_branch_and_push_line.py new file mode 100644 index 00000000..11dc4bdd --- /dev/null +++ b/tests/test_bump_leg_branch_and_push_line.py @@ -0,0 +1,520 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`scripts/bump-leg.py` reads the root's branch the way git spells it, ends +on a push no branch name can bend, spells what a dirty root lists, names +what its commit holds, and says what a dry run does not run (#672). + +FIVE LINES, EACH MEASURED ON MAIN BEFORE THE FIX: + + * THE BRANCH. `current_branch` asked `git symbolic-ref --short HEAD`, which + gives the shortest name that is not ambiguous: beside a tag `main` it + printed `heads/main`, the compare with the tracking branch read that + against `main`, and the bump COMMITTED ON THE DEFAULT BRANCH and printed + `push -u origin heads/main`. A HEAD on `refs/tags/` read as ``, and + the bump moved the tag. The full ref is read now, and only a ref under + `refs/heads/` is a branch. + * THE PUSH. The NEXT line ended `push -u origin `, and a bare + `` is a refspec: a leading `+` is FORCE. A root on `+x` was told + `push -u origin +x`, which pushed a local `x` over origin's `x`, forced, + and never pushed `+x`; on `+main` it rewinds origin's `main`. It is + `refs/heads/:refs/heads/` now, the branch and nothing + else on both sides, the form #689 and #674 give `family.py`'s and + `update-shape.py`'s push lines. + * THE DIRTY ROOT. Under `core.quotePath=false` git prints a name as the + bytes it holds, and `bump-leg-root-dirty` printed a C1 control and a + bidi override raw. Each line is `repo_shape.spelled` now, split on git's + own newlines, so U+0085 and U+2028 stay on their line and are named. + * THE COMMIT. `committed :` listed the paths STAGED, and the leg is + staged whether or not its gitlink moves: over a gitlink a person had + moved by hand it named `spec` beside the pin, and the report and the + message said `X -> X`. The paths are read from git now, and the old + commit is the pin's own `commit:` where the gitlink already holds it. + * THE DRY RUN. It never runs the project's own validators, so over a pin + they already refuse it exits 0 and the real run ends + `bump-leg-validators-red`. Its closing paragraph says so. + +NO NETWORK, NO `gh`: the bare remotes are the session's own, and every root +here pushes to a private copy of its remote (`make_root`), so a push reaches +nothing another test reads. Every printed line is run as the argv a POSIX +shell reads out of it, without a shell, so these hold on every OS. +""" + +from __future__ import annotations + +import shlex +import subprocess +import sys +from pathlib import Path + +import pytest + +from conftest import FILE_PROTOCOL, PROJECT, REPO, cloned_just_pushed, git +# A fixture is used by being named in this module's namespace, which is what +# the imports of `bump_leg_module` and `make_root` are for. +from test_bump_leg import ( # noqa: F401 + branched, bump, bump_leg_module, committed, reference_both_legs, validate, +) +from test_bump_leg_paste_lines import make_root, next_line, words # noqa: F401 + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import spelled # noqa: E402 + +#: Where the advanced spec commit is pushed. Not another module's branch: the +#: bare remotes are session-scoped, and a second, different commit pushed to +#: a name already taken is refused as non-fast-forward. +ADVANCE_BRANCH = "bump-leg-branch-and-push-line-advance" + +SPEC_PIN = "contracts/spec-pin.yaml" + +#: The sentence the dry run's closing paragraph ends with. +NOT_VALIDATED = ( + "A dry run does not run the project's own validators " + "(scripts/validate-pins.py and scripts/validate-manifest.py): the real " + "run runs them after the rewrite, and can still end " + "`bump-leg-validators-red`, with every byte rolled back.") + + +@pytest.fixture(scope="module") +def advanced(scaffolded, tmp_path_factory) -> str: + """A real new commit on the spec leg's bare remote; returns its sha.""" + remote = scaffolded["remotes"] / f"{PROJECT}-spec.git" + work = cloned_just_pushed( + remote, tmp_path_factory.mktemp("spec-advance-push-line") / "leg") + (work / "PUSH-LINE.md").write_text("# the push line\n", encoding="utf-8") + git("add", "--", "PUSH-LINE.md", cwd=work) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Advance the spec leg for the push-line tests", cwd=work) + git("push", "-q", "origin", f"HEAD:refs/heads/{ADVANCE_BRANCH}", cwd=work) + return git("rev-parse", "HEAD", cwd=work).stdout.strip() + + +def head_of(repo: Path, ref: str = "HEAD") -> str: + return git("rev-parse", "--verify", "-q", ref, cwd=repo, + check=False).stdout.strip() + + +def commit_empty(root: Path, message: str) -> str: + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", + "--allow-empty", "-m", message, cwd=root) + return head_of(root) + + +def lines_of(result) -> list[str]: + return result.stdout.splitlines() + + +# --- the branch, read in full ------------------------------------------------ + + +def test_a_tag_named_like_the_tracking_branch_does_not_let_the_bump_onto_it( + project, scaffolded, advanced): + """THE ISSUE'S MEASUREMENT: a tag `main` beside the tracking branch + `main`. `--short` read `heads/main`, the refusal was not given, the bump + committed on the default branch and printed a push to it. Refused now, + in the words a root on `main` with no such tag is given, and the dry run + names the tracking branch as itself.""" + git("tag", "main", cwd=project) + head = head_of(project) + + real = bump(project, scaffolded, advanced) + assert real.returncode == 2, real.stderr + real.stdout + assert "REFUSED bump-leg-on-tracking-branch" in real.stderr, real.stderr + assert "is on 'main', which project.yaml declares" in real.stderr + assert real.stdout == "" + assert head_of(project) == head, "nothing was committed on main" + + dry = bump(project, scaffolded, advanced, "--dry-run") + assert dry.returncode == 0, dry.stderr + dry.stdout + assert f"{project.resolve()} is on 'main', the tracking branch." in \ + dry.stdout, dry.stdout + assert "heads/main" not in dry.stdout + + +@pytest.mark.parametrize("ref", ["refs/tags/release-1", + "refs/remotes/origin/main"]) +def test_a_head_on_a_ref_outside_refs_heads_is_no_branch_to_bump_on( + project, scaffolded, advanced, ref): + """`symbolic-ref` lets HEAD name `refs/tags/` or a remote-tracking + ref, and `--short` read either as a branch (`release-1`, `origin/main`): + the bump committed THROUGH it, which moved the ref, and printed a push + of it. A ref outside `refs/heads/` is no branch: refused as a root on no + branch is, with the ref where it was. Neither name is 11 characters + long, as `refs/heads/` is, so the refusal is the guard's and not an + empty name's (rv672's R1).""" + if ref.startswith("refs/tags/"): + git("tag", ref[len("refs/tags/"):], cwd=project) + was = head_of(project, ref) + assert was, f"fixture: {ref} exists" + git("symbolic-ref", "HEAD", ref, cwd=project) + + result = bump(project, scaffolded, advanced) + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED bump-leg-root-detached" in result.stderr, result.stderr + assert result.stdout == "" + assert head_of(project, ref) == was, f"{ref} did not move" + + +# --- the push, by its full ref ----------------------------------------------- + + +def push_argv(root: Path, branch: str) -> list[str]: + """The NEXT line's argv: the branch by its full refname on both sides, + the form #689 and #674 give `family.py`'s and `update-shape.py`'s.""" + return ["git", "-C", str(root.resolve()), "push", "-u", "origin", + f"refs/heads/{branch}:refs/heads/{branch}"] + + +def run_the_line(line: str, cwd: Path) -> subprocess.CompletedProcess: + """The NEXT line as the argv a POSIX shell reads out of it, run without + a shell, from a directory that is not the root's.""" + return subprocess.run(words(line), cwd=str(cwd), capture_output=True, + text=True, encoding="utf-8", errors="replace", + check=False) + + +def upstream_of(root: Path, branch: str) -> str: + return git("for-each-ref", "--format=%(upstream)", + f"refs/heads/{branch}", cwd=root).stdout.strip() + + +@pytest.mark.parametrize("name", ["+x", "+main"]) +def test_a_branch_named_with_a_leading_plus_is_pushed_and_nothing_is_forced( + make_root, scaffolded, advanced, name): + """THE ISSUE'S MEASUREMENT, and its worst case. Origin's `` + has moved on from the local branch of that name. The bare line read + `+` as the FORCED refspec ``: origin's `x` was + overwritten (`forced update`), and on `+main` origin's DEFAULT BRANCH was + rewound, while `+` itself was never pushed. By its full refname + the line pushes `+`, as a new branch, upstream recorded, and + leaves the other where it was.""" + root, remote = make_root("plain") + other = name[1:] + start = head_of(root) + git("checkout", "-q", "-b", "w672-elsewhere", cwd=root) + moved_on = commit_empty(root, f"origin's {other} moves on") + git("push", "-q", "origin", f"HEAD:refs/heads/{other}", cwd=root) + git("checkout", "-q", "main", cwd=root) + if other != "main": + git("branch", other, start, cwd=root) + git("checkout", "-q", "-b", name, cwd=root) + + result = bump(root, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + line = next_line(result.stdout) + assert words(line) == push_argv(root, name) + + pushed = run_the_line(line, root.parent) + assert pushed.returncode == 0, pushed.stderr + pushed.stdout + assert "forced update" not in pushed.stderr, pushed.stderr + assert head_of(remote, f"refs/heads/{name}") == head_of(root) + assert head_of(remote, f"refs/heads/{other}") == moved_on, ( + f"origin's {other} was not touched") + assert upstream_of(root, name) == f"refs/remotes/origin/{name}" + + +def test_a_branch_beside_a_tag_of_its_name_is_pushed_by_its_full_ref( + make_root, scaffolded, advanced): + """A branch `v1` beside a tag `v1`: `--short` read `heads/v1`, and the + line said `push -u origin heads/v1` (git's own bare `v1` there is `src + refspec v1 matches more than one`). The line names `refs/heads/v1` on + both sides, and run, it pushes the branch and records its upstream.""" + root, remote = make_root("plain") + git("tag", "v1", cwd=root) + git("checkout", "-q", "-b", "v1", cwd=root) + + result = bump(root, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + line = next_line(result.stdout) + assert words(line) == push_argv(root, "v1") + + pushed = run_the_line(line, root.parent) + assert pushed.returncode == 0, pushed.stderr + pushed.stdout + assert head_of(remote, "refs/heads/v1") == head_of(root, "refs/heads/v1") + assert upstream_of(root, "v1") == "refs/remotes/origin/v1" + + +def test_a_branch_ending_in_a_no_break_space_is_pushed_as_itself( + make_root, scaffolded, advanced): + """`feat` beside a branch `feat`: `strip()` took the no-break + space off the name it read, so the bump committed on `feat` and + the line pushed `feat`, the other branch, at the commit before the bump + (rv672's S2). Only git's newline is taken off now: the line names the + branch the bump committed on, and run, pushes that commit.""" + root, remote = make_root("plain") + name = "feat\u00a0" + git("branch", "feat", cwd=root) + git("checkout", "-q", "-b", name, cwd=root) + + result = bump(root, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + line = next_line(result.stdout) + assert words(line) == push_argv(root, name) + + pushed = run_the_line(line, root.parent) + assert pushed.returncode == 0, pushed.stderr + pushed.stdout + assert head_of(remote, f"refs/heads/{name}") == head_of(root) + assert head_of(remote, "refs/heads/feat") == "", "`feat` was not pushed" + + +# --- the dirty root, spelled ------------------------------------------------- + +#: `(a name, as `bump-leg-root-dirty` lists it under core.quotePath=false)`. +SPELLED = [ + pytest.param("caf\u009b\u202eevil.yml", "caf\\x9b\\u202eevil.yml", + id="c1-and-bidi"), + pytest.param("a\u2028b.yml", "a\\u2028b.yml", id="line-separator"), + pytest.param("c\u0085d.yml", "c\\x85d.yml", id="next-line"), + pytest.param("end\u2028", "end\\u2028", id="ends-in-a-separator"), +] + + +def raw_controls(name: str, text: str) -> list[str]: + """The characters of `name` that `spelled` escapes and that `text` still + holds as they are: what a terminal must not be handed raw.""" + return [char for char in name if spelled(char) != char and char in text] + + +def dirty_with(project: Path, name: str) -> None: + """A tracked workflow file `name`, committed and then edited, under + `core.quotePath=false`, on a bump branch; or skip where the disk cannot + hold the name.""" + branched(project) + path = project / ".github" / "workflows" / name + try: + path.write_bytes(b"on: [push]\n") + except (OSError, UnicodeError) as exc: + pytest.skip(f"this filesystem cannot hold the name: {exc}") + git("add", "--", f".github/workflows/{name}", cwd=project) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "A workflow whose name holds a control", cwd=project) + path.write_bytes(path.read_bytes() + b"# edited\n") + git("config", "core.quotePath", "false", cwd=project) + + +@pytest.mark.parametrize("extra", [(), ("--dry-run",)], + ids=["bump", "dry-run"]) +@pytest.mark.parametrize("name, shown", SPELLED) +def test_a_dirty_name_holding_controls_is_listed_spelled( + project, scaffolded, advanced, name, shown, extra): + """THE ISSUE'S MEASUREMENT (`cafevil.yml`), and the two + characters `splitlines` breaks a line at: the name is on ONE line of its + own, every control written as its escape, and none of them raw.""" + dirty_with(project, name) + result = bump(project, scaffolded, advanced, *extra) + assert result.returncode == 2, result.stderr + result.stdout + assert "REFUSED bump-leg-root-dirty" in result.stderr, result.stderr + assert f" M .github/workflows/{shown}" in result.stderr.split("\n"), ( + result.stderr) + assert raw_controls(name, result.stderr) == [], ascii(result.stderr) + + +def test_under_the_default_quoting_git_names_the_file_itself( + project, scaffolded, advanced): + """THE CONTROL: with `core.quotePath` as git ships it, git quotes the + bytes itself, and the line is git's, unchanged.""" + dirty_with(project, "caf\u009b\u202eevil.yml") + git("config", "--unset", "core.quotePath", cwd=project) + result = bump(project, scaffolded, advanced) + assert result.returncode == 2, result.stderr + result.stdout + assert (' M ".github/workflows/caf\\302\\233\\342\\200\\256evil.yml"' + in result.stderr.splitlines()), result.stderr + + +def test_every_dirty_line_keeps_gits_status_columns(project, scaffolded, + advanced): + """`strip()` took the leading space off the FIRST line only, so its + ` M` (changed in the working tree) read as `M` (changed in the index) + and stood one column left of the line under it (rv672's N1). Every line + is git's own now, after the same indent.""" + branched(project) + workflows = project / ".github" / "workflows" + for name in ("one.yml", "two.yml"): + (workflows / name).write_bytes(b"on: [push]\n") + git("add", "--", ".github/workflows", cwd=project) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Two workflows", cwd=project) + for name in ("one.yml", "two.yml"): + (workflows / name).write_bytes(b"on: [pull_request]\n") + + result = bump(project, scaffolded, advanced) + assert result.returncode == 2, result.stderr + result.stdout + lines = result.stderr.split("\n") + assert " M .github/workflows/one.yml" in lines, result.stderr + assert " M .github/workflows/two.yml" in lines, result.stderr + + +# --- the commit, as git holds it --------------------------------------------- + + +def body_of(project: Path) -> str: + return git("log", "-1", "--format=%b", cwd=project).stdout + + +@pytest.mark.parametrize("setting", [None, "submodule.spec.ignore", + "diff.ignoreSubmodules"]) +def test_the_committed_line_names_the_paths_git_holds( + project, scaffolded, advanced, setting): + """The paths of the commit, as git lists them, in git's order: a + workflow, the pin and the gitlink. With an `ignore = all` the person set + for the leg, `git show` drops the gitlink from its list, and the line + still names it, because the commit holds it.""" + reference_both_legs(project) + branched(project) + if setting: + git("config", setting, "all", cwd=project) + + result = bump(project, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + held = git("show", "--ignore-submodules=none", "--name-only", + "--format=", "HEAD", cwd=project).stdout.split() + assert held == [".github/workflows/legs.yml", SPEC_PIN, "spec"] + assert f" committed {head_of(project)[:12]}: {', '.join(held)}" in \ + lines_of(result), result.stdout + assert not any(line.startswith("gitlink ") for line in lines_of(result)) + + +def move_the_gitlink_by_hand(project: Path, scaffolded: dict, + to: str) -> None: + """On a bump branch, the leg checked out at `to` and the gitlink moved + to it in a commit of its own, the pin left as it was: the repair case + of AT-scaffold N9.""" + branched(project) + leg = project / "spec" + git(*FILE_PROTOCOL, "fetch", "-q", + str(scaffolded["remotes"] / f"{PROJECT}-spec.git"), ADVANCE_BRANCH, + cwd=leg) + git("checkout", "-q", "--detach", to, cwd=leg) + git("add", "--", "spec", cwd=project) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Move the gitlink by hand", cwd=project) + + +def held_line(commit: str) -> str: + return (f"gitlink spec already records {commit[:12]}, so this " + "does not move it") + + +def test_a_gitlink_moved_by_hand_is_reported_as_the_pins_move( + project, scaffolded, advanced): + """THE ISSUE'S MEASUREMENT (AT-scaffold N9): a person moved the gitlink to + the new commit by hand and committed it; the pin still names the old + one. The report, the dry run's included, and the commit message said + `X -> X`, and `committed` named `spec` beside the pin, which the commit + does not hold. The old commit is the pin's own now, a line says the + gitlink does not move, and the commit's paths are git's. The message's + last sentence states the rule without saying the gitlink moved in it + (rv672's N3).""" + pinned = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + move_the_gitlink_by_hand(project, scaffolded, advanced) + moved = f"commit {pinned[:12]} -> {advanced[:12]}" + held = held_line(advanced) + + dry = bump(project, scaffolded, advanced, "--dry-run") + assert dry.returncode == 0, dry.stderr + dry.stdout + assert moved in lines_of(dry), dry.stdout + assert held in lines_of(dry), dry.stdout + + result = bump(project, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + assert moved in lines_of(result), result.stdout + assert held in lines_of(result), result.stdout + assert f"{advanced[:12]} -> {advanced[:12]}" not in result.stdout + assert committed(project) == {SPEC_PIN} + assert f" committed {head_of(project)[:12]}: {SPEC_PIN}" in \ + lines_of(result), result.stdout + body = body_of(project) + assert body.startswith(f"{pinned[:12]} -> {advanced[:12]}, tree "), body + assert (f"The gitlink at spec already recorded {advanced[:12]}, so this " + "commit does not move it.") in body, body + assert "so the pin file is the whole of it." in body, body + assert ("are brought to the gitlink at spec in ONE commit because " + "`scripts/validate-pins.py` refuses when they disagree") in \ + " ".join(body.split()), body + assert "The gitlink at spec, `commit:`" not in body, body + assert validate(project).returncode == 0, "the project's own gate agrees" + + +def test_a_pin_commit_that_is_no_commit_is_not_printed_as_the_old_one( + project, scaffolded, advanced): + """The pin's own `commit:` is the old commit only when it IS one: text + such as `not-a-sha` there is not printed on the `commit` line, which + names the gitlink's commit instead, with the line that says the gitlink + does not move (rv672's R2).""" + pin = project / SPEC_PIN + pinned = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + move_the_gitlink_by_hand(project, scaffolded, advanced) + pin.write_bytes(pin.read_bytes().replace( + f'commit: "{pinned}"'.encode("ascii"), b'commit: "not-a-sha"', 1)) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "A pin whose commit is no commit", "--", SPEC_PIN, cwd=project) + + result = bump(project, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + assert f"commit {advanced[:12]} -> {advanced[:12]}" in \ + lines_of(result), result.stdout + assert held_line(advanced) in lines_of(result), result.stdout + assert "not-a-sha" not in result.stdout + assert validate(project).returncode == 0, "the project's own gate agrees" + + +def test_a_committed_line_git_cannot_read_back_still_says_the_bump_landed( + project, scaffolded, advanced, bump_leg_module, monkeypatch, capsys): + """THE LINE IS READ AFTER THE COMMIT, so nothing in it may refuse: a + `git-failed` there would be exit 2 over a bump that landed. The line + names the commit and says where its paths are, and the run ends on its + NEXT, exit 0.""" + branched(project) + before = head_of(project) + real_git_out = bump_leg_module.git_out + + def no_diff_tree(args, *rest, **kwargs): + if args[:1] == ["diff-tree"]: + raise bump_leg_module.Refusal( + "git-failed", "`git diff-tree` exited 128: fatal: bad object") + return real_git_out(args, *rest, **kwargs) + + monkeypatch.setattr(bump_leg_module, "git_out", no_diff_tree) + code = bump_leg_module.main([ + "--root", str(project), "--leg", "spec", "--to", advanced, + "--local-remote-dir", str(scaffolded["remotes"])]) + out = capsys.readouterr().out + + head = head_of(project) + assert code == 0, out + assert git("rev-parse", f"{head}~1", cwd=project).stdout.strip() == before + said = (f" committed {head[:12]}: its paths could not be read back " + "(`git diff-tree` exited 128: fatal: bad object); `git show " + f"--name-only --ignore-submodules=none {head[:12]}` lists them") + assert said in out.splitlines(), out + assert next_line(out) == shlex.join(push_argv(project, "bump/spec")) + + +# --- the dry run, and what it does not run ----------------------------------- + + +def test_a_dry_run_says_the_validators_are_the_real_runs(project, scaffolded, + advanced): + """THE ISSUE'S MEASUREMENT: a pin `validate-pins.py` already refuses + (`yaml-unparsable`, a tab-indented line after a `note:`), committed. The + dry run previews the move, exit 0, as before, and now says that it runs + no validator and that the real run can still end red; the real run then + does, rolled back. No behaviour changes.""" + branched(project) + pin = project / SPEC_PIN + pin.write_bytes(pin.read_bytes() + + b"note: a remark\n\tindented with a tab\n") + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "A pin its validator cannot parse", "--", SPEC_PIN, cwd=project) + gate = validate(project) + assert gate.returncode == 2, gate.stdout + gate.stderr + assert "yaml-unparsable" in gate.stdout + gate.stderr + head = head_of(project) + + dry = bump(project, scaffolded, advanced, "--dry-run") + assert dry.returncode == 0, dry.stderr + dry.stdout + assert NOT_VALIDATED in dry.stdout, dry.stdout + + real = bump(project, scaffolded, advanced) + assert real.returncode == 2, real.stderr + real.stdout + assert "REFUSED bump-leg-validators-red" in real.stderr, real.stderr + assert head_of(project) == head + assert git("status", "--porcelain", cwd=project).stdout == "" diff --git a/tests/test_bump_leg_paste_lines.py b/tests/test_bump_leg_paste_lines.py index 59eed637..49130023 100644 --- a/tests/test_bump_leg_paste_lines.py +++ b/tests/test_bump_leg_paste_lines.py @@ -288,8 +288,10 @@ def test_pasting_the_detached_remedy_puts_the_root_on_a_branch( @pytest.mark.parametrize("name", ROOTS) def test_the_closing_next_is_one_command(make_root, scaffolded, advanced, name): - """`NEXT git -C push -u origin `, split into the argv it - means, and spelled exactly as `shlex` spells it.""" + """`NEXT git -C push -u origin refs/heads/:refs/heads/`, + split into the argv it means, and spelled exactly as `shlex` spells it. + The branch by its full refname (#672): a bare one is a refspec, and a + leading `+` forces.""" root, _ = make_root(name) branched(root) result = bump(root, scaffolded, advanced) @@ -297,7 +299,7 @@ def test_the_closing_next_is_one_command(make_root, scaffolded, advanced, line = next_line(result.stdout) argv = ["git", "-C", str(root.resolve()), "push", "-u", "origin", - WORKING_BRANCH] + f"refs/heads/{WORKING_BRANCH}:refs/heads/{WORKING_BRANCH}"] assert words(line) == argv assert line == shlex.join(argv) assert (" then open a pull request against main; never push the " @@ -309,13 +311,15 @@ def test_the_closing_next_is_one_command(make_root, scaffolded, advanced, def test_an_ordinary_root_prints_the_line_it_always_did(make_root, scaffolded, advanced): """Nothing is quoted that needs no quoting: a root whose name is plain - prints the same bytes as before #532.""" + prints the bytes it did before #532, the branch by its full refname on + both sides (#672).""" root, _ = make_root("plain") branched(root) result = bump(root, scaffolded, advanced) assert result.returncode == 0, result.stderr + result.stdout assert next_line(result.stdout) == ( - f"git -C {root.resolve()} push -u origin {WORKING_BRANCH}") + f"git -C {root.resolve()} push -u origin " + f"refs/heads/{WORKING_BRANCH}:refs/heads/{WORKING_BRANCH}") @WINDOWS_SKIP @@ -348,11 +352,12 @@ def test_the_closing_next_carries_the_branch_whole(make_root, scaffolded, assert result.returncode == 0, result.stderr + result.stdout line = next_line(result.stdout) + pushed = f"refs/heads/{branch}:refs/heads/{branch}" assert words(line) == ["git", "-C", str(root.resolve()), "push", "-u", - "origin", branch] - assert words(line)[-1] == branch + "origin", pushed] + assert words(line)[-1] == pushed assert line == shlex.join(["git", "-C", str(root.resolve()), "push", "-u", - "origin", branch]) + "origin", pushed]) @WINDOWS_SKIP diff --git a/tests/test_non_utf8_git_reads.py b/tests/test_non_utf8_git_reads.py index 4d19a998..85afd289 100644 --- a/tests/test_non_utf8_git_reads.py +++ b/tests/test_non_utf8_git_reads.py @@ -253,7 +253,8 @@ def refused_dirty(project: Path, scaffolded: dict, to: str, shown: str, assert result.returncode == 2, output assert "REFUSED bump-leg-root-dirty: " in result.stderr, output assert f"{project} has uncommitted changes:\n" in result.stderr, output - assert f" M {shown}" in result.stderr.splitlines(), output + # Four spaces, then git's own ` M` (#672: its leading space is kept). + assert f" M {shown}" in result.stderr.splitlines(), output assert "Remediation: commit or stash them first." in result.stderr assert "Traceback" not in output, output assert "UnicodeDecodeError" not in output, output @@ -309,7 +310,7 @@ def test_a_name_that_is_utf8_under_quote_path_false_is_still_refused( assert result.returncode == 2, output assert "REFUSED bump-leg-root-dirty: " in result.stderr, output [shown] = [line for line in result.stderr.splitlines() - if line.startswith(" M .github/workflows/caf")] + if line.startswith(" M .github/workflows/caf")] assert shown.endswith(".yml"), output assert "Traceback" not in output, output assert found(project) == before @@ -326,7 +327,7 @@ def test_the_refusal_names_every_dirty_file_whatever_its_bytes( result = bump(project, scaffolded, advanced) lines = result.stderr.splitlines() assert result.returncode == 2, result.stderr - assert r" M .github/workflows/caf\xe9.yml" in lines, result.stderr + assert r" M .github/workflows/caf\xe9.yml" in lines, result.stderr assert any(line.rstrip().endswith("M README.md") for line in lines), ( result.stderr) assert "Traceback" not in result.stderr