diff --git a/adopt-project.py b/adopt-project.py index aaf2c5bc..df01dc91 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -37,8 +37,9 @@ a directory it writes into, `.github`, `contracts` or `scripts`, or the source's own path at a copy it writes under `shape/`, in any case), and the source's own submodules kept on one leg with - their `.gitmodules`, registered there and never in the root. It - prints what will happen and changes nothing. + their `.gitmodules`, registered there and never in the root, each + relative url naming the same repository from the leg as from the + source. It prints what will happen and changes nothing. execute creates the two legs, extracts them with `git filter-repo`, makes the one split commit on a branch of the source, sets the `xf-project-` topic on all three (skipped for local remotes), @@ -51,7 +52,9 @@ right about `openspec/` and cannot be right about `examples/golden-run/` without knowing whether the specification cites it. `execute` REFUSES while any `leg:` is still null, and on every other finding `check` reports, before -it has created a leg repository or pushed anything. +it has created a leg repository or pushed anything -- a source submodule's +relative url resolved again from the leg remotes it is about to create, which +under `--local-remote-dir` are not the GitHub ones `check` resolves from. WHY `git filter-repo` AND NOT A VENDORED COPY. Extracting history correctly is a solved problem with one correct implementation, and a vendored copy of it @@ -100,7 +103,8 @@ COMMIT_RE, NEUTRAL_PRODUCT_OWNER, PROJECT_ID_RE, TREE_DIGEST_DEFINITION, VISIBILITY_CHOICES, NamingPolicy, Refusal, YamlError, accepts_role, checked_value, double_quoted, git_out, load_yaml, - parse_yaml, tree_digest, + parse_yaml, redacted, remote_is_a_path, remote_key, remote_local_path, + tree_digest, ) from shape_materialize import ( # noqa: E402 ADOPT_MAKEFILE_BLOCK, RULESET_HINT, SHAPE_REPOSITORY, @@ -334,6 +338,12 @@ def __contains__(self, path: str) -> bool: name = self._name_at.get(path) return name is not None and bool(self._url_of.get(name)) + def url(self, path: str) -> str: + """The url git reads for the submodule registered at `path`; empty + when it registers none there.""" + name = self._name_at.get(path) + return "" if name is None else self._url_of.get(name, "") + def why_not(self, path: str) -> str | None: """Why git registers nothing at `path` though an entry names it; None when no entry of the file does.""" @@ -2561,17 +2571,272 @@ def _kept_submodule_problems(kept: list, registered: Registrations, return problems -def _submodule_findings(plan: Plan, source: Source, - tree: list) -> tuple[list[str], list[str]]: - """`submodule_plan_problems` as `check` prints them, one FINDING each, - and what to print ONCE after every finding: SUBMODULE_REMEDIATION when - there is one of these, nothing when there is none. `check` is where the - person is told to fix the plan, so it says how, in the words `execute`'s - refusal would use one step later.""" +# --------------------------------------------------------------------------- +# A kept submodule's RELATIVE url, resolved from the leg that holds it (#190) +# --------------------------------------------------------------------------- +# +# git resolves a submodule url that begins `./` or `../` against the remote of +# the repository whose `.gitmodules` holds it: that repository's default +# remote, or, with none, its own directory (`git help submodule`). Before the +# split that is the SOURCE; after it, the file is in a LEG, so it is the leg's +# remote. `url = ../dependency` names a sibling of the source before and a +# sibling of the leg after, and on main the run said `adoption verified` and +# the assembly's `git clone --recurse-submodules` then failed wherever the +# dependency did not happen to sit beside the leg: under `--local-remote-dir`, +# or with an `--org` other than the source's. +# +# REFUSED, NEVER REWRITTEN. Writing the absolute url into the leg's +# `.gitmodules` would change a blob the verification accounts for by sha, in +# every commit of the leg's history that holds one, and put in the leg a url +# nobody committed. So a url whose two resolutions differ is a finding of +# `check` and a refusal of `execute` before any leg exists, and the person +# commits the absolute url in the source or drops the submodule. Where both +# name the same repository -- legs made in the source's own GitHub +# organisation, or on disk beside the dependency -- nothing changes, and the +# verification has nothing new to account for. +# +# ASKED OF GIT. Each resolution is `git submodule init`'s, run in a scratch +# repository whose `origin` is the remote in question: the code the +# assembly's recursive clone runs, not a copy of it. `repo_shape.join_remote` +# is a copy, and git's arithmetic differs from it where it matters here: git +# takes `..\` for a relative url on every platform and strips it on Windows +# alone, and it fails on a `..` too many where the copy consumes nothing. A +# url that is not relative is never resolved, because git copies it as it +# stands, so a source with no submodule, or none with a relative url, starts +# no git process here. + +RELATIVE_URL = "plan-submodule-relative-url" +#: git's own test for a url it resolves against a remote +#: (`starts_with_dot_slash`, `starts_with_dot_dot_slash`): `./` or `../`, +#: with either separator on every platform. +RELATIVE_URL_RE = re.compile(r"\.\.?[/\\]") +#: What a plan that keeps a relative url git resolves elsewhere from its leg +#: is told: by `check`, once after its findings, and by `execute`'s refusal. +RELATIVE_URL_REMEDIATION = ( + "Remediation for a relative submodule url: git resolves it against the " + "remote of the repository whose `.gitmodules` holds it -- in the " + "assembly, the leg -- so it must name the same repository from the leg " + "as from the source. Commit the submodule's absolute url in the source's " + "`.gitmodules` and re-run `plan`, or `drop` the submodule: its " + "registration then stays behind in the leg, inert. `check` resolves it " + "from the GitHub remotes the plan's `org:` names and `execute` from the " + "remotes it is about to create, before it creates any; under " + "`--local-remote-dir` those are bare repositories on disk, where " + "`check`'s `plan ok` settles nothing about it.") + + +class _Resolution: + """One relative url as git resolved it from one repository: `url`, or, + when git could not, `said`, which is what git said.""" + + def __init__(self, url: str | None, said: str = "") -> None: + self.url = url + self.said = said + + def same_as(self, other: "_Resolution") -> bool: + """Both resolved, to one repository however each is spelled. + + `remote_key` is the standard's one rule for when two spellings are + one repository: the source's `origin` may be + `git@github.com:Org/Thing.git` and a leg's is + `https://github.com/org/...`. It compares two filesystem paths by + their text alone, so they are then asked of this machine, whose + symlinks -- and the `..` of a relative answer joined to the source's + working tree (`_default_remote`) -- may make two spellings one + directory: the order `same_repository_here` asks them in, so the + disk can add an answer and never take one away. + """ + if self.url is None or other.url is None: + return False + if remote_key(self.url) == remote_key(other.url): + return True + paths = [remote_local_path(url) for url in (self.url, other.url)] + return None not in paths \ + and os.path.realpath(paths[0]) == os.path.realpath(paths[1]) + + def __str__(self) -> str: + # `redacted`: a source's remote may carry a token, and so does every + # url resolved against it. + return (f"to {_spelled(redacted(self.url))}" if self.url is not None + else f"not at all ({self.said})") + + +def _kept_relative_urls(entries: list, tree: list, + registered: Registrations | str + ) -> tuple[str | None, list[tuple[str, str, str]]]: + """`(leg, [(path, entry, url), ...])`: each submodule the plan keeps in + the leg its `.gitmodules` goes to, registered with a relative url, and + that leg. A submodule kept anywhere else, or not registered, is already + a finding of `submodule_plan_problems`; a dropped one is inert; and a + `.gitmodules` git cannot read has no url to read.""" + gitlinks = [path for path, mode, _, _ in tree if mode == GITLINK_MODE] + if not gitlinks or isinstance(registered, str): + return None, [] + answered = _answered_legs(entries, [GITMODULES, *gitlinks]) + leg = answered.get(GITMODULES, (None, None))[0] + return leg, [(path, entry, registered.url(path)) for path, kept, entry + in _kept_in_a_leg(answered, gitlinks) + if kept == leg and path in registered + and RELATIVE_URL_RE.match(registered.url(path))] + + +def _default_remote(source: Source) -> tuple[str, str, Path | None]: + """`(base, which, working_tree)`: what git resolves the source's relative + urls against, how a finding names it, and where the clone opens git's + answer when that answer is a relative path. git's rule -- the + checked-out branch's remote, else `origin`; that remote's url, else, + with none, the repository's own directory -- asked of the branch being + ADOPTED, which a clone of the source checks out, and not of whatever + branch this working copy happens to have checked out. + + A RELATIVE REMOTE GIVES A RELATIVE ANSWER. Against `../src/Thing.git`, + `git submodule init` resolves `../dependency` to `../src/dependency` and + keeps it so, and `git submodule update` opens that from the top of the + working tree. Compared as git leaves it, it equalled no leg's answer, + and a layout whose clone works was refused (#190, P3-2). So + `working_tree` is the source's own for such a remote -- a path, and not + an absolute one (`remote_local_path` names absolute paths only), which + is git's own test for the answers it leaves relative -- and None for + every other base.""" + name = _config_value(source.path, f"branch.{source.branch}.remote") \ + or "origin" + url = _config_value(source.path, f"remote.{name}.url") + if url is None: + return (source.path.as_posix(), + f"its own directory, as it has no remote {_spelled(name)}", + None) + if remote_is_a_path(url) and remote_local_path(url) is None: + return (url, f"its remote {_spelled(name)}, relative to its working " + "tree", source.path) + return url, f"its remote {_spelled(name)}", None + + +def _config_value(repository: Path, key: str) -> str | None: + """`key` in `repository`'s git config, as git reads it -- the last + value, when there are several; None when unset.""" + got = subprocess.run(["git", GIT_CONFIG, "-z", "--get", key], + cwd=repository, capture_output=True, check=False) + return None if got.returncode else _git_path(got.stdout.rstrip(b"\x00")) + + +def _resolved(base: str, urls: list[str], + working_tree: Path | None = None) -> list[_Resolution]: + """Each of `urls` as `git submodule init` resolves it in a repository + whose `origin` is `base`, in a scratch repository made for the purpose + and removed after it; and, given a `working_tree`, git's relative answer + joined to it, which is what the clone opens (`_default_remote`). A url + git cannot resolve is what git said: that is where the clone fails.""" + scratch = Path(tempfile.mkdtemp(prefix="openreposhape-url-")) + try: + run(["git", "init", "-q", str(scratch)]) + run(["git", GIT_CONFIG, "remote.origin.url", base], cwd=scratch) + for index, url in enumerate(urls): + for key, value in (("path", f"u{index}"), ("url", url)): + run(["git", GIT_CONFIG, "-f", GITMODULES, + f"submodule.u{index}.{key}", value], cwd=scratch) + # A gitlink names a commit nobody fetches here; any object id of the + # repository's own format will do, and this one is at hand. + oid = run(["git", "hash-object", GITMODULES], cwd=scratch) + for index in range(len(urls)): + run(["git", "update-index", "--add", "--cacheinfo", + f"{GITLINK_MODE},{oid},u{index}"], cwd=scratch) + # One `init` per url, because git stops at the first it cannot + # resolve and each finding names its own. + return [_resolution_of(scratch, f"u{index}", working_tree) + for index in range(len(urls))] + finally: + shutil.rmtree(scratch, ignore_errors=True) + + +def _resolution_of(scratch: Path, name: str, + working_tree: Path | None) -> _Resolution: + """`name`'s url, as `git submodule init` copies it into the config, and + joined to `working_tree` when there is one.""" + init = subprocess.run(["git", "submodule", "init", "-q", "--", name], + cwd=scratch, capture_output=True, check=False) + if init.returncode: + return _Resolution(None, _one_line(init.stderr.decode( + errors="replace"))) + url = _config_value(scratch, f"submodule.{name}.url") + if working_tree is None or url is None: + return _Resolution(url) + # Not normalised: `..` after a symlink is the disk's to resolve, and + # `same_as` asks the disk (`realpath`) for exactly that. + return _Resolution((working_tree / url).as_posix()) + + +def _relative_url_findings(plan: Plan, source: Source, tree: list, + registered: Registrations | str, + local_remote_dir: Path | None + ) -> tuple[list[str], list[str]]: + """`(findings, notes)` for the relative urls of the submodules the plan + keeps in a leg: a `plan-submodule-relative-url` FINDING for each that + git resolves to another repository from its leg than from the source, + or cannot resolve from one of them; and, when there is none of those, + the one NOTE `check` prints about what it could not know. + + THE LEG'S REMOTE IS THE ONE `execute` WOULD CREATE: under + `local_remote_dir`, the bare repository there; otherwise the GitHub + repository the plan's `org:` and leg name give. `check` takes no + `--local-remote-dir`, so it decides on GitHub's, and `execute` decides + again on the remotes it is about to create, before it creates any. They + are read only when there is a relative url to resolve, and the NOTE + says, where `check` resolved them all alike, that it was GitHub's it + asked: a plan it passes can still be refused by `execute + --local-remote-dir`.""" + leg, kept = _kept_relative_urls(plan.entries, tree, registered) + if not kept: + return [], [] + remote = _remote_urls(checked_value("org", plan.get("org")), plan.names(), + local_remote_dir, source)[leg] + base, which, working_tree = _default_remote(source) + urls = [url for _, _, url in kept] + found, alike = [], [] + for (path, entry, url), here, there in zip( + kept, _resolved(base, urls, working_tree), + _resolved(remote, urls)): + if here.same_as(there): + alike.append(f"{_spelled(url)} of {_spelled(path)}, {there}") + continue + found.append( + f"FINDING {RELATIVE_URL}: the submodule {_as_entry(path, entry)} " + f"has leg: {leg}, and its url in the source's {GITMODULES} is the " + f"relative {_spelled(url)}, which git resolves against the remote " + "of the repository holding that file: from the source, at " + f"{_spelled(redacted(base))} ({which}), {here}, and from the " + f"{leg} leg, at {_spelled(remote)}, {there}; the assembly's " + "recursive clone resolves it from the leg") + if found or local_remote_dir is not None: + return found, [] + return [], [ + f"NOTE `check` resolved the relative url of each submodule the plan " + f"keeps in the {leg} leg from that leg's GitHub remote, " + f"{_spelled(remote)}, and each names the same repository from there " + f"as from the source: {'; '.join(alike)}. `execute " + "--local-remote-dir` resolves it again from the bare repository it " + "would create instead, and refuses there, before it creates any leg, " + "a url that names another repository."] + + +def _submodule_findings(plan: Plan, source: Source, tree: list, + local_remote_dir: Path | None = None + ) -> tuple[tuple[list[str], list[str]], + tuple[list[str], list[str]], list[str]]: + """`(findings, remediation)` for `submodule_plan_problems`, one FINDING + each and SUBMODULE_REMEDIATION when there is one; the same for each + `_relative_url_findings` one, with RELATIVE_URL_REMEDIATION; and the + NOTEs `check` prints. A remediation is printed ONCE after every finding. + The two come back apart because `_entry_findings` lists the relative + urls last, after the values #255 finds. `check` is where the person is + told to fix the plan, so it says how, in the words `execute`'s refusal + would use one step later.""" + registered = source.registered_submodules(tree) found = [f"FINDING {code}: {detail}" for code, detail - in submodule_plan_problems(plan.entries, tree, - source.registered_submodules(tree))] - return found, [SUBMODULE_REMEDIATION] if found else [] + in submodule_plan_problems(plan.entries, tree, registered)] + moved, notes = _relative_url_findings(plan, source, tree, registered, + local_remote_dir) + return ((found, [SUBMODULE_REMEDIATION] if found else []), + (moved, [RELATIVE_URL_REMEDIATION] if moved else []), notes) #: The answers that never extract an entry into a leg. A path answered @@ -2799,13 +3064,16 @@ def _unsafe_path_findings(plan: Plan) -> tuple[list[str], list[str]]: return found, [UNSAFE_PATH_REMEDIATION] if found else [] -def _entry_findings(plan: Plan, source: Source - ) -> tuple[list[str], list[str]]: +def _entry_findings(plan: Plan, source: Source, + local_remote_dir: Path | None = None + ) -> tuple[list[str], list[str], list[str]]: """Every FINDING `check` prints about the plan's ENTRIES, in its order, - and what it prints once after them: each entry's path, then coverage, - then each entry's `leg:`, the two mount paths and what the root keeps - where the shape writes, then the source's own submodules, then each - value `execute` writes into a file that no file can hold (#255). + what it prints once after them, and the NOTEs it prints beside them: + each entry's path, then coverage, then each entry's `leg:`, the two + mount paths and what the root keeps where the shape writes, then the + source's own submodules, then each value `execute` writes into a file + that no file can hold (#255), and last each relative url of a kept + submodule that names another repository from its leg (#190). ONE LIST, read by `check` and by `execute`'s `_refuse_what_check_finds`. #168 and #166 each added findings to `check` and a refusal to `execute` @@ -2813,7 +3081,10 @@ def _entry_findings(plan: Plan, source: Source same lines; three compositions then disagreed, and a plan `check` rejected as uncovered AND split was refused by `execute` for the split alone, to be refused again for the rest once that was fixed. Composed - once, the two commands cannot differ about a plan. + once, the two commands cannot differ about a plan -- but in the one + place a remote decides it: `local_remote_dir` is `execute`'s, and a + relative submodule url is resolved from the leg remotes it names, where + `check` has none and resolves from GitHub's (#190). `_leg_findings` is handed the TREE, because the paths the plan keeps in the root are read out of it (#172, #191): without it a mount path at, or @@ -2831,10 +3102,12 @@ def _entry_findings(plan: Plan, source: Source for e in plan.entries], [path for path, _, _, _ in tree]) + _leg_findings(plan, tree)) - submodule_findings, remediation = _submodule_findings(plan, source, tree) + (split, split_remediation), (moved, moved_remediation), notes = \ + _submodule_findings(plan, source, tree, local_remote_dir) unwritable_findings, unwritable_remediation = _value_findings(plan) - return (findings + submodule_findings + unwritable_findings, - unsafe_remediation + remediation + unwritable_remediation) + return (findings + split + unwritable_findings + moved, + unsafe_remediation + split_remediation + unwritable_remediation + + moved_remediation, notes) def _topics_line(topic: str, local: bool) -> str: @@ -2898,7 +3171,7 @@ def cmd_check(args) -> int: "against a tree that has moved proves nothing about the tree that " "will be split.") - entry_findings, remediation = _entry_findings(plan, source) + entry_findings, remediation, notes = _entry_findings(plan, source) findings.extend(entry_findings) names = plan.names() @@ -2920,6 +3193,8 @@ def cmd_check(args) -> int: print(line) for line in plan.seeding_record_disagreements(seeded): print(line) + for line in notes: + print(line) for line in [*findings, *remediation]: print(line, file=sys.stderr) @@ -3373,21 +3648,28 @@ def _repository_urls(args, plan: Plan, names: dict, itself under `--local-remote-dir`, because an in-place adoption pushes its split branch back to the repository it read. """ - local = args.local_remote_dir is not None org = checked_value("org", plan.get("org")) repositories = {role: f"{org}/{name}" for role, name in names.items()} - if local: - # NOT created yet: nothing exists on disk until the human has said - # yes, so a refused run leaves the directory it would have used - # absent rather than empty. - remote_dir = args.local_remote_dir.resolve() - urls = {role: str(remote_dir / f"{name}.git") - for role, name in names.items()} - urls["assembly"] = str(source.path) - else: - urls = {role: f"https://github.com/{org}/{name}.git" + return (args.local_remote_dir is not None, repositories, + _remote_urls(org, names, args.local_remote_dir, source)) + + +def _remote_urls(org: str, names: dict, local_remote_dir: Path | None, + source: Source) -> dict: + """The url of each of the three repositories, by role: what `execute` + pushes to, and what a relative submodule url kept in a leg is resolved + from (#190), which `check` asks with no `local_remote_dir`.""" + if local_remote_dir is None: + return {role: f"https://github.com/{org}/{name}.git" for role, name in names.items()} - return local, repositories, urls + # NOT created yet: nothing exists on disk until the human has said yes, + # so a refused run leaves the directory it would have used absent rather + # than empty. + remote_dir = local_remote_dir.resolve() + urls = {role: str(remote_dir / f"{name}.git") + for role, name in names.items()} + urls["assembly"] = str(source.path) + return urls def _leg_paths(plan: Plan) -> dict: @@ -3412,7 +3694,8 @@ def _leg_paths(plan: Plan) -> dict: REFUSED_FINDINGS_SHOWN = 8 -def _refuse_what_check_finds(plan: Plan, source: Source) -> None: +def _refuse_what_check_finds(plan: Plan, source: Source, + local_remote_dir: Path | None = None) -> None: """Refuse a plan `check` rejects, BEFORE a leg repository exists (#168). Of `check`'s findings, `execute` used to refuse only `plan-unresolved`, @@ -3477,14 +3760,21 @@ def _refuse_what_check_finds(plan: Plan, source: Source) -> None: states: a plan `check` rejects as uncovered and split is refused for both at once, and a submodule problem is refused here like any other, after the entries' paths, which are the first of these findings. + + BUT FROM THE REMOTES THIS RUN WOULD CREATE (#190). A relative submodule + url kept in a leg is resolved from that leg's remote, and under + `local_remote_dir` that is a bare repository `check` cannot know of: such + a finding is this run's own, and the refusal says so. A url `check` finds + from GitHub's remotes may name the same repository from these, and one it + passes may not. """ - findings, remediation = _entry_findings(plan, source) + findings, remediation, _ = _entry_findings(plan, source, local_remote_dir) if not findings: return # `FINDING : ` is the one shape both functions write. code = findings[0].split()[1].rstrip(":") lines = [f"{len(findings)} finding(s) in {plan.path}, as `check` prints " - "them:"] + f"them{_as_resolved_here(findings, local_remote_dir)}:"] lines += [f" {finding}" for finding in findings[:REFUSED_FINDINGS_SHOWN]] if len(findings) > REFUSED_FINDINGS_SHOWN: lines.append(f" and {len(findings) - REFUSED_FINDINGS_SHOWN} more") @@ -3501,6 +3791,20 @@ def _refuse_what_check_finds(plan: Plan, source: Source) -> None: *remediation])) +def _as_resolved_here(findings: list[str], + local_remote_dir: Path | None) -> str: + """What the refusal's first line adds when a finding in it is not one + `check` prints: a relative url resolved from `local_remote_dir`'s + remotes, where `check` resolves it from GitHub's. Nothing otherwise.""" + if local_remote_dir is None or not any( + finding.startswith(f"FINDING {RELATIVE_URL}:") + for finding in findings): + return "" + return (", but with each relative submodule url resolved from the leg " + "remotes this run would create under --local-remote-dir, where " + "`check` resolves it from GitHub's") + + def _refuse_unconsented_seeding(args, plan: Plan, seeded: list) -> None: """A leg with no path is SEEDED, and seeding takes a human's word. @@ -3759,7 +4063,7 @@ def cmd_execute(args) -> int: local, repositories, urls = _repository_urls(args, plan, names, source) _refuse_an_unwritable_clone_url(urls["assembly"]) paths_for = _leg_paths(plan) - _refuse_what_check_finds(plan, source) + _refuse_what_check_finds(plan, source, args.local_remote_dir) seeded = seeded_legs(paths_for) _refuse_unconsented_seeding(args, plan, seeded) diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index 38b45290..5d1a2392 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -1972,6 +1972,17 @@ def repo_basename(repository: str) -> str: #: draws for a value on a command line, for the same reason. REMOTE_DRIVE_RE = re.compile(r"^[A-Za-z]:$") +#: A `:` after the HOST of a `://` url: where its transport +#: connects, never which repository it reaches. Only such a url has one; in +#: git's scp spelling `host:22/Repo.git` the `22` is the path's, and stays. +#: Digits are required, so a drive's bare colon is never read as one. +REMOTE_PORT_RE = re.compile(r":\d+$") + +#: A host that is another door into the repositories of the host it maps +#: to: GitHub's SSH-over-the-HTTPS-port endpoint, which GitHub's own +#: documentation spells `ssh://git@ssh.github.com:443/Org/Repo.git`. +REMOTE_HOST_ALIASES = {"ssh.github.com": "github.com"} + def _remote_body(url: str) -> str: """A remote with its scheme, its credential and its trailing `/` gone. @@ -2085,11 +2096,24 @@ def remote_key(url: str) -> str: was written in. It is asked of the remote AS IT WAS WRITTEN, because the scp colon this function is about to turn into a `/` is the one thing that tells a Windows drive from a host. + + A PORT COMES OFF TOO, AND `ssh.github.com` IS `github.com` (#190). + `ssh://git@github.com:22/Org/Repo.git` and GitHub's documented + `ssh://git@ssh.github.com:443/Org/Repo.git` are + `git@github.com:Org/Repo.git` reached through another door; with the + port's colon taken for the scp one they keyed as `github.com/22/org/repo`, + which no other spelling of that repository does, and a caller comparing + keys -- `adopt-project.py`'s relative-url check -- called them two. """ text = _remote_body(url) fold_case = _remote_names_a_host(url) - if ":" in text.split("/", 1)[0]: # the scp spelling's one separator - text = text.replace(":", "/", 1) + first, slash, rest = text.partition("/") + if REMOTE_SCHEME_RE.match(url.strip()): # only a url's host has a port + first = REMOTE_PORT_RE.sub("", first) + # The scp spelling's one separator, `host:path`, becomes a `/`. + host, slash, rest = (first.replace(":", "/", 1) + slash + + rest).partition("/") + text = REMOTE_HOST_ALIASES.get(host.lower(), host) + slash + rest if text.lower().endswith(".git"): text = text[:-4] return (text.lower() if fold_case else text).strip("/") diff --git a/tests/test_adopt_gitmodules_values.py b/tests/test_adopt_gitmodules_values.py index 255edaa3..5a6eca0d 100644 --- a/tests/test_adopt_gitmodules_values.py +++ b/tests/test_adopt_gitmodules_values.py @@ -38,7 +38,12 @@ UNREADABLE = "plan-gitmodules-unreadable" REMOTES = "remotes" -REGISTRATION = '[submodule "b"]\n\tpath = vendor/b\n\turl = ../b\n' +#: An ABSOLUTE url, which git copies as it stands. A relative one kept in a +#: leg is resolved from the leg too (#190), and in a source with no remote +#: `../b` is the directory beside it, which no GitHub leg reaches: a finding +#: of its own beside every value here, which is not what these tests ask. +REGISTRATION = ('[submodule "b"]\n\tpath = vendor/b\n' + '\turl = https://example.invalid/b\n') #: A source whose plan asks about `.gitmodules`, and about `vendor/` where #: the tip holds the submodule: `specs/` and `src/` classify themselves and #: `README.md` stays in the root. diff --git a/tests/test_adopt_relative_submodule_url.py b/tests/test_adopt_relative_submodule_url.py new file mode 100644 index 00000000..e03c5a35 --- /dev/null +++ b/tests/test_adopt_relative_submodule_url.py @@ -0,0 +1,698 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A source submodule's RELATIVE url, resolved from the leg that holds it (#190). + +git resolves `url = ../dependency` against the remote of the repository +whose `.gitmodules` holds it: the source's before the split, the LEG's after +it. On main a plan that kept such a submodule in a leg checked out, executed, +and said `adoption verified`, and the assembly's `git clone +--recurse-submodules` then failed wherever the dependency did not sit beside +the leg. `check` now resolves it from both places, as git does, and finds it +where the two differ; `execute` resolves it again from the remotes it would +create and refuses it before it creates any. + +No network: a GitHub url here is only ever a string git resolves against. +Every repository is local, and the end-to-end tests clone from disk. +""" + +from __future__ import annotations + +import importlib.util +import os +import re +import shutil +import sys +from argparse import Namespace +from pathlib import Path + +import pytest + +from conftest import (ADOPT, FILE_PROTOCOL, git, make_source_repo, resolve, + run_script, write_plan) + +needs_filter_repo = pytest.mark.skipif( + shutil.which("git-filter-repo") is None, + reason="git filter-repo is not installed: `pip install git-filter-repo`") + +#: What the test with a symlink ON THE DISK needs; see +#: tests/test_adopt_ignored_shape_files.py for the same guard. +needs_disk_symlinks = pytest.mark.skipif( + sys.platform == "win32", + reason="a symlink needs a privilege Windows does not grant by default") + +#: The finding, as `check` prints it and `execute` raises it. +RELATIVE = "plan-submodule-relative-url" +SPLIT = "plan-submodule-split" +GITMODULES = ".gitmodules" +UPSTREAM = "upstream/" +EXTRA = "extra/" +#: The submodule every source here holds, a second one some hold, and the +#: one file of the repository each is a commit of. +DEPENDENCY = "upstream/dependency" +SECOND = "extra/second" +VALUE = "value.txt" +DEPENDENCY_TREE = {VALUE: "dependency\n"} +#: The url #190 is about: the dependency beside whatever repository holds +#: `.gitmodules`. +SIBLING = "../dependency" +#: The dependency INSIDE the repository holding `.gitmodules`, and two +#: levels up from it: git's arithmetic, not a sibling's. +INSIDE = "./dependency" +TWO_UP = "../../dependency" +#: The second submodule's url, beside whatever holds `.gitmodules`. +SECOND_SIBLING = "../second" +#: SIBLING with Windows' separator. git takes `..\` for a relative url on +#: every platform and strips it only where `\` separates directories, so on +#: Windows this is #190's own case. +BACKSLASH = "..\\dependency" +#: The source's own name for its remote, beside the repositories it makes. +THING_REMOTE = "Thing.git" +#: Where each test's `execute` works, and what a verified one says. +WORK = "work" +VERIFIED = "adoption verified" +#: A source whose plan asks nothing but the submodules' questions. +SMALL_TREE = {"README.md": "# Thing\n", "src/app/util.py": "VALUE = 1\n", + "specs/001-feature/tasks.md": "- [ ] one\n"} +#: The plan's own organisation, and one that is not the source's. +ORG, OTHER_ORG = "testorg", "otherorg" +#: Where each test's `execute` makes its leg remotes. +REMOTES = "remotes" +#: The source's remote beside them, as `git remote add origin +#: ../remotes/Thing.git` writes it from `/Thing`: RELATIVE. +RELATIVE_ORIGIN = f"../{REMOTES}/{THING_REMOTE}" +#: The code leg's GitHub remote, as `check` resolves from it. +CODE_LEG = "https://github.com/{org}/Northwind-code.git" +#: The code leg's bare remote, as `execute --local-remote-dir` names it. +CODE_BARE = "Northwind-code.git" +#: The source's `origin` on GitHub, in the spellings a clone may carry. +HTTPS_ORIGIN = "https://github.com/testorg/Thing.git" +SCP_ORIGIN = "git@github.com:testorg/Thing.git" +#: An owner spelled in another case: one organisation to GitHub. +SCP_OTHER_CASE = "git@github.com:TestOrg/Thing.git" +#: What `check` says of the source's base when it is its `origin`. +ITS_ORIGIN = "its remote `origin`" +#: Every source's dependency, from the plan's own organisation on GitHub. +GITHUB_DEPENDENCY = "https://github.com/testorg/dependency" +#: The one leg every plan here keeps the submodules in. +BOTH_IN_CODE = {GITMODULES: "code", UPSTREAM: "code"} + + +@pytest.fixture(scope="module") +def adopter(): + """`adopt-project.py` as a module: its remediation, its spelling of a + url in a finding, and `execute`'s refusal.""" + module_spec = importlib.util.spec_from_file_location("adopt_relative", + ADOPT) + module = importlib.util.module_from_spec(module_spec) + module_spec.loader.exec_module(module) + return module + + +def source_with(base: Path, url: str, origin: str | None = None, + dependency: Path | None = None, + second: str | None = None) -> Path: + """`base/Thing`, holding DEPENDENCY registered with `url`, and SECOND + registered with `second` when it is given. + + Each submodule is ADDED from the dependency's absolute path, so nothing + is fetched from a url that is only a string, and its url is then set in + `.gitmodules` and committed, as a hand edit would. `origin`, when given, + is the source's remote: git resolves a relative url against it only + when asked to. + """ + dependency = dependency or base / "dependency" + make_source_repo(dependency, tree=DEPENDENCY_TREE, edits=()) + source = make_source_repo(base / "Thing", tree=SMALL_TREE) + for path, value in ((DEPENDENCY, url), (SECOND, second)): + if value is None: + continue + git(*FILE_PROTOCOL, "submodule", "add", "-q", str(dependency), path, + cwd=source) + git("config", "-f", GITMODULES, f"submodule.{path}.url", value, + cwd=source) + commit(source, "Register the dependency") + if origin is not None: + git("remote", "add", "origin", origin, cwd=source) + return source + + +def commit(source: Path, message: str) -> None: + git("add", "--", GITMODULES, cwd=source) + git("-c", "user.name=Source Human", "-c", + "user.email=source@invalid.example", "commit", "-qm", message, + cwd=source) + + +def planned(source: Path, base: Path, org: str = ORG, + answers: dict | None = None, name: str | None = None) -> Path: + """A plan of `source` for `org`, in a directory of its own under `base` + (`name`, else `plan-`), with `.gitmodules` and DEPENDENCY both kept + in code unless `answers` says otherwise.""" + plan = base / (name or f"plan-{org}") / "adoption-plan.yaml" + plan.parent.mkdir(parents=True) + written = write_plan(source, plan, org=org) + assert written.returncode == 0, written.stderr + written.stdout + for path, leg in (answers or BOTH_IN_CODE).items(): + resolve(plan, path, leg) + return plan + + +def check(plan: Path): + return run_script(ADOPT, "check", "--plan", str(plan)) + + +def found(stderr: str) -> list[tuple[str, str]]: + """`(code, detail)` for each `FINDING : ` line.""" + return [tuple(line.split(" ", 1)[1].split(": ", 1)) + for line in stderr.splitlines() if line.startswith("FINDING ")] + + +def notes(stdout: str) -> list[str]: + """The NOTE lines `check` prints about the relative urls it resolved.""" + return [line for line in stdout.splitlines() + if line.startswith("NOTE `check` resolved")] + + +def moved(adopter, url: str, base: str, which: str, here: str, remote: str, + there: str, path: str = DEPENDENCY, entry: str = UPSTREAM) -> str: + """The detail of the finding, for the submodule at `path` kept in code. + `here` and `there` are git's resolutions, spelled; every other url is + spelled here as the tool spells it, which on Windows doubles each + backslash of a path. The submodule and its entry are between backticks, + as every path a finding names is (#212).""" + spelled = adopter._spelled + return (f"the submodule `{path}` (entry `{entry}`) has leg: code, and " + f"its url in the source's {GITMODULES} is the relative " + f"{spelled(url)}, which git resolves against the remote of the " + f"repository holding that file: from the source, at " + f"{spelled(base)} ({which}), {here}, and from the code leg, at " + f"{spelled(remote)}, {there}; the assembly's recursive clone " + "resolves it from the leg") + + +def alike(org: str, *resolved: tuple[str, str, str]) -> str: + """The NOTE `check` prints when every `(url, path, resolution)` names, + from the code leg on GitHub, the repository it names from the source.""" + return ("NOTE `check` resolved the relative url of each submodule the " + "plan keeps in the code leg from that leg's GitHub remote, " + f"`{CODE_LEG.format(org=org)}`, and each names the same " + "repository from there as from the source: " + + "; ".join(f"`{url}` of `{path}`, to `{to}`" + for url, path, to in resolved) + + ". `execute --local-remote-dir` resolves it again from the bare " + "repository it would create instead, and refuses there, before " + "it creates any leg, a url that names another repository.") + + +#: `from the leg, at ``, to ``; ` in a finding. +LEG_RESOLUTION_RE = re.compile( + r"from the (spec|code) leg, at `(?:[^`\\]|\\.)*`, " + r"to `((?:[^`\\]|\\.)*)`; ") + + +def resolved_from_the_leg(detail: str, leg: str = "code") -> Path: + """The url git resolved from `leg`, as the path it names: the + spelling's backslash escapes undone, so the comparison is a Path's and + holds whichever separator git joined it with.""" + spelled = LEG_RESOLUTION_RE.search(detail) + assert spelled and spelled[1] == leg, detail + return Path(re.sub(r"\\(.)", r"\1", spelled[2])) + + +def passes(result) -> None: + assert result.returncode == 0, result.stderr + result.stdout + assert "plan ok" in result.stdout + assert RELATIVE not in result.stderr + result.stdout + + +# --- `check`, on the GitHub remotes the plan names -------------------------- + + +@pytest.mark.parametrize("origin, resolved", [ + (HTTPS_ORIGIN, GITHUB_DEPENDENCY), + (SCP_ORIGIN, "git@github.com:testorg/dependency"), + (SCP_OTHER_CASE, "git@github.com:TestOrg/dependency"), + # A port, and GitHub's SSH-over-the-HTTPS-port host: GitHub all the same. + ("ssh://git@github.com:22/testorg/Thing.git", + "ssh://git@github.com:22/testorg/dependency"), + ("ssh://git@ssh.github.com:443/testorg/Thing.git", + "ssh://git@ssh.github.com:443/testorg/dependency"), +], ids=["https", "scp", "scp-other-case", "ssh-port", "ssh-github-443"]) +def test_check_keeps_a_relative_url_its_github_leg_resolves_alike( + adopter, tmp_path, origin, resolved): + """(a) Legs in the source's own organisation sit beside its dependency + on GitHub, so `../dependency` names one repository from both, however + the source's `origin` spells it, a port and `ssh.github.com` included: + no finding, and the NOTE says which remotes `check` asked. In another + organisation it names another repository, and that is found, with the + submodule, its url, the leg's remote and both resolutions, and the + remediation once after it. A url with a scheme is printed through + `redacted`, which hides its user too (`ssh://***@github.com:22/...`).""" + source = source_with(tmp_path, SIBLING, origin) + same = check(planned(source, tmp_path)) + passes(same) + assert notes(same.stdout) == [alike(ORG, (SIBLING, DEPENDENCY, + GITHUB_DEPENDENCY))] + + result = check(planned(source, tmp_path, OTHER_ORG)) + assert result.returncode == 1, result.stderr + result.stdout + assert found(result.stderr) == [(RELATIVE, moved( + adopter, SIBLING, adopter.redacted(origin), ITS_ORIGIN, + f"to `{adopter.redacted(resolved)}`", CODE_LEG.format(org=OTHER_ORG), + "to `https://github.com/otherorg/dependency`"))] + assert notes(result.stdout) == [] + # Said once, after the findings, and it is not #166's remediation. + assert result.stderr.count(adopter.RELATIVE_URL_REMEDIATION) == 1 + assert adopter.SUBMODULE_REMEDIATION not in result.stderr + assert result.stderr.index("FINDING ") \ + < result.stderr.index(adopter.RELATIVE_URL_REMEDIATION) \ + < result.stderr.index(" finding(s) in ") + + +@pytest.mark.parametrize("url, there", [ + # Inside the repository that holds it: another one for every leg. + (INSIDE, + "to `https://github.com/testorg/Northwind-code.git/dependency`"), + # More `..` than the remote has components: git says so, in words + # this pins only the start of, since they are git's to change. + ("../" * 8 + "dependency", "not at all (fatal: "), +], ids=["dot-slash", "too-many-dotdots"]) +def test_check_finds_a_relative_url_no_github_leg_resolves_alike( + tmp_path, url, there): + """`./dependency` names a repository INSIDE the one holding + `.gitmodules`, so no leg resolves it to the source's; a url git cannot + resolve from the leg is one the assembly's clone fails on. Both are found + in the plan's own organisation, where `../dependency` is not.""" + source = source_with(tmp_path, url, HTTPS_ORIGIN) + result = check(planned(source, tmp_path)) + assert result.returncode == 1, result.stderr + result.stdout + [(code, detail)] = found(result.stderr) + assert code == RELATIVE + assert f"from the code leg, at `{CODE_LEG.format(org=ORG)}`, {there}" \ + in detail, detail + assert detail.endswith("; the assembly's recursive clone resolves it " + "from the leg"), detail + + +def test_a_source_with_no_remote_resolves_from_its_own_directory(adopter, + tmp_path): + """With no remote, git resolves a relative url against the source's own + directory, so `../dependency` is the directory beside the source -- and + a GitHub leg resolves it to GitHub. `check` says which it used.""" + source = source_with(tmp_path, SIBLING) + result = check(planned(source, tmp_path)) + assert result.returncode == 1, result.stderr + result.stdout + resolved = adopter._spelled((tmp_path / "dependency").as_posix()) + assert found(result.stderr) == [(RELATIVE, moved( + adopter, SIBLING, source.as_posix(), + "its own directory, as it has no remote `origin`", f"to {resolved}", + CODE_LEG.format(org=ORG), f"to `{GITHUB_DEPENDENCY}`"))] + + +def test_the_source_resolves_from_the_remote_its_branch_tracks(tmp_path): + """git's default remote is the one the branch tracks, and `origin` only + when it tracks none: a fork's `origin` is not where its `main` comes + from, and `upstream` is.""" + source = source_with(tmp_path, SIBLING, + "https://github.com/someone/Thing.git") + git("remote", "add", "upstream", HTTPS_ORIGIN, cwd=source) + git("config", "branch.main.remote", "upstream", cwd=source) + passes(check(planned(source, tmp_path))) + git("config", "--unset", "branch.main.remote", cwd=source) + result = check(planned(source, tmp_path, name="untracked")) + assert [code for code, _ in found(result.stderr)] == [RELATIVE] + assert ("from the source, at `https://github.com/someone/Thing.git` " + f"({ITS_ORIGIN}), to `https://github.com/someone/dependency`" + ) in result.stderr + + +def test_each_relative_url_is_resolved_on_its_own(adopter, tmp_path): + """Two submodules in one leg, resolved in one scratch repository: the + one whose url names another repository from the leg is found, by its + own path and url, and the one that does not is not; with both alike, + the NOTE names both, in the tree's order.""" + source = source_with(tmp_path, INSIDE, HTTPS_ORIGIN, + second=SECOND_SIBLING) + answers = {**BOTH_IN_CODE, EXTRA: "code"} + result = check(planned(source, tmp_path, answers=answers)) + assert found(result.stderr) == [(RELATIVE, moved( + adopter, INSIDE, HTTPS_ORIGIN, ITS_ORIGIN, + "to `https://github.com/testorg/Thing.git/dependency`", + CODE_LEG.format(org=ORG), + "to `https://github.com/testorg/Northwind-code.git/dependency`"))] + assert notes(result.stdout) == [] + + both = source_with(tmp_path / "both", SIBLING, HTTPS_ORIGIN, + second=SECOND_SIBLING) + result = check(planned(both, tmp_path / "both", answers=answers)) + passes(result) + assert notes(result.stdout) == [alike( + ORG, (SECOND_SIBLING, SECOND, "https://github.com/testorg/second"), + (SIBLING, DEPENDENCY, GITHUB_DEPENDENCY))] + + +def test_a_dropped_submodule_leaves_its_relative_url_inert(tmp_path): + """git resolves the url of a submodule it finds a gitlink for, and of no + other: dropped on its own, its registration stays behind in the leg and + is never resolved, and nothing is said of it. Kept, it is found.""" + source = source_with(tmp_path, SIBLING) + dropped = check(planned(source, tmp_path, + answers={GITMODULES: "code", UPSTREAM: "drop"})) + passes(dropped) + assert notes(dropped.stdout) == [] + kept = check(planned(source, tmp_path, name="kept")) + assert [code for code, _ in found(kept.stderr)] == [RELATIVE] + + +def test_a_split_submodule_is_a_split_and_nothing_more(tmp_path): + """A submodule kept away from its `.gitmodules` is #166's split: the + leg that holds it holds no url for it, relative or not, so it is not + also a relative url resolved from a leg.""" + source = source_with(tmp_path, SIBLING) + result = check(planned(source, tmp_path, + answers={GITMODULES: "code", UPSTREAM: "spec"})) + assert [code for code, _ in found(result.stderr)] == [SPLIT] + + +def test_a_credential_in_the_source_remote_is_never_printed(adopter, + tmp_path): + """A source's `origin` may carry a token, and every url resolved against + it carries it too. The finding names both with the credential redacted, + as the doctor does, and the token appears nowhere `check` writes.""" + secret = "s3cret-token" + origin = f"https://x-access-token:{secret}@github.com/testorg/Thing.git" + source = source_with(tmp_path, SIBLING, origin) + result = check(planned(source, tmp_path, OTHER_ORG)) + assert secret not in result.stderr + result.stdout + assert found(result.stderr) == [(RELATIVE, moved( + adopter, SIBLING, + "https://***@github.com/testorg/Thing.git", ITS_ORIGIN, + "to `https://***@github.com/testorg/dependency`", + CODE_LEG.format(org=OTHER_ORG), + "to `https://github.com/otherorg/dependency`"))] + # And from the plan's own organisation it is one repository: the + # credential is not part of a repository's identity. + same = check(planned(source, tmp_path)) + passes(same) + assert secret not in same.stderr + same.stdout + + +def test_an_absolute_url_resolves_nothing(adopter, tmp_path, monkeypatch): + """(d) A url that is not relative is copied by git as it stands, so it + is never resolved and no scratch repository is made for it: a source + with an absolute url checks exactly as it did on #166's base, with no + finding and no NOTE. The same plan with a relative url is resolved.""" + def refuse(*_): + raise AssertionError("a url was resolved") + + monkeypatch.setattr(adopter, "_resolved", refuse) + dependency = tmp_path / "dependency" + absolute = source_with(tmp_path / "absolute", dependency.as_posix(), + dependency=dependency) + plan = adopter.Plan.load(planned(absolute, tmp_path / "absolute")) + opened = plan.open_source(None, tmp_path / WORK) + assert adopter._entry_findings(plan, opened) == ([], [], []) + + relative = source_with(tmp_path / "relative", SIBLING) + plan = adopter.Plan.load(planned(relative, tmp_path / "relative")) + opened = plan.open_source(None, tmp_path / WORK) + with pytest.raises(AssertionError, match="a url was resolved"): + adopter._entry_findings(plan, opened) + + +# --- `execute`, on the remotes it would create ------------------------------- + + +def refusal_of(adopter, plan: Path, tmp_path: Path, + remotes: Path | None = None): + """What `_refuse_what_check_finds` raises for `plan` against the leg + remotes `execute` would make under `remotes`, or GitHub's; None when it + raises nothing.""" + loaded = adopter.Plan.load(plan) + opened = loaded.open_source(None, tmp_path / WORK) + try: + adopter._refuse_what_check_finds(loaded, opened, remotes) + except adopter.Refusal as refusal: + return refusal + return None + + +@pytest.mark.parametrize("origin, url, org", [ + (HTTPS_ORIGIN, SIBLING, ORG), + (SCP_ORIGIN, SIBLING, ORG), + (SCP_OTHER_CASE, SIBLING, ORG), + # git's arithmetic, not the organisation: two `..` leave both behind. + (HTTPS_ORIGIN, TWO_UP, OTHER_ORG), +], ids=["https", "scp", "scp-other-case", "dotdot-dotdot-other-org"]) +def test_execute_resolves_again_from_the_remotes_it_would_create( + adopter, tmp_path, monkeypatch, origin, url, org): + """(c) `check` cannot know where `execute` will make the legs, so it + resolves from GitHub's, where these plans name one repository from both + and pass, and says so. `execute --local-remote-dir` resolves again from + the bare repositories it would make there, where they do not, and + refuses before it makes any -- saying the finding is its own and not one + `check` printed, and that `check`'s `plan ok` settles nothing here.""" + source = source_with(tmp_path, url, origin) + plan = planned(source, tmp_path, org) + checked = check(plan) + passes(checked) + assert len(notes(checked.stdout)) == 1 + # Every refusal here comes before the tool would be used. + monkeypatch.setattr(adopter, "_require_filter_repo", lambda: None) + remotes = tmp_path / REMOTES + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(Namespace( + plan=str(plan), source=None, local_remote_dir=remotes, + allow_empty_leg=[], yes=True, work_dir=tmp_path / WORK)) + refusal = caught.value + assert refusal.code == RELATIVE + first, finding = refusal.detail.splitlines() + assert first == ( + f"1 finding(s) in {plan}, as `check` prints them, but with each " + "relative submodule url resolved from the leg remotes this run would " + "create under --local-remote-dir, where `check` resolves it from " + "GitHub's:") + leg = adopter._spelled(str(remotes.resolve() / CODE_BARE)) + assert finding.startswith(f" FINDING {RELATIVE}: the submodule " + f"`{DEPENDENCY}` (entry `{UPSTREAM}`)"), finding + assert f", and from the code leg, at {leg}, to " in finding + assert refusal.remediation.endswith( + "\n" + adopter.RELATIVE_URL_REMEDIATION) + assert "`check`'s `plan ok` settles nothing" in refusal.remediation + assert "no leg repository was created" in refusal.remediation + assert not remotes.exists() + + +def test_execute_without_local_remotes_refuses_what_check_prints( + adopter, tmp_path): + """Against GitHub, `execute` resolves from the remotes `check` resolved + from, so its refusal is `check`'s list, line for line, under the first + line `check`'s refusals have always had.""" + source = source_with(tmp_path, SIBLING) + plan = planned(source, tmp_path) + printed = found(check(plan).stderr) + assert [code for code, _ in printed] == [RELATIVE] + refusal = refusal_of(adopter, plan, tmp_path) + assert refusal.code == RELATIVE + assert refusal.detail.splitlines() == [ + f"1 finding(s) in {plan}, as `check` prints them:", + *(f" FINDING {code}: {detail}" for code, detail in printed)] + + +def test_the_refusal_says_nothing_of_local_remotes_it_did_not_use( + adopter, tmp_path): + """Under `--local-remote-dir` a plan refused for anything BUT a relative + url is refused in `check`'s words: the first line adds nothing about + remotes it resolved nothing from.""" + source = source_with(tmp_path, SIBLING) + plan = planned(source, tmp_path, + answers={GITMODULES: "code", UPSTREAM: "spec"}) + refusal = refusal_of(adopter, plan, tmp_path, tmp_path / REMOTES) + assert refusal.code == SPLIT + assert refusal.detail.splitlines()[0] \ + == f"1 finding(s) in {plan}, as `check` prints them:" + + +@needs_disk_symlinks +def test_one_directory_through_a_symlink_is_one_repository(adopter, + tmp_path): + """The source's `origin` reaches the remotes directory through a + symlink, and `--local-remote-dir` names it directly: two spellings, one + directory, so `../dependency` names one repository from both and is not + refused. A directory that is not the same one is.""" + remotes = tmp_path / REMOTES + remotes.mkdir() + link = tmp_path / "link" + try: + os.symlink(remotes, link, target_is_directory=True) + except OSError as exc: + # A platform, or a filesystem, that refuses one: not a failure. + pytest.skip(f"this platform cannot create a symlink here: {exc}") + linked = source_with(tmp_path / "linked", SIBLING, + str(link / THING_REMOTE)) + plan = planned(linked, tmp_path / "linked") + assert refusal_of(adopter, plan, tmp_path, remotes) is None + + elsewhere = source_with(tmp_path / "elsewhere", SIBLING, + str(tmp_path / "elsewhere" / THING_REMOTE)) + plan = planned(elsewhere, tmp_path / "elsewhere") + refusal = refusal_of(adopter, plan, tmp_path, remotes) + assert refusal is not None and refusal.code == RELATIVE + + +@pytest.mark.parametrize("relative", [True, False], + ids=["relative-origin", "absolute-origin"]) +def test_a_relative_origin_resolves_from_the_source_working_tree( + adopter, tmp_path, relative): + """git resolves `../dependency` against the RELATIVE origin + `../remotes/Thing.git` to `../remotes/dependency`, keeps it relative, + and the clone opens it from the source's working tree: the directory + beside the bare legs `execute --local-remote-dir` would make there, + which name it too, so nothing is refused, as on main. With the legs + anywhere else it names another repository and is refused, and the + finding names the directory git opens and says why. The same remote + spelled absolute resolves to an absolute path, said as before.""" + remotes = tmp_path / REMOTES + origin = RELATIVE_ORIGIN if relative else str(remotes / THING_REMOTE) + source = source_with(tmp_path, SIBLING, origin, remotes / "dependency") + plan = planned(source, tmp_path) + assert refusal_of(adopter, plan, tmp_path, remotes) is None + + elsewhere = tmp_path / "elsewhere" + refusal = refusal_of(adopter, plan, tmp_path, elsewhere) + assert refusal is not None and refusal.code == RELATIVE + [_, finding] = refusal.detail.splitlines() + if relative: + which = f"{ITS_ORIGIN}, relative to its working tree" + opened = (tmp_path.resolve() / "Thing" + / f"../{REMOTES}/dependency").as_posix() + else: + # git keeps the base's own separators and joins with `/`. + which, opened = ITS_ORIGIN, f"{remotes}/dependency" + assert (f"from the source, at {adopter._spelled(origin)} ({which}), to " + f"{adopter._spelled(opened)}, and from the code leg, at ") \ + in finding, finding + assert resolved_from_the_leg(finding) \ + == elsewhere.resolve() / "dependency" + + +def execute(plan: Path, work: Path, remotes: Path): + return run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(remotes), + "--work-dir", str(work)) + + +def clone_the_split(source: Path, clone: Path): + """`git clone --recurse-submodules` of the branch `execute` pushed the + split to: what a person runs next.""" + return git(*FILE_PROTOCOL, "clone", "-q", "--recurse-submodules", "-b", + "adopt/three-repo-shape", str(source), str(clone), + cwd=clone.parent, check=False) + + +@needs_filter_repo +@pytest.mark.parametrize("url, leg, there", [ + # #190 itself: the re-verifier's `relative-url` case. + (SIBLING, "code", "remotes/dependency"), + (INSIDE, "code", "remotes/Northwind-code.git/dependency"), + (TWO_UP, "code", "dependency"), + # The SPEC leg's own remote, which `./` names from inside. + (INSIDE, "spec", "remotes/Northwind-spec.git/dependency"), + # Where `\` separates nothing, git appends `..\dependency` whole. + (BACKSLASH, "code", + "remotes/dependency" if sys.platform == "win32" + else "remotes/Northwind-code.git/..\\dependency"), +], ids=["dotdot", "dot-slash", "dotdot-dotdot", "spec-leg", "backslash"]) +def test_execute_refuses_a_relative_url_its_leg_resolves_elsewhere( + tmp_path, url, leg, there): + """A source with no remote of its own, as the re-verifier built it: + `check` finds it from GitHub's remotes, and `execute --local-remote-dir` + from the bare ones it would make. On main both passed, `execute` said + `adoption verified`, and the assembly's recursive clone exited 1. Now + `execute` exits 2 before the remotes directory exists, and the source + is untouched -- from either leg, and for `..\\` as for `../`.""" + source = source_with(tmp_path, url) + head = git("rev-parse", "HEAD", cwd=source).stdout + plan = planned(source, tmp_path, answers={GITMODULES: leg, UPSTREAM: leg}) + assert [code for code, _ in found(check(plan).stderr)] == [RELATIVE] + remotes, work = tmp_path / REMOTES, tmp_path / WORK + result = execute(plan, work, remotes) + assert result.returncode == 2, result.stderr + result.stdout + assert f"REFUSED {RELATIVE}: 1 finding(s) in " in result.stderr + [detail] = [line.split(": ", 1)[1] for line in result.stderr.splitlines() + if line.startswith(f" FINDING {RELATIVE}: ")] + assert resolved_from_the_leg(detail, leg) == tmp_path.resolve() / there + assert "creating the leg repositories" not in result.stdout + assert VERIFIED not in result.stdout + assert not remotes.exists(), "a refused plan made the remotes directory" + assert list(work.iterdir()) == [], "a refused plan wrote into its work" + assert git("rev-parse", "HEAD", cwd=source).stdout == head + + +@needs_filter_repo +def test_the_remediation_adopts_and_clones(tmp_path): + """The refusal's remediation works: the absolute url committed in the + source's `.gitmodules`, a fresh plan answered as before, and `check` + passes with nothing to say of it, `execute` verifies into the remotes + the refused run never made, and the assembly's recursive clone fetches + the dependency into the leg.""" + source = source_with(tmp_path, SIBLING) + remotes = tmp_path / REMOTES + refused = execute(planned(source, tmp_path), tmp_path / WORK, remotes) + assert f"REFUSED {RELATIVE}: " in refused.stderr, refused.stderr + + git("config", "-f", GITMODULES, f"submodule.{DEPENDENCY}.url", + (tmp_path / "dependency").as_posix(), cwd=source) + commit(source, "Register the dependency by its absolute url") + plan = planned(source, tmp_path, name="replanned") + checked = check(plan) + passes(checked) + assert notes(checked.stdout) == [] + result = execute(plan, tmp_path / "work-again", remotes) + assert result.returncode == 0, result.stderr + result.stdout + assert VERIFIED in result.stdout + clone = tmp_path / "clone" + cloned = clone_the_split(source, clone) + assert cloned.returncode == 0, cloned.stderr + assert (clone / "code" / DEPENDENCY / VALUE).read_text() \ + == DEPENDENCY_TREE[VALUE] + + +@needs_filter_repo +@pytest.mark.parametrize("url, origin, legs", [ + # The dependency beside the source's remote, and the legs beside both. + (SIBLING, THING_REMOTE, "."), + # Two levels: the source's remote and the legs in sibling directories. + (TWO_UP, f"sources/{THING_REMOTE}", "legs"), + # The first layout with the remote written relative to the source. + (SIBLING, RELATIVE_ORIGIN, "."), +], ids=["dotdot", "dotdot-dotdot", "relative-origin"]) +def test_a_relative_url_its_leg_resolves_alike_adopts_and_clones( + tmp_path, url, origin, legs): + """(a) end to end. The source's `origin` and the legs are local, in a + layout where its relative url names the dependency from both: `check`, + which resolves from GitHub's remotes, finds it, and `execute`, which + resolves from the ones it makes, adopts and verifies, and the assembly's + `git clone --recurse-submodules` and `bootstrap.py` both exit 0 with the + dependency checked out in the leg, whose `.gitmodules` still says what + the source committed. An `origin` beginning `..` is written as it + stands, relative to the source's working tree; any other is a path in + the remotes directory.""" + remotes = tmp_path / REMOTES + dependency = remotes / "dependency" + source = source_with(tmp_path, url, origin if origin.startswith("..") + else str(remotes / origin), dependency) + plan = planned(source, tmp_path) + assert [code for code, _ in found(check(plan).stderr)] == [RELATIVE] + result = execute(plan, tmp_path / WORK, remotes / legs) + assert result.returncode == 0, result.stderr + result.stdout + assert VERIFIED in result.stdout + clone = tmp_path / "clone" + cloned = clone_the_split(source, clone) + assert cloned.returncode == 0, cloned.stderr + leg = clone / "code" + assert git("config", "-f", GITMODULES, "--get", + f"submodule.{DEPENDENCY}.url", cwd=leg).stdout.strip() == url + assert (leg / DEPENDENCY / VALUE).read_text() == DEPENDENCY_TREE[VALUE] + bootstrapped = run_script(clone / "scripts/bootstrap.py", cwd=clone) + assert bootstrapped.returncode == 0, (bootstrapped.stderr + + bootstrapped.stdout) diff --git a/tests/test_adopt_submodules.py b/tests/test_adopt_submodules.py index d97d01cc..daf4ffec 100644 --- a/tests/test_adopt_submodules.py +++ b/tests/test_adopt_submodules.py @@ -349,8 +349,11 @@ def test_mount_removes_gitmodules_before_a_submodule_that_sorts_ahead( THE_DEPENDENCY = "" -def registration(name: str = DEPENDENCY, url: str = "../dependency") -> str: - """`DEPENDENCY` registered by hand, under `name`, with `url`.""" +def registration(name: str = DEPENDENCY, url: str = THE_DEPENDENCY) -> str: + """`DEPENDENCY` registered by hand, under `name`, with `url`: by + default the dependency's absolute path, because `check` finds a RELATIVE + url its leg resolves elsewhere (#190), and these sources have no remote + to resolve one alike from.""" return f'[submodule "{name}"]\n\tpath = {DEPENDENCY}\n\turl = {url}\n' @@ -1012,7 +1015,7 @@ def test_check_refuses_to_keep_a_gitlink_its_gitmodules_does_not_register( LATIN_1_COMMENT = b"# caf\xe9 \xff\xfe\n" #: Upper-case section and keys, which git lower-cases. UPPER_CASE_KEYS = ('[Submodule "upstream/dependency"]\n' - "\tPATH = upstream/dependency\n\tURL = ../dependency\n") + f"\tPATH = upstream/dependency\n\tURL = {THE_DEPENDENCY}\n") @pytest.mark.parametrize("gitmodules, expected", [ @@ -1535,7 +1538,7 @@ def test_execute_refuses_a_submodule_plan_before_anything_exists( ("code", "drop", None), # Hand-written registrations `check` reads as the leg's clone reads them: # upper-case keys, and two `git config --blob` refused. - ("code", "code", UPPER_CASE_KEYS.replace("../dependency", THE_DEPENDENCY)), + ("code", "code", UPPER_CASE_KEYS), ("code", "code", LATIN_1_COMMENT + registration(url=THE_DEPENDENCY).encode()), ("code", "code", registration(url=THE_DEPENDENCY) diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index d7c1e3fb..3bf618c5 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -2732,6 +2732,38 @@ def test_the_doctor_and_siblings_run_ONE_definition_of_one_repository( "https://host/IRRS.git", "https://host/x") == "https://host/IRRS.git" +def test_a_port_and_githubs_ssh_host_are_not_part_of_a_remotes_key(): + """`remote_key` TAKES A PORT, AND `ssh.github.com`, OFF THE HOST (#190). + + `ssh://git@github.com:22/Acme/Widget.git`, and + `ssh://git@ssh.github.com:443/Acme/Widget.git` as GitHub documents SSH + over the HTTPS port, are the scp `git@github.com:Acme/Widget.git` + reached another way, and keyed as `github.com/22/acme/widget`. + `same_repository`'s forge-tail match hid that for an `Owner/Repo`, but + not for a repository at a host's top level, and `adopt-project.py` + compares keys and nothing else, so it refused a source cloned that way. + """ + scp = repo_shape.remote_key("git@github.com:Acme/Widget.git") + for url in ("ssh://git@github.com:22/Acme/Widget.git", + "ssh://git@ssh.github.com:443/Acme/Widget.git", + "ssh://git@SSH.GitHub.com:443/Acme/Widget.git", + "https://github.com:443/Acme/Widget.git", + "https://x-access-token:token@github.com:443/Acme/Widget.git", + "git@ssh.github.com:Acme/Widget.git"): + assert repo_shape.remote_key(url) == scp, url + assert repo_shape.same_repository("ssh://git@host:2222/Repo.git", + "git@host:Repo.git") + # ...and nothing that is not a port: the scp spelling has none, so its + # `22` is a path component; a drive's colon carries no digits; and + # another repository, or another host, is still another. + for url in ("git@github.com:22/Acme/Widget.git", + "ssh://git@github.com:22/Acme/Other.git", + "ssh://git@gitlab.com:22/Acme/Widget.git"): + assert repo_shape.remote_key(url) != scp, url + assert repo_shape.remote_key("file:///E:/mirrors/Widget.git") \ + == repo_shape.remote_key("E:\\mirrors\\Widget.git") + + def test_a_symlinked_prefix_is_ONE_directory_to_both_callers(tmp_path, siblings): """THE PREFIX CASE #157 MOVED OUT OF THE RULE AND INTO THE CALLERS.