From 9eded6fc02ed060d18e494f2fdd7dd55540c7ea8 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 11:04:22 +0000 Subject: [PATCH 1/4] WIP (usage checkpoint 2026-10-17; untested on these bytes): adopt-project.py: a `--source` or a recorded `local_path` that names no repository's top level is refused by name (#796) Work in progress for #796 (L1-64, L1-88 and its rider, L1-95, L1-96, L1-100, L1-101, L1-77): - `source-not-top-level`, a new code, for a file or a directory inside a repository (`_top_level`, `git rev-parse --show-prefix`), given as `--source` or recorded as `source.local_path`; - an empty `--source` refused `source-unresolvable`, and `check` and `execute` no longer drop one (`override is not None`); - `os.path.expanduser`, so an unknown `~user` is a path that is not there; - `_github_name` refuses the repository `.git`; `www.github.com` is GitHub's host; - `Source.tree` refuses a size git prints as `BAD`, naming the path and `git fsck`; - `plan` reads `--source` before it makes the work directory; - the `source` line names the directory read, with its `origin`'s repository beside it; - `--help` and docs/cli.md name the new code. The two test files passed (114 passed) one edit before these bytes; the last edits (the `_source_named` helper, a test helper, a comment) are untested, and neither mode has been run in full yet. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 211 +++++++++++++--- docs/cli.md | 13 +- tests/test_adopt_source_clone_record.py | 305 +++++++++++++++++++++++- tests/test_adopt_work_dir_refusal.py | 37 +++ 4 files changed, 532 insertions(+), 34 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 4e36ec55..4be383a9 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1559,6 +1559,19 @@ def _read_for_the_history(source: "Source", tree: list, PLAN_SOURCE_GONE = "plan-source-gone" #: The refusal of a `--source` that names nothing, as `Source.open` words it. SOURCE_UNRESOLVABLE = "source-unresolvable" +#: The refusal of a local source that is no repository's top-level +#: directory: a file, or a directory inside a repository (#796). +SOURCE_NOT_TOP_LEVEL = "source-not-top-level" +#: What `source-not-top-level` says a source is, and its way out, for a +#: `--source` and a plan's `source.local_path` alike. +TOP_LEVEL_ONLY = "a source is the top-level directory of a repository" +TOP_LEVEL_REMEDY = ( + "Remediation: pass --source the repository's top-level directory, which " + "`git rev-parse --show-toplevel` prints in any directory of it, or its " + "`org/repo`.") +#: GitHub's host as a browser's address bar spells it, read as +#: `GITHUB_HOST` (#796): on main it was refused as another host. +WWW_GITHUB_HOST = f"www.{GITHUB_HOST}" #: One segment of a GitHub `owner/repo` as `gh` reads it. `.` and `..` match #: it and are refused apart, so the clone directory is always one name. GITHUB_SEGMENT_RE = re.compile(r"[A-Za-z0-9_.-]+") @@ -1567,11 +1580,13 @@ def _read_for_the_history(source: "Source", tree: list, PATH_SPELLING_RE = re.compile(r"[/~]|\.{1,2}(?:[/\\]|$)|\\\\") #: A `--source` written as a URL that names a repository on a host: a scheme #: and its host, git's `user@host:` form, or `github.com/` with no scheme, -#: which `gh` itself reads as `HOST/OWNER/REPO`. `path` is the rest. +#: which `gh` itself reads as `HOST/OWNER/REPO`, or `www.github.com/` (#796). +#: `path` is the rest. SOURCE_URL_RE = re.compile( r"(?:(?:https?|ssh|git)://(?:[^@/]+@)?(?P[^/:@]+)(?::\d*)?/" r"|[^@/:]+@(?P[^/:]+):" - r"|(?Pgithub\.com)/)(?P.*)", re.IGNORECASE | re.DOTALL) + r"|(?P(?:www\.)?github\.com)/)(?P.*)", + re.IGNORECASE | re.DOTALL) def _github_name(written: str) -> str | None: @@ -1586,14 +1601,19 @@ def _github_name(written: str) -> str | None: auth status`. Two segments must be left, each one `gh` takes, neither `.` nor `..`, and the owner beginning with neither `.`, which no GitHub owner does, nor `-`, which gh's parser reads as an option: `-u/x` - set its `--upstream-remote-name` (S8705). Case is kept as typed.""" + set its `--upstream-remote-name` (S8705). Case is kept as typed. + + NOR IS THE REPOSITORY `.git` (#796), as `owner/.git` and + `owner/.git.git` leave it: on main `gh` was handed `owner/.git`, to clone + into `source-readonly/.git`, and refused it `source-unclonable`, with a + remedy about `gh auth status`.""" segments = [part for part in written.split("/") if part] if len(segments) != 2: return None owner, repo = segments if repo.endswith(".git") and len(repo) > len(".git"): repo = repo[:-len(".git")] - if owner.startswith(("-", ".")) or not all( + if owner.startswith(("-", ".")) or repo == ".git" or not all( GITHUB_SEGMENT_RE.fullmatch(part) and part not in (".", "..") for part in (owner, repo)): return None @@ -1616,8 +1636,11 @@ def _repository_in_url(spec: str, url: re.Match, candidate: Path) -> str: """The `owner/repo` a url `--source` names (`SOURCE_URL_RE`'s `url`): refused where its host is not GitHub's, where it names no `owner/repo`, and, for `github.com//` written with no scheme, where it is - a path here as well (`candidate`), as `_source_spec` refuses a name.""" + a path here as well (`candidate`), as `_source_spec` refuses a name. + `www.github.com` is GitHub's host, as a browser spells it (#796).""" host = (url["host"] or url["scp"] or url["bare"]).lower() + if host == WWW_GITHUB_HOST: + host = GITHUB_HOST name = _github_name(url["path"]) if host != GITHUB_HOST: raise Refusal( @@ -1661,8 +1684,29 @@ def _source_spec(spec: str) -> tuple[Path | None, str | None]: TEXT (#255), which `_github_name` never reads as one, NOR ONE THAT BEGINS WITH `-`, the first argument `gh repo clone` is given, which gh's own parser reads as an option. A one-segment name (`Atlas`) is a path, as on - main, where it never reached `gh`.""" - candidate = Path(spec).expanduser() + main, where it never reached `gh`. + + A PATH IS A REPOSITORY'S TOP-LEVEL DIRECTORY, OR IT IS REFUSED (#796), + by name and before git reads anything else in it: the path is given + back resolved by `_top_level`. On main a file ended `plan`, `check` and + `execute` in a `NotADirectoryError` traceback, and a directory inside a + repository, or `.` run in one, was planned, exit 0, holding only what + is under it. An EMPTY value names no path, and is refused as + `pin-source-no-path` refuses an empty `--pin-source`: as a `Path` it is + `.`, so `--source "$SRC"` with `SRC` unset planned the directory it ran + in. A `~user` is expanded as `os.path.expanduser` expands it, which + leaves one this machine has not as typed, a path that is not there: + `Path.expanduser` raised `RuntimeError` for one mistyped user name, + which bash passes through unchanged.""" + if not spec: + raise Refusal( + SOURCE_UNRESOLVABLE, + "--source '' names no path: an empty one would be read as the " + "directory this runs in", + "Remediation: pass the repository's top-level directory (`.` for " + "this directory), or its `org/repo`. Without --source, `check` " + "and `execute` read the plan's own source.") + candidate = Path(os.path.expanduser(spec)) written_as_a_path = bool(PATH_SPELLING_RE.match(spec) or WINDOWS_ABSOLUTE_RE.match(spec)) url = None if written_as_a_path else SOURCE_URL_RE.fullmatch(spec) @@ -1673,7 +1717,7 @@ def _source_spec(spec: str) -> tuple[Path | None, str | None]: if candidate.exists(): if name is not None: raise _ambiguous(spec, name) - return candidate, None + return _top_level(candidate, f"--source {spec!r}"), None if name is None: raise Refusal( SOURCE_UNRESOLVABLE, @@ -1683,6 +1727,43 @@ def _source_spec(spec: str) -> tuple[Path | None, str | None]: return None, name +def _top_level(path: Path, named: str) -> Path: + """`path`, a local source that exists, resolved, where it is the + top-level directory of a repository; refused as `source-not-top-level` + where it is not a directory, or is a directory inside a repository + (#796). `named` is how the refusal names it: `--source ''`, or the + plan's `source.local_path`, which `Plan._recorded_source` reads. + + BEFORE GIT READS ANYTHING ELSE IN IT. A file is asked of the disk, and + git is never run in it: `subprocess` raised `NotADirectoryError` for + one, a traceback. A directory is asked `git rev-parse --show-prefix`, + which prints nothing at the top level, of a work tree or a bare + repository, and the path of the directory below it otherwise; only + then is `--show-toplevel` asked, for the refusal to name. A directory + git reads no repository from is refused `git-failed` here, in git's own + words, as a later read refused it on main. On main a directory inside a + repository was read as the source: `git ls-tree` there lists what is + under it alone, so `plan` planned that, exit 0, and the plan recorded + it.""" + resolved = Path(os.path.realpath(path)) + if not resolved.is_dir(): + raise Refusal(SOURCE_NOT_TOP_LEVEL, + f"{named} is not a directory, and {TOP_LEVEL_ONLY}", + TOP_LEVEL_REMEDY) + prefix = os.fsdecode(git_out(["rev-parse", "--show-prefix"], + cwd=resolved, binary=True).rstrip(b"\n")) + if prefix: + top = os.fsdecode(git_out(["rev-parse", "--show-toplevel"], + cwd=resolved, binary=True).rstrip(b"\n")) + raise Refusal( + SOURCE_NOT_TOP_LEVEL, + f"{named} is {_spelled(prefix)} inside the repository at " + f"{_spelled(top)}, and {TOP_LEVEL_ONLY}: read from there, it " + f"would be only what is under {_spelled(prefix)}", + TOP_LEVEL_REMEDY) + return resolved + + def _occupied_clone_target(work_root: Path, path: Path, repository: str, taken: str = "", done: str = "") -> Refusal: """`adopt-work-dir-occupied` for a clone target `path` that already @@ -1765,7 +1846,16 @@ def open(cls, spec: str, work_root: Path, unwritable: str | None = None, """`spec`, a `--source`, as `_source_spec` reads it: a local path, read in place, or a GitHub repository, cloned (`clone`, which takes `unwritable`, `made` and `done`).""" - path, repository = _source_spec(spec) + return cls.of(_source_spec(spec), work_root, unwritable, made, done) + + @classmethod + def of(cls, named: tuple[Path | None, str | None], work_root: Path, + unwritable: str | None = None, made: list | None = None, + done: str = "") -> "Source": + """The source `named` names, `_source_spec`'s answer, which `plan` + asks before it makes its work directory (#796): the top-level + directory it gave back, read in place, or the repository, cloned.""" + path, repository = named if repository is not None: return cls.clone(repository, work_root, unwritable, made, done=done) @@ -1774,8 +1864,8 @@ def open(cls, spec: str, work_root: Path, unwritable: str | None = None, @classmethod def in_place(cls, path: Path, work_root: Path) -> "Source": """The repository at `path`, read where it is, named by its - `origin`.""" - path = path.expanduser().resolve() + `origin`. `path` is a top-level directory `_top_level` gave back, + resolved (#796).""" return cls._at(path, _remote_repository(path), work_root, False) @classmethod @@ -1838,17 +1928,35 @@ def _at(cls, path: Path, repository: str | None, work_root: Path, return cls(path, repository, branch, commit, work_root, cloned) def tree(self) -> list[tuple[str, str, str, int]]: - """(path, mode, blob sha, size) for every file at the pinned commit.""" - raw = git_out(["ls-tree", "-r", "-l", "-z", self.commit], - cwd=self.path, binary=True) + """(path, mode, blob sha, size) for every file at the pinned commit. + + A SIZE IS `-` OR DIGITS, OR THE SOURCE HAS LOST AN OBJECT (#796). + `git ls-tree -l` prints `BAD` for the size of a blob it cannot read + and still exits 0, and on main `int('BAD')` ended `plan`, `check` + and `execute` in a `ValueError` traceback. It is refused, naming + the path and `git fsck`, which names every object a repository + lacks.""" + command = ["ls-tree", "-r", "-l", "-z", self.commit] + raw = git_out(command, cwd=self.path, binary=True) out = [] for record in raw.split(b"\x00"): if not record: continue head, _, path = record.partition(b"\t") mode, kind, oid, size = head.decode().split(maxsplit=3) + size = size.strip() + if size != "-" and not (size.isascii() and size.isdigit()): + raise Refusal( + "git-failed", + f"`git {' '.join(command)}` in {shown_path(self.path)} " + f"gives no size for {_spelled(_git_path(path))}, whose " + f"blob {oid} it cannot read: it prints {_spelled(size)}", + f"Remediation: run `git fsck` in " + f"{shown_path(self.path)}: it names each object the " + "repository lacks. Restore them, or pass a clone that " + "holds them all, and run the command again.") out.append((_git_path(path), mode, oid, - 0 if size.strip() == "-" else int(size))) + 0 if size == "-" else int(size))) del kind return sorted(out) @@ -4338,9 +4446,9 @@ def _print_plan_report(args, source: Source, tree: list, entries: list, a question printed above a summary table is a question that gets scrolled past. """ - print(f"source {source.repository or _shown(str(source.path))} @ " - f"{source.commit[:12]} ({source.branch}), " - f"{len(tree)} files, {source.commit_count()} commits") + print(f"source {_source_named(source)} @ {source.commit[:12]} " + f"({source.branch}), {len(tree)} files, " + f"{source.commit_count()} commits") print(f"project {args.project} ({args.id}) in {args.org}, mode in-place") # Each pin as the scaffold's plan prints it: what is pinned, at which # commit and digest, the file `execute` writes, and how the digest was @@ -4386,6 +4494,20 @@ def _print_plan_report(args, source: Source, tree: list, entries: list, print(f" {_shown(entry.path)}\n {entry.question}") +def _source_named(source: Source) -> str: + """The source as `plan`'s `source` line names it. A clone `plan` made + is its repository. A source read in place is its directory, with the + repository its `origin` names beside it (#796): on main the line named + only that repository, so it read the same whatever directory of it was + read, a directory inside it included.""" + if source.cloned: + return str(source.repository) + where = _shown(str(source.path)) + if source.repository: + where += f" (origin: {_shown(source.repository)})" + return where + + def cmd_plan(args) -> int: # A `--pin-source` that names no path, `''`, which as a `Path` is the # directory this runs in, one no `--pin` will read, and one `plan` cannot @@ -4403,15 +4525,22 @@ def cmd_plan(args) -> int: nothing_made=PLAN_NOT_WRITTEN) if args.pin and args.pin_source is not None: _refuse_an_unrecordable_pin_source(args.pin_source) + # What `--source` names, next, before the work directory is made + # (#796): every refusal of it that needs none -- an empty value, a path + # that is no repository's top level, a name `gh` cannot clone -- is + # asked here, and only `git rev-parse` has run in a local source. On + # main a file ended `plan` in a traceback after the `--work-dir` it was + # given had been made, and that directory stayed. + named = _source_spec(args.source) work_root = _work_root(args) # An `org/repo` source is cloned into the work directory, so one this # user cannot write in is refused before the clone, as `check` refuses # it (#650); a local source is read in place, and writes nothing there. # The clone goes on `args.clones`, and `_leave_the_clones` removes it # unless this run ends having written the plan that records it (#652). - source = Source.open(args.source, work_root, - _unwritable_work_dir(args, work_root), - getattr(args, "clones", None)) + source = Source.of(named, work_root, + _unwritable_work_dir(args, work_root), + getattr(args, "clones", None)) policy = PathPolicy.load(args.path_policy or PATH_POLICY) naming = NamingPolicy.load(NAMING_POLICY) @@ -4633,8 +4762,12 @@ def open_source(self, override: str | None, work_root: Path, A RECORDED NAME IS ALWAYS CLONED FROM GITHUB, never read as a directory here (#652): on main `check`, run where `./acme/Atlas` existed, said the plan's repository is cloned again and read that - directory instead.""" - if override: + directory instead. + + AN EMPTY `override` IS ONE GIVEN (#796), and `Source.open` refuses + it: on main `check --source ''` and `execute --source ''` dropped it + and read the plan's own source.""" + if override is not None: return Source.open(override, work_root, unwritable, made, done) local_path = self.source.get("local_path") if local_path: @@ -4694,12 +4827,21 @@ def _recorded_source(self, local_path: str, work_root: Path, Where the clone would land on the plan's own clone, no longer whole (the same `--work-dir`), it is refused, and nothing there is removed; where that path is free, the clone made there is the one - the plan records, and is not on `made`.""" - path = Path(local_path).expanduser() + the plan records, and is not on `made`. + + A PATH THE PERSON GAVE IS HELD TO BEING A REPOSITORY'S TOP LEVEL, as + `--source` is (`_top_level`, #796), and refused by its key: a file + ended `check` and `execute` in a traceback, and a directory inside a + repository was read as the source. A `~user` this machine has not is + a path that is not there (`os.path.expanduser`): `Path.expanduser` + raised `RuntimeError` for it.""" + path = Path(os.path.expanduser(local_path)) there = path.exists() if self.source.get("cloned") is not True: if there: - return Source.in_place(path, work_root) + return Source.in_place(_top_level( + path, f"the plan's `source.local_path`, " + f"{_spelled(local_path)},"), work_root) raise self._gone(local_path, "names nothing that exists", "the plan does not record it as a clone `plan` " "made (`source.cloned` is not `true`), so " @@ -4707,7 +4849,7 @@ def _recorded_source(self, local_path: str, work_root: Path, broken = _not_a_whole_clone(path, self.source_commit) if there \ else None if there and broken is None: - return Source.in_place(path, work_root) + return Source.in_place(Path(os.path.realpath(path)), work_root) state = (f"is a clone `plan` made that is no longer whole ({broken})" if there else "no longer exists") repository = self._recorded_repository() @@ -13188,13 +13330,15 @@ def _work_dir_help(does: str, refused: str, but: str = "") -> str: "a source is cloned, is refused as " "`adopt-work-dir-occupied`.") #: `--source`'s help on `check` and `execute`: how the source is read with -#: it and without it (#652). +#: it and without it (#652), and a recorded path refused (#796). SOURCE_OVERRIDE_HELP = ( "override the source the plan names, read as `plan --source` reads it. " "Without it, the plan's `source.local_path` is read; where that is a " "clone `plan` made (`source.cloned: true`) and is gone or no longer " "whole, its `source.repository` is cloned again, and any other " - "`local_path` that is gone is refused as `plan-source-gone`.") + "`local_path` that is gone is refused as `plan-source-gone`; one that " + "is a file, or a directory inside a repository, as " + "`source-not-top-level`.") def main(argv: list[str] | None = None) -> int: @@ -13213,7 +13357,14 @@ def main(argv: list[str] | None = None) -> int: "too. `source-ambiguous` refuses a name that is " "also a path here, as `acme/Atlas` is beside a " "directory `acme/Atlas`: pass `./acme/Atlas` for " - "the directory, or the url for the repository.") + "the directory, or the url for the repository. " + "The path is a repository's top-level directory " + "(`git rev-parse --show-toplevel` prints it): " + "`source-not-top-level` refuses a file, or a " + "directory inside a repository. " + "`source-unresolvable` refuses an empty value, " + "and one that is neither a path that exists nor " + "an `org/repo`.") plan.add_argument("--project", required=True, help="the assembly-root name; for an in-place adoption " "it is the source repository's own name") diff --git a/docs/cli.md b/docs/cli.md index ad2cfa9b..a015bb7d 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -419,7 +419,12 @@ options: `source-ambiguous` refuses a name that is also a path here, as `acme/Atlas` is beside a directory `acme/Atlas`: pass `./acme/Atlas` for the directory, - or the url for the repository. + or the url for the repository. The path is a + repository's top-level directory (`git rev-parse + --show-toplevel` prints it): `source-not-top-level` + refuses a file, or a directory inside a repository. + `source-unresolvable` refuses an empty value, and one + that is neither a path that exists nor an `org/repo`. --project PROJECT the assembly-root name; for an in-place adoption it is the source repository's own name --org ORG the organisation the source repository is in, which @@ -557,7 +562,8 @@ options: `plan` made (`source.cloned: true`) and is gone or no longer whole, its `source.repository` is cloned again, and any other `local_path` that is gone is refused as - `plan-source-gone`. + `plan-source-gone`; one that is a file, or a directory + inside a repository, as `source-not-top-level`. --local-remote-dir LOCAL_REMOTE_DIR the directory `execute --local-remote-dir` will be given (the TEST path): each relative submodule url the @@ -600,7 +606,8 @@ options: `plan` made (`source.cloned: true`) and is gone or no longer whole, its `source.repository` is cloned again, and any other `local_path` that is gone is refused as - `plan-source-gone`. + `plan-source-gone`; one that is a file, or a directory + inside a repository, as `source-not-top-level`. --local-remote-dir LOCAL_REMOTE_DIR create the legs as bare repositories here instead of calling `gh` (the TEST path) diff --git a/tests/test_adopt_source_clone_record.py b/tests/test_adopt_source_clone_record.py index b9ee0800..8281ba2e 100644 --- a/tests/test_adopt_source_clone_record.py +++ b/tests/test_adopt_source_clone_record.py @@ -26,6 +26,23 @@ run given the same `--work-dir` next was refused at it, `source- unclonable`, with a remedy about `gh auth status`. +And each `--source` that names no repository's top level, nor a repository +on GitHub, is refused by name before git reads anything else, with no plan +written and no `gh` run (#796). ON MAIN AT bf1f099b (2026-10-17): +- L1-64. A file ended `plan`, `check` and `execute` in a + `NotADirectoryError` traceback, exit 1. +- L1-88. A directory inside a repository, or `.` run in one, was planned, + exit 0, holding only what is under it; and the `source` line named the + `origin`'s repository, whatever directory was read. +- L1-95. `~nouser/x` ended each in `RuntimeError: Could not determine home + directory.`, and so did a plan recording it. +- L1-96. `--source ''` inside a repository planned that repository, and + `check` and `execute` dropped the value. +- L1-100 and L1-101. `owner/.git` was handed to `gh`; `www.github.com` was + refused as another host, or as neither a path nor a name. +- L1-77. A source that lost a blob ended each in a `ValueError` at + `Source.tree`, exit 1. + NO NETWORK AND NO GITHUB: `gh repo clone` is a `git clone` of a repository on disk, and only `testorg/Thing` is found (`gh_clones`). """ @@ -73,6 +90,8 @@ "held to the plan's `commit:`.") MINE = "mine\n" MARKER = "mine.txt" +#: What `source-unresolvable` says of a `--source` that names nothing. +NEITHER = "is neither a path that exists nor an `org/repo` name" def run(adopter, capsys, *argv: str) -> tuple[int, str, str]: @@ -216,6 +235,11 @@ def test_a_recorded_repository_is_cloned_again_wherever_check_runs( "url-upper-case": f"HTTPS://GitHub.com/{REPOSITORY}", "scp": f"git@github.com:{REPOSITORY}.git", "no-scheme": f"github.com/{REPOSITORY}", + # #796's L1-101: GitHub's own host, as a browser's address bar spells it. + # On main the url was refused as a repository on another host, and the + # form with no scheme as neither a path nor a name. + "www-url": f"https://www.github.com/{REPOSITORY}", + "www-no-scheme": f"WWW.GitHub.com/{REPOSITORY}", } @@ -241,6 +265,12 @@ def test_each_spelling_of_a_name_is_cloned_and_recorded_as_owner_repo( "clones from github.com alone"), "three-segments": (f"a/{REPOSITORY}", "is neither a path that exists nor an `org/repo` name"), + # #796's L1-100: a repository named `.git`, which `gh` was handed, to + # clone into `source-readonly/.git`, and refused `source-unclonable` + # with a remedy about `gh auth status`. + "dot-git-repository": (f"{ORG}/.git", NEITHER), + "url-dot-git-repository": (f"https://github.com/{ORG}/.git.git", + "names no `owner/repo` on github.com"), } @@ -733,11 +763,284 @@ def test_execute_into_a_work_dir_holding_its_clone_says_nothing_exists( "it once nothing in it is needed.") +# --- #796: a `--source` that names no repository's top level ----------------- + +NOT_TOP_LEVEL = "REFUSED source-not-top-level: " +#: What `source-not-top-level` says a source is, after what was given. +TOP_LEVEL_ONLY = "a source is the top-level directory of a repository" +#: Its way out, for a `--source` and a recorded `local_path` alike. +TOP_LEVEL_REMEDY = ( + "Remediation: pass --source the repository's top-level directory, which " + "`git rev-parse --show-toplevel` prints in any directory of it, or its " + "`org/repo`.") +README = "README.md" +#: A path inside the local source that is no top level: a file, a directory. +INSIDE = {"file": README, "directory": "src"} +#: A `~user` no machine running the suite has, which bash passes through as +#: typed, and `os.path.expanduser` leaves as typed. +NO_SUCH_USER = "~no-such-user-796/Thing" +#: The empty `--source`, as `--source "$SRC"` passes it with `SRC` unset. +EMPTY_REFUSED = [ + f"{UNRESOLVABLE}--source '' names no path: an empty one would be read " + "as the directory this runs in", + "Remediation: pass the repository's top-level directory (`.` for this " + "directory), or its `org/repo`. Without --source, `check` and `execute` " + "read the plan's own source."] +AGAIN = "again.yaml" + + +def local_source(tmp_path: Path) -> Path: + """A clone of `testorg/Thing` on this machine: its `origin` names it.""" + source = make_source_repo(tmp_path / "local" / PROJECT, tree=SMALL_TREE, + edits=()) + git("remote", "add", "origin", f"https://github.com/{REPOSITORY}.git", + cwd=source) + return source + + +def no_github(adopter, monkeypatch, tmp_path: Path) -> list: + """`gh repo clone` stood in for, finding nothing: its calls.""" + return gh_clones(adopter, monkeypatch, tmp_path / "nowhere") + + +def planned_here(adopter, monkeypatch, capsys, + tmp_path: Path) -> tuple[Path, Path, list]: + """`(source, plan, gh calls)`: `plan` of `local_source`, in this + process, with `gh repo clone` stood in for; stdin is no terminal.""" + calls = no_github(adopter, monkeypatch, tmp_path) + monkeypatch.setattr(sys, "stdin", io.StringIO("")) + source = local_source(tmp_path) + plan = tmp_path / PLAN + code, _, err = run(adopter, capsys, *plan_args( + str(source), plan, WORK_FLAG, str(tmp_path / WORK))) + assert code == 0, err + return source, plan, calls + + +def given_source(command: str, plan: Path, given: str, + tmp_path: Path) -> list[str]: + """`command --source given`: `plan`, writing `AGAIN`; or `check` or + `execute` of `plan`, given a `--local-remote-dir` in `tmp_path`.""" + if command == PLAN_VERB: + return plan_args(given, tmp_path / AGAIN) + return [command, PLAN_FLAG, str(plan), SOURCE_FLAG, given, + *remotes(tmp_path)] + + +def remotes(tmp_path: Path) -> list[str]: + """`--local-remote-dir`, in `tmp_path`: `execute` makes no `gh` call.""" + return [LOCAL_REMOTE_DIR, str(tmp_path / REMOTES)] + + +def nothing_was_made(tmp_path: Path, calls: list) -> None: + """No plan written, no leg repository made, no `gh` run.""" + assert not (tmp_path / AGAIN).exists() + assert not (tmp_path / REMOTES).exists() + assert calls == [] + + +def not_top_level(named: str, source: Path, inside: str) -> str: + """The first line of `source-not-top-level` for `inside`, a path in + `source`, which the refusal calls `named`.""" + if inside == INSIDE["file"]: + return (f"{NOT_TOP_LEVEL}{named} is not a directory, and " + f"{TOP_LEVEL_ONLY}") + return (f"{NOT_TOP_LEVEL}{named} is `{inside}/` inside the repository at " + f"{spelled(os.path.realpath(source))}, and {TOP_LEVEL_ONLY}: read " + f"from there, it would be only what is under `{inside}/`") + + +def local_path_now(adopter, plan: Path, value: str) -> None: + """The plan's `source.local_path` edited to `value`, as a person may.""" + edited(plan, f" local_path: {adopter.y(recorded(plan)['local_path'])}" + .encode(), f" local_path: {adopter.y(value)}".encode()) + + +@pytest.mark.parametrize("command", [PLAN_VERB, CHECK, EXECUTE]) +@pytest.mark.parametrize("inside", list(INSIDE.values()), ids=list(INSIDE)) +def test_a_source_that_is_no_top_level_is_refused_by_name( + adopter, monkeypatch, capsys, tmp_path, command, inside): + """L1-64 and L1-88. On main a file ended each command in a + `NotADirectoryError` traceback, exit 1; `plan` of a directory inside the + repository planned only what is under it, exit 0, and `check` then + refused `git-failed` and `execute` `adopt-remote-unreadable`. Now each + is refused before git reads anything else in it.""" + source, plan, calls = planned_here(adopter, monkeypatch, capsys, tmp_path) + given = str(source / inside) + code, out, err = run(adopter, capsys, + *given_source(command, plan, given, tmp_path)) + assert code == 2, err + assert err.splitlines() == [ + not_top_level(f"--source {given!r}", source, inside), TOP_LEVEL_REMEDY] + assert out == "" + nothing_was_made(tmp_path, calls) + + +@pytest.mark.parametrize("command", [CHECK, EXECUTE]) +@pytest.mark.parametrize("inside", list(INSIDE.values()), ids=list(INSIDE)) +def test_a_recorded_local_path_that_is_no_top_level_is_named_by_its_key( + adopter, monkeypatch, capsys, tmp_path, command, inside): + """A plan whose `local_path:` names a file or a directory inside the + repository, as a plan main wrote for one does: on main the first ended + in a traceback, and the second was read as the source.""" + source, plan, calls = planned_here(adopter, monkeypatch, capsys, tmp_path) + value = str(source / inside) + local_path_now(adopter, plan, value) + code, _, err = run(adopter, capsys, command, PLAN_FLAG, str(plan), + *remotes(tmp_path)) + assert code == 2, err + assert err.splitlines() == [not_top_level( + f"the plan's `source.local_path`, {spelled(value)},", source, inside), + TOP_LEVEL_REMEDY] + nothing_was_made(tmp_path, calls) + + +def test_dot_in_a_directory_inside_the_repository_is_refused( + adopter, monkeypatch, capsys, tmp_path): + """L1-88: on main `plan --source .` run in `/src` planned what is + under `src/`, exit 0.""" + calls = no_github(adopter, monkeypatch, tmp_path) + source = local_source(tmp_path) + monkeypatch.chdir(source / INSIDE["directory"]) + code, _, err = run(adopter, capsys, *plan_args(".", tmp_path / AGAIN)) + assert code == 2, err + assert err.splitlines()[0] == not_top_level( + "--source '.'", source, INSIDE["directory"]) + nothing_was_made(tmp_path, calls) + + +@pytest.mark.parametrize("dot", [False, True], ids=["top-level", "dot-there"]) +def test_the_top_level_is_read_in_place(adopter, monkeypatch, capsys, + tmp_path, dot): + """The controls: the top-level directory, and `.` run there, plan the + whole repository, read where it is, as on main.""" + calls = no_github(adopter, monkeypatch, tmp_path) + source = local_source(tmp_path) + if dot: + monkeypatch.chdir(source) + plan = tmp_path / PLAN + code, _, err = run(adopter, capsys, + *plan_args("." if dot else str(source), plan)) + assert code == 0, err + assert recorded(plan)["local_path"] == os.path.realpath(source) + assert recorded(plan)["cloned"] is False + assert calls == [] + + +def test_the_source_line_names_the_directory_read_beside_its_repository( + adopter, monkeypatch, capsys, tmp_path): + """L1-88's rider: on main `plan` of a local directory said only the + repository its `origin` names, so the line read the same whatever + directory was read. A clone `plan` made is named by its repository.""" + source = local_source(tmp_path) + calls = on_github(adopter, monkeypatch, tmp_path) + assert source_line(adopter, capsys, str(source), tmp_path / PLAN) \ + .startswith(f"source {os.path.realpath(source)} (origin: " + f"{REPOSITORY}) @ ") + assert source_line(adopter, capsys, REPOSITORY, tmp_path / AGAIN) \ + .startswith(f"source {REPOSITORY} @ ") + assert len(calls) == 1 + + +def source_line(adopter, capsys, spec: str, out: Path) -> str: + """The `source` line of `plan --source spec`'s report.""" + code, said, err = run(adopter, capsys, *plan_args(spec, out)) + assert code == 0, err + [line] = [line for line in said.splitlines() + if line.startswith("source ")] + return line + + +@pytest.mark.parametrize("command", [PLAN_VERB, CHECK, EXECUTE]) +def test_an_unknown_user_in_the_source_is_unresolvable( + adopter, monkeypatch, capsys, tmp_path, command): + """L1-95. On main `Path.expanduser` raised `RuntimeError: Could not + determine home directory.`, exit 1. One mistyped user name was enough.""" + _, plan, calls = planned_here(adopter, monkeypatch, capsys, tmp_path) + code, _, err = run(adopter, capsys, + *given_source(command, plan, NO_SUCH_USER, tmp_path)) + assert code == 2, err + assert err.startswith( + f"{UNRESOLVABLE}--source {NO_SUCH_USER!r} {NEITHER}\n"), err + nothing_was_made(tmp_path, calls) + + +@pytest.mark.parametrize("command", [CHECK, EXECUTE]) +def test_a_recorded_path_under_an_unknown_user_is_gone( + adopter, monkeypatch, capsys, tmp_path, command): + """A path the person gave: on main `check` and `execute` ended in the + same `RuntimeError` at `Plan._recorded_source`.""" + _, plan, calls = planned_here(adopter, monkeypatch, capsys, tmp_path) + local_path_now(adopter, plan, NO_SUCH_USER) + code, _, err = run(adopter, capsys, command, PLAN_FLAG, str(plan)) + assert code == 2, err + assert err.splitlines()[:2] == [ + f"{GONE_REFUSED}the plan's `source.local_path`, " + f"{spelled(NO_SUCH_USER)}, {NOT_MADE}", GONE_REMEDY] + assert calls == [] + + +def test_a_recorded_clone_under_an_unknown_user_is_cloned_again( + adopter, monkeypatch, capsys, tmp_path): + """A clone `plan` made, recorded under a path that is gone, is cloned + again, as any clone of its that is gone is.""" + planned = planned_from_github(adopter, monkeypatch, tmp_path) + local_path_now(adopter, planned.plan, NO_SUCH_USER) + code, out, err = run(adopter, capsys, CHECK, PLAN_FLAG, str(planned.plan)) + assert code == 0, err + assert f"{GONE}{spelled(NO_SUCH_USER)}, no longer exists, so its " in out + assert [call[:4] for call in planned.calls] == [CLONE, CLONE] + + +@pytest.mark.parametrize("command", [PLAN_VERB, CHECK, EXECUTE]) +def test_an_empty_source_names_no_path_and_is_refused( + adopter, monkeypatch, capsys, tmp_path, command): + """L1-96. On main `plan --source ''` run inside a repository planned that + repository, exit 0, as `--source "$SRC"` did with `SRC` unset; `check` + and `execute` dropped the value and read the plan's own source.""" + source, plan, calls = planned_here(adopter, monkeypatch, capsys, tmp_path) + monkeypatch.chdir(source) + code, _, err = run(adopter, capsys, + *given_source(command, plan, "", tmp_path)) + assert code == 2, err + assert err.splitlines() == EMPTY_REFUSED + nothing_was_made(tmp_path, calls) + + +def blob_lost(source: Path, path: str) -> None: + """The blob of `path` at the source's tip removed from its object store, + as a cleaner or a bad copy loses one.""" + oid = git("rev-parse", f"HEAD:{path}", cwd=source).stdout.strip() + writable(source / ".git" / "objects" / oid[:2] / oid[2:]).unlink() + + +@pytest.mark.parametrize("command", [PLAN_VERB, CHECK, EXECUTE]) +def test_a_source_that_lost_a_blob_is_refused_naming_it_and_git_fsck( + adopter, monkeypatch, capsys, tmp_path, command): + """L1-77. `git ls-tree -r -l` prints `BAD` for the size of a blob it + cannot read and still exits 0: on main each command ended in a + `ValueError` traceback at `Source.tree`, exit 1.""" + source, plan, calls = planned_here(adopter, monkeypatch, capsys, tmp_path) + blob_lost(source, README) + code, _, err = run(adopter, capsys, + *given_source(command, plan, str(source), tmp_path)) + assert code == 2, err + first, remedy = err.splitlines()[:2] + assert first.startswith("REFUSED git-failed: `git ls-tree -r -l -z "), err + assert f" gives no size for `{README}`, " in first, err + assert remedy.startswith( + f"Remediation: run `git fsck` in {os.path.realpath(source)}: "), err + nothing_was_made(tmp_path, calls) + + # --- what `--help` says ------------------------------------------------------- @pytest.mark.parametrize(("command", "code"), [ (PLAN_VERB, "source-ambiguous"), (CHECK, "plan-source-gone"), - (EXECUTE, "plan-source-gone")]) + (EXECUTE, "plan-source-gone"), + # #796: a path that is no repository's top level, and an empty value. + (PLAN_VERB, "source-not-top-level"), (CHECK, "source-not-top-level"), + (EXECUTE, "source-not-top-level"), (PLAN_VERB, "source-unresolvable")]) def test_the_source_help_names_its_refusal_whole_on_one_line(command, code): """`docs/cli.md` is grepped for the code a tool prints.""" result = run_script(ADOPT, command, "--help", env=HELP_ENV) diff --git a/tests/test_adopt_work_dir_refusal.py b/tests/test_adopt_work_dir_refusal.py index d8c03807..98e6ce14 100644 --- a/tests/test_adopt_work_dir_refusal.py +++ b/tests/test_adopt_work_dir_refusal.py @@ -24,6 +24,10 @@ - L1-25 and L1-49. `--work-dir` had no help on any of the three commands, and `--org` none on `plan`, so `docs/cli.md` said nothing of either. +And (#796) `plan` refuses a `--source` that is a file, a directory inside +a repository, empty or under a `~user` this machine has not, before it makes +the `--work-dir`: on main it made it first. + `chmod` keeps neither root nor a Windows user from making a directory, so the tests of such a directory skip where it cannot (`unwritable_work`). Nothing here reaches `gh`: every refusal comes before any clone. @@ -229,6 +233,39 @@ def test_check_asks_the_work_dir_before_it_says_it_clones_again( assert executed.stderr == refused(EXECUTE, cannot_write(unwritable_work)) +# --------------------------------------------------------------------------- +# #796: `plan` refuses a `--source` before it makes the `--work-dir` +# --------------------------------------------------------------------------- + +#: `--source` values `plan` refuses before anything exists, each as the +#: source repository `THING` in `base` would be given: a file in it, a +#: directory inside it, nothing, and a `~user` this machine has not. +REFUSED_SOURCES = { + "a-file": lambda source: str(source / "README.md"), + "a-directory-inside": lambda source: str(source / "src"), + "empty": lambda _: "", + "unknown-user": lambda _: "~no-such-user-796/Thing", +} + + +@pytest.mark.parametrize("given", list(REFUSED_SOURCES.values()), + ids=list(REFUSED_SOURCES)) +def test_plan_refuses_a_source_before_it_makes_the_work_dir(tmp_path, given): + """On main `plan` made the `--work-dir` it was given, and then ended in + a traceback (a file, `~user`) or planned a directory inside the + repository, or, for `''`, the one it ran in, exit 0. Each is refused + now, run in the repository, and the `--work-dir` is never made.""" + source = make_source_repo(tmp_path / THING, tree=BASIC) + work = tmp_path / "made-here" + result = run_script(ADOPT, PLAN_VERB, "--source", given(source), + *PLANNED, "--out", str(tmp_path / AGAIN), WORK_FLAG, + str(work), cwd=source) + assert result.returncode == 2, result.stderr + result.stdout + assert result.stderr.startswith("REFUSED source-"), result.stderr + assert not work.exists() + assert not (tmp_path / AGAIN).exists() + + # --------------------------------------------------------------------------- # L1-25 and L1-49: what `--help` says # --------------------------------------------------------------------------- From 2be76b98d49c1f2ffe835d850b733917a5adb819 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 13:52:25 +0000 Subject: [PATCH 2/4] tests/test_adopt_source_clone_record.py: `.` and `..` as the repository of a `--source` name stay refused, as controls beside `.git`, so a later change to `_github_name` that read them as a repository no longer passes unseen (#796) `_github_name` has refused `.` and `..` since #652, whatever `.git` follows them (`testorg/..git`, `https://github.com/testorg/..`); no test held that through `--source`. Two control cases join #796's `.git` cases in `NAMES_NO_REPOSITORY`, and the phrase a url names is a constant. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_source_clone_record.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/test_adopt_source_clone_record.py b/tests/test_adopt_source_clone_record.py index 8281ba2e..57942477 100644 --- a/tests/test_adopt_source_clone_record.py +++ b/tests/test_adopt_source_clone_record.py @@ -90,8 +90,10 @@ "held to the plan's `commit:`.") MINE = "mine\n" MARKER = "mine.txt" -#: What `source-unresolvable` says of a `--source` that names nothing. +#: What `source-unresolvable` says of a `--source` that names nothing, and +#: of a url that names no repository on GitHub. NEITHER = "is neither a path that exists nor an `org/repo` name" +NO_OWNER_REPO = "names no `owner/repo` on github.com" def run(adopter, capsys, *argv: str) -> tuple[int, str, str]: @@ -270,7 +272,11 @@ def test_each_spelling_of_a_name_is_cloned_and_recorded_as_owner_repo( # with a remedy about `gh auth status`. "dot-git-repository": (f"{ORG}/.git", NEITHER), "url-dot-git-repository": (f"https://github.com/{ORG}/.git.git", - "names no `owner/repo` on github.com"), + NO_OWNER_REPO), + # Controls: `.` and `..` name no repository either, whatever `.git` + # follows them, as since #652. + "dot-dot-git-repository": (f"{ORG}/..git", NEITHER), + "url-dot-dot-repository": (f"https://github.com/{ORG}/..", NO_OWNER_REPO), } From adbc5e377cd2e112a62d6927944c90d35d7d1c35 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 15:39:08 +0000 Subject: [PATCH 3/4] tests/test_adopt_source_clone_record.py: `planned_here` stands in for the `git filter-repo` check, as `planned_from_github` does, so `execute` of a source that lost a blob reaches `Source.tree` with `git filter-repo` off PATH too, where it was refused `filter-repo-missing` first (#796) `execute` asks for `git filter-repo` after it opens the source and before it reads the source's tree. With it hidden, as in the second of the two modes, `test_a_source_that_lost_a_blob_is_refused_naming_it_and_git_fsck[execute]` met `filter-repo-missing` and never the lost blob. The other tests that use `planned_here` are refused before that check either way. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_source_clone_record.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_adopt_source_clone_record.py b/tests/test_adopt_source_clone_record.py index 57942477..2d1a7aef 100644 --- a/tests/test_adopt_source_clone_record.py +++ b/tests/test_adopt_source_clone_record.py @@ -812,8 +812,12 @@ def no_github(adopter, monkeypatch, tmp_path: Path) -> list: def planned_here(adopter, monkeypatch, capsys, tmp_path: Path) -> tuple[Path, Path, list]: """`(source, plan, gh calls)`: `plan` of `local_source`, in this - process, with `gh repo clone` stood in for; stdin is no terminal.""" + process, with `gh repo clone` stood in for; stdin is no terminal, and + `git filter-repo` is not asked for, as in `planned_from_github`: + `execute` asks for it before it reads the source's tree, so without + it on PATH a lost blob would never be reached.""" calls = no_github(adopter, monkeypatch, tmp_path) + monkeypatch.setattr(adopter, "_require_filter_repo", lambda: None) monkeypatch.setattr(sys, "stdin", io.StringIO("")) source = local_source(tmp_path) plan = tmp_path / PLAN From 2d51f1811a89eb661ac1f7541b0a6da2ad27cb43 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 19:19:57 +0000 Subject: [PATCH 4/4] adopt-project.py: a directory inside a git directory, a `.git` repository in any case and an empty recorded `local_path` are refused too, so `.git/objects` is no longer planned as the repository, `owner/.GIT` no longer reaches `gh`, and `local_path: ""` no longer clones the plan's repository from GitHub (#796) The three findings of #831's review (r796): - F1. Inside a git directory `git rev-parse --show-prefix` prints nothing, so `/.git/objects` and `.git/refs` passed `_top_level`: `plan` planned the whole repository from them, exit 0, and `check` and `execute` then failed as L1-88 did. `_top_level` asks `--is-inside-git-dir` first, and refuses `source-not-top-level`, naming the git directory, for any directory below one; a git directory itself (`/.git`, a bare repository, `.git/modules/`, `.git/worktrees/`) is read whole, as on main. - F2. `_github_name` compared `repo == ".git"` and dropped a trailing `.git` in that case alone, so `owner/.GIT` and `owner/.Git.git` reached `gh`. Both are now read without regard to case, as GitHub reads a repository's name. - F3. `Plan.open_source` tested `if local_path:`, so `local_path: ""` was read as none and the plan's `source.repository` was cloned from GitHub, which `_recorded_source` never does for a path the person gave. It is refused `plan-source-gone`, "names no path", before that test; `_gone`'s way out is the constant `PLAN_SOURCE_GONE_REMEDY`. Tests: `.git/objects` given and recorded, `.git/refs`, the three git directories read whole as controls, `.GIT` and `.Git.git` refused, `Thing.GIT` cloned as `testorg/Thing`, and `local_path: ""` under `check` and `execute`. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 68 +++++++++++++++------ tests/test_adopt_source_clone_record.py | 79 ++++++++++++++++++++++++- 2 files changed, 126 insertions(+), 21 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 4ac94de5..86a4b3ed 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1555,8 +1555,13 @@ def _read_for_the_history(source: "Source", tree: list, #: The refusal of a `--source` that is both a path here and a GitHub name. SOURCE_AMBIGUOUS = "source-ambiguous" #: The refusal of a plan's `source.local_path` that is gone, or no longer a -#: whole clone, where nothing clones it again. +#: whole clone, where nothing clones it again, or that names no path; and +#: its way out. PLAN_SOURCE_GONE = "plan-source-gone" +PLAN_SOURCE_GONE_REMEDY = ( + "Remediation: pass the source again with --source: the path its clone is " + "at now; or, where `plan` cloned it from GitHub, the `org/repo` it " + "cloned, which clones it again. Either is held to the plan's `commit:`.") #: The refusal of a `--source` that names nothing, as `Source.open` words it. SOURCE_UNRESOLVABLE = "source-unresolvable" #: The refusal of a local source that is no repository's top-level @@ -1603,17 +1608,20 @@ def _github_name(written: str) -> str | None: GitHub owner does, nor `-`, which gh's parser reads as an option: `-u/x` set its `--upstream-remote-name` (S8705). Case is kept as typed. - NOR IS THE REPOSITORY `.git` (#796), as `owner/.git` and - `owner/.git.git` leave it: on main `gh` was handed `owner/.git`, to clone - into `source-readonly/.git`, and refused it `source-unclonable`, with a - remedy about `gh auth status`.""" + NOR IS THE REPOSITORY `.git` (#796), in any case, as `owner/.git`, + `owner/.GIT` and `owner/.git.git` leave it: on main `gh` was handed + `owner/.git`, to clone into `source-readonly/.git`, and refused it + `source-unclonable`, with a remedy about `gh auth status`. GitHub reads + a repository's name without regard to case, and so does a disk that + ignores case, where `source-readonly/.GIT` is `source-readonly/.git`; + so the trailing `.git` is dropped in any case too.""" segments = [part for part in written.split("/") if part] if len(segments) != 2: return None owner, repo = segments - if repo.endswith(".git") and len(repo) > len(".git"): + if repo.lower().endswith(".git") and len(repo) > len(".git"): repo = repo[:-len(".git")] - if owner.startswith(("-", ".")) or repo == ".git" or not all( + if owner.startswith(("-", ".")) or repo.lower() == ".git" or not all( GITHUB_SEGMENT_RE.fullmatch(part) and part not in (".", "..") for part in (owner, repo)): return None @@ -1736,12 +1744,20 @@ def _top_level(path: Path, named: str) -> Path: BEFORE GIT READS ANYTHING ELSE IN IT. A file is asked of the disk, and git is never run in it: `subprocess` raised `NotADirectoryError` for - one, a traceback. A directory is asked `git rev-parse --show-prefix`, - which prints nothing at the top level, of a work tree or a bare - repository, and the path of the directory below it otherwise; only - then is `--show-toplevel` asked, for the refusal to name. A directory - git reads no repository from is refused `git-failed` here, in git's own - words, as a later read refused it on main. On main a directory inside a + one, a traceback. A directory is asked `git rev-parse + --is-inside-git-dir` first: a git directory itself (`/.git`, a + bare repository, `.git/modules/`, `.git/worktrees/`) is read + whole, as on main, and a directory below one (`.git/objects`, a bare + repository's `refs/`) is refused, naming the git directory `git + rev-parse --absolute-git-dir` gives. On main `plan` planned the whole + repository from `.git/objects`, exit 0, and recorded that directory, + which `check` then refused at `git ls-remote` of it, and `execute` as + `adopt-remote-unreadable`. Then `git rev-parse --show-prefix`, which + prints nothing at the top level, of a work tree or a git directory, and + the path of the directory below it otherwise; only then is + `--show-toplevel` asked, for the refusal to name. A directory git reads + no repository from is refused `git-failed` here, in git's own words, as + a later read refused it on main. On main a directory inside a repository was read as the source: `git ls-tree` there lists what is under it alone, so `plan` planned that, exit 0, and the plan recorded it.""" @@ -1750,6 +1766,14 @@ def _top_level(path: Path, named: str) -> Path: raise Refusal(SOURCE_NOT_TOP_LEVEL, f"{named} is not a directory, and {TOP_LEVEL_ONLY}", TOP_LEVEL_REMEDY) + if git_out(["rev-parse", "--is-inside-git-dir"], cwd=resolved) == "true": + git_dir = os.fsdecode(git_out(["rev-parse", "--absolute-git-dir"], + cwd=resolved, binary=True).rstrip(b"\n")) + if Path(os.path.realpath(git_dir)) != resolved: + raise Refusal( + SOURCE_NOT_TOP_LEVEL, + f"{named} is inside the git directory {_spelled(git_dir)}, " + f"and {TOP_LEVEL_ONLY}", TOP_LEVEL_REMEDY) prefix = os.fsdecode(git_out(["rev-parse", "--show-prefix"], cwd=resolved, binary=True).rstrip(b"\n")) if prefix: @@ -4766,10 +4790,20 @@ def open_source(self, override: str | None, work_root: Path, AN EMPTY `override` IS ONE GIVEN (#796), and `Source.open` refuses it: on main `check --source ''` and `execute --source ''` dropped it - and read the plan's own source.""" + and read the plan's own source. So is an empty `local_path:`, which + no `plan` writes, refused as `plan-source-gone`: on main it was read + as no `local_path` at all, and the plan's `source.repository` was + cloned from GitHub, a path the person gave or a gitlab clone's + `origin` included, which `_recorded_source` never clones again.""" if override is not None: return Source.open(override, work_root, unwritable, made, done) local_path = self.source.get("local_path") + if local_path == "": + raise Refusal( + PLAN_SOURCE_GONE, + "the plan's `source.local_path` is empty: it names no path, " + "and an empty path would be read as the directory this runs " + "in", PLAN_SOURCE_GONE_REMEDY) if local_path: return self._recorded_source(str(local_path), work_root, unwritable, made, done) @@ -4882,11 +4916,7 @@ def _gone(self, local_path: str, state: str, why: str) -> Refusal: return Refusal( PLAN_SOURCE_GONE, f"the plan's `source.local_path`, {_spelled(local_path)}, " - f"{state}, and {why}", - "Remediation: pass the source again with --source: the path its " - "clone is at now; or, where `plan` cloned it from GitHub, the " - "`org/repo` it cloned, which clones it again. Either is held to " - "the plan's `commit:`.") + f"{state}, and {why}", PLAN_SOURCE_GONE_REMEDY) def names(self) -> dict[str, str]: """The three repository names the plan declares, validated as values. diff --git a/tests/test_adopt_source_clone_record.py b/tests/test_adopt_source_clone_record.py index 2d1a7aef..55826495 100644 --- a/tests/test_adopt_source_clone_record.py +++ b/tests/test_adopt_source_clone_record.py @@ -242,6 +242,8 @@ def test_a_recorded_repository_is_cloned_again_wherever_check_runs( # form with no scheme as neither a path nor a name. "www-url": f"https://www.github.com/{REPOSITORY}", "www-no-scheme": f"WWW.GitHub.com/{REPOSITORY}", + # `.git` dropped in any case, as `.git` is (#831's review, F2). + "dot-git-upper-case": f"{REPOSITORY}.GIT", } @@ -271,6 +273,10 @@ def test_each_spelling_of_a_name_is_cloned_and_recorded_as_owner_repo( # clone into `source-readonly/.git`, and refused `source-unclonable` # with a remedy about `gh auth status`. "dot-git-repository": (f"{ORG}/.git", NEITHER), + # ... in any case, as GitHub and a disk that ignores case read it (#831's + # review, F2): on main and before it, `gh` was handed each of these. + "dot-git-upper-case": (f"{ORG}/.GIT", NEITHER), + "dot-git-mixed-case": (f"{ORG}/.Git.git", NEITHER), "url-dot-git-repository": (f"https://github.com/{ORG}/.git.git", NO_OWNER_REPO), # Controls: `.` and `..` name no repository either, whatever `.git` @@ -780,8 +786,9 @@ def test_execute_into_a_work_dir_holding_its_clone_says_nothing_exists( "`git rev-parse --show-toplevel` prints in any directory of it, or its " "`org/repo`.") README = "README.md" -#: A path inside the local source that is no top level: a file, a directory. -INSIDE = {"file": README, "directory": "src"} +#: A path inside the local source that is no top level: a file, a directory +#: of its work tree, a directory of its git directory. +INSIDE = {"file": README, "directory": "src", "git-directory": ".git/objects"} #: A `~user` no machine running the suite has, which bash passes through as #: typed, and `os.path.expanduser` leaves as typed. NO_SUCH_USER = "~no-such-user-796/Thing" @@ -855,11 +862,20 @@ def not_top_level(named: str, source: Path, inside: str) -> str: if inside == INSIDE["file"]: return (f"{NOT_TOP_LEVEL}{named} is not a directory, and " f"{TOP_LEVEL_ONLY}") + if inside == INSIDE["git-directory"]: + return in_a_git_directory(named, source / ".git") return (f"{NOT_TOP_LEVEL}{named} is `{inside}/` inside the repository at " f"{spelled(os.path.realpath(source))}, and {TOP_LEVEL_ONLY}: read " f"from there, it would be only what is under `{inside}/`") +def in_a_git_directory(named: str, git_dir: Path) -> str: + """The first line of `source-not-top-level` for a directory inside + `git_dir`, which the refusal calls `named`.""" + return (f"{NOT_TOP_LEVEL}{named} is inside the git directory " + f"{spelled(os.path.realpath(git_dir))}, and {TOP_LEVEL_ONLY}") + + def local_path_now(adopter, plan: Path, value: str) -> None: """The plan's `source.local_path` edited to `value`, as a person may.""" edited(plan, f" local_path: {adopter.y(recorded(plan)['local_path'])}" @@ -919,6 +935,65 @@ def test_dot_in_a_directory_inside_the_repository_is_refused( nothing_was_made(tmp_path, calls) +def git_directories(source: Path, tmp_path: Path) -> dict[str, Path]: + """The git directories of `source`'s repository: its own `.git`, a bare + clone of it, and a linked worktree's.""" + bare = tmp_path / "bare.git" + git("clone", "-q", "--bare", str(source), str(bare), cwd=tmp_path) + git("worktree", "add", "-q", str(tmp_path / "wt"), "-b", "wt", cwd=source) + return {"dot-git": source / ".git", "bare": bare, + "worktree": source / ".git" / "worktrees" / "wt"} + + +@pytest.mark.parametrize("which", ["dot-git", "bare", "worktree"]) +def test_a_git_directory_itself_is_read_whole(adopter, monkeypatch, capsys, + tmp_path, which): + """The controls of #831's review, F1: a git directory is a repository's + own, and is read whole, as on main.""" + calls = no_github(adopter, monkeypatch, tmp_path) + given = git_directories(local_source(tmp_path), tmp_path)[which] + plan = tmp_path / PLAN + code, _, err = run(adopter, capsys, *plan_args(str(given), plan)) + assert code == 0, err + assert recorded(plan)["local_path"] == os.path.realpath(given) + assert calls == [] + + +def test_a_directory_inside_a_bare_repository_is_refused( + adopter, monkeypatch, capsys, tmp_path): + """#831's review, F1: inside a git directory `--show-prefix` prints + nothing, so `plan` planned the whole repository from `.git/refs`, + exit 0, and recorded that directory, which `check` and `execute` could + not read as a repository. `.git/objects` is in `INSIDE`.""" + calls = no_github(adopter, monkeypatch, tmp_path) + bare = git_directories(local_source(tmp_path), tmp_path)["bare"] + given = str(bare / "refs") + code, _, err = run(adopter, capsys, *plan_args(given, tmp_path / AGAIN)) + assert code == 2, err + assert err.splitlines() == [ + in_a_git_directory(f"--source {given!r}", bare), TOP_LEVEL_REMEDY] + nothing_was_made(tmp_path, calls) + + +@pytest.mark.parametrize("command", [CHECK, EXECUTE]) +def test_an_empty_recorded_local_path_is_refused_and_never_cloned( + adopter, monkeypatch, capsys, tmp_path, command): + """#831's review, F3, L1-96's cause in the plan: on main `if + local_path:` read `local_path: ""` as none, and the plan's + `source.repository` was cloned from GitHub, though `plan` made no clone + of it.""" + _, plan, calls = planned_here(adopter, monkeypatch, capsys, tmp_path) + local_path_now(adopter, plan, "") + code, _, err = run(adopter, capsys, command, PLAN_FLAG, str(plan), + *remotes(tmp_path)) + assert code == 2, err + assert err.splitlines()[:2] == [ + f"{GONE_REFUSED}the plan's `source.local_path` is empty: it names no " + "path, and an empty path would be read as the directory this runs in", + GONE_REMEDY] + nothing_was_made(tmp_path, calls) + + @pytest.mark.parametrize("dot", [False, True], ids=["top-level", "dot-there"]) def test_the_top_level_is_read_in_place(adopter, monkeypatch, capsys, tmp_path, dot):