diff --git a/templates/family-root/scripts/bootstrap.py b/templates/family-root/scripts/bootstrap.py index 40487407..aec82b91 100755 --- a/templates/family-root/scripts/bootstrap.py +++ b/templates/family-root/scripts/bootstrap.py @@ -25,7 +25,10 @@ A fetch that fails is REPORTED and does not stop the run: the members that did arrive are still worth bootstrapping, and a family that refused to do anything because one member is unreachable would be worse - than one that says which. + than one that says which. The advice to set a credential is given only + where git asked for one: not where its init died at a gitlink it has + no url for, nor where it read nothing from `.gitmodules`, both below, + since it fetched nothing then (#839). "every member and leg fetched" is said only where git exited 0 AND every row's member is checked out (#648). A member left NOT CHECKED @@ -72,7 +75,11 @@ review-authority readout. This command does not reimplement any of that; it walks the members `family.yaml` names and calls theirs. `--members-make validate` runs `make validate` instead, which is what - the family Makefile's `validate` target uses. + the family Makefile's `validate` target uses. A member whose command + fails is named with that command's exit code, read from make's + `Error ` line, since make exits 2 for any command that fails, + so a member's finding (1) and its refusal (2) are told apart, and + from a failure of make's own, which prints no such line (#839). (c) EXITS 0 unless a member's own command failed, or (a) left a member not checked out that no credential would check out, or found a line or a @@ -182,8 +189,10 @@ def fetch_members(root: Path, rows: list[dict]) -> list[str]: Returns a failure line for each of `rows` whose member it leaves not checked out where no credential would check it out (`unfetchable`), for the run to end FAILED on (#648); every member and leg is said to be - fetched only where there is none and git exited 0. Raises the - `git-not-runnable` refusal where git cannot be started (#694).""" + fetched only where there is none and git exited 0. The advice to set a + credential is printed only where git asked for one + (`credential_advice`, #839). Raises the `git-not-runnable` refusal + where git cannot be started (#694).""" source, token = credential() args = ["git"] if token: @@ -202,19 +211,10 @@ def fetch_members(root: Path, rows: list[dict]) -> list[str]: if refusal is None: raise raise refusal from exc - if proc.returncode != 0: - # The token can appear in git's own error text (it is in the URL it - # tried), so the output is scrubbed before it is printed. - message = (proc.stderr + proc.stdout).strip() - if token: - message = message.replace(token, "") - print(f" FETCH INCOMPLETE (git exit {proc.returncode}) using " - f"credential source '{source}':\n{message}", file=sys.stderr) - print(" If the members are PRIVATE, set SHAPE_LEGS_TOKEN (a " - "fine-grained PAT with contents:read on them) or export an " - "App-minted SHAPE_LEGS_APP_TOKEN, then re-run. What did arrive " - "is bootstrapped below.", file=sys.stderr) left, stopper, modules = unfetchable(root, rows, proc.returncode == 0) + if proc.returncode != 0: + fetch_incomplete(proc, source, token, + credential_advice(bool(left), stopper, modules)) if left: print(" NOT CHECKED OUT, and not for want of a credential: " + ", ".join(f"{spelled(path)} ({reason(why, stopper, modules)})" @@ -232,6 +232,44 @@ def fetch_members(root: Path, rows: list[dict]) -> list[str]: for row, _, _ in left] +def fetch_incomplete(proc: subprocess.CompletedProcess, source: str, + token: str, advice: str) -> None: + """(a)'s report of a `git submodule update` that exited non-zero: git's + own words, then `advice`, where there is any (`credential_advice`).""" + # The token can appear in git's own error text (it is in the URL it + # tried), so the output is scrubbed before it is printed. + message = (proc.stderr + proc.stdout).strip() + if token: + message = message.replace(token, "") + print(f" FETCH INCOMPLETE (git exit {proc.returncode}) using " + f"credential source '{source}':\n{message}", file=sys.stderr) + if advice: + print(advice, file=sys.stderr) + + +def credential_advice(denied: bool, stopper: str | None, + modules: Gitmodules) -> str: + """(a)'s advice to set a credential, or nothing where git asked for none + (#839): where its init died at a gitlink it has no url for (`stopper`), + or it read nothing from `.gitmodules`, a merge leaving it unmerged, a + value git dies on or a file it cannot parse, it fetched nothing. Every + row (a) then leaves not checked out is said to be no credential's, and + the advice beside that was its denial (w768's candidate 1 on #819). + Anywhere else a fetch git made failed, a member's, or a leg's in a + member checked out, and the advice stands; where a row is said to be no + credential's beside it (`denied`), it is for the members that line does + not name.""" + if (stopper is not None or modules.unmerged or modules.refused + or modules.unreadable): + return "" + which = ("the members the next line does not name" if denied + else "the members") + return (f" If {which} are PRIVATE, set SHAPE_LEGS_TOKEN (a fine-grained " + "PAT with contents:read on them) or export an App-minted " + "SHAPE_LEGS_APP_TOKEN, then re-run. What did arrive is " + "bootstrapped below.") + + def git_not_started(exc: OSError) -> Refusal | None: """The `git-not-runnable` refusal for an `OSError` that says git could not be STARTED (#694), else `None`, and the error goes on as it did. @@ -798,7 +836,9 @@ def member_path(root: Path, row: dict) -> Path: def run_member_target(root: Path, row: dict, target: str) -> str | None: - """(b) One member's own `make `. Returns a failure line, or None.""" + """(b) One member's own `make `. Returns a failure line, or None: + one naming the exit code of the member's command where make names it, + else make's own (`run_make`, #839).""" # A member row is the family manifest's text (#391): what is PRINTED of it, # and of `--members-make`, is spelled. The path is built from the raw row. project = spelled(row.get("project") or "?") @@ -819,11 +859,72 @@ def run_member_target(root: Path, row: dict, target: str) -> str | None: return None print(f" --- {project}: make {named} ---") sys.stdout.flush() - proc = subprocess.run([make, target], cwd=str(path), check=False) + returncode, own = run_make(make, target, path) sys.stdout.flush() - if proc.returncode != 0: - return f"{project} (make {named} exited {proc.returncode})" - return None + if returncode == 0: + return None + if own is not None: + return f"{project} (make {named}: the member's command exited {own})" + return (f"{project} (make {named} exited {returncode} and printed no " + "`Error ` line for a command of the member's: make's own " + "message above says what failed)") + + +#: The line GNU make prints where a command of the target exits non-zero, +#: `make: *** [Makefile:44: bootstrap] Error 1` (`make[1]:` in a make run by +#: make, `*** [bootstrap] Error 1` before make 4.0): the command's own exit +#: code, which make's does not carry, since make exits 2 for any command +#: that fails (#839). +MAKE_ERROR_RE = re.compile(rb"\*\*\* \[[^\n]*\] Error (\d+)\s*$") + + +def run_make(make: str, target: str, cwd: Path) -> tuple[int, int | None]: + """(b) `make ` in a member, as `(make's exit code, the exit code + of the member's command it names)`, the second None where make printed + no `MAKE_ERROR_RE` line (#839). + + MAKE'S OWN CODE TELLS NOTHING APART. GNU make exits 2 for any command + that fails, measured (GNU Make 4.3) for a command exiting 1, 2, 3 or + 126, and 2 for a failure of its own (`No rule to make target`), so the + line said `make bootstrap exited 2` for the member's own `bootstrap + FAILED` (1, a finding), for a refusal (2) and for make's own failure + alike (w764b, for lane 4). The command's code is the one make names + in its `Error ` line, the last on stderr, so make's stderr is + passed on to this run's own as it comes and that line read from the + end of it; a make that prints none, one of its own failures, a command + killed by a signal, or a make speaking another language, leaves make's + code alone. The member's command is never run around make: another + target than `bootstrap` (`--members-make validate`) has no script of + its own to call, as `make siblings` calls a member's + `scripts/bootstrap.py`.""" + proc = subprocess.Popen([make, target], cwd=str(cwd), + stderr=subprocess.PIPE) + tail = b"" + for chunk in iter(lambda: proc.stderr.read1(65536), b""): + passed_on(chunk) + tail = (tail + chunk)[-4096:] + proc.stderr.close() + returncode = proc.wait() + found = [MAKE_ERROR_RE.search(line) for line in tail.splitlines()] + named = [int(match.group(1)) for match in found if match] + return returncode, (named[-1] if named else None) + + +def passed_on(chunk: bytes) -> None: + """`chunk` of what a member's make wrote to its stderr, written to this + run's as it came (#839): as its bytes, after what this run wrote there + before, or as text where stderr has no byte buffer.""" + stream = sys.stderr + if stream is None: + return + stream.flush() + sink = getattr(stream, "buffer", None) + if sink is None: + stream.write(chunk.decode("utf-8", "replace")) + stream.flush() + return + sink.write(chunk) + sink.flush() def main(argv: list[str] | None = None) -> int: diff --git a/templates/family-root/scripts/validate-family.py b/templates/family-root/scripts/validate-family.py index 2cb89e57..f0ec8356 100755 --- a/templates/family-root/scripts/validate-family.py +++ b/templates/family-root/scripts/validate-family.py @@ -595,7 +595,8 @@ def _gitlink_in_another_case(root: Path, path: str, commit: str, move it out of the holder, by an `mv` with nothing to fill in, which a shell test skips where the disk folds the two names into one directory, the row's own, and where the directory is a checkout - (#681). A checkout is deleted by hand once nothing only it holds is + (#681), and where nothing is there, and never skips for a symbolic + link (#839). A checkout is deleted by hand once nothing only it holds is needed. Where nothing is checked out at the row's path, as in a fresh clone, `git submodule update --init --recursive` then checks the member out there, LAST (#762): a checkout `git mv` moved to the variant names @@ -614,7 +615,8 @@ def _gitlink_in_another_case(root: Path, path: str, commit: str, f"at {path}.", "Remediation: the member is recorded in the wrong case. " + _wrong_case_move(path, commit, variants, - _move_context(root, path, variants, repository))) + _move_context(root, path, variants, repository, + gitlinks))) class MoveContext(NamedTuple): @@ -623,18 +625,22 @@ class MoveContext(NamedTuple): checkout keeps off disk; `in_the_way`, every entry the index holds at or under the row's path; and `section`, the lines that make `.gitmodules` give the row's path, with what they do, or empty where a - section of HEAD's already gives it.""" + section of HEAD's already gives it. `forced`, the variants whose `git + rm --cached` takes `-f` (`_forced`, #839).""" sparse: tuple[str, ...] = () in_the_way: tuple[str, ...] = () section: str = "" section_said: str = "" + forced: tuple[str, ...] = () def _move_context(root: Path, path: str, variants: list[str], - repository) -> MoveContext: + repository, gitlinks: dict[str, str]) -> MoveContext: """What the index and HEAD's `.gitmodules` hold around the move of a - member recorded only at `variants` to its row's `path` (#762). + member recorded only at `variants` to its row's `path` (#762), and + which variants the move's `git rm --cached` takes out only with `-f` + (`_forced`, by `gitlinks`, `_member_gitlinks`, #839). `--sparse` (the issue's (f)): a variant a sparse checkout keeps off disk (`git ls-files -t`'s `S`, as `family.py`'s `_skip_worktree` reads @@ -699,7 +705,39 @@ def _move_context(root: Path, path: str, variants: list[str], "variant's path") section += f"\n git add -- {GITMODULES}" return MoveContext(tuple(sorted(sparse)), tuple(in_the_way), section, - said) + said, _forced_variants(root, variants, gitlinks)) + + +def _forced_variants(root: Path, variants: list[str], + gitlinks: dict[str, str]) -> tuple[str, ...]: + """Each of `variants` whose `git rm --cached` takes `-f` (`_forced`), + in their order (#839).""" + return tuple(variant for variant in variants + if _forced(root, variant, gitlinks[variant]) is not None) + + +def _forced(root: Path, path: str, commit: str) -> str | None: + """None where `git rm --cached` takes the gitlink at `path`, which + `_member_gitlinks` reads at `commit`, out of the index as it is, else + what HEAD records at `path`, its commit or `""` for nothing, where that + takes `-f` (#839): the index stages the gitlink at a commit HEAD does + not record there, and a checkout is on disk at it, a `.git` entry. Git + refuses it then wherever that checkout has moved on or holds a file it + does not track (`the following file has staged content different from + both the file and the HEAD`, exit 1), measured on git 2.43 after `git + mv members/

Members/

`, staged, and a commit or an untracked file + in the checkout it moved (w761's candidate 2 on #828), and `-f`, with + `--cached`, touches no file. A gitlink HEAD alone records, at a staged + removal or a conflict, is read at HEAD's commit and takes none.""" + if not os.path.lexists(root / path / ".git"): + return None + with _git_runs(f"the gitlink HEAD records at {path} could not be read"): + try: + head = git_out(["rev-parse", "--verify", "--quiet", + f"HEAD:{path}"], cwd=root).lower() + except Refusal: + head = "" + return None if head == commit else head def _wrong_case_move(path: str, commit: str, variants: list[str], @@ -710,39 +748,45 @@ def _wrong_case_move(path: str, commit: str, variants: list[str], `member-gitlink-unnamed` under `--no-members` (#648), in one text. What else it meets is `context` (`_move_context`, #762). - EACH `mv` IS GUARDED BY TWO TESTS A POSIX SHELL RUNS (#681, rv611c's - N-1 and N-2), not by a system's name: this text is read in CI too, on - another disk than the one it is typed on. `[ -ef ]` is - true where the disk takes the two names for one directory, the row's - own, as a Linux mount that folds case does too (WSL's `/mnt/c`), where - the `mv` moved the member's own checkout out. `[ -e /.git ]` - is true for a checkout, and a submodule's `.git` is a file naming this - holder's `.git/modules/` by a relative path, which a move leaves - pointing nowhere (`fatal: not a git repository`). So the `mv` runs only - for a directory apart from the row's with no `.git` entry, the empty one - a fresh clone leaves or a plain one, and the text says where a - submodule checkout's commits are kept, in the store its `.git` file - names (not always `.git/modules/`: a checkout a plain `mv` - brought there names the old one, and git cannot open it), what deleting - it would lose, and to delete it by hand once nothing only it holds is - needed. A clone's own `.git` DIRECTORY holds its commits and nothing - else does, so the text names the `git log` that lists the ones not - pushed and says to move it out whole, not delete it (rv681's S2 and - N1, Direction B: the `-e` guard Brett Heap's word took is kept). + EACH `mv` IS GUARDED BY TESTS A POSIX SHELL RUNS (#681, rv611c's N-1 + and N-2; `_guarded_move`), not by a system's name: this text is read in + CI too, on another disk than the one it is typed on. `[ -ef + ]` is true where the disk takes the two names for one directory, + the row's own, as a Linux mount that folds case does too (WSL's + `/mnt/c`), where the `mv` moved the member's own checkout out. `[ -e + /.git ]` is true for a checkout, and a submodule's `.git` is a + file naming this holder's `.git/modules/` by a relative path, which a + move leaves pointing nowhere (`fatal: not a git repository`). `[ ! -e + ]` is true where nothing is there, where the bare `mv` failed + (#839). A symbolic link there is asked first, `[ ! -L ]`, and + always moved, since the other tests read through it (#839). So the + `mv` runs only for a link, or a directory apart from the row's with no + `.git` entry, the empty one a fresh clone leaves or a plain one, and + the text says where a submodule checkout's commits are kept, in the + store its `.git` file names (not always `.git/modules/`: a + checkout a plain `mv` brought there names the old one, and git cannot + open it), what deleting it would lose, and to delete it by hand once + nothing only it holds is needed. A clone's own `.git` DIRECTORY holds + its commits and nothing else does, so the text names the `git log` that + lists the ones not pushed and says to move it out whole, not delete it + (rv681's S2 and N1, Direction B: the `-e` guard Brett Heap's word took + is kept). + + THE `git rm` TAKES `-f` where the index stages a variant at a commit + HEAD does not record there and a checkout is on disk at it + (`MoveContext.forced`, `_forced`, #839): git refuses it there wherever + that checkout has moved on or holds a file it does not track, and the + text says what `-f` does (`FORCED_SAID`). """ taken_out = " ".join(shlex.quote(variant) for variant in variants) - moved = " and ".join( - f"`[ {shlex.quote(variant)} -ef {shlex.quote(path)} ] || " - f"[ -e {shlex.quote(variant + '/.git')} ] || " - f"mv -- {shlex.quote(variant)} " - f"{shlex.quote('../' + variant.replace('/', '-'))}`" - for variant in variants) + moved = " and ".join(_guarded_move(variant, path) for variant in variants) status = " or ".join(f"`git -C {shlex.quote(variant)} status`" for variant in variants) unpushed = " or ".join(f"`git -C {shlex.quote(variant)} log --oneline " "HEAD --branches --not --remotes`" for variant in variants) - flag = " --sparse" if context.sparse else "" + flag = (" --sparse" if context.sparse else "") + ( + " -f" if context.forced else "") return ( _entries_aside(path, context.in_the_way) + ("Then move" if context.in_the_way else "Move") @@ -750,10 +794,7 @@ def _wrong_case_move(path: str, commit: str, variants: list[str], "the index alone, which touches no file, with the " + ("first two lines below" if context.section else "two lines below") - + (f" (the first with --sparse, which needs git 2.34 or later: a " - f"sparse checkout keeps {' and '.join(context.sparse)} off disk, " - "and without it git rm passes over what that keeps off disk, exit " - "0, and removes nothing)" if context.sparse else "") + + _first_line_flags(context) + (f"; the lines after them {context.section_said}, and stage " f"{GITMODULES}. Commit that." if context.section else ", and commit that.") @@ -763,9 +804,11 @@ def _wrong_case_move(path: str, commit: str, variants: list[str], "(WSL's /mnt/c). Where the disk holds the two apart it stays there, " "untracked, and family.py add refuses it as " "family-members-dir-variant: move each one that is there out of the " - f"holder, in a POSIX shell, {moved}, which moves nothing where the " - "two names are one directory or where it holds a .git entry, a " - "checkout. A checkout git submodule add made holds a .git file " + f"holder, in a POSIX shell (Git Bash on Windows), {moved}, which " + "moves nothing where nothing is there, where the two names are one " + "directory or where it holds a .git entry, a checkout, and moves a " + "symbolic link there wherever it points, the link and never what it " + "points at. A checkout git submodule add made holds a .git file " "naming its store by a path relative to it, which a move would " "leave pointing nowhere. Its commits are kept under this holder's " ".git/modules/, in the directory its .git file names, whatever " @@ -788,6 +831,58 @@ def _wrong_case_move(path: str, commit: str, variants: list[str], f"{shlex.quote(f'160000,{commit},{path}')}{context.section}") +#: What `-f` on a `git rm --cached` of a gitlink does, said where one is +#: printed with it (`_forced`, #839), as `family.py`'s move back says it +#: (#761). +FORCED_SAID = ("-f takes out a staged gitlink whose checkout has moved on or " + "holds a file git does not track, which git rm otherwise " + "refuses, and with --cached touches no file") + + +def _guarded_move(variant: str, path: str) -> str: + """The backticked `mv` that moves a directory left at `variant`, a case + variant of the row's `path`, out of the holder, each test a POSIX shell + runs, dash and bash alike (#681, #839). + + `[ ! -L ]` FIRST, so a symbolic link there is always moved, + the link and never what it points at: `-ef` and `-e` read through a + link, so the guard kept one that points at the row's own directory, or + at a clone elsewhere, and `family.py add` then refused it as + `family-members-dir-variant`, a link on disk and untracked (w840's + candidate 1). Then `[ ! -e ]`, true where nothing is there, + a fresh clone's, where the bare `mv` failed `cannot stat`, exit 1, and + stopped what was typed after it (Codex's P2 2 on #828, met again here); + a link pointing nowhere, which `-e` takes for nothing, is a link, and + moved. `[ -ef ]`, true where the disk takes the two + names for one directory, the row's own; `-L` asks the last component + alone, so a `Members` that is itself a link to `members`, which + `tests/test_validate_family_remedies_typed.py` emulates a folding disk + with, still moves nothing. `[ -e /.git ]`, true for a + checkout.""" + quoted = shlex.quote(variant) + return (f"`[ ! -L {quoted} ] && {{ [ ! -e {quoted} ] || " + f"[ {quoted} -ef {shlex.quote(path)} ] || " + f"[ -e {shlex.quote(variant + '/.git')} ]; }} || " + f"mv -- {quoted} " + f"{shlex.quote('../' + variant.replace('/', '-'))}`") + + +def _first_line_flags(context: MoveContext) -> str: + """What the wrong-case move says of the flags its first line, the `git + rm`, carries, or nothing: `--sparse` (#762) and `-f` (#839).""" + said = [] + if context.sparse: + said.append("--sparse, which needs git 2.34 or later: a sparse " + f"checkout keeps {' and '.join(context.sparse)} off " + "disk, and without it git rm passes over what that keeps " + "off disk, exit 0, and removes nothing") + if context.forced: + said.append(f"-f, since the index stages " + f"{' and '.join(context.forced)} at a commit HEAD does " + f"not record there, over a checkout: {FORCED_SAID}") + return f" (the first with {', and with '.join(said)})" if said else "" + + def _entries_aside(path: str, entries: tuple[str, ...]) -> str: """What the wrong-case move says first where the index holds `entries` at or under the row's `path` (#762's (g)), or nothing: they go aside @@ -1949,7 +2044,8 @@ def _check_gitlinks_named(root: Path, gitlinks: dict[str, str], + _wrong_case_move(row_path, row.pin, variants, _move_context(root, row_path, variants, - row.repository))) + row.repository, + gitlinks))) continue if kind != STRAY: report.finding( @@ -1960,7 +2056,7 @@ def _check_gitlinks_named(root: Path, gitlinks: dict[str, str], report.finding( "member-gitlink-unnamed", f"{said}{_variant_said(row_path, row)} " - + _second_path_out(path, path in removed)) + + _second_path_out(root, path, commit, path in removed)) continue parts = path.split("/") project = parts[1] if len(parts) == 2 and parts[1] else None @@ -1978,14 +2074,31 @@ def _check_gitlinks_named(root: Path, gitlinks: dict[str, str], "did not match any files).") -def _second_path_out(path: str, staged: bool) -> str: - """What a `STRAY` gitlink, a second path to a member's own directory, - says to do: take it out of the index alone, or, where that is staged - already (#681), commit it.""" +def _second_path_out(root: Path, path: str, commit: str, + staged: bool) -> str: + """What a `STRAY` gitlink at `path`, a second path to a member's own + directory, which `_member_gitlinks` reads at `commit`, says to do: take + it out of the index alone, with `-f` where git refuses it without + (`_forced`, #839), or, where that is staged already (#681), commit it. + + A `git mv members/

Members/

`, staged, is this state: the index + holds the variant, and HEAD the row's own gitlink, whose removal is + staged (`member-gitlink-removal-staged`). A commit or an untracked file + in the checkout `git mv` moved, and the plain `git rm --cached` refused + (w761's candidate 2 on #828, measured on the gate's state).""" if staged: return f"{STAGED_REMOVAL} Commit that removal." - return ("Take it out of the index alone, which touches no file, and " - f"commit that:\n git rm --cached -- {shlex.quote(path)}") + head = _forced(root, path, commit) + if head is None: + return ("Take it out of the index alone, which touches no file, and " + f"commit that:\n git rm --cached -- {shlex.quote(path)}") + return ("Take it out of the index alone, which touches no file" + + (", and commit that. The index stages it at a commit HEAD does " + "not record there" if head else + ": HEAD records nothing there, so nothing of it is left to " + "commit. The index stages it") + + f" over a checkout, so the line takes -f: {FORCED_SAID}:" + f"\n git rm --cached -f -- {shlex.quote(path)}") def _mounted_by_hand(root: Path, path: str, project: str | None, @@ -2132,7 +2245,8 @@ def _section_lines(path: str, repository, def _check_gitlinks_registered(root: Path, members: list, gitlinks: dict[str, str], - report: Report) -> set[str]: + report: Report, + refused: bool = False) -> set[str]: """Every row's gitlink is at a path a section of HEAD's `.gitmodules` gives — the `member-gitmodules-absent` finding (#611, #407's decision 3). Returns the paths it was made for, of which `_check_member` asks no @@ -2166,6 +2280,16 @@ def _check_gitlinks_registered(root: Path, members: list, submodule update --init` dies there (`No url found for submodule path`, exit 128) before it checks out any member, and no mode named it. A path more than one section gives is `gitmodules-path-twice`'s. + + WHERE THE WORK TREE'S FILE IS GONE FROM DISK, the first of these + findings puts it back first (`_gitmodules_put_back`), and no other + does, nor any where `.gitmodules` gives a value git dies on + (`refused`), whose `gitmodules-value-refused` puts HEAD's back first + (#768): `git config -f` makes a file of the lines it is given, so + `member-gitmodules-absent`'s lines, staged and committed, dropped every + other section (rv820's review, comment 6237354577 on #839), and a + second `member-gitmodules-url-absent`'s put the file back over the url + the first had written (#839). """ head = _head_gitmodules(root) if head is None: @@ -2173,6 +2297,7 @@ def _check_gitlinks_registered(root: Path, members: list, held, sections = head given = Counter(keys["path"] for keys in sections.values() if "path" in keys) + back = ("", "") if refused else _gitmodules_put_back(root)[1:] unregistered: set[str] = set() for row in members: project = row.get("project") if isinstance(row, dict) else None @@ -2184,35 +2309,59 @@ def _check_gitlinks_registered(root: Path, members: list, continue if path in given: if given[path] == 1 and _url_absent(root, path, gitlinks[path], - row, sections, report): + row, sections, report, back): unregistered.add(path) + back = ("", "") continue unregistered.add(path) - lines = _section_lines(path, row.get("repository"), sections) - report.finding( - "member-gitmodules-absent", - f"{path} is a gitlink at {gitlinks[path]} that a row of members: " - "names, and " - + (f"no section of HEAD's {GITMODULES} gives that path" if held - else f"HEAD holds no {GITMODULES}") - + ", so a recursive clone of this holder checks out nothing " - "there, and neither does make bootstrap in that clone, whatever " - "checkout this holder has of it. Write its section back by hand, " - "in a POSIX shell, as family.py add writes one, then stage and " - f"commit {GITMODULES}:{lines}") + report.finding("member-gitmodules-absent", _absent_said( + path, gitlinks[path], held, + _section_lines(path, row.get("repository"), sections), back)) + back = ("", "") return unregistered +def _absent_said(path: str, commit: str, held: bool, lines: str, + back: tuple[str, str]) -> str: + """`member-gitmodules-absent`'s text for the gitlink at `path`, at + `commit`, where `held` says whether HEAD holds a `.gitmodules`, with + `lines` (`_section_lines`) under it, after the line that puts the work + tree's file back where `back` gives one (`_back_first`, #839).""" + return ( + f"{path} is a gitlink at {commit} that a row of members: names, and " + + (f"no section of HEAD's {GITMODULES} gives that path" if held + else f"HEAD holds no {GITMODULES}") + + ", so a recursive clone of this holder checks out nothing there, " + "and neither does make bootstrap in that clone, whatever checkout " + "this holder has of it. Write its section back by hand, in a POSIX " + "shell, as family.py add writes one, then stage and commit " + f"{GITMODULES}{_back_first(back)}{lines}") + + +def _back_first(back: tuple[str, str]) -> str: + """The end of the sentence before a finding's `git config -f` lines, + with the line that puts the work tree's `.gitmodules` back first where + `back` is `(that line, whose file it puts back)`, or a colon where it is + empty (#762, #839).""" + put_back, whose = back + if not put_back: + return ":" + return (f", which is gone from the work tree: the first line puts back " + f"{whose}, so that every other section stays:\n {put_back}") + + def _url_absent(root: Path, path: str, commit: str, row: dict, - sections: dict[str, dict[str, str]], report: Report) -> bool: + sections: dict[str, dict[str, str]], report: Report, + back: tuple[str, str]) -> bool: """`member-gitmodules-url-absent` where the one section of HEAD's `.gitmodules` (`sections`) that gives `path` gives no url (#762); True where it was made. The url written is the one `family.py add` writes, `https://github.com/` and the row's `repository:`, or `` where that is not `/`, as `_section_lines` writes it. Where the - work tree's file is gone from disk, a line before it puts that back - (`_gitmodules_put_back`), since `git config -f` would make a file of - that one key, and staged it dropped every other section.""" + work tree's file is gone from disk, `back` is the line before it that + puts that back (`_gitmodules_put_back`), since `git config -f` would + make a file of that one key, and staged it dropped every other + section, and empty where an earlier finding put it back (#839).""" name = next(name for name, keys in sections.items() if keys.get("path") == path) if "url" in sections[name]: @@ -2221,7 +2370,6 @@ def _url_absent(root: Path, path: str, commit: str, row: dict, url = (shlex.quote(f"https://github.com/{repository}.git") if isinstance(repository, str) and QUALIFIED_RE.match(repository) else "") - _, put_back, whose = _gitmodules_put_back(root) report.finding( "member-gitmodules-url-absent", f"{path} is a gitlink at {commit} that a row of members: names, and " @@ -2231,11 +2379,8 @@ def _url_absent(root: Path, path: str, commit: str, row: dict, "path, exit 128) before it checks out any member, as make " "bootstrap's does, and a recursive clone checks out nothing there. " "Write its url back by hand, in a POSIX shell, as family.py add " - f"writes one, then stage and commit {GITMODULES}" - + (f", which is gone from the work tree: the first line puts back " - f"{whose}, so that every other section stays:\n {put_back}" - if put_back else ":") - + f"\n git config -f {GITMODULES} " + f"writes one, then stage and commit {GITMODULES}{_back_first(back)}" + f"\n git config -f {GITMODULES} " f"{shlex.quote(f'submodule.{name}.url')} {url}") return True @@ -2524,7 +2669,7 @@ def _check_members(root: Path, members: list, no_members: bool, _check_gitlinks_named(root, gitlinks, named, removed, report) _check_gitmodules_named(sections, gitlinks, named, removed, report) unregistered = _check_gitlinks_registered(root, members, gitlinks, - report) + report, refused) unregistered |= _check_removals_staged(root, members, gitlinks, removed, as_read, report) if no_members: diff --git a/tests/test_bootstrap_spelling.py b/tests/test_bootstrap_spelling.py index c3472892..ef4ec397 100644 --- a/tests/test_bootstrap_spelling.py +++ b/tests/test_bootstrap_spelling.py @@ -700,8 +700,9 @@ def test_a_members_row_and_the_make_target_are_spelled(holder, tmp_path): assert (members / "Make" / "ran").exists(), \ "the member's command was handed the escaped target, not the raw one" assert err.strip().endswith( - r"family bootstrap FAILED: Fa\x1bil (make ta\u2028rget.py exited 2)" - ), err + r"family bootstrap FAILED: Fa\x1bil (make ta\u2028rget.py exited 2 " + "and printed no `Error ` line for a command of the member's: " + "make's own message above says what failed)"), err assert not {"\x1b", "\u2028"} & set(out), out diff --git a/tests/test_family_head_lost_gitmodules.py b/tests/test_family_head_lost_gitmodules.py index 09ccbe74..3e94859e 100644 --- a/tests/test_family_head_lost_gitmodules.py +++ b/tests/test_family_head_lost_gitmodules.py @@ -657,21 +657,50 @@ def test_a_row_written_twice_is_one_finding_per_path(holder, offline, # --- 574/2: a gitlink recorded only at a case variant of the row's path ----- +#: What the move's first line says of its `-f` (#839), as `family.py`'s +#: move back says it (#761). +FORCED_SAID = ("-f takes out a staged gitlink whose checkout has moved on or " + "holds a file git does not track, which git rm otherwise " + "refuses, and with --cached touches no file") + + +def guarded_move(variant: str, path: str) -> str: + """The guarded `mv` the move prints for a directory left at `variant`: + a symbolic link there always moved, nothing there kept, and the row's + own directory or a checkout kept (#681, #839).""" + return (f"`[ ! -L {variant} ] && {{ [ ! -e {variant} ] || [ {variant} " + f"-ef {path} ] || [ -e {variant}/.git ]; }} || mv -- {variant} " + f"../{variant.replace('/', '-')}`") + + +def first_line_flags(sparse: tuple, forced: tuple) -> str: + """What the move says of its first line's `--sparse` (#762) and `-f` + (#839), or nothing.""" + said = [text for given, text in ( + (sparse, "--sparse, which needs git 2.34 or later: a sparse checkout " + f"keeps {' and '.join(sparse)} off disk, and without it git " + "rm passes over what that keeps off disk, exit 0, and " + "removes nothing"), + (forced, f"-f, since the index stages {' and '.join(forced)} at a " + f"commit HEAD does not record there, over a checkout: " + f"{FORCED_SAID}")) if given] + return f" (the first with {', and with '.join(said)})" if said else "" + + def variant_refused(path: str, commit: str, variants: list[str], gitlinks: dict[str, str], *, sparse: tuple = (), aside: str = "", section: tuple = (), - section_said: str = "") -> list[str]: + section_said: str = "", forced: tuple = ()) -> list[str]: """`member-gitlink-absent`'s lines for a row whose gitlink is recorded only at `variants`. What the move meets beside them (#762): `sparse`, the variants a sparse checkout keeps off disk; `aside`, the sentence - for an entry in the way; and `section`, the lines that make - `.gitmodules` give `path`, with `section_said`, what they do.""" + for an entry in the way; `section`, the lines that make `.gitmodules` + give `path`, with `section_said`, what they do; and `forced`, the + variants the first line takes out with `-f` (#839).""" project = path.split("/", 1)[1] found = ", ".join(f"{variant} (at {gitlinks[variant]})" for variant in variants) - moved = " and ".join(f"`[ {variant} -ef {path} ] || [ -e {variant}/.git ] " - f"|| mv -- {variant} ../{variant.replace('/', '-')}`" - for variant in variants) + moved = " and ".join(guarded_move(variant, path) for variant in variants) status = " or ".join(f"`git -C {variant} status`" for variant in variants) unpushed = " or ".join(f"`git -C {variant} log --oneline HEAD --branches " "--not --remotes`" for variant in variants) @@ -686,10 +715,7 @@ def variant_refused(path: str, commit: str, variants: list[str], "row's path, at the commit the row pins, in the index alone, which " "touches no file, with the " + ("first two lines below" if section else "two lines below") - + (" (the first with --sparse, which needs git 2.34 or later: a " - f"sparse checkout keeps {' and '.join(sparse)} off disk, and " - "without it git rm passes over what that keeps off disk, exit 0, " - "and removes nothing)" if sparse else "") + + first_line_flags(sparse, forced) + (f"; the lines after them {section_said}, and stage .gitmodules. " "Commit that." if section else ", and commit that.") + " A directory left at " @@ -698,9 +724,11 @@ def variant_refused(path: str, commit: str, variants: list[str], "usually do and a Linux mount can (WSL's /mnt/c). Where the disk " "holds the two apart it stays there, untracked, and family.py add " "refuses it as family-members-dir-variant: move each one that is " - f"there out of the holder, in a POSIX shell, {moved}, which moves " - "nothing where the two names are one directory or where it holds a " - ".git entry, a checkout. A checkout git submodule add made holds a " + "there out of the holder, in a POSIX shell (Git Bash on Windows), " + f"{moved}, which moves nothing where nothing is there, where the two " + "names are one directory or where it holds a .git entry, a checkout, " + "and moves a symbolic link there wherever it points, the link and " + "never what it points at. A checkout git submodule add made holds a " ".git file naming its store by a path relative to it, which a move " "would leave pointing nowhere. Its commits are kept under this " "holder's .git/modules/, in the directory its .git file names, " @@ -718,7 +746,8 @@ def variant_refused(path: str, commit: str, variants: list[str], f"which that update points at {path}, so {status} reads {path} after " "it. The " + ("lines" if section else "two lines") + ":", " git rm -r --cached" + (" --sparse" if sparse else "") - + " --ignore-unmatch -- " + " ".join(variants), + + (" -f" if forced else "") + " --ignore-unmatch -- " + + " ".join(variants), f" git update-index --add --cacheinfo 160000,{commit},{path}", *(f" {line}" for line in section)] @@ -818,7 +847,7 @@ def case_sensitive_disk(tmp_path): #: The guarded `mv` the variant refusal gives for each variant, backticked: -#: two shell tests, then the move (#681). +#: the shell tests, then the move (#681, #839). GUARDED_MOVE = re.compile(r"`(\[ [^`]* \|\| mv -- [^`]*)`") diff --git a/tests/test_family_holder_bootstrap_texts.py b/tests/test_family_holder_bootstrap_texts.py new file mode 100644 index 00000000..2faa6988 --- /dev/null +++ b/tests/test_family_holder_bootstrap_texts.py @@ -0,0 +1,738 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The family holder's pinned `scripts/bootstrap.py` and +`scripts/validate-family.py`: seven lines of #839, each in the state it +names, each remedy typed as printed. + +THE ISSUE'S SEVEN LINES, each reproduced on #819's head (233a334e) before +this change, but where one already held there: + +- 1. `make bootstrap` told a person to set `SHAPE_LEGS_TOKEN` and, on the + next line, that no credential would check the members out: the advice was + printed for every failed `git submodule update`. It is printed only where + git asked for a credential, not where its init died at a gitlink it has + no url for or it read nothing from `.gitmodules`; beside the denial, it + is for the members the denial does not name. +- 2. A member's refusal and its finding both read `make bootstrap exited + 2`: make exits 2 for any command that fails, and for a failure of its + own. The line names the code of the member's command, read from make's + `Error ` line, or says make printed none. +- 3. In the gate's state after `git mv members/

Members/

`, staged, + with a commit or an untracked file in the checkout it moved, the printed + `git rm --cached -- Members/

` refused, exit 1 (`staged content + different from both the file and the HEAD`). It carries `-f`, with what + `-f` does, there and in the wrong-case move's `git rm`. +- 4. The wrong-case move's guarded `mv` kept a symbolic link at the variant + that points at the row's own directory or at a clone, so `family.py add` + then refused it; and with nothing at the variant, a fresh clone's, the + `mv` failed `cannot stat`, exit 1 (comment 6237599648 is the same fault). + A link is always moved, the link and never what it points at, and nothing + there moves nothing. +- 5. Comment 6237354577 (a): bootstrap read `.gitmodules` only from the + work tree. That HELD at 233a334e (#819's `gitmodules_source` reads the + index's, then HEAD's): the credential's skip here asks it again where + the work tree lost the file. +- 6. Comment 6237354577 (b): `member-gitmodules-absent`'s `git config -f` + lines, where the work tree's `.gitmodules` is gone, made a file of one + section, which staged and committed dropped every other. The first + finding that writes the file puts it back first, and no other does, + which a second `member-gitmodules-url-absent` did over the url the first + had written. + +THE SAME FAMILY, OFFLINE RULES AND HELPERS as +`tests/test_family_bootstrap_gate_order.py` (#648) and +`tests/test_validate_family_remedies_typed.py` (#681). A missing credential +is git's refusal of the file transport to the fixture's local remotes, as +there. A state that needs a directory or a link at a case variant is +skipped where the disk cannot hold one (`case_sensitive_disk`). +""" + +from __future__ import annotations + +import os +import re +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +from conftest import REPO, load_without_bytecode, rmtree, run_script +from test_family import FAMILY, SPARE, family, validate # noqa: F401 +from test_family_add_refusals import holder, offline # noqa: F401 +from test_family_bootstrap_gate_order import ( + NO_GITLINK, NO_SECTION, NO_URL, OUTSIDE_CURE, SECTION_CURE, STOPPED, + UNMERGED, UNMERGED_CURE, UNNAMED_CURE, UNNAMED_STOPS, VALIDATOR_CURE, + bootstrap, cloned, edited_clone, failed_on, fetching, not_fetched, + skip_by_subset, type_line, variant, without_url, +) +from test_family_dry_run import isolated +from test_family_gitmodules_values import ( + DAMAGED, LOST, MEND_CURE, UNPARSED, refused_why, value_cure, with_line, +) +from test_family_head_lost_gitmodules import ( # noqa: F401 + FORCED_SAID, GUARDED_MOVE, absent, assert_passes_in_every_mode, + case_sensitive_disk, commit_all, committed_gitmodules, head_lost, + moved_as_printed, pinned, recorded_at, refused_at, staged_and_committed, + variant_refused, +) +from test_family_member_path import MEMBER, OTHER, raw +from test_family_remove_gitmodules import verb_args +from test_validate_family_remedies_typed import ( + MOVED_TO, VARIANT_PATH, mounted_at_the_variant, +) +from test_validate_family_typed_states import GONE, ROW, URL_ABSENT, gone +from test_validate_family_unnamed_mounts import ( + GITLINK_SAID, VARIANT, codes, commands, findings, spare_commits, + stage_gitlink, take_out_as_printed, +) + +# --- line 1: the credential advice ----------------------------------------- + +#: The advice (a) gives where git asked for a credential, alone and beside +#: the line that names members no credential would check out. +ADVICE = (" If {which} are PRIVATE, set SHAPE_LEGS_TOKEN (a fine-grained " + "PAT with contents:read on them) or export an App-minted " + "SHAPE_LEGS_APP_TOKEN, then re-run. What did arrive is " + "bootstrapped below.") +ALONE = ADVICE.format(which="the members") +BESIDE = ADVICE.format(which="the members the next line does not name") + +#: How each line bootstrap writes to stderr itself begins; git's own lines, +#: which (a) passes on under the first, begin otherwise. +OURS = (" FETCH INCOMPLETE ", " If ", " NOT CHECKED OUT, ", + " NO MEMBER UPDATED, ", "family bootstrap ") +FETCH_LINE = re.compile(r" FETCH INCOMPLETE \(git exit \d+\) using " + r"credential source 'none':") + + +def after_fetch_line(result) -> list[str]: + """Bootstrap's own stderr lines, which must begin with the fetch's, + after it: git's lines between are its own.""" + ours = [line for line in result.stderr.splitlines() + if line.startswith(OURS)] + assert ours, result.stderr + assert FETCH_LINE.fullmatch(ours[0]), result.stderr + return ours[1:] + + +def unmerged_gitmodules(root: Path, env: dict) -> None: + """`.gitmodules` held in the index only at a merge's three conflict + stages, each the blob it held, as a conflicted merge leaves it.""" + blob = raw(root, env, "rev-parse", ":.gitmodules").stdout.decode( + "ascii").strip() + raw(root, env, "update-index", "--index-info", stdin=( + f"0 {'0' * 40}\t.gitmodules\n" + + "".join(f"100644 {blob} {stage}\t.gitmodules\n" + for stage in (1, 2, 3))).encode("ascii")) + + +def plain_clone_of(set_up): + """A state `set_up` makes in the holder, then a plain clone of it.""" + def made(holder: Path, env: dict, family, tmp_path: Path) -> Path: + set_up(holder, env) + return cloned(holder, env, family, tmp_path, recurse=False) + return made + + +def edited(edit): + """HEAD's `.gitmodules` put through `edit` and committed, then a plain + clone of it.""" + def made(holder: Path, env: dict, family, tmp_path: Path) -> Path: + return edited_clone(holder, env, family, tmp_path, edit)[0] + return made + + +def unmerged_in_a_clone(holder: Path, env: dict, family, + tmp_path: Path) -> Path: + clone = cloned(holder, env, family, tmp_path, recurse=False) + unmerged_gitmodules(clone, env) + return clone + + +def unnamed_stop(name: str): + """rv648's S1 or H1, then a plain clone of it.""" + def made(holder: Path, env: dict, family, tmp_path: Path) -> Path: + UNNAMED_STOPS[name](holder, env, family) + return cloned(holder, env, family, tmp_path, recurse=False) + return made + + +def outside(holder: Path, env: dict, family, tmp_path: Path) -> Path: + """rv648's H2: a gitlink outside members/, then a plain clone of it.""" + stage_gitlink(holder, f"vendor/{SPARE}", spare_commits(family)[1]) + commit_all(holder, env, "A gitlink mounted by hand outside members/") + return cloned(holder, env, family, tmp_path, recurse=False) + + +ROWS = (f"members/{MEMBER}", f"members/{OTHER}") +STOP_AT_OTHER = STOPPED.format(path=f"members/{OTHER}") + +#: `(a plain clone in the state, why (a) says each row is no credential's, +#: what it says cures them)`: every reason `unfetchable` gives where git +#: exits non-zero (#648, #768). Git asked for no credential in any. +NO_CREDENTIAL_ASKED = { + "head-lost": (plain_clone_of(head_lost), (NO_SECTION, NO_SECTION), + VALIDATOR_CURE), + "variant": (plain_clone_of(variant), + (STOPPED.format(path=f"Members/{OTHER}"), NO_GITLINK), + f"{VALIDATOR_CURE} {UNNAMED_CURE}"), + "no-url": (edited(lambda data: without_url(data, OTHER)), + (STOP_AT_OTHER, NO_URL), + SECTION_CURE.format(path=f"members/{OTHER}")), + "unmerged": (unmerged_in_a_clone, (UNMERGED, UNMERGED), UNMERGED_CURE), + "value-refused": (edited(with_line("update = None")), + (refused_why("update", "None"),) * 2, + value_cure("update")), + "unparsed": (edited(lambda data: data + DAMAGED["conflict-marker"]), + (UNPARSED, UNPARSED), MEND_CURE), + "stray": (unnamed_stop("stray"), + (STOPPED.format(path=f"Members/{OTHER}"),) * 2, UNNAMED_CURE), + "hand-mount": (unnamed_stop("hand-mount"), + (STOPPED.format(path=f"members/{SPARE}"),) * 2, + UNNAMED_CURE), + "outside": (outside, (STOPPED.format(path=f"vendor/{SPARE}"),) * 2, + OUTSIDE_CURE.format(path=f"vendor/{SPARE}")), +} + + +@pytest.mark.parametrize("make, whys, cure", NO_CREDENTIAL_ASKED.values(), + ids=NO_CREDENTIAL_ASKED) +def test_no_advice_is_printed_beside_its_denial(family, holder, offline, + tmp_path, make, whys, cure): + """LINE 1, each reason a row is no credential's. Git died in its init, + or reading `.gitmodules`, and fetched nothing, so (a)'s line says both + members are no credential's, and the advice to set one printed just + above it said the opposite. Bootstrap's own lines on stderr are now the + fetch's, the denial and the failure, exactly.""" + env = fetching(isolated(offline, tmp_path)) + clone = make(holder, env, family, tmp_path) + + result = bootstrap(clone, env, "--skip-members") + + assert result.returncode == 1, result.stdout + result.stderr + assert after_fetch_line(result) == [ + not_fetched(*zip(ROWS, whys), cure=cure), failed_on(MEMBER, OTHER)] + + +def as_cloned(root: Path, env: dict) -> None: + """The plain clone, as it was cloned.""" + + +#: A plain clone, and one whose work tree lost `.gitmodules`, not +#: committed, where git reads the index's or HEAD's (comment (a)). +CREDENTIAL_S = {"as-cloned": as_cloned, **LOST} + + +@pytest.mark.parametrize("lose", CREDENTIAL_S.values(), ids=CREDENTIAL_S) +def test_the_credential_s_skip_prints_the_advice_alone(family, holder, + offline, tmp_path, + lose): + """LINE 1's other half, and comment (a): a plain clone with no + credential for the members, where git asked for one and was refused. + The advice stands, alone, and the run ends ok. Where the work tree lost + `.gitmodules`, git reads the index's, or HEAD's, and bootstrap reads the + file git reads (#819), so the members are still the credential's, not + `no section of .gitmodules gives that path`.""" + env = isolated(offline, tmp_path) + clone = cloned(holder, env, family, tmp_path, recurse=False) + lose(clone, env) + + result = bootstrap(clone, env, "--skip-members") + + assert result.returncode == 0, result.stdout + result.stderr + assert after_fetch_line(result) == [ALONE] + assert result.stdout.endswith("\nfamily bootstrap ok\n"), result.stdout + + +def test_a_leg_the_credential_s_keeps_the_advice_with_every_member_out( + family, holder, offline, tmp_path): + """No misfire: every member checked out, and their legs refused for + want of a credential. No row is left out, so no line denies anything, + and the advice, which is for the legs too, stands.""" + env = isolated(offline, tmp_path) + clone = cloned(holder, env, family, tmp_path, recurse=False) + raw(clone, fetching(env), "submodule", "update", "--init") + assert not (clone / ROW / "spec" / ".git").exists(), "premise: no legs" + + result = bootstrap(clone, env, "--skip-members") + + assert result.returncode == 0, result.stdout + result.stderr + assert after_fetch_line(result) == [ALONE] + + +def test_beside_a_denial_the_advice_is_for_the_members_it_does_not_name( + family, holder, offline, tmp_path): + """LINE 1, both kinds at once: a clone that chose MEMBER alone, whose + fetch git refuses for want of a credential, and OTHER's section with no + url, which git skips there and no credential cures. The advice, then + the denial for OTHER, the advice saying it is for the members the next + line does not name.""" + env = isolated(offline, tmp_path) + clone = edited(lambda data: without_url(data, OTHER))( + holder, env, family, tmp_path) + env = skip_by_subset(clone, env) + + result = bootstrap(clone, env, "--skip-members") + + assert result.returncode == 1, result.stdout + result.stderr + assert after_fetch_line(result) == [ + BESIDE, + not_fetched((f"members/{OTHER}", NO_URL), + cure=SECTION_CURE.format(path=f"members/{OTHER}")), + failed_on(OTHER)] + + +# --- line 2: a member's finding, its refusal, and make's own failure -------- + +#: `{project: its Makefile}`, every recipe for both targets (#839's line 2). +MAKEFILES = { + "Find": b"bootstrap validate:\n\t@echo 'Find: a finding' >&2; exit 1\n", + "Fine": b"bootstrap validate:\n\t@true\n", + "Refuse": b"bootstrap validate:\n\t@echo 'Refuse: refused' >&2; exit 2\n", + "NoRule": b"other:\n\t@true\n", +} + + +@pytest.mark.parametrize("target", ["bootstrap", "validate"]) +def test_a_finding_a_refusal_and_make_s_own_failure_are_told_apart( + holder, tmp_path, target): + """LINE 2. Four members, each checked out, whose `make ` runs a + command that exits 1 (a finding, as a member's `bootstrap FAILED`), one + that exits 0, one that exits 2 (a refusal), and none at all, make + failing on its own (`No rule to make target`). Make exits 2 for all + three failures, and the line said `exited 2` for each; it names the + command's own code now, read from make's `Error ` line, and says + so where make printed none. Make's own lines, and the members', reach + stderr as they did. `--members-make validate`, which has no script of + its own to call, the same.""" + if shutil.which("make") is None: + pytest.skip("no make on PATH") + (holder / "family.yaml").write_bytes( + b"name: InkRouter\nmembers:\n" + b"".join( + f" - project: {project}\n path: members/{project}\n" + .encode("ascii") for project in MAKEFILES)) + for project, makefile in MAKEFILES.items(): + (holder / "members" / project / ".git").mkdir(parents=True) + (holder / "members" / project / "Makefile").write_bytes(makefile) + + result = run_script(holder / "scripts" / "bootstrap.py", "--no-fetch", + "--members-make", target, cwd=holder, + env={"MAKE": ""}) + + assert result.returncode == 1, result.stdout + result.stderr + errors = result.stderr.splitlines() + assert errors[-1] == ( + f"family bootstrap FAILED: Find (make {target}: the member's command " + f"exited 1), Refuse (make {target}: the member's command exited 2), " + f"NoRule (make {target} exited 2 and printed no `Error ` line " + "for a command of the member's: make's own message above says what " + "failed)"), result.stderr + for said in ("Find: a finding", "Refuse: refused", + f"No rule to make target '{target}'"): + assert any(said in line for line in errors[:-1]), (said, errors) + assert f" --- Fine: make {target} ---" in result.stdout.splitlines() + + +# --- line 3: `-f` where git rm --cached refuses ------------------------------ + +#: What lands in the checkout `git mv` moved, as w761 measured it on #828. +WORK = {"commit": lambda checkout, env: raw( + checkout, env, "commit", "-q", "--allow-empty", "-m", + "Work there"), + "untracked": lambda checkout, env: ( + checkout / "scratch.txt").write_bytes(b"mine\n")} + +#: The `STRAY` finding for `Members/

`, staged where HEAD records nothing. +STAGED_STRAY = ( + GITLINK_SAID + VARIANT + " Take it out of the index alone, which touches " + "no file: HEAD records nothing there, so nothing of it is left to commit. " + "The index stages it over a checkout, so the line takes -f: " + + FORCED_SAID + ":\n git rm --cached -f -- {path}") + + +@pytest.mark.parametrize("work", WORK.values(), ids=WORK) +def test_a_git_mv_staged_with_work_in_the_checkout_is_undone_as_printed( + family, holder, offline, tmp_path, case_sensitive_disk, work): + """LINE 3, reproduced on the gate's own state as the issue asks: `mkdir + Members && git mv members/IRRS Members/IRRS`, staged, then a commit, or + an untracked file, in the checkout it moved. The gate names the + variant, a second path to the member's directory, and the row's own + gitlink, whose removal is staged. Its `git rm --cached` refused there, + exit 1, in every mode; it carries `-f` now and says what it does, and + every line printed, typed as printed, puts the member back as HEAD has + it, nothing left to commit. The gate then passes in every mode, and + the checkout's commit is still in the member's store.""" + env = fetching(isolated(offline, tmp_path)) + commit = pinned(holder, MEMBER) + (holder / "Members").mkdir() + raw(holder, env, "mv", ROW, VARIANT_PATH) + work(holder / VARIANT_PATH, env) + stray = STAGED_STRAY.format(path=VARIANT_PATH, commit=commit, + row_path=ROW, project=MEMBER) + + for extra in ((), ("--pins",), ("--no-members",)): + found = validate(holder, *extra) + assert found.returncode == 1, (extra, found.stdout + found.stderr) + assert codes(found) == ["member-gitlink-unnamed", + "gitmodules-path-unnamed", + "member-gitlink-removal-staged"], extra + assert findings(found)[0] == stray, extra + every = [line for finding in findings(validate(holder)) + for line in commands(finding)] + head = raw(holder, env, "rev-parse", "--verify", "--quiet", + f"HEAD:{VARIANT_PATH}", check=False) + for line in every: + type_line(holder, env, line) + + assert head.returncode != 0, "premise: HEAD records nothing there" + assert raw(holder, env, "diff", "--cached", "--quiet", + check=False).returncode == 0, "nothing left to commit" + assert_passes_in_every_mode(holder, 2) + assert raw(holder, env, "rev-parse", f"HEAD:{ROW}").stdout.decode( + "ascii").strip() == commit + + +def test_a_staged_second_path_with_nothing_on_disk_takes_no_f( + holder, offline, tmp_path): + """No misfire: a second path to MEMBER's directory, `Members/IRRS`, + staged and not committed, and nothing on disk there. Git takes the + gitlink out with no `-f`, so the line prints none; typed as printed, + the gate passes in every mode.""" + env = isolated(offline, tmp_path) + commit = pinned(holder, MEMBER) + stage_gitlink(holder, VARIANT_PATH, commit) + + (finding,) = findings(validate(holder, "--no-members")) + + assert finding == ( + GITLINK_SAID.format(path=VARIANT_PATH, commit=commit) + + VARIANT.format(row_path=ROW, project=MEMBER) + + " Take it out of the index alone, which touches no file, and " + f"commit that:\n git rm --cached -- {VARIANT_PATH}") + type_line(holder, env, commands(finding)[0]) + assert_passes_in_every_mode(holder, 2) + + +def test_a_checkout_moved_on_at_the_variant_is_moved_with_f( + family, holder, offline, tmp_path, case_sensitive_disk): + """LINE 3, the wrong-case move's own `git rm`: MEMBER recorded only at + `Members/IRRS`, by a committed `git submodule add`, then a commit in + that checkout staged in the holder, and an untracked file left in it. + The index stages the variant at a commit HEAD does not record there, + over a checkout that holds a file git does not track, so the plain + `git rm -r --cached` refused, exit 1, and the move stopped there. It + carries `-f` and says why; typed as printed, with the commit, the + section `git submodule add` wrote taken out as printed, and the + checkout deleted by hand as the text says, the gate passes in every + mode.""" + env = fetching(isolated(offline, tmp_path)) + commit = pinned(holder, MEMBER) + checkout = holder / VARIANT_PATH + mounted_at_the_variant(family, holder, env, commit, "submodule") + raw(checkout, env, "commit", "-q", "--allow-empty", "-m", "Work there") + work = raw(checkout, env, "rev-parse", "HEAD").stdout.decode().strip() + raw(holder, env, "add", "--", VARIANT_PATH) + (checkout / "scratch.txt").write_bytes(b"mine\n") + + refused = validate(holder) + + assert refused.returncode == 2, refused.stdout + refused.stderr + lines = refused.stderr.splitlines() + assert lines == variant_refused(ROW, commit, [VARIANT_PATH], + {VARIANT_PATH: work}, + forced=(VARIANT_PATH,)) + moved_as_printed(holder, env, lines, [VARIANT_PATH]) + left = validate(holder) + assert codes(left) == ["gitmodules-path-unnamed"], left.stderr + take_out_as_printed(holder, findings(left)[0]) + rmtree(checkout) + assert_passes_in_every_mode(holder, 2) + store = holder / ".git" / "modules" / "Members" / MEMBER + assert raw(holder, env, f"--git-dir={store}", f"--work-tree={holder}", + "cat-file", "-e", work, check=False).returncode == 0, ( + "the commit is kept") + + +# --- line 4: what the guarded `mv` moves ------------------------------------- + +def link_to_the_row(holder: Path, tmp_path: Path) -> Path | None: + """A link to the row's own directory. Returns what a link there + points at, which must be there after the move.""" + (holder / "Members").mkdir() + os.symlink(f"../{ROW}", holder / VARIANT_PATH, target_is_directory=True) + return holder / ROW + + +def link_to_a_clone(holder: Path, tmp_path: Path) -> Path | None: + """A link to a clone beside the holder, a directory holding a `.git` + file.""" + clone = tmp_path / f"{MEMBER}-clone" + clone.mkdir() + (clone / ".git").write_bytes(b"gitdir: ../nowhere\n") + (holder / "Members").mkdir() + os.symlink(str(clone), holder / VARIANT_PATH, target_is_directory=True) + return clone + + +def dangling_link(holder: Path, tmp_path: Path) -> Path | None: + (holder / "Members").mkdir() + os.symlink("nowhere", holder / VARIANT_PATH) + return None + + +def nothing(holder: Path, tmp_path: Path) -> Path | None: + """Nothing at the variant, as a fresh clone of a holder whose index + records only the variant leaves it after the move.""" + return None + + +LAYOUTS = {"link-to-the-row": link_to_the_row, + "link-to-a-clone": link_to_a_clone, + "dangling-link": dangling_link, + "nothing": nothing} + + +@pytest.mark.parametrize("shell", ["sh", "bash"]) +@pytest.mark.parametrize("layout", LAYOUTS.values(), ids=LAYOUTS) +def test_a_link_or_nothing_at_the_variant_is_typed_as_printed( + family, holder, offline, tmp_path, case_sensitive_disk, layout, + shell): + """LINE 4 and comment 6237599648, in w840's state: MEMBER's gitlink + held only at `Members/IRRS`, committed, its checkout at its row's path. + After the move's two lines and their commit, what is at the variant, + then the guarded `mv` typed as printed, in dash (this host's `sh`) and + in bash. A link there, to the row's own directory, to a clone, or to + nowhere, is moved, the link and never what it points at: the guard + kept the first two, reading through them, and `family.py add` then + refused the link, on disk and untracked. Nothing there moves nothing + and exits 0, where the bare `mv` failed `cannot stat`, exit 1. The gate + then passes in every mode, and SPARE is added.""" + if shutil.which(shell) is None: + pytest.skip(f"no {shell} on PATH") + env = fetching(isolated(offline, tmp_path)) + commit = recorded_at(holder, env, MEMBER, VARIANT_PATH) + lines = refused_at(holder, commit, [VARIANT_PATH]) + moved_as_printed(holder, env, lines, [VARIANT_PATH]) + try: + pointed = layout(holder, tmp_path) + except OSError as exc: + pytest.skip(f"no symbolic link here: {exc}") + link = os.path.islink(holder / VARIANT_PATH) + target = os.readlink(holder / VARIANT_PATH) if link else None + (move,) = GUARDED_MOVE.findall(lines[1]) + + typed = subprocess.run([shell, "-c", move], cwd=str(holder), + capture_output=True, check=False) + + assert typed.returncode == 0, typed.stderr + assert not os.path.lexists(holder / VARIANT_PATH) + moved = tmp_path / MOVED_TO + assert os.path.islink(moved) is link + if link: + assert os.readlink(moved) == target + assert pointed is None or pointed.is_dir() + assert (holder / ROW / "project.yaml").is_file() + assert_passes_in_every_mode(holder, 2) + added = run_script(FAMILY, *verb_args("add", holder, family), env=env) + assert added.returncode == 0, added.stdout + added.stderr + assert_passes_in_every_mode(holder, 3) + + +# --- comment (b): the work tree's .gitmodules put back, once --------------- + +def put_back_first(finding: str, how: str) -> str: + """`finding` with the line that puts the work tree's `.gitmodules` + back from where git reads it (`GONE`), as its first command.""" + whose, line = GONE[how] + said = "then stage and commit .gitmodules:\n" + assert finding.count(said) == 1, finding + return finding.replace( + said, "then stage and commit .gitmodules, which is gone from the " + f"work tree: the first line puts back {whose}, so that every other " + f"section stays:\n {line}\n") + + +def typed_and_committed(root: Path, env: dict, result) -> None: + """Every command every finding prints, typed as printed, then + `.gitmodules` staged and committed, as each says.""" + for finding in findings(result): + for line in commands(finding): + type_line(root, env, line) + staged_and_committed(root, env, "Written back as printed") + + +@pytest.mark.parametrize("how", GONE) +def test_a_section_absent_from_head_is_written_into_the_file_put_back( + holder, offline, tmp_path, how): + """COMMENT (b): OTHER's section taken out of HEAD's `.gitmodules`, and + the work tree's file gone, deleted or its deletion staged. The finding + printed the `git config -f` lines alone, which made a file of OTHER's + section, so staged and committed HEAD lost MEMBER's: a second round. + The line that puts the file back from where git reads it comes first + now, in every mode; typed as printed and committed, HEAD keeps + MEMBER's section, and the gate passes in every mode.""" + env = isolated(offline, tmp_path) + data = raw(holder, env, "show", "HEAD:.gitmodules").stdout + committed_gitmodules(holder, env, section_taken_out(data, OTHER)) + gone(holder, env, how) + expected = put_back_first( + absent(OTHER, pinned(holder, OTHER), held=True), how) + + for extra in ((), ("--pins",), ("--no-members",)): + result = validate(holder, *extra) + assert result.returncode == 1, (extra, result.stdout + result.stderr) + assert findings(result) == [expected], extra + + typed_and_committed(holder, env, validate(holder)) + assert f'[submodule "{ROW}"]'.encode() in raw( + holder, env, "show", "HEAD:.gitmodules").stdout + assert_passes_in_every_mode(holder, 2) + + +def section_taken_out(data: bytes, project: str) -> bytes: + """`.gitmodules` bytes with `project`'s section taken out by hand.""" + section = re.compile(rb'\[submodule "members/' + project.encode() + + rb'"\]\n(?:\t[^\n]*\n)*') + return section.sub(b"", data) + + +#: `(the edit to HEAD's .gitmodules, each finding as `(url-less?, project)` +#: in the order printed)`: two findings that each write the file. +TWO_WRITE = { + "two-url-less": (lambda data: without_url(without_url(data, MEMBER), + OTHER), + ((True, MEMBER), (True, OTHER))), + "absent-then-url-less": (lambda data: without_url( + section_taken_out(data, MEMBER), OTHER), + ((False, MEMBER), (True, OTHER))), + "url-less-then-absent": (lambda data: section_taken_out( + without_url(data, MEMBER), OTHER), + ((True, MEMBER), (False, OTHER))), +} + + +@pytest.mark.parametrize("edit, made", TWO_WRITE.values(), ids=TWO_WRITE) +def test_two_findings_that_write_the_file_put_it_back_once( + holder, offline, tmp_path, edit, made): + """COMMENT (b)'s ORDER: two findings that write `.gitmodules`, with the + work tree's gone. Each `member-gitmodules-url-absent` put the file back + first, so the second put it back over the url the first had written, + and a `member-gitmodules-absent` before one had its section put back + over the same way: a second round. Only the first puts it back now; + typed as printed and committed, the gate passes in every mode.""" + env = isolated(offline, tmp_path) + data = raw(holder, env, "show", "HEAD:.gitmodules").stdout + committed_gitmodules(holder, env, edit(data)) + gone(holder, env, "deleted") + expected = [ + (URL_ABSENT.replace(OTHER, project).format( + commit=pinned(holder, project)) if url_less + else absent(project, pinned(holder, project), held=True)) + for url_less, project in made] + expected[0] = put_back_first(expected[0], "deleted") + + result = validate(holder, "--no-members") + + assert result.returncode == 1, result.stdout + result.stderr + assert findings(result) == expected + typed_and_committed(holder, env, result) + assert_passes_in_every_mode(holder, 2) + + +def test_a_value_finding_that_put_head_s_back_is_not_put_back_over( + holder, offline, tmp_path): + """No second put back after `gitmodules-value-refused`'s: MEMBER's + section given `update = None`, OTHER's taken out, committed, and the + work tree's file deleted. The value's finding puts HEAD's back first + (#768) and takes the key out; OTHER's `member-gitmodules-absent` then + writes into that file, and puts nothing back over the key it took + out. Typed as printed and committed, the gate passes in every mode.""" + env = isolated(offline, tmp_path) + data = raw(holder, env, "show", "HEAD:.gitmodules").stdout + line = f"\tpath = members/{MEMBER}\n".encode() + assert data.count(line) == 1, data + committed_gitmodules(holder, env, section_taken_out( + data.replace(line, line + b"\tupdate = None\n"), OTHER)) + gone(holder, env, "deleted") + + result = validate(holder, "--no-members") + + assert result.returncode == 1, result.stdout + result.stderr + assert codes(result) == ["gitmodules-value-refused", + "member-gitmodules-absent"] + assert commands(findings(result)[0])[0] == ( + "git checkout HEAD -- .gitmodules") + assert findings(result)[1] == absent(OTHER, pinned(holder, OTHER), + held=True) + typed_and_committed(holder, env, result) + assert_passes_in_every_mode(holder, 2) + + +# --- the guard's text, typed in both shells on every layout ---------------- + +#: `(what is at the variant, whether the guard moves it)` for the guard +#: alone, in a scratch tree: the issue's seven layouts. +GUARD_LAYOUTS = { + "nothing": (lambda root: None, False), + "dangling-link": (lambda root: os.symlink( + "nowhere", root / VARIANT_PATH), True), + "link-to-the-row": (lambda root: os.symlink( + f"../{ROW}", root / VARIANT_PATH), True), + "link-to-a-checkout": (lambda root: os.symlink( + "../../clone", root / VARIANT_PATH), True), + "Members-a-link-to-members": (None, False), + "a-directory-apart": (lambda root: (root / VARIANT_PATH).mkdir(), True), + "a-checkout-apart": (lambda root: ( + (root / VARIANT_PATH).mkdir(), + (root / VARIANT_PATH / ".git").write_bytes(b"gitdir: x\n")), False), +} + + +@pytest.fixture(scope="module") +def guarded_move(): + """The gate's own `_guarded_move`, loaded by path; `sys.path` as it + was.""" + kept = list(sys.path) + gate = load_without_bytecode( + REPO / "templates" / "family-root" / "scripts" / "validate-family.py", + "validate_family_holder_texts") + sys.path[:] = kept + return gate._guarded_move + + +@pytest.mark.parametrize("shell", ["sh", "bash"]) +@pytest.mark.parametrize("place, moves", GUARD_LAYOUTS.values(), + ids=GUARD_LAYOUTS) +def test_the_guard_moves_a_link_always_and_nothing_where_nothing_is( + tmp_path, case_sensitive_disk, guarded_move, shell, place, moves): + """The guarded `mv` the variant refusal prints, on its own, on the + issue's seven layouts, in dash and in bash: exit 0 on each, a link + always moved, the link itself; nothing there, `Members` itself a link + to `members` (how `tests/test_validate_family_remedies_typed.py` + emulates a folding disk), and a checkout apart kept.""" + if shutil.which(shell) is None: + pytest.skip(f"no {shell} on PATH") + root = tmp_path / "holder" + (root / ROW).mkdir(parents=True) + (root / ROW / ".git").write_bytes(b"gitdir: x\n") + (tmp_path / "clone").mkdir() + (tmp_path / "clone" / ".git").write_bytes(b"gitdir: y\n") + (move,) = GUARDED_MOVE.findall(guarded_move(VARIANT_PATH, ROW)) + try: + if place is None: + os.symlink("members", root / "Members", target_is_directory=True) + else: + (root / "Members").mkdir() + place(root) + except OSError as exc: + pytest.skip(f"no symbolic link here: {exc}") + there = os.path.lexists(root / VARIANT_PATH) + + typed = subprocess.run([shell, "-c", move], cwd=str(root), + capture_output=True, check=False) + + assert typed.returncode == 0, typed.stderr + assert os.path.lexists(tmp_path / MOVED_TO) is moves + assert os.path.lexists(root / VARIANT_PATH) is (there and not moves) + assert (root / ROW / ".git").is_file()