diff --git a/adopt-project.py b/adopt-project.py index 70f7e56e..b558bf61 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -1607,10 +1607,28 @@ 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 +#: 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_.-]+") @@ -1619,11 +1637,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: @@ -1638,14 +1658,22 @@ 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), 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 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 @@ -1668,8 +1696,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( @@ -1713,8 +1744,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) @@ -1725,7 +1777,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, @@ -1735,6 +1787,59 @@ 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 + --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.""" + 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) + 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: + 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 @@ -1820,7 +1925,18 @@ def open(cls, spec: str, work_root: Path, unwritable: str | None = None, `unwritable`, `made` and `done`). `instead` is `_default_branch`'s: the branch to read it at where its remote's default branch is no branch of its own.""" - path, repository = _source_spec(spec) + return cls.of(_source_spec(spec), work_root, unwritable, made, done, + instead) + + @classmethod + def of(cls, named: tuple[Path | None, str | None], work_root: Path, + unwritable: str | None = None, made: list | None = None, + done: str = "", instead: str | None = None) -> "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. + `instead` is as `open` takes it.""" + path, repository = named if repository is not None: return cls.clone(repository, work_root, unwritable, made, done=done, instead=instead) @@ -1830,8 +1946,8 @@ def open(cls, spec: str, work_root: Path, unwritable: str | None = None, def in_place(cls, path: Path, work_root: Path, instead: str | None = None) -> "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, instead) @@ -1900,17 +2016,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) @@ -4616,9 +4750,9 @@ def _print_plan_report(args, source: Source, tree: list, entries: list, source, so a read that fails here (`symlink_targets`' `git cat-file --batch`) still leaves the line that says the plan exists (#799). """ - 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 @@ -4665,6 +4799,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 + + #: The code `plan` refuses an `--out` it cannot write under (#799), whether #: it asks before any work or the write itself fails; and what both say to #: do, after `PLAN_NOT_WRITTEN`. @@ -4855,6 +5003,13 @@ def cmd_plan(args) -> int: # traceback at the write, after the work directory was made, an # `org/repo` source cloned and the whole plan built. _refuse_an_unwritable_out(args) + # 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 @@ -4863,10 +5018,10 @@ def cmd_plan(args) -> int: # unless this run ends having written the plan that records it (#652). # Read at `--tracking-branch` where its remote's default branch is no # branch of its own, a `git clone -b` (#797). - source = Source.open(args.source, work_root, - _unwritable_work_dir(args, work_root), - getattr(args, "clones", None), - instead=args.tracking_branch) + source = Source.of(named, work_root, + _unwritable_work_dir(args, work_root), + getattr(args, "clones", None), + instead=args.tracking_branch) policy = PathPolicy.load(args.path_policy or PATH_POLICY) naming = NamingPolicy.load(NAMING_POLICY) @@ -5100,11 +5255,25 @@ 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. 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, self.read_at) 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) @@ -5162,12 +5331,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, self.read_at) + return Source.in_place(_top_level( + path, f"the plan's `source.local_path`, " + f"{_spelled(local_path)},"), work_root, self.read_at) 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 " @@ -5175,7 +5353,8 @@ 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, self.read_at) + return Source.in_place(Path(os.path.realpath(path)), work_root, + self.read_at) state = (f"is a clone `plan` made that is no longer whole ({broken})" if there else "no longer exists") repository = self._recorded_repository() @@ -5208,11 +5387,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. @@ -13853,13 +14028,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: @@ -13878,7 +14055,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 ae309604..663a0d8d 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -421,7 +421,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 @@ -573,7 +578,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 @@ -616,7 +622,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..55826495 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,10 @@ "held to the plan's `commit:`.") MINE = "mine\n" MARKER = "mine.txt" +#: 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]: @@ -216,6 +237,13 @@ 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}", + # `.git` dropped in any case, as `.git` is (#831's review, F2). + "dot-git-upper-case": f"{REPOSITORY}.GIT", } @@ -241,6 +269,20 @@ 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), + # ... 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` + # 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), } @@ -733,11 +775,357 @@ 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 +#: 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" +#: 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, 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 + 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}") + 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'])}" + .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) + + +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): + """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 # ---------------------------------------------------------------------------