diff --git a/shape-doctor.py b/shape-doctor.py index 94f993c4..aebbfb2a 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -94,7 +94,10 @@ `members` and `machine`. A directory that is NEITHER gets `naming` (what it is called, under the policy), `what is here`, `the way in` -- `adopt-project.py plan` for a repository that already -exists, `setup.sh` for a new one -- and `machine`. +exists, `setup.sh` for a new one -- and `machine`. A git repository is told +of both: `the way in` is its adoption, with `--org ` where its +`origin` names no organisation `plan` can read, and `the other way in` is the +scaffold (#545); a directory that is not a repository gets the scaffold alone. A MANIFEST THAT IS THERE AND DOES NOT LOAD IS NOT AN ABSENT ONE (#205). A `project.yaml` with a merge conflict left in it, a tab, a construct the @@ -5821,6 +5824,45 @@ def repo_local_origin_name(root: Path) -> str | None: return origin_name(root) if (root / ".git").exists() else None +#: The `org/repo` pair `adopt-project.py`'s `_remote_repository` reads off the +#: end of an `origin` URL, byte for byte: the text after the last two +#: separators, less a `.git`. It is COPIED, not imported -- `adopt-project.py` +#: is a script with a hyphen in its name, not a module this file may load -- +#: and `tests/test_shape_doctor.py` runs both over the same URLs, so a change +#: to one that the other does not follow is a red test. +_ORIGIN_REPOSITORY_RE = re.compile(r"[:/]([^/:]+/[^/]+?)(?:\.git)?$") + + +def origin_org(root: Path) -> str | None: + """The organisation `adopt-project.py plan` would read off `root`'s own + `origin`, or None where it reads none (#545). + + `plan --source ` takes its `--org` from the source's `origin` and, + with none to take, refuses `adopt-no-org` and asks for the flag. It + reads none in THREE cases, and this is the one question the NEXT of + `the way in` needs answered: the repository has no `origin`; its URL is + not UTF-8 (git keeps a URL as the bytes it was given, and `plan` reads + it strictly, so it is no origin there, as `origin_name` and + `shape_advisory.repository_names` already say of it, #385); or its URL + ends in no `org/repo` -- a bare `repo.git`, a Windows path with no `/` in + it, a trailing slash. The first is the case #545 was filed for. The + other two are the same refusal on the same line, so the line is built + from the same reading rather than from "has an origin". + + ONLY WHEN `root` CARRIES ITS OWN `.git`, for `repo_local_origin_name`'s + reason: `git remote get-url origin` walks UP to the nearest ancestor + repository, and an ancestor's organisation is not this directory's. + """ + if not (root / ".git").exists(): + return None + try: + url = git_out(["remote", "get-url", "origin"], cwd=root) + except (Refusal, OSError, UnicodeDecodeError): + return None + pair = _ORIGIN_REPOSITORY_RE.search(url) + return pair.group(1).split("/")[0] if pair else None + + def check_not_a_root_naming(ctx: Context) -> Row: """Classify what this directory is CALLED, under the naming policy. @@ -6068,6 +6110,21 @@ def python_command(platform: str | None = None) -> str: return "python" if platform == "nt" else "python3" +#: The one value a way-in command needs and this report cannot know, spelled +#: the one way the handbook's scaffold commands and this file's own scaffold +#: line have always spelled it. IT IS NOT QUOTED AND MUST NOT BE: it is a +#: placeholder the reader replaces, not a value a command could take, and +#: quoting it would tell them to type the angle brackets. +ORG_PLACEHOLDER = "" + +#: What both scaffold rows say about their one line (#545): the placeholder +#: the line carries is the reader's to replace, and the scaffold asks before +#: it creates anything. Said once, so the non-repository row and the second +#: row of a repository cannot word it two ways. +SCAFFOLD_WHY = (f"replace {ORG_PLACEHOLDER} with the GitHub organisation to " + "create it in; without --yes, it asks") + + def scaffold_command(shape: Path, project: str, platform: str | None = None) -> str: # RAW, for `quote_arg`'s reason one function along: the paragraph below @@ -6120,8 +6177,9 @@ def scaffold_command(shape: Path, project: str, if target == "nt": return (f"{python_command(platform)} " f"{quote_arg(shape / 'setup-project.py', target)} " - f"--org --project {quote_arg(project, target)}") - return (f"{quote_arg(shape / 'setup.sh', target)} --org " + f"--org {ORG_PLACEHOLDER} --project " + f"{quote_arg(project, target)}") + return (f"{quote_arg(shape / 'setup.sh', target)} --org {ORG_PLACEHOLDER} " f"--project {quote_arg(project, target)}") @@ -6191,6 +6249,24 @@ def check_the_way_in(ctx: Context) -> Row: person wants is a fact about their repository, not about this directory listing, so both are named and the human chooses. + A ROW CARRIES ONE NEXT, SO A GIT REPOSITORY GETS TWO ROWS (#545). This + one is the way a directory's own kind points at -- the adoption for a + repository, the scaffold for one that is not -- and `check_the_other_way_in` + is the second command a repository is told of, which `AGENTS.md` asks the + report to give ("Report the verdict and both commands"). A directory + that is not a repository has no adoption to offer and gets one row, as it + always did; a leg clone gets neither (below). + + THE ADOPTION LINE NAMES THE ORGANISATION WHEN IT CANNOT BE READ (#545). + `adopt-project.py plan` takes `--org` from the source's `origin`, so a + repository with no `origin` -- or one whose URL names no `org/repo`, or + is not UTF-8 -- pasted this line and was refused `adopt-no-org`, exit 2. + `origin_org` asks the question `plan` asks, and where it has no answer + the line ends `--org `, the placeholder this report already + prints on the scaffold line (`ORG_PLACEHOLDER`). It is a value the + reader supplies, so the row's `why` says to replace it; where `origin` + does name an organisation the line is unchanged, and carries none. + THE SUGGESTED `--project` READS THE SAME IDENTITY THE `naming` ROW DOES: `origin`'s repository name where there is one, the directory's own name otherwise -- `check_not_a_root_naming`'s own docstring, "the name that @@ -6210,10 +6286,8 @@ def check_the_way_in(ctx: Context) -> Row: """ root = ctx.root is_repo = (root / ".git").exists() - identity = repo_local_origin_name(root) or root.name - policy = NamingPolicy.load(ctx.shape / "contracts" / - "repository-naming.yaml") - project = project_token_for(identity, policy) + project = way_in_project(ctx) + org = origin_org(root) if is_repo and leg_clone(ctx): # A LEG IS NOT ADOPTED. Splitting a spec or code leg into two more # legs is the one adoption nobody wants, and offering it two rows @@ -6229,22 +6303,71 @@ def check_the_way_in(ctx: Context) -> Row: f"plan --source {quote_arg(root)} --project " f"{quote_arg(project)}") why = None + if org is None: + # NOT QUOTED, for `ORG_PLACEHOLDER`'s reason. + fix += f" --org {ORG_PLACEHOLDER}" + why = (f"replace {ORG_PLACEHOLDER} with the GitHub organisation " + "to adopt under: this repository's `origin` names none " + "that `plan` can read") reason = ("this is a git repository with no shape manifest: it is " "ADOPTED in place, keeping its name, identity and history") else: - # `` IS NOT QUOTED and must not be: it is a placeholder - # the reader replaces, not a value this command knows. Quoting it - # would tell them to type the angle brackets. fix = scaffold_command(ctx.shape, project) - why = "without --yes; it asks" + why = SCAFFOLD_WHY reason = ("there is no repository here to adopt: a new project is " "SCAFFOLDED, which creates three repositories and asks " "first") return Row("way-in", "the way in", NA, reason, fix, - {"git_repository": is_repo, "suggested_project": project}, + {"git_repository": is_repo, "suggested_project": project, + "origin_org": org}, why=why) +def way_in_project(ctx: Context) -> str: + """The `--project` both ways in are offered, from the one identity + `check_the_way_in` reads: `origin`'s repository name where `root` + carries its own `.git` and one, the directory's name otherwise.""" + identity = repo_local_origin_name(ctx.root) or ctx.root.name + policy = NamingPolicy.load(ctx.shape / "contracts" / + "repository-naming.yaml") + return project_token_for(identity, policy) + + +#: Named once, as `LABEL_TRIAD_ADVISORY` below is: the row, the registry entry +#: and the tests all print the same words. +LABEL_OTHER_WAY_IN = "the other way in" + + +def check_the_other_way_in(ctx: Context) -> Row | None: + """The scaffold, for a git repository that is also told of the adoption + (#545). + + `check_the_way_in` carries one NEXT, and for a repository it is the + adoption. `AGENTS.md` says the two ways in are `adopt-project.py plan` + and `setup.sh` and that the report gives the verdict "and both commands": + which of them is wanted is a fact about the person's repository, and the + row that answered only the adoption chose for them. THE SAME LINE THE + NON-REPOSITORY ROW PRINTS, `scaffold_command`, for the same `--project` + -- the one identity `way_in_project` reads -- so the two rows cannot + name two projects. + + SILENT WHERE THE FIRST ROW OFFERS NO ADOPTION: a directory that is not a + repository already has the scaffold as its one row, and a leg clone is + sent to its assembly root (`leg_clone`, asked as `check_the_way_in` asks + it). A row that repeated the scaffold there would be this report + printing the same line twice, or contradicting the Triad row above it. + """ + if not (ctx.root / ".git").exists() or leg_clone(ctx): + return None + project = way_in_project(ctx) + return Row("way-in-scaffold", LABEL_OTHER_WAY_IN, NA, + "a NEW project is SCAFFOLDED instead of adopting this " + "repository: it creates three repositories and asks first", + scaffold_command(ctx.shape, project), + {"git_repository": True, "suggested_project": project}, + why=SCAFFOLD_WHY) + + #: Named once, like `LABEL_MANIFEST_KINDS` above: this function's three #: returns and the `CHECKS` entry below must all print the same row name. #: "Triad" because human-facing text says Triad (the ratified vocabulary); @@ -6541,6 +6664,10 @@ def check_merge(ctx: Context) -> Row | None: Check("naming", "naming", (NOT_A_ROOT,), check_not_a_root_naming), Check("contents", "what is here", (NOT_A_ROOT,), check_what_is_here), Check("way-in", "the way in", (NOT_A_ROOT,), check_the_way_in), + # RIGHT AFTER IT: a git repository is told of the scaffold too (#545), and + # the two commands are read together. Silent for anything else. + Check("way-in-scaffold", LABEL_OTHER_WAY_IN, (NOT_A_ROOT,), + check_the_other_way_in), # AFTER EVERYTHING ABOUT THE REPOSITORY AND BEFORE THE MACHINE, and only # where there is no manifest at all. A family holder is never advised; an # assembly root whose manifest is short of a Triad is a broken root, which diff --git a/tests/test_doctor_next_is_a_command.py b/tests/test_doctor_next_is_a_command.py index 3a7f1b86..00f97498 100644 --- a/tests/test_doctor_next_is_a_command.py +++ b/tests/test_doctor_next_is_a_command.py @@ -70,10 +70,11 @@ #: project or a holder made any other way would be a different project. from test_shape_doctor import ( # noqa: F401 BREAKAGES, DOCTOR, DRIFT_TARGET, FAMILY_MEMBER, FAMILY_NAME, - GIT_IDENTITY, SHAPE_PIN, UPSTREAM_TARGET, behind_row, break_manifest, - break_the_pin, doctor, doctor_module, holder, not_forty_hex, pin_with, - project, recommit_a_good_pin, red_root, rows_of, scaffolded_here, stage, - standard, standard_ahead, standard_of_another_history, + GIT_IDENTITY, SCAFFOLD_WHY, SHAPE_PIN, UPSTREAM_TARGET, behind_row, + break_manifest, break_the_pin, doctor, doctor_module, holder, + not_forty_hex, pin_with, project, recommit_a_good_pin, red_root, rows_of, + scaffolded_here, stage, standard, standard_ahead, + standard_of_another_history, ) #: Lane 2's ways to make a file under `contracts/` not load (#291), from the #: module that owns them: the `manifest kinds` row's NEXTs are #291's, so the @@ -427,6 +428,18 @@ def _way_in(standard, project, holder, tmp_path): return rows_of(doctor(standard, here, "--json"))["way-in"] +def _a_repository_row(row_id): + """The builder of a `the way in` row for a git repository with no + `origin`, which is the adoption with `--org ` and the scaffold + beside it (#545).""" + def build(standard, project, holder, tmp_path): + here = tmp_path / "Loose" + here.mkdir() + git("init", "-q", "-b", "main", ".", cwd=here) + return rows_of(doctor(standard, here, "--json"))[row_id] + return build + + #: One fixture per NEXT #277 converted, by the builder and branch it reaches, #: and how the sentence that used to follow ` # ` on it now opens -- moved #: VERBATIM, less the `#`, so these are the words main, doctors A and B and @@ -574,8 +587,16 @@ def _way_in(standard, project, holder, tmp_path): _machine, "it names each missing prerequisite and offers to install it, on a " "typed yes"), - "check_the_way_in": ( - _way_in, "without --yes; it asks"), + # Its sentence is the scaffold rows' one now (#545): it opens with the + # placeholder to replace, and the words printed after ` # ` follow. + "check_the_way_in": (_way_in, SCAFFOLD_WHY), + # #545's, which were never a ` # ` tail; the property holds them to the + # same rule, and the placeholder is the one the scaffold line carries. + "check_the_way_in-adopt-no-origin": ( + _a_repository_row("way-in"), + "replace with the GitHub organisation to adopt under"), + "check_the_other_way_in": ( + _a_repository_row("way-in-scaffold"), SCAFFOLD_WHY), } diff --git a/tests/test_shape_advisory.py b/tests/test_shape_advisory.py index b68229b4..482f5d7d 100644 --- a/tests/test_shape_advisory.py +++ b/tests/test_shape_advisory.py @@ -411,6 +411,8 @@ def test_a_leg_known_by_name_alone_is_told_so_with_a_hedge(tmp_path, assert way_in["next"] is None, way_in assert "adopt-project.py" not in (way_in["next"] or "") assert "AS A LEG" in way_in["reason"], way_in["reason"] + # NOR IS THE SCAFFOLD OFFERED INSTEAD (#545): a leg is sent to its root. + assert "way-in-scaffold" not in rows, sorted(rows) result = adopt_plan(here, tmp_path / "plan.yaml", quiet_env) assert "This is a single repository." not in result.stdout assert f"NOTE `{name}` is named as the {role} leg" in result.stdout, \ diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index d2ca4ed2..2e34cfbb 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -64,9 +64,12 @@ #: An `origin` written as BYTES git hands back unchanged, and the start of a #: URL holding the byte 0xE9 (#385), from the file that owns the advisory's #: half of that issue, so the two halves write the same URL the same way. -from test_shape_advisory import NOT_UTF8, with_origin +from test_shape_advisory import NOT_UTF8, repository, with_origin DOCTOR = "shape-doctor.py" +#: What both scaffold rows say beside their line (#545). +SCAFFOLD_WHY = ("replace with the GitHub organisation to create " + "it in; without --yes, it asks") ORG = "testorg" PROJECT = "Atlas" FAMILY_NAME = "InkRouter" @@ -1305,8 +1308,8 @@ def test_an_empty_git_repository_is_not_a_shape_root(standard, tmp_path): # `triad-advisory` since #163: an empty repository has elected nothing # and recorded nothing, so it hears the Triad advisory -- an `advisory` # row that the verdict steps over, which the exit code above shows. - assert set(rows) == {"naming", "contents", "way-in", "triad-advisory", - "machine"} + assert set(rows) == {"naming", "contents", "way-in", "way-in-scaffold", + "triad-advisory", "machine"} assert rows["triad-advisory"]["status"] == "advisory" assert "placement" not in rows, ( "there are no legs here and no manifest declaring any, so there is " @@ -1338,8 +1341,8 @@ def test_a_root_named_like_an_option_still_gets_a_real_naming_verdict( rows = rows_of(result) # `triad-advisory` since #163, as for any repository that has not # elected the Triad; it does not touch the naming row this test is about. - assert set(rows) == {"naming", "contents", "way-in", "triad-advisory", - "machine"} + assert set(rows) == {"naming", "contents", "way-in", "way-in-scaffold", + "triad-advisory", "machine"} row = rows["naming"] # The real verdict -- unclassified -- not the false OK argparse's own # help action used to produce. @@ -1409,9 +1412,12 @@ def test_a_directory_that_is_not_a_repository_says_scaffold(standard, rows["way-in"]["next"], rows["way-in"]["next"] assert "--org " in rows["way-in"]["next"] # Said beside the command, never on it (#277). - assert "without --yes" in rows["way-in"]["why"] + assert rows["way-in"]["why"] == SCAFFOLD_WHY, rows["way-in"] assert rows["way-in"]["detail"]["git_repository"] is False assert "an ordinary directory" in rows["contents"]["reason"] + # THE SCAFFOLD IS NOT PRINTED TWICE: it is this row, and a directory + # with no repository has no adoption for a second row to sit beside (#545). + assert "way-in-scaffold" not in rows, sorted(rows) def test_a_manifest_declaring_the_wrong_kind_is_not_a_root(standard, @@ -1452,7 +1458,8 @@ def test_a_manifest_declaring_the_wrong_kind_is_not_a_root(standard, PROJECT_ROWS = {"naming", "manifest", "pins", "manifest-kinds", "shape-currency", "legs", "leg-shape-files", "placement", "agent-files", "machine"} -NOT_A_ROOT_ROWS = ("contents", "way-in", "triad-advisory") +NOT_A_ROOT_ROWS = ("contents", "way-in", "way-in-scaffold", + "triad-advisory") #: The rows that read what `project.yaml` DECLARES, which wait on the #: `manifest` row while it does not load rather than answering a question @@ -8406,7 +8413,7 @@ def test_the_way_in_row_matches_the_host_platform_end_to_end(standard, scaffolds = module.check_the_way_in(module.Context(here)) assert scaffolds.next_command == module.scaffold_command( module.SHAPE_ROOT, "Loose", os.name), scaffolds.next_command - assert scaffolds.why == "without --yes; it asks", scaffolds.why + assert scaffolds.why == SCAFFOLD_WHY, scaffolds.why # The other way in, from the same directory once it is a repository. git("init", "-q", "-b", "main", ".", cwd=here) adopts = module.check_the_way_in(module.Context(here)) @@ -8414,7 +8421,111 @@ def test_the_way_in_row_matches_the_host_platform_end_to_end(standard, f"{module.PYTHON} " f"{module.quote_arg(module.SHAPE_ROOT / 'adopt-project.py', os.name)} " f"plan --source {module.quote_arg(here, os.name)} " - f"--project {module.quote_arg('Loose', os.name)}"), adopts.next_command + f"--project {module.quote_arg('Loose', os.name)} " + # No `origin` here, so `plan` reads no organisation (#545). + "--org "), adopts.next_command + + +def test_a_git_repository_is_told_of_both_ways_in(standard, tmp_path): + """#545: AGENTS.md says to "report the verdict and both commands", and + the report named the adoption alone for a repository. + + A row carries one NEXT, so the scaffold is the row after the adoption, + for the same `--project`, printed as the non-repository row prints it. + The verdict is untouched: both rows are `n/a`. + """ + here = tmp_path / "Loose" + here.mkdir() + git("init", "-q", "-b", "main", ".", cwd=here) + result = doctor(standard, here, "--json") + assert result.returncode == 2, result.stdout + result.stderr + listed = json.loads(result.stdout)["rows"] + ids = [row["id"] for row in listed] + assert ids.index("way-in-scaffold") == ids.index("way-in") + 1, ids + rows = rows_of(result) + adopts, scaffolds = rows["way-in"], rows["way-in-scaffold"] + module = doctor_module(standard) + assert "adopt-project.py plan" in adopts["next"], adopts + assert scaffolds["label"] == "the other way in", scaffolds + assert scaffolds["status"] == "n/a", scaffolds + assert scaffolds["next"] == module.scaffold_command( + module.SHAPE_ROOT, "Loose"), scaffolds + assert scaffolds["why"] == SCAFFOLD_WHY, scaffolds + assert scaffolds["detail"]["suggested_project"] == "Loose", scaffolds + + text = doctor(standard, here) + assert text.returncode == 2, text.stdout + text.stderr + assert adopts["next"] in text.stdout, text.stdout + assert scaffolds["next"] in text.stdout, text.stdout + + +def an_origin_of(path: Path, url: str | bytes | None) -> Path: + """A repository at `path` whose `origin` is `url`: bytes through + `with_origin`, so one that is not UTF-8 reaches git as written.""" + if isinstance(url, bytes): + return with_origin(repository(path), url) + return repository(path, origin=url) + + +#: `(origin URL, the organisation `adopt-project.py plan` reads from it)`, for +#: the spellings that matter: none at all (#545), an ordinary pair, and the +#: two other ways `plan` reads none -- a URL ending in no `org/repo` and one +#: that is not UTF-8 -- which refuse `adopt-no-org` as a missing origin does +#: (`tests/test_adopt_git_reads.py`). +ORIGIN_ORGS = [ + pytest.param(None, None, id="no-origin"), + pytest.param("git@github.com:opensoft/Atlas.git", "opensoft", id="scp"), + pytest.param("https://github.com/opensoft/Atlas", "opensoft", id="https"), + pytest.param("Atlas.git", None, id="no-pair"), + pytest.param("https://github.com/opensoft/Atlas/", None, + id="trailing-slash"), + pytest.param(r"D:\a\_work\1\s\Atlas.git", None, id="windows-path"), + pytest.param(NOT_UTF8 + b"Atlas.git", None, id="not-utf8"), +] + + +@pytest.mark.parametrize("url, org", ORIGIN_ORGS) +def test_the_adoption_line_names_an_org_only_where_plan_reads_none( + standard, tmp_path, url, org): + """#545: pasted on a repository with no `origin`, the adoption line was + refused `adopt-no-org`, exit 2, and so was one whose origin names no + organisation `plan` can read. Where there is none the line ends in the + placeholder the scaffold line already prints, and says to replace it; + where `plan` reads one the line is what it always was. + + `ORIGIN_ORGS` IS NOT TRUSTED ON ITS OWN WORD: `origin_org` is + `adopt-project.py`'s `_remote_repository` read for its organisation, a + copy of its pattern, and the same repository is put to both. + """ + module = doctor_module(standard) + here = an_origin_of(tmp_path / "Atlas", url) + read = adoption_module(standard)._remote_repository(here) + assert (read or "/").split("/")[0] == (org or ""), read + assert module.origin_org(here) == org + + row = module.check_the_way_in(module.Context(here)) + assert row.detail["origin_org"] == org, row.detail + named = f"--org {module.ORG_PLACEHOLDER}" + assert row.next_command.endswith(named) is (org is None), row.next_command + assert (row.why is None) is (org is not None), row.why + if org is None: + assert "replace with the GitHub organisation" in row.why + + +def test_an_ancestors_origin_names_no_org_for_a_loose_directory(standard, + tmp_path): + """A directory with no `.git` of its own, inside a clone whose `origin` + names `opensoft`, has no origin of its own to read an organisation from: + `git remote get-url` would answer for the ancestor, as it does for the + name (`test_the_way_in_row_never_leaks_an_ancestor_repository_origin`). + """ + module = doctor_module(standard) + parent = an_origin_of(tmp_path / "Parent", + "git@github.com:opensoft/Parent.git") + loose = parent / "Loose" + loose.mkdir() + assert module.origin_org(parent) == "opensoft" + assert module.origin_org(loose) is None def test_the_way_in_row_emits_a_classifying_name_verbatim(standard, tmp_path):