diff --git a/docs/cli.md b/docs/cli.md index 68585086..3ca5f67e 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -348,8 +348,14 @@ options: trees in it, --local-remote-dir) and whether each of the three repositories already exists (`gh repo view` and its commit listing, or the bare repositories with - --local-remote-dir), refusing what it refuses; make no - folder, create no remote and push nothing + --local-remote-dir), refusing what it refuses. Whether + each folder can be written in is asked of `os.access`, + which reads the mode bits and on Windows says yes for + every folder, so a quota, a Windows ACL or a read-only + drive on Windows can pass here and be refused by the + real run. It leaves nothing that stays: it makes and + removes temporary folders, creates no remote and + pushes nothing --local-remote-dir LOCAL_REMOTE_DIR create bare repositories here instead of calling `gh repo create` (the TEST path; no network). An empty diff --git a/scaffold-project.py b/scaffold-project.py index 546a1344..ee0540e0 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -92,7 +92,7 @@ SHAPE_ROOT = Path(__file__).resolve().parent sys.path.insert(0, str(SHAPE_ROOT / "scripts")) from repo_shape import ( # noqa: E402 - COMMIT_RE, NEUTRAL_PRODUCT_OWNER, PROJECT_ID_RE, TREE_DIGEST_DEFINITION, + NEUTRAL_PRODUCT_OWNER, PROJECT_ID_RE, TREE_DIGEST_DEFINITION, VISIBILITY_CHOICES, NamingPolicy, Refusal, accepts_role, checked_value, git_out, link_pins_from_trees, repo_basename, shell_line, shown_path, tree_digest, @@ -373,8 +373,15 @@ def main(argv: list[str] | None = None) -> int: "three repositories already exists (`gh repo " "view` and its commit listing, or the bare " "repositories with --local-remote-dir), " - "refusing what it refuses; make no folder, " - "create no remote and push nothing") + "refusing what it refuses. Whether each " + "folder can be written in is asked of " + "`os.access`, which reads the mode bits and on " + "Windows says yes for every folder, so a quota, " + "a Windows ACL or a read-only drive on Windows " + "can pass here and be refused by the real run. " + "It leaves nothing that stays: it makes " + "and removes temporary folders, creates no " + "remote and pushes nothing") # TEXT, not `type=Path`, as `--pin-source` and `--work-dir` are: `Path("")` # is `.`, so an empty value could not be told from a typed `.` once # parsed (`folder_flag_path`). @@ -523,9 +530,12 @@ def seed_tree(role: str, values: dict, tracking: str, scratch: Path) -> str: PUSH_ENV = {**REMOTE_ENV, "LC_ALL": "C"} -def only_commit(url: str, scratch: Path) -> tuple[str, str] | None: - """`(commit, tree)` when the repository at `url` holds exactly ONE commit - and every ref it has names it, else None (#462). +def only_commit(url: str, + scratch: Path) -> tuple[str, str, tuple[str, ...]] | None: + """`(commit, tree, refs)` when the repository at `url` holds exactly ONE + commit and every ref it has names it, else None (#462). `refs` is the + name of each ref the listing gave, `HEAD` among them, and says which + branch the commit was found on (#921). `git ls-remote` lists every ref, `HEAD`, a `seed/*` branch and a pull request's included, and one object name across all of them is the first @@ -537,10 +547,11 @@ def only_commit(url: str, scratch: Path) -> tuple[str, str] | None: Both commands that reach `url` run under `PROBE_ENV`, and a `url` git cannot read raises `CommandFailed` for `_held_seed` to answer (#542). """ - tips = {line.split()[0].lower() - for line in run(["git", "ls-remote", url], - env=PROBE_ENV).splitlines() - if line.strip()} + listed = [line.split(None, 1) + for line in run(["git", "ls-remote", url], + env=PROBE_ENV).splitlines() + if line.strip()] + tips = {fields[0].lower() for fields in listed} if len(tips) != 1: return None commit = tips.pop() @@ -555,15 +566,18 @@ def only_commit(url: str, scratch: Path) -> tuple[str, str] | None: if any(line.startswith("parent ") for line in header): return None tree = next(line.split()[1] for line in header if line.startswith("tree ")) - return commit, tree.lower() + return commit, tree.lower(), tuple(fields[1].strip() for fields in listed + if len(fields) > 1) def _held_seed(role: str, url: str, values: dict, tracking: str, - scratch: Path) -> tuple[str | None, str | None]: - """`(commit, None)` when a leg that is not empty holds ONE commit and its - tree is the one this run would seed the leg with (#462); `(None, None)` - when it holds anything else; `(None, why)` when git cannot read it - (#542). + scratch: Path) -> tuple[str | None, str | None, + tuple[str, ...]]: + """`(commit, None, found_on)` when a leg that is not empty holds ONE + commit and its tree is the one this run would seed the leg with (#462); + `(None, None, ())` when it holds anything else; `(None, why, ())` when + git cannot read it (#542). `found_on` is each branch the commit was + found on, or `()` when `tracking` is one of them (`_seed_branches`, #921). That is a leg a run of this scaffold pushed before a later push was refused, by an organisation's ruleset or anything else, or one an @@ -586,12 +600,57 @@ def _held_seed(role: str, url: str, values: dict, tracking: str, try: held = only_commit(url, scratch) except CommandFailed as exc: - return None, _unread_because(exc) + return None, _unread_because(exc), () if held is None: - return None, None - commit, tree = held - seeded = tree == seed_tree(role, values, tracking, scratch) - return (commit if seeded else None), None + return None, None, () + commit, tree, refs = held + if tree != seed_tree(role, values, tracking, scratch): + return None, None, () + return commit, None, _seed_branches(refs, tracking) + + +#: What git's listing puts before a branch's name. +HEADS = "refs/heads/" + + +def _seed_branches(refs: tuple[str, ...], tracking: str) -> tuple[str, ...]: + """Each branch a leg's seed was found on, by name, when `tracking`, the + branch this run creates at it, is NOT one of them, else `()` (#921). + + A leg whose seed sits on a branch of another name, `seed/scaffold` above + all (the branch the ruleset hint made before #506), is taken all the + same, and the re-run creates `tracking` at it and leaves that branch as + it is (`_take_held_seed`). A tag or a pull request's ref is no branch, + and `HEAD` is the leg's default branch under another name, so neither + counts.""" + branches = sorted(ref[len(HEADS):] for ref in refs + if ref.startswith(HEADS)) + return () if tracking in branches else tuple(branches) + + +def _seed_branch_note(target, branches: tuple[str, ...], tracking: str, + local: bool = False) -> str: + """The NOTE for a seed found only on `branches`, not on `tracking` + (#921): the re-run creates `tracking` at it and touches none of them, so + each remains. On GitHub it adds that one may be the leg's default branch + (#921 did not verify that there: it says "may"); under + `--local-remote-dir` it does not, as that is a bare repository on disk + and no GitHub. The names come from the remote, so each is spelt as + `shown_path` spells a path: as it is, or as `repr` where a character in + it is not printable. + """ + names = [f"`{shown_path(branch)}`" for branch in branches] + if len(names) > 1: + found = f"branches {', '.join(names[:-1])} and {names[-1]}" + left = "them, so they remain" + default = "and on GitHub one of them may be the leg's default branch" + else: + found = f"branch {names[0]}" + left = f"{names[0]}, so that branch remains" + default = "and on GitHub it may be the leg's default branch" + return (f"NOTE the seed in {target} was found only on its {found}: this " + f"run creates `{tracking}` at it and does not touch {left}" + + ("." if local else f", {default}.")) #: The `fatal:` line git prints when the program it reaches a remote through @@ -657,6 +716,14 @@ def _reusable_remotes(names: dict, urls: dict, repositories: dict, asked only of a leg that is not empty, and a leg git cannot read is refused as one with commits, saying so (#542). + A SEED FOUND ONLY ON ANOTHER BRANCH is taken, and said (#921). Where the + leg's one commit is on no branch named `tracking`, `seed/scaffold` + above all, the re-run creates `tracking` at it and leaves the branch it + was found on, which remains and, on GitHub, may be the leg's default + branch (`_seed_branch_note`: a local bare repository is said no such + thing). The one NOTE is printed after the `reuse` line, by a dry run as + by the real run. + A LOCAL REMOTE IS NAMED AS `shown_path` NAMES A PATH (#766), as `family.py init` names its holder's: it is a path under `--local-remote-dir`, where `scaffold-unwritable-value` refuses a @@ -670,15 +737,18 @@ def _reusable_remotes(names: dict, urls: dict, repositories: dict, else remote_is_empty(target)) if empty is None: continue - seed, unread = ((None, None) if role == "assembly" or empty else - _held_seed(role, urls[role], values, tracking, - Path(scratch) / role)) + seed, unread, found_on = ( + (None, None, ()) if role == "assembly" or empty else + _held_seed(role, urls[role], values, tracking, + Path(scratch) / role)) if reuse_flag and (empty or seed): reused[role] = seed print(f" reuse {target} " + ( "(zero commits)" if empty else f"(one commit, {seed[:12]}: the seed this scaffold " "pushes)")) + if found_on: + print(_seed_branch_note(target, found_on, tracking, local)) continue raise Refusal( "scaffold-remote-exists", @@ -1956,8 +2026,10 @@ def _take_held_seed(work: Path, url: str, seed: str, tracking: str) -> str: `seed_tree` built from these values, which is what was just written. The pin, the gitlink and the push that follows are then about the commit the remote already has, and that push changes nothing where the tracking - branch already holds it. The fetch reaches the remote, so it runs under - `REMOTE_ENV`, the prompt rule every such command runs under (#915). + branch already holds it. A branch the seed was found on instead + (`seed/scaffold`) is not touched, and `_reusable_remotes` has said so + (#921). The fetch reaches the remote, so it runs under `REMOTE_ENV`, the + prompt rule every such command runs under (#915). """ run(["git", "init", "-q", "-b", tracking, str(work)]) run(["git", "remote", "add", "origin", url], cwd=work) diff --git a/tests/test_scaffold_reuse_seeded_legs.py b/tests/test_scaffold_reuse_seeded_legs.py index 8a47a6d4..91301a31 100644 --- a/tests/test_scaffold_reuse_seeded_legs.py +++ b/tests/test_scaffold_reuse_seeded_legs.py @@ -39,6 +39,13 @@ re-run's, in a terminal, and a key the stand-in ssh refuses where an `insteadOf` or a `pushInsteadOf` rule sends the push over ssh, each in git's own locale and in one that translates git. + +A SEED FOUND ONLY ON ANOTHER BRANCH IS SAID (#921). The re-run takes a seed +that sits on `seed/scaffold`, the branch the ruleset hint made before #506, +creates the tracking branch at it and leaves that branch as it is. Its NOTE +names each such branch, says it remains and, on GitHub only, that it may be +the leg's default branch, in a dry run as in the real run, and says nothing +where the tracking branch holds the seed too. """ from __future__ import annotations @@ -184,6 +191,9 @@ def test_legs_holding_the_seed_are_reused_at_their_commits( for leg in LEGS: assert (f"reuse {bare(tmp_path, leg)} (one commit, {seeds[leg][:12]}: " "the seed this scaffold pushes)") in result.stdout + assert "NOTE the seed in" not in result.stdout, ( + "a seed on the tracking branch is taken from it, so no branch " + "elsewhere is in question (#921)") assert_finished(tmp_path, result) for leg in LEGS: assert tip(tmp_path, leg) == seeds[leg], "a reused leg is never rewritten" @@ -192,26 +202,203 @@ def test_legs_holding_the_seed_are_reused_at_their_commits( assert seeds[leg] in pin, pin +def seed_pushed_to(tmp_path, template_dir, *branches: str) -> str: + """A first run every ruleset refused, so all three repositories are made + and empty, then the spec leg's seed pushed, as an operator would, to each + of `branches` (`main` not among them unless named): the commit pushed.""" + assert scaffold(tmp_path, "w1", env=refusing( + template_dir, *REPOS.values())).returncode == 2 + for branch in branches: + pushed = subprocess.run( + ["git", "-C", str(tmp_path / "w1" / f"{PROJECT}-spec"), "push", + "-q", str(bare(tmp_path, "spec")), + f"main:refs/heads/{branch}"], + capture_output=True, text=True, check=False, + env={**os.environ, **refusing(template_dir)}) + assert pushed.returncode == 0, pushed.stderr + seed = tip(tmp_path, "spec", f"refs/heads/{branches[0]}") + assert seed + return seed + + +def seed_branch_note(tmp_path, tracking: str, *branches: str) -> str: + """The NOTE a `--local-remote-dir` re-run prints for the spec leg's seed + found only on `branches` (#921): no word of GitHub, as the leg is a bare + repository on disk.""" + if len(branches) > 1: + found = ("branches " + ", ".join(f"`{name}`" for name in branches[:-1]) + + f" and `{branches[-1]}`") + left = "them, so they remain" + else: + found = f"branch `{branches[0]}`" + left = f"`{branches[0]}`, so that branch remains" + return (f"NOTE the seed in {bare(tmp_path, 'spec')} was found only on its " + f"{found}: this run creates `{tracking}` at it and does not " + f"touch {left}.") + + def test_a_seed_pushed_to_a_seed_branch_alone_is_taken_too( tmp_path, template): # noqa: F811 """The exit the hint used to print: the seed pushed to `seed/scaffold`, which the ruleset lets through, and `main` absent. Its one commit is the seed, so the re-run takes it and creates `main` AT it: a new branch, not - a rewrite.""" - everything = refusing(template, *REPOS.values()) - assert scaffold(tmp_path, "w1", env=everything).returncode == 2 - pushed = subprocess.run( - ["git", "-C", str(tmp_path / "w1" / f"{PROJECT}-spec"), "push", "-q", - str(bare(tmp_path, "spec")), "main:refs/heads/seed/scaffold"], - capture_output=True, text=True, check=False, - env={**os.environ, **everything}) - assert pushed.returncode == 0, pushed.stderr - seed = tip(tmp_path, "spec", "refs/heads/seed/scaffold") + a rewrite. And it SAYS which branch remains (#921): `seed/scaffold` is + left as it is. It says nothing of GitHub's default branch, as a bare + repository on disk is no GitHub's.""" + seed = seed_pushed_to(tmp_path, template, "seed/scaffold") assert seed and not tip(tmp_path, "spec") result = scaffold(tmp_path, "w2", "--reuse-empty-repo", env=refusing(template)) assert_finished(tmp_path, result) assert tip(tmp_path, "spec") == seed + assert tip(tmp_path, "spec", "refs/heads/seed/scaffold") == seed, ( + "the branch the seed was found on is not touched") + note = seed_branch_note(tmp_path, "main", "seed/scaffold") + assert result.stdout.count(note) == 1, result.stdout + assert result.stdout.count("NOTE the seed in") == 1, ( + "the code leg was empty and the root is no leg: one leg, one NOTE") + assert "GitHub" not in result.stdout, result.stdout + assert result.stdout.index(f"reuse {bare(tmp_path, 'spec')} (one commit") \ + < result.stdout.index(note), "the NOTE follows the leg's reuse line" + + +def test_a_dry_run_says_the_seed_branch_too_and_makes_nothing( + tmp_path, template): # noqa: F811 + """The question is asked before anything is made (#537), the NOTE with + it, so a person reads it in the plan and not after the run.""" + seed = seed_pushed_to(tmp_path, template, "seed/scaffold") + result = scaffold(tmp_path, "w2", "--reuse-empty-repo", "--dry-run", + env=refusing(template)) + assert result.returncode == 0, result.stderr + result.stdout + assert seed_branch_note(tmp_path, "main", "seed/scaffold") in ( + result.stdout), result.stdout + assert not tip(tmp_path, "spec"), "a dry run creates no branch" + assert tip(tmp_path, "spec", "refs/heads/seed/scaffold") == seed + + +def test_a_seed_on_the_tracking_branch_as_well_prints_no_seed_note( + tmp_path, template): # noqa: F811 + """`main` holds the seed beside `seed/scaffold`: the re-run creates + nothing and takes the seed from `main`, so there is no seed found only + elsewhere to say (#921).""" + seed = seed_pushed_to(tmp_path, template, "seed/scaffold", "main") + assert tip(tmp_path, "spec") == seed + result = scaffold(tmp_path, "w2", "--reuse-empty-repo", + env=refusing(template)) + assert_finished(tmp_path, result) + assert "NOTE the seed in" not in result.stdout, result.stdout + + +def test_a_seed_on_two_branches_names_both(tmp_path, template): # noqa: F811 + """Neither is `main`: both remain, and the NOTE names both, in the + order a sort gives them, whichever order git listed them.""" + seed = seed_pushed_to(tmp_path, template, "seed/scaffold", "other") + result = scaffold(tmp_path, "w2", "--reuse-empty-repo", + env=refusing(template)) + assert_finished(tmp_path, result) + assert tip(tmp_path, "spec") == seed + assert seed_branch_note(tmp_path, "main", "other", "seed/scaffold") in ( + result.stdout), result.stdout + + +def test_a_branch_name_is_shown_safe_in_the_seed_note( + tmp_path, template): # noqa: F811 + """The remote gives the name, and git takes U+202E in a branch's name: + shown raw it reorders the line a person reads. It is spelt as + `shown_path` spells a path, as `repr` does (#692, #766).""" + name = "seed/" + chr(0x202E) + "scaffold" + assert not name.isprintable() + seed_pushed_to(tmp_path, template, name) + result = scaffold(tmp_path, "w2", "--reuse-empty-repo", "--dry-run", + env=refusing(template)) + assert result.returncode == 0, result.stderr + result.stdout + assert chr(0x202E) not in result.stdout + result.stderr + assert f"its branch `{name!r}`: " in result.stdout, result.stdout + + +@pytest.mark.parametrize("refs,found_on", [ + pytest.param(("HEAD", "refs/heads/seed/scaffold"), ("seed/scaffold",), + id="github-default-branch-is-seed-scaffold"), + pytest.param(("refs/heads/seed/scaffold", "refs/heads/main"), (), + id="the-tracking-branch-holds-it-too"), + pytest.param(("HEAD", "refs/heads/main"), (), id="only-the-tracking"), + pytest.param(("refs/heads/zeta", "refs/heads/alpha"), ("alpha", "zeta"), + id="sorted"), + pytest.param(("refs/tags/v0", "refs/pull/1/head"), (), + id="a-tag-and-a-pull-request-are-no-branch"), + pytest.param(("refs/heads/seed/scaffold", "refs/pull/1/head"), + ("seed/scaffold",), id="a-pull-request-beside-the-branch"), + pytest.param(("refs/heads/mainline",), ("mainline",), + id="a-name-that-starts-with-the-tracking-name"), + pytest.param(("refs/heads/seed/main",), ("seed/main",), + id="a-branch-ending-in-the-tracking-name"), + pytest.param(("refs/tags/main", "refs/heads/seed/scaffold"), + ("seed/scaffold",), id="a-tag-named-as-the-tracking-branch"), +]) +def test_the_branches_a_seed_was_found_on(scaffold_module, refs, found_on): + """#921: `_seed_branches` names each branch, by name and in order, and + names none where `main` holds the seed too.""" + assert scaffold_module._seed_branches(refs, "main") == found_on + + +@pytest.mark.parametrize("local,branches,said", [ + pytest.param( + True, ("seed/scaffold",), + "NOTE the seed in T was found only on its branch `seed/scaffold`: " + "this run creates `main` at it and does not touch `seed/scaffold`, " + "so that branch remains.", + id="local-one-branch-and-no-word-of-github"), + pytest.param( + True, ("other", "seed/scaffold"), + "NOTE the seed in T was found only on its branches `other` and " + "`seed/scaffold`: this run creates `main` at it and does not touch " + "them, so they remain.", + id="local-two-branches-and-no-word-of-github"), + pytest.param( + False, ("seed/scaffold",), + "NOTE the seed in T was found only on its branch `seed/scaffold`: " + "this run creates `main` at it and does not touch `seed/scaffold`, " + "so that branch remains, and on GitHub it may be the leg's default " + "branch.", + id="github-one-branch-may-be-the-default"), + pytest.param( + False, ("other", "seed/scaffold"), + "NOTE the seed in T was found only on its branches `other` and " + "`seed/scaffold`: this run creates `main` at it and does not touch " + "them, so they remain, and on GitHub one of them may be the leg's " + "default branch.", + id="github-two-branches-one-may-be-the-default"), +]) +def test_the_seed_note_says_github_only_on_github(scaffold_module, local, + branches, said): + """#921: "on GitHub it may be the leg's default branch" is said of a + GitHub leg, as the issue says it, and not of a bare repository under + `--local-remote-dir`, which has no GitHub to be the default branch of.""" + assert scaffold_module._seed_branch_note( + "T", branches, "main", local) == said + + +def test_dry_run_help_says_the_mode_bits_and_nothing_that_stays(tmp_path): + """#923 U063 and #921 U088, the scaffold's half: `--dry-run`'s help says + the write question is asked of `os.access`, which on Windows says yes for + every folder, so a quota, a Windows ACL or a read-only drive there can + pass it, and that it leaves "nothing that stays" where it once said "make + no folder": it makes and removes `openreposhape-seed-*` and + `openreposhape-branch-*` in the temporary folder. The wording is + `docs/cli.md`'s too.""" + result = run_script(SCAFFOLD, "--help", cwd=tmp_path) + assert result.returncode == 0, result.stderr + said = " ".join(result.stdout.split()) + assert ("Whether each folder can be written in is asked of `os.access`, " + "which reads the mode bits and on Windows says yes for every " + "folder, so a quota, a Windows ACL or a read-only drive on " + "Windows can pass here and be refused by the real run. It leaves " + "nothing that stays: it makes and removes temporary folders, " + "creates no remote and pushes nothing") in said, said + assert "make no folder" not in said + assert "its mode bits'" not in said, ( + "only the write question is `os.access`'s: a file in the way and a " + "tree's contents are not the mode bits'") def test_a_leg_holding_the_seed_without_the_flag_names_the_flag(