diff --git a/shape-doctor.py b/shape-doctor.py index e4287748..edc9ca12 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -4362,46 +4362,46 @@ def gitmodules_entry(root: Path, rel: str) -> tuple[str | None, str | None]: index, else the one in HEAD. So a root whose `.gitmodules` was deleted from the working tree alone still updates: measured, the line exits 0 there. `git config -f .gitmodules` by itself would call every mount of - that root url-less and withhold a line that works. A file git cannot - parse gives no url here. + that root url-less and withhold a line that works. - `-z`, because a section name may hold a space, a dot or a quote, and the - listing is read by `section_url`. Read only: `--get-regexp` writes - nothing. + AND ONLY THAT ONE COPY: the first that EXISTS, as `rev-parse --verify` + finds it, which is how git's `config_from_gitmodules` chooses (#668). + A copy with no section for `rel`, or one git cannot parse, is no url, + and git does not look further: measured, over an index copy of either + kind beside a HEAD copy that gives the url, `submodule init` exits 128, + `No url found`. Reading the next copy wherever one would not list named + a url there, and a line that exits 128. + + The copy is read by `gitmodules_listing`, the one reading + `submodule_config` makes too, and `section_url` picks the section out of + it. """ if os.path.lexists(root / ".gitmodules"): - sources = [["--file", ".gitmodules"]] + source = ["--file", ".gitmodules"] else: - sources = [["--blob", ":.gitmodules"], ["--blob", "HEAD:.gitmodules"]] - for source in sources: - try: - proc = subprocess.run( - ["git", "config", "-z", *source, "--get-regexp", - r"^submodule\..*\.(path|url)$"], - cwd=str(root), capture_output=True, check=False) - except OSError: - return None, None - if proc.returncode == 1: - # Read, and no section gives a path or a url. + found = next((copy for copy in (":.gitmodules", "HEAD:.gitmodules") + if git_answer(["rev-parse", "-q", "--verify", copy], + root) is not None), None) + if found is None: return None, None - if proc.returncode != 0: - # No such copy, or one git cannot parse: the next place, as git. - continue - return section_url(proc.stdout.decode("utf-8", "replace"), rel) - return None, None + source = ["--blob", found] + listing = gitmodules_listing(root, source) + if listing is None: + # The copy git reads, and one it cannot parse: git stops there. + return None, None + return section_url(listing, rel) def section_url(listing: str, rel: str) -> tuple[str | None, str | None]: """`(name, url)` of the section whose `path` is `rel`, read out of one - `git config -z --get-regexp` listing of `submodule..path` and - `.url` keys, which is what `gitmodules_entry` asks git for: `(None, - None)` when no section gives that path, and `(name, None)` when the one - that does gives no url (#414). + `gitmodules_listing`, which is what `gitmodules_entry` asks git for: + `(None, None)` when no section gives that path, and `(name, None)` when + the one that does gives no url (#414). PURE, so the reading is tested apart from the question of where git - found the file. Each record ends in a NUL and is the key, a newline and - the value. The NAME is all of the key between `submodule.` and its last - dot, so a name that holds a space or a dot is read whole. + found the file. Only a section's `path` and `url` keys count + (`submodule_keys`): `--list` gives every key the file holds, and the + `path` of a section that is not a submodule's names no mount (#668). THE LAST VALUE GIVEN WINS, for a path two sections give and for a key a section gives twice, because git reads `.gitmodules` with each later @@ -4409,9 +4409,7 @@ def section_url(listing: str, rel: str) -> tuple[str | None, str | None]: """ named = None urls: dict[str, str] = {} - for record in listing.split("\0"): - key, _, value = record.partition("\n") - name, _, field = key[len("submodule."):].rpartition(".") + for name, field, value in submodule_keys(listing): if field == "path" and value == rel: named = name elif field == "url": @@ -4421,6 +4419,57 @@ def section_url(listing: str, rel: str) -> tuple[str | None, str | None]: return named, urls.get(named) or None +def submodule_keys(listing: str) -> list[tuple[str, str, str]]: + """`(name, field, value)` for each `submodule..path` and `.url` + key of one `gitmodules_listing`, in git's order, and no other key + (#668). + + PURE, as `section_url` is. Each record ends in a NUL and is the key, a + newline and the value: a key holds no newline and a value may, so the + record is split at its FIRST one. The NAME is all of the key between + `submodule.` and its last dot, so a name that holds a space, a dot, a + quote or an `=` is read whole, and the field is what follows that dot, + which git lists in lower case. + """ + keys = [] + for record in listing.split("\0"): + key, _, value = record.partition("\n") + section, _, field = key.rpartition(".") + if section.startswith("submodule.") and field in ("path", "url"): + keys.append((section[len("submodule."):], field, value)) + return keys + + +def gitmodules_listing(root: Path, source: list[str]) -> str | None: + """`git config -z --list` in `root`: every key of one copy of + `.gitmodules`, or None where git reads no such copy (it is not there, + git cannot parse it, there is no git to run). + + ONE READING, FOR `gitmodules_entry` AND `submodule_config` (#668), and + the one `validate-family.py`'s `_gitmodules_sections` and `family.py` + make: the whole file, cut by `submodule_keys`. + + `--list` AND NO REGEXP, because git matches a regexp in the locale's + character set: under a UTF-8 locale `.*` matches no byte that is not + UTF-8, so `--get-regexp '^submodule\\..*\\.(path|url)$'` left out a + section whose name holds one, which `LC_ALL=C` read (measured, git + 2.43), and the mount that section gives was called url-less. `-z`, + because a value may hold a line break git keeps, NEL and U+2028 among + them, and a name a space: git's plain listing, split at line breaks and + at the first space, cut such a url short and such a name in two. + + Decoded with the replacement, as `make siblings` reads the same file + (#432) and `origin_url` reads a remote (#385). Read only: `--list` + writes nothing. + """ + try: + raw = git_out(["config", "-z", *source, "--list"], cwd=root, + binary=True) + except (Refusal, OSError): + return None + return raw.decode("utf-8", "replace") + + def configured_url(root: Path, name: str) -> str | None: """The url the root's own config gives the submodule NAMED `name`, or None (#414). @@ -6985,13 +7034,24 @@ def origin_url(path: Path) -> str: def submodule_config(root: Path) -> dict: - """`{submodule name: {field: value}}` out of the holder's `.gitmodules`. + """`{submodule name: {field: value}}` out of the holder's `.gitmodules`: + each section's `path` and `url`, the later value winning as git reads + the file. GIT'S OWN PARSER, never a reader of this file's own: `.gitmodules` is config syntax, it ESCAPES a backslash on the way in -- a Windows path is stored `D:\\\\a\\\\...` -- and reading it as text is how a report ends up quoting a url nobody has. + THE WORKING TREE'S FILE ALONE, as `make siblings` reads it, and read as + `gitmodules_entry` reads each copy git reads: `gitmodules_listing` + (#668). The `--get-regexp` listing this split at the first space and at + every line break `str.splitlines` knows read a section named `members/My + IRRS`, or a url holding NEL or U+2028, as no url or one cut short, and + under a UTF-8 locale it held no section whose name is not UTF-8. The + member's clone beside the holder was then a stranger, and `make + siblings` fetches into it. + AND IT IS READ AS `make siblings` READS IT: a byte that is not UTF-8 is replaced, which is `siblings.py::submodule_urls`'s `errors="replace"` over the same file (#432), and the decode `origin_url` makes of the @@ -6999,10 +7059,10 @@ def submodule_config(root: Path) -> dict: so a member mounted from a directory named in a legacy code page, or a hand-edited `.gitmodules`, has one that is not text, and read through `git_out`'s strict decode it raised a `UnicodeDecodeError` that the - `except` below does not name -- it is a `ValueError`, neither a + `except` there did not name -- it is a `ValueError`, neither a `Refusal` nor an `OSError` -- so `--root` ended in a traceback, exit 1, which is also the code of a finding, with no verdict. The bytes are - asked for, so nothing is decoded before this function says how. + asked for, and `gitmodules_listing` says how they are decoded. THE URL IS KEPT, NOT DROPPED. The row exists to agree with `make siblings` about one directory (#146): a clone whose own `origin` is the @@ -7018,20 +7078,11 @@ def submodule_config(root: Path) -> dict: is COMPARED with `origin_url`'s reading of a clone's remote, which is the replacement (#385): the same bytes read two ways are two strings, so a clone made from this very url would stop being the member's - working clone. It is also what the row PRINTS, and a lone surrogate is - a string no UTF-8 stream can write. + working clone. """ fields: dict = {} - try: - raw = git_out(["config", "-f", str(root / ".gitmodules"), - "--get-regexp", r"^submodule\..*\.(path|url)$"], - cwd=root, binary=True) - except (Refusal, OSError): - return fields - listing = raw.decode("utf-8", "replace").strip() - for line in listing.splitlines(): - key, _, value = line.partition(" ") - name, _, field = key[len("submodule."):].rpartition(".") + listing = gitmodules_listing(root, ["--file", ".gitmodules"]) + for name, field, value in submodule_keys(listing or ""): fields.setdefault(name, {})[field] = value return fields @@ -7065,15 +7116,22 @@ def mounted_from(root: Path, rel: str, repository: str) -> str: to resolve it against is handed back as it is: it then matches nothing, and the row's answer rests on the `repository:` comparison alone -- which is the state `verify_sibling` is in for the same holder. + + THE LAST SECTION THAT GIVES `rel` A URL WINS, as git reads the file, + each later value overwriting the one before, and as + `siblings.py::submodule_urls` maps it (#668). The first named a remote + that `git submodule update` and `make siblings` both pass over. """ + url = None for name, mount in submodule_config(root).items(): # A submodule's NAME is its path unless somebody mounted it by hand # under another one, which git allows and this reads rather than # assumes. - if mount.get("path", name) != rel or not mount.get("url"): - continue - return resolved_remote(mount["url"], origin_url(root)) - return f"https://github.com/{repository}.git" + if mount.get("path", name) == rel and mount.get("url"): + url = mount["url"] + if url is None: + return f"https://github.com/{repository}.git" + return resolved_remote(url, origin_url(root)) def same_repository_here(one: str, two: str) -> bool: diff --git a/tests/test_doctor_gitmodules_not_utf8.py b/tests/test_doctor_gitmodules_not_utf8.py index 4add9472..f9433768 100644 --- a/tests/test_doctor_gitmodules_not_utf8.py +++ b/tests/test_doctor_gitmodules_not_utf8.py @@ -19,6 +19,16 @@ it found, quoting the url with the replacement character, and the run reaches the verdict a holder with an ASCII url gets. +AND THE WHOLE FILE IS READ AS GIT READS IT (#668). `submodule_config` split +git's plain `--get-regexp` listing at the first space and at every line break +`str.splitlines` knows, and a UTF-8 locale kept that regexp from matching a +byte that is not UTF-8. So a section named `members/My IRRS`, a url holding +NEL or U+2028, or a section named in Latin-1 gave the doctor no url or a url +cut short, where git and `make siblings` read the whole one, and the +member's clone beside the holder was called a stranger. `gitmodules_entry` +had the Latin-1 half. Both now read `git config -z --list`, and the corpus +at the end asks each case the same four questions. + THE FIXTURES ARE `test_shape_doctor`'s, imported rather than rebuilt: a holder made any other way would be a different one. The `.gitmodules` is written as BYTES (a Windows command line is text), with forward slashes so @@ -28,6 +38,7 @@ from __future__ import annotations import json +import os import re import shutil import subprocess @@ -239,3 +250,142 @@ def test_a_clone_from_another_remote_is_still_a_stranger_beside_such_a_mount( assert text.returncode == 0, text.stdout + text.stderr assert "caf?/IRRS.git, so it is not counted as its working clone" \ in members_line(text.stdout), text.stdout + + +# --- #668: a section name and a url git reads whole -------------------------- + +#: Section NAMES, as the bytes between the header's quotes, each holding a +#: character a reading of git's listing has cut at: a space (the doctor's +#: old split), a dot, a quote (escaped in the header) and an `=`. +NAMES = [ + pytest.param(b"members/My IRRS", id="space"), + pytest.param(b"members/IRRS.v2", id="dot"), + pytest.param(b'members/\\"IRRS\\"', id="quote"), + pytest.param(b"members/My=IRRS", id="equals"), +] + +#: A url through a directory holding each line break `str.splitlines` knows +#: besides `\n` and `\r`, all of which git keeps in a value: VT, FF, FS, GS, +#: RS, NEL, U+2028 and U+2029, in UTF-8. +BREAKS = [pytest.param(f"caf{char}e".encode("utf-8"), + id=f"U+{ord(char):04X}") + for char in "\x0b\x0c\x1c\x1d\x1e\x85\u2028\u2029"] + + +def name_section(root: Path, written: bytes) -> bytes: + """`root`'s member section renamed, `written` between its header's + quotes and committed, and the name git reads it under: `written` with a + quote's escape read.""" + config = root / ".gitmodules" + header = f'[submodule "{SECTION}"]'.encode("ascii") + before = config.read_bytes() + assert before.count(header) == 1, before + config.write_bytes(before.replace(header, + b'[submodule "' + written + b'"]')) + commit_all(root, "Name the member's section otherwise") + return written.replace(b'\\"', b'"') + + +def gits_url(root: Path, name: bytes) -> str: + """The url git reads for the section NAMED `name`, asked by its exact + key as `git submodule` asks, and decoded as the doctor decodes one.""" + read = subprocess.run(["git", "config", "--file", ".gitmodules", "--get", + b"submodule." + name + b".url"], + cwd=str(root), capture_output=True, check=False) + assert read.returncode == 0, read.stderr + return read.stdout.rstrip(b"\r\n").decode("utf-8", "replace") + + +def assert_read_as_git_reads(standard: Path, root: Path, sibling: Path, + name: bytes) -> None: + """#668's four questions of a holder whose member section is NAMED + `name`, with a clone of that section's url beside it: the doctor mounts + the member from the url git reads, makes up no section, finds the url + in the section git finds it in, and calls the clone the member's working + clone, as `make siblings` does.""" + module = doctor_module(standard) + url = gits_url(root, name) + assert module.mounted_from(root, SECTION, REPOSITORY) == url + assert "" not in module.submodule_config(root) + assert module.gitmodules_entry(root, SECTION) == ( + name.decode("utf-8", "replace"), url) + result = doctor(standard, root, "--json") + assert "Traceback" not in result.stderr, result.stderr + entry = rows_of(result)["members"]["detail"]["members"][0] + assert entry["working_clone"] == sibling.as_posix(), entry + + +@pytest.mark.parametrize("written", NAMES) +def test_a_section_name_git_reads_whole_is_read_whole(standard, holder, + tmp_path, written): + """A section NAMED with a space was split at it: the doctor read no url + for the member, mounted it from the `repository:` guess, and called its + clone a stranger, where `make siblings` fetches into it.""" + root, sibling = member_beside(holder, tmp_path) + assert_read_as_git_reads(standard, root, sibling, + name_section(root, written)) + + +@pytest.mark.parametrize("directory", BREAKS) +def test_a_url_holding_a_line_break_git_keeps_is_read_whole( + standard, holder, tmp_path, directory): + """A url holding a line break git keeps was cut short at it, and the + rest made up a section named `''`.""" + root, sibling = member_beside(holder, tmp_path) + url = mount_through(holder, root, directory) + git("remote", "remove", "origin", cwd=sibling) + with_origin(sibling, url) + assert_read_as_git_reads(standard, root, sibling, SECTION.encode()) + + +def listed_locale(codeset: str) -> str: + """`C`, or for `UTF-8` any UTF-8 locale `locale -a` lists (`C.UTF-8` + first where it is one), so the test runs wherever one is installed. The + test is skipped where `locale -a` lists none.""" + try: + listed = subprocess.run(["locale", "-a"], capture_output=True, + text=True, check=False).stdout.split() + except OSError: + pytest.skip("no `locale` here to list the locales") + if codeset == "C": + found = [name for name in listed if name == "C"] + else: + found = sorted((name for name in listed + if name.lower().replace("-", "").endswith(".utf8")), + key=lambda name: not name.lower().startswith("c.")) + if not found: + pytest.skip(f"`locale -a` lists no {codeset} locale here") + return found[0] + + +@pytest.mark.parametrize("codeset", ["UTF-8", "C"]) +def test_a_section_name_in_latin_1_is_read_under_either_locale( + standard, holder, tmp_path, monkeypatch, codeset): + """A section NAMED in Latin-1 was read under `LC_ALL=C` and not under a + UTF-8 locale, where the regexp matched no byte that is not UTF-8: no + url, and the member's clone a stranger. Skipped on Windows, whose + command line is text and cannot carry the byte to `gits_url`.""" + if os.name == "nt": + pytest.skip("a Windows command line cannot carry the byte 0xE9") + monkeypatch.setenv("LC_ALL", listed_locale(codeset)) + root, sibling = member_beside(holder, tmp_path) + assert_read_as_git_reads(standard, root, sibling, + name_section(root, b"members/caf\xe9")) + + +def test_of_two_sections_giving_the_mount_the_last_is_read( + standard, holder, tmp_path, siblings): + """Two sections give the member's path, and git, `gitmodules_entry` and + `make siblings` all take the LAST, as git reads the file. `mounted_from` + took the first, and called the clone of the url git fetches from a + stranger (#668).""" + root, sibling = member_beside(holder, tmp_path) + config = root / ".gitmodules" + remotes = (holder["base"] / "remotes").as_posix().encode("utf-8") + config.write_bytes(b'[submodule "members/Old"]\n\tpath = ' + + SECTION.encode() + b"\n\turl = " + remotes + + b"/Old.git\n" + config.read_bytes()) + commit_all(root, "Give the member's path an earlier section too") + assert_read_as_git_reads(standard, root, sibling, SECTION.encode()) + assert siblings.submodule_urls(root)[SECTION] == gits_url( + root, SECTION.encode()) diff --git a/tests/test_doctor_legs_members_next.py b/tests/test_doctor_legs_members_next.py index 8c908562..8e84cf6e 100644 --- a/tests/test_doctor_legs_members_next.py +++ b/tests/test_doctor_legs_members_next.py @@ -1495,6 +1495,72 @@ def test_414_3_a_gitmodules_deleted_from_the_working_tree_alone_is_read( ), row["next"] +#: What the index holds in place of `.gitmodules`, for a root whose working +#: tree has none and whose HEAD's copy gives the mount `p` its url, and the +#: `(name, url)` git then finds for `p` (#668). None stages no copy. +INDEX_COPIES = [ + pytest.param(b'[submodule "q"]\n\tpath = q\n\turl = /r/index.git\n', + (None, None), id="no-section-for-the-path"), + pytest.param(None, ("p", "/r/head.git"), id="no-copy"), + pytest.param(b'[submodule "p"\n\tpath = p\n', (None, None), + id="unparsable"), +] + + +def gitmodules_in_head_and_index(root: Path, index_copy: bytes | None): + """`root`, a new repository with a gitlink at `p` and HEAD's + `.gitmodules` giving it `/r/head.git`, with no `.gitmodules` in the + working tree and `index_copy` staged in its place. The copy reaches git + as bytes on stdin.""" + git("init", "-q", "-b", "main", ".", cwd=root) + (root / ".gitmodules").write_bytes( + b'[submodule "p"]\n\tpath = p\n\turl = /r/head.git\n') + git("add", ".gitmodules", cwd=root) + git("update-index", "--add", "--cacheinfo", f"160000,{'1' * 40},p", + cwd=root) + git("commit", "-q", "-m", "Mount p", cwd=root) + (root / ".gitmodules").unlink() + if index_copy is None: + git("rm", "-q", "--cached", ".gitmodules", cwd=root) + return + blob = subprocess.run(["git", "hash-object", "-w", "--stdin"], + cwd=str(root), input=index_copy, + capture_output=True, check=True).stdout + git("update-index", "--cacheinfo", + f"100644,{blob.decode('ascii').strip()},.gitmodules", cwd=root) + + +def url_git_finds(root: Path, rel: str) -> str | None: + """The url `git submodule init -- ` copies into the root's config, + or None where git stops at `No url found`, exit 128.""" + init = subprocess.run(["git", "submodule", "init", "--", rel], + cwd=str(root), capture_output=True, check=False) + if init.returncode != 0: + return None + return git("config", "--get", f"submodule.{rel}.url", + cwd=root).stdout.strip() + + +@pytest.mark.parametrize(("index_copy", "found"), INDEX_COPIES) +def test_668_with_none_in_the_working_tree_the_first_copy_alone_is_read( + standard, tmp_path, index_copy, found): + """With no `.gitmodules` in the working tree, git reads the index's + copy, else HEAD's, and ONLY the first that exists, whatever it holds + (#668). Each answer is git's own, from `git submodule init`: + + - an index copy with no section for the path: no url, although HEAD's + copy gives one; + - no copy in the index: HEAD's url. Main called the mount url-less + there: its `--get-regexp` exits 1 over a blob that is not there, + which it took for "read, and no section"; + - an index copy git cannot parse: no url, for git stops there. Reading + on to HEAD's copy named a url and handed over a line that exits 128. + """ + gitmodules_in_head_and_index(tmp_path, index_copy) + assert doctor_module(standard).gitmodules_entry(tmp_path, "p") == found + assert url_git_finds(tmp_path, "p") == found[1] + + @WINDOWS_SKIP def test_414_3_pasted_there_the_line_runs_and_the_row_is_ok( standard, project, tmp_path): @@ -2005,18 +2071,28 @@ def test_pasted_there_the_git_line_fills_the_member_the_bootstrap_would_not( def test_section_url_reads_gits_listing_and_the_last_value_wins(standard, tmp_path): """`section_url` is the pure half of `gitmodules_entry`: one `git config - -z --get-regexp` listing in, `(name, url)` out. Fed git's own listing of - a `.gitmodules` whose section NAME holds a space and a dot and gives its + -z --list` listing in, `(name, url)` out. Fed git's own listing of a + `.gitmodules` whose section NAME holds a space and a dot and gives its url twice, and of a path two sections give: the last value given wins - each time, as git reads the file, and an empty url is no url.""" + each time, as git reads the file, and an empty url is no url. + + `--list` gives every key (#668), so the file holds keys no submodule + has: a `path` and `url` in a section that is not `submodule`, given + last, and in a `[submodule]` section with no subsection, which git + ignores. A reading that cut `submodule.` off every key by its length + took the first for a section named `gin` and the second for one named + `''`.""" module = doctor_module(standard) (tmp_path / ".gitmodules").write_bytes( b'[submodule "my lib.v2"]\n\tpath = vendor/lib\n' b"\turl = ../first.git\n\turl = ../second.git\n" b'[submodule "old"]\n\tpath = vendor/both\n\turl = ../old.git\n' - b'[submodule "new"]\n\tpath = vendor/both\n\turl =\n') - listing = git("config", "-z", "--file", ".gitmodules", "--get-regexp", - r"^submodule\..*\.(path|url)$", cwd=tmp_path).stdout + b'[submodule "new"]\n\tpath = vendor/both\n\turl =\n' + b"\tbranch = main\n" + b"[submodule]\n\tpath = vendor/none\n\turl = ../nameless.git\n" + b'[remote "origin"]\n\tpath = vendor/lib\n\turl = ../other.git\n') + listing = git("config", "-z", "--file", ".gitmodules", "--list", + cwd=tmp_path).stdout assert module.section_url(listing, "vendor/lib") == ( "my lib.v2", "../second.git"), listing assert module.section_url(listing, "vendor/both") == ("new", None), \