diff --git a/adopt-project.py b/adopt-project.py index e2310e80..b92a4745 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -393,8 +393,12 @@ # Whether a recorded `--pin-source` is a clone's top level, asked as `plan` # asks it (#800). from shape_materialize import pin_source_not_top_level # noqa: E402 -# One reading of an `origin`, shared with `shape-doctor.py` (#521). -from repo_shape import origin_is_a_path, url_repository # noqa: E402 +# One reading of an `origin`, shared with `shape-doctor.py` (#521), and the +# one rule for a name GitHub can hold, which it and `_github_name` ask +# (#1129, #1211). +from repo_shape import ( # noqa: E402 + github_can_hold, origin_is_a_path, url_repository, +) # The one spelling of a `neutral_product_pins:` entry, which # `scaffold-project.py` calls too, and the plain scalar it and `y` write a # text bare by (#924). @@ -2200,7 +2204,9 @@ def _github_name(written: str) -> str | None: names no repository (`_recorded_repository`, `_held_repository`); and beside a directory this user cannot search it is offered no url (`_source_there`, #1003). Asked of the name and not of the disk, it - holds on Windows as well, where no error is `File name too long`.""" + holds on Windows as well, where no error is `File name too long`. The + rule is `github_can_hold`'s, which a source's `origin` is read by too + (`url_repository`, #1211).""" segments = [part for part in written.split("/") if part] if len(segments) != 2: return None @@ -2211,7 +2217,8 @@ def _github_name(written: str) -> str | None: GITHUB_SEGMENT_RE.fullmatch(part) and part not in (".", "..") for part in (owner, repo)): return None - if len(owner) > GITHUB_OWNER_MAX or len(repo) > GITHUB_NAME_MAX: + # The repository as written: `github_can_hold` drops its `.git` itself. + if not github_can_hold(owner, segments[1]): return None return f"{owner}/{repo}" diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index 5ca7173f..23dbf48a 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -75,6 +75,13 @@ PROJECT_ID_RE = re.compile(r"^[a-z0-9][a-z0-9-]*$") TOPIC_PREFIX = "xf-project-" +#: The longest name GitHub gives a repository, in characters. Here, with the +#: owner's below, so that `url_repository` reads an `origin` by them +#: (`github_can_hold`, #1211); `shape_materialize.py` imports both. +GITHUB_NAME_MAX = 100 +#: The longest name GitHub gives a user or an organisation, in characters. +GITHUB_OWNER_MAX = 39 + #: The digest definition recorded in every pin this standard writes. Bumping #: the algorithm bumps this string, so a pin never silently means something #: else than it did when it was written. @@ -3063,10 +3070,27 @@ def origin_is_a_path(url: str) -> bool: return remote_is_a_path(url) or FILE_URL_RE.match(url) is not None +def github_can_hold(owner: str, repo: str) -> bool: + """Whether GitHub can hold a repository `repo` under the owner `owner`, + by their lengths: an owner of at most `GITHUB_OWNER_MAX` characters and + a repository of at most `GITHUB_NAME_MAX`, counted once one trailing + `.git`, in any case, is dropped (#1129, #1211). + + THE ONE RULE FOR EVERY READING OF A NAME. `adopt-project.py`'s + `_github_name` asks it of an `owner/repo` that is typed, recorded or in + a `--source` url, and `url_repository` of a source's `origin`. A name + past either limit names no repository on GitHub, so neither reading + gives one, and no remedy offers it.""" + if repo.lower().endswith(".git") and len(repo) > len(".git"): + repo = repo[:-len(".git")] + return len(owner) <= GITHUB_OWNER_MAX and len(repo) <= GITHUB_NAME_MAX + + def url_repository(url: str) -> str | None: """`/`: the last two segments of the path a url names, case - kept, however the url ends; None for a url whose path has fewer, and for - a filesystem PATH, however it is written (#521). + kept, however the url ends; None for a url whose path has fewer, for a + pair GitHub cannot hold (#1211), and for a filesystem PATH, however it is + written (#521). THE ONE READING OF AN `origin`, for both tools that need one: `adopt-project.py plan` takes `--org` from it and compares `--org` and @@ -3097,6 +3121,18 @@ def url_repository(url: str) -> str | None: separator, and `plan` says so in a NOTE; nor does one give `plan` an organisation, so it asks for `--org` (`adopt-no-org`), and so does the doctor's adoption line. + + NOR DOES A PAIR GITHUB CANNOT HOLD (#1211): an organisation over + `GITHUB_OWNER_MAX` characters, or a repository over `GITHUB_NAME_MAX`, + by the rule `adopt-project.py` reads a typed or recorded `owner/repo` + by (`github_can_hold`, #1129). A clone whose `origin` is + `https://gitlab.com/<40 c's>/Thing.git`, planned with `--org testorg`, + was refused `adopt-not-the-source` and told to pass `--org <40 c's>`, + which `execute` refuses; and `check` held a plan to that name while + `origin` was there, and to nothing once it was gone. Such an `origin` + names no repository now, as a path does: `plan` records none and + compares nothing with it, and with no `--org` it asks for one, as the + doctor's adoption line does. """ if origin_is_a_path(url): return None @@ -3109,11 +3145,16 @@ def url_repository(url: str) -> str | None: else: path = url.partition(":")[2] path = path.rstrip("/") + # The repository as written, `.git` and all, as `_github_name` hands it + # to `github_can_hold`, which drops one `.git` itself. + written = path.rpartition("/")[2] if path.endswith(".git"): path = path[:-4] above, _, name = path.rpartition("/") org = above.rpartition("/")[2] - return f"{org}/{name}" if org and name else None + if org and name and github_can_hold(org, written): + return f"{org}/{name}" + return None def remote_local_path(url: str) -> str | None: diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 09abf53a..10f2f24a 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -86,6 +86,9 @@ # The topic's fixed prefix, which GitHub's limit on a topic bounds an id by # (#838). from repo_shape import TOPIC_PREFIX # noqa: E402 +# GitHub's limits on a repository's name and an owner's (#838), which +# `repo_shape.url_repository` reads an `origin` by too (#1211). +from repo_shape import GITHUB_NAME_MAX, GITHUB_OWNER_MAX # noqa: E402 SHAPE_REPOSITORY = "opensoft/openRepoShape" @@ -6343,12 +6346,11 @@ def refuse_manifest_problems(prefix: str, problems: list[tuple[str, str]], #: The longest name a file or a directory may have, in bytes. NAME_MAX = 255 -#: The longest name GitHub gives a repository, in characters. -GITHUB_NAME_MAX = 100 +# The longest name GitHub gives a repository, `GITHUB_NAME_MAX`, and a user +# or an organisation, `GITHUB_OWNER_MAX`, are `repo_shape`'s, imported above, +# so that its `url_repository` reads an `origin` by them too (#1211). #: The longest topic GitHub sets on a repository, in characters. GITHUB_TOPIC_MAX = 50 -#: The longest name GitHub gives a user or an organisation, in characters. -GITHUB_OWNER_MAX = 39 #: The longest `` GitHub can hold a project's repositories under: #: `-spec` and `-code` are five characters longer. GITHUB_PROJECT_MAX = GITHUB_NAME_MAX - len("-spec") diff --git a/tests/test_adopt_source_github_limits.py b/tests/test_adopt_source_github_limits.py index afc547e9..58ce4e05 100644 --- a/tests/test_adopt_source_github_limits.py +++ b/tests/test_adopt_source_github_limits.py @@ -51,6 +51,7 @@ from conftest import (ADOPT, git, load_without_bytecode, make_source_repo, run_script) +from repo_shape import load_yaml from test_adopt_project_is_the_source import edited from test_adopt_temp_work_roots import ( ORG, PROJECT, REPOSITORY, SMALL_TREE, needs_filter_repo, recorded, @@ -481,3 +482,191 @@ def test_a_record_github_cannot_hold_offers_no_org(tmp_path, verb, owner): assert (f"Run `plan` again with `--project {PROJECT} --org {OWNER_AT}`, " "the source's own name,") in control.stderr, said(control) assert log.read_bytes() == b"" + + +# --- a local source's `origin` (#1211) -------------------------------------- +# +# AT #1194'S HEAD 906172b4 (2026-11-10), a local source's `origin` was read by +# `url_repository`, which asked neither limit: a clone whose `origin` is +# `https://gitlab.com/<40 c's>/Thing.git`, planned with `--org testorg`, was +# refused `adopt-not-the-source` and told to run `plan` again with `--org <40 +# c's>`, which `execute` refuses; planned with no `--org`, it was written with +# `org: <40 c's>`; and `check` held a plan to that name while `origin` was +# there, and to nothing once it was gone (RV1194's NIT-1 and NIT-2). The +# reading now asks `github_can_hold`, the rule `_github_name` asks, of the +# repository as written, so the two read every spelling alike. + +#: (the source's `origin`, its owner, and the repository as `url_repository` +#: names it). Each is past a limit GitHub keeps, as `_github_name` counts it. +ORIGINS_PAST = [ + pytest.param(f"{URL}{OWNER_OVER}/{PROJECT}.git", OWNER_OVER, PROJECT, + id="owner-40"), + pytest.param(f"https://gitlab.com/{OWNER_OVER}/{PROJECT}.git", + OWNER_OVER, PROJECT, id="gitlab-owner-40"), + pytest.param(f"git@github.com:{ISSUE_OWNER}/{PROJECT}.git", ISSUE_OWNER, + PROJECT, id="scp-owner-50"), + pytest.param(f"{URL}{ORG}/{NAME_OVER}.git", ORG, NAME_OVER, + id="repository-101"), + pytest.param(f"{URL}{ORG}/{NAME_AT}.git.git", ORG, f"{NAME_AT}.git", + id="repository-100-dot-git-dot-git"), + pytest.param(f"{URL}{OWNER_OVER}/{NAME_OVER}/", OWNER_OVER, NAME_OVER, + id="both-over"), + pytest.param(f"{URL}{ORG}/{NAME_OVER}.git/", ORG, NAME_OVER, + id="repository-101-slash"), +] +#: The controls: at GitHub's limits each names its repository, as it did. +ORIGINS_AT = [ + pytest.param(f"{URL}{OWNER_AT}/{PROJECT}.git", OWNER_AT, PROJECT, + id="owner-39"), + pytest.param(f"https://gitlab.com/{OWNER_AT}/{PROJECT}.git", OWNER_AT, + PROJECT, id="gitlab-owner-39"), + pytest.param(f"{URL}{ORG}/{NAME_AT}.git", ORG, NAME_AT, + id="repository-100"), + pytest.param(f"{URL}{ORG}/{NAME_AT}.GIT", ORG, f"{NAME_AT}.GIT", + id="repository-100-dot-GIT"), +] + + +def with_origin(tmp_path: Path, origin: str) -> Path: + """A local source whose `origin` is `origin`, as it is written.""" + source = make_source_repo(tmp_path / "source" / PROJECT, tree=SMALL_TREE, + edits=()) + git("remote", "add", "origin", origin, cwd=source) + return source + + +@pytest.mark.parametrize( + ("origin", "owner", "repo", "past"), + [pytest.param(*row.values, True, id=row.id) for row in ORIGINS_PAST] + + [pytest.param(*row.values, False, id=row.id) for row in ORIGINS_AT]) +def test_an_origin_is_read_by_the_rule_a_name_is(adopter, origin, owner, + repo, past): + """`url_repository` reads an `origin` as `_github_name` reads the same + `owner/repo` typed: past either limit, counted once one `.git` is + dropped, it names no repository; at them it names the one it did.""" + written = origin.rstrip("/").rpartition("/")[2] + + assert adopter.url_repository(origin) == (None if past else + f"{owner}/{repo}") + assert (adopter._github_name(f"{owner}/{written}") is None) is past + + +@pytest.mark.parametrize(("origin", "owner", "repo"), ORIGINS_PAST) +def test_an_origin_github_cannot_hold_offers_no_org(tmp_path, origin, owner, + repo): + """RV1194's P25 and P27: a local source whose `origin` names an owner of + 40 characters, on github.com or gitlab.com, planned with `--org + testorg`, was refused `adopt-not-the-source` and told to run `plan` + again with `--org <40 c's>`, which `execute` refuses; a repository of + 101 was told `--project <101 r's>`. Such an `origin` names no + repository now, as a path does: the plan is written, recording none, + and nothing is offered. `gh` is never asked.""" + source = with_origin(tmp_path, origin) + env, log = recording_gh(tmp_path / "gh-bin") + + proc = planned(tmp_path, tmp_path, str(source), PROJECT, ORG, env) + + assert proc.returncode == 0, said(proc) + assert "adopt-not-the-source" not in said(proc), said(proc) + assert "Run `plan` again" not in said(proc), said(proc) + assert "(origin: " not in proc.stdout, said(proc) + assert recorded(tmp_path / "adoption-plan.yaml")["repository"] is None + assert log.read_bytes() == b"" + + +@pytest.mark.parametrize(("origin", "owner", "repo"), ORIGINS_AT) +def test_an_origin_github_can_hold_is_still_the_source(tmp_path, origin, + owner, repo): + """The control: at GitHub's limits the `origin` names the source's own + repository, and `--org testorg --project Thing` is refused + `adopt-not-the-source` naming it, as it was.""" + source = with_origin(tmp_path, origin) + env, log = recording_gh(tmp_path / "gh-bin") + + proc = planned(tmp_path, tmp_path, str(source), PROJECT, ORG, env) + + assert proc.returncode == 2, said(proc) + assert (f"REFUSED adopt-not-the-source: --org {ORG} and --project " + f"{PROJECT} name {REPOSITORY}, and the source is {owner}/{repo}:" + ) in proc.stderr, said(proc) + assert not (tmp_path / "adoption-plan.yaml").exists() + assert log.read_bytes() == b"" + + +@pytest.mark.parametrize(("origin", "org"), [ + pytest.param(f"{URL}{OWNER_OVER}/{PROJECT}.git", None, id="owner-40"), + pytest.param(f"https://gitlab.com/{ISSUE_OWNER}/{PROJECT}.git", None, + id="gitlab-owner-50"), + pytest.param(f"{URL}{ORG}/{NAME_OVER}.git", None, id="repository-101"), + pytest.param(f"{URL}{OWNER_AT}/{PROJECT}.git", OWNER_AT, id="owner-39"), +]) +def test_plan_reads_no_org_from_an_origin_github_cannot_hold(tmp_path, + origin, org): + """With no `--org`, `plan` took the organisation from an `origin` of + `<40 c's>/Thing` and wrote the plan with `org: <40 c's>`, which + `execute` refuses. It reads none there now, as from a path, and asks + for `--org` (`adopt-no-org`), as `shape-doctor.py`'s adoption line + does. The control: an owner of 39 is still read.""" + source = with_origin(tmp_path, origin) + env, log = recording_gh(tmp_path / "gh-bin") + plan = tmp_path / "adoption-plan.yaml" + + proc = run_script(ADOPT, "plan", "--source", str(source), "--project", + PROJECT, "--elected-by", "Test Human", "--elected-on", + "2026-09-02", "--out", str(plan), "--work-dir", + str(tmp_path / "work"), cwd=tmp_path, env=env) + + if org is None: + assert proc.returncode == 2, said(proc) + assert "REFUSED adopt-no-org: " in proc.stderr, said(proc) + assert not plan.exists() + else: + assert proc.returncode == 0, said(proc) + assert load_yaml(plan)["org"] == org + assert log.read_bytes() == b"" + + +@needs_filter_repo +@VERBS +@pytest.mark.parametrize(("owner", "held"), [ + pytest.param(OWNER_OVER, False, id="owner-40"), + pytest.param(OWNER_AT, True, id="owner-39"), +]) +def test_a_plan_is_held_alike_with_origin_there_or_gone(tmp_path, verb, + owner, held): + """RV1194-cont's P29: a source whose `origin` is + `https://gitlab.com/<40 c's>/Thing.git`, planned with `--org <40 c's>`, + its `org:` then edited to `testorg`. `check` found `plan-not-the-source` + with `origin` there, offering `--org <40 c's>`, and `plan ok` with it + removed. The `origin` names no repository now, and the plan records + none, so both readings hold it to nothing: `check` says `plan ok` and + `execute` goes on to the confirmation it is not given, either way. The + control: an owner of 39 holds it, with `origin` there and gone.""" + source = with_origin(tmp_path, + f"https://gitlab.com/{owner}/{PROJECT}.git") + env, log = recording_gh(tmp_path / "gh-bin") + written = planned(tmp_path, tmp_path, str(source), PROJECT, owner, env) + assert written.returncode == 0, said(written) + plan = tmp_path / "adoption-plan.yaml" + edited(plan, (f"org: {owner}", f"org: {ORG}")) + + there = unconfirmed(verb, plan, tmp_path, env) + git("remote", "remove", "origin", cwd=source) + gone = unconfirmed(verb, plan, tmp_path, env) + + for proc in (there, gone): + if held: + assert "plan-not-the-source: " in proc.stderr, said(proc) + assert (f"Run `plan` again with `--project {PROJECT} --org " + f"{owner}`, the source's own name,") in proc.stderr + else: + assert "plan-not-the-source" not in said(proc), said(proc) + assert f"--org {owner}" not in said(proc), said(proc) + if verb == "check": + assert proc.returncode == 0, said(proc) + assert "\nplan ok\n" in proc.stdout, said(proc) + else: + assert "REFUSED adopt-unconfirmed: " in proc.stderr, \ + said(proc) + assert not (tmp_path / "remotes").exists() + assert log.read_bytes() == b"" diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index 9fd7562e..7954ca5b 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -9765,6 +9765,17 @@ def an_origin_of(path: Path, url: str | bytes | None) -> Path: pytest.param("file:///srv/git/opensoft/Atlas.git", None, id="file-url"), 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"), + # A pair GitHub cannot hold, an owner over 39 characters or a repository + # over 100, names no organisation either (#1211): `plan` read + # `<40 c's>` and the line carried no `--org`. The control: at 39. + pytest.param("https://github.com/" + "c" * 40 + "/Atlas.git", None, + id="owner-40"), + pytest.param("https://gitlab.com/" + "c" * 40 + "/Atlas.git", None, + id="gitlab-owner-40"), + pytest.param("git@github.com:opensoft/" + "r" * 101 + ".git", None, + id="repository-101"), + pytest.param("https://github.com/" + "c" * 39 + "/Atlas.git", "c" * 39, + id="owner-39"), ]