diff --git a/shape-doctor.py b/shape-doctor.py index f8903571..1e41af46 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -5146,13 +5146,25 @@ def submodule_keys(listing: str) -> list[tuple[str, str, str]]: `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. + + THE NAME KEEPS GIT'S BYTES AND THE VALUE IS TEXT (#732). The name is + only ever handed back to git, by `configured_url` and `kept_gitdir` + through `git_bytes`, so a byte that is not UTF-8 stays the + `surrogateescape` the listing was decoded with: replaced, a section + named `caf\\xe9` was looked up as `caf\\ufffd`, a name git has nowhere, + and a member with a commit on its detached HEAD got the update line + with no keep line. The value is compared and printed, so it takes the + replacement, as `make siblings` reads the same file (#432) and + `origin_url` reads a remote (#385). """ 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)) + keys.append((section[len("submodule."):], field, + value.encode("utf-8", "surrogateescape").decode( + "utf-8", "replace"))) return keys @@ -5174,16 +5186,17 @@ def gitmodules_listing(root: Path, source: list[str]) -> str | None: 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. + Decoded with `surrogateescape`, so nothing git listed is lost: + `submodule_keys` keeps a section's name as git's own bytes and gives + each value with the replacement (#732). 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") + return raw.decode("utf-8", "surrogateescape") def configured_url(root: Path, name: str) -> str | None: @@ -5201,11 +5214,13 @@ def configured_url(root: Path, name: str) -> str | None: 128, measured), so neither is helped by this. `git config --get` in the root, which reads what git reads: the - repository's config and the ones above it. Read only. + repository's config and the ones above it. Read only. The key is + handed to git as the bytes the section was named with (`git_bytes`, + #732). """ try: proc = subprocess.run(["git", "config", "--get", - f"submodule.{name}.url"], + git_bytes(f"submodule.{name}.url")], cwd=str(root), capture_output=True, check=False) except OSError: return None @@ -5214,6 +5229,31 @@ def configured_url(root: Path, name: str) -> str | None: return proc.stdout.decode("utf-8", "replace").strip() or None +def git_bytes(text: str) -> str | bytes: + """`text`, read out of git as UTF-8 with `surrogateescape`, as the + argument that hands git back the same bytes (#732). + + BYTES ON POSIX, whose command line is bytes. A `str` there is encoded + with the filesystem encoding, which is ASCII under `LC_ALL=C + PYTHONUTF8=0`: a section named `caf\\xc3\\xa9`, valid UTF-8, then raised + `UnicodeEncodeError` out of `subprocess`, a traceback with no verdict, + and one named `caf\\xe9` too. THE TEXT ITSELF ON WINDOWS, whose command + line is text: `subprocess` would have to decode bytes there, and a byte + that is not UTF-8 is not text to it. + + SO ON WINDOWS A NAME THAT IS NOT UTF-8 IS STILL NOT FOUND: Git for + Windows' `xwcstoutf` turns the lone surrogate into the bytes ED B3 A9, + not E9. A UTF-8 name is found there. What would find the other is a + lookup with no name on the command line: read the root's config with + `git config -z --list` and match the key's bytes in Python, and build + the kept repository's path from `git rev-parse --git-common-dir`, + `modules/` and the name's bytes. + """ + if os.name == "nt": + return text + return text.encode("utf-8", "surrogateescape") + + def submodule_url(root: Path, rel: str) -> tuple[str | None, str | None]: """`(name, url)`: the NAME a `.gitmodules` section gives the mount at `rel`, and the url `git submodule update --init` fetches it from, each @@ -5246,11 +5286,21 @@ def kept_gitdir(root: Path, name: str) -> Path | None: BY NAME, which is not always the path: `--git-path modules/` says where git keeps it, in the common directory of a linked worktree too. + + AND BY THE NAME'S OWN BYTES (#732): the name goes to git through + `git_bytes`, and the path git answers with is read as a path + (`os.fsdecode`), which `kept_head` and the keep line hand back to git + unchanged. A strict decode of it found no repository for a section + named `caf\\xe9`, and the commit on its detached HEAD got no keep line. """ try: - where = git_out(["rev-parse", "--git-path", f"modules/{name}"], - cwd=root) - except (Refusal, OSError, UnicodeDecodeError): + proc = subprocess.run(["git", "rev-parse", "--git-path", + git_bytes(f"modules/{name}")], + cwd=str(root), capture_output=True, check=False) + if proc.returncode != 0: + return None + where = os.fsdecode(proc.stdout.rstrip(b"\r\n")) + except (OSError, ValueError): return None path = Path(where) if not path.is_absolute(): @@ -7927,7 +7977,7 @@ def submodule_config(root: Path) -> dict: `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, and `gitmodules_listing` says how they are decoded. + asked for, and `submodule_keys` says how each part is 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 @@ -7943,7 +7993,8 @@ 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. + working clone. A section's NAME, which is handed back to git and never + compared, keeps the `surrogateescape` (#732). """ fields: dict = {} listing = gitmodules_listing(root, ["--file", ".gitmodules"]) diff --git a/tests/test_doctor_gitmodules_not_utf8.py b/tests/test_doctor_gitmodules_not_utf8.py index f9433768..ded76389 100644 --- a/tests/test_doctor_gitmodules_not_utf8.py +++ b/tests/test_doctor_gitmodules_not_utf8.py @@ -302,13 +302,14 @@ def assert_read_as_git_reads(standard: Path, root: Path, sibling: Path, `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.""" + clone, as `make siblings` does. The section's name comes back as git's + own bytes, which is how it is handed to git again (#732).""" 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) + name.decode("utf-8", "surrogateescape"), url) result = doctor(standard, root, "--json") assert "Traceback" not in result.stderr, result.stderr entry = rows_of(result)["members"]["detail"]["members"][0] diff --git a/tests/test_doctor_legs_members_next.py b/tests/test_doctor_legs_members_next.py index a6391082..38daf4fa 100644 --- a/tests/test_doctor_legs_members_next.py +++ b/tests/test_doctor_legs_members_next.py @@ -52,6 +52,7 @@ from conftest import (WINDOWS_SKIP, appended_to_command_scope, cloned_just_pushed, git, rmtree, submodule_update_through_git) +from test_doctor_gitmodules_not_utf8 import listed_locale from test_shape_doctor import ( # noqa: F401 FAMILY_MEMBER, FAMILY_NAME, GIT_IDENTITY, PROJECT, commit_all, doctor, doctor_module, holder, project, record_gitlink, rows_of, @@ -1821,6 +1822,82 @@ def test_414_3_pasted_the_holders_bootstrap_cures_that_member( assert after["status"] == "ok", (after, result.stdout, result.stderr) +#: Section NAMES that are not ASCII (#732): one in Latin-1, which is not +#: UTF-8 at all, and one in UTF-8. +BYTE_NAMES = [pytest.param(b"members/caf\xe9", id="latin-1"), + pytest.param(b"members/caf\xc3\xa9", id="utf-8")] + +#: The locales the doctor runs under (#732): a UTF-8 one, `C`, and `C` with +#: Python's UTF-8 mode off, where its filesystem encoding is ASCII. +NAME_LOCALES = [pytest.param("UTF-8", {}, id="utf-8-locale"), + pytest.param("C", {}, id="C"), + pytest.param("C", {"PYTHONUTF8": "0"}, id="C-ascii-fs")] + + +def member_named(holder, tmp_path, monkeypatch, name: bytes, codeset: str, + env: dict) -> Path: + """A copy of the holder whose member is the submodule NAMED `name`, as + `git submodule add --name` names it: the `.gitmodules` header (then + committed), the holder's config section, the repository git keeps and + the mount's `.git` file all renamed, at the same path. The doctor is + run under `codeset` and `env`. Skipped on Windows, whose command line is + text and cannot carry a byte that is not UTF-8.""" + if os.name == "nt": + pytest.skip("a Windows command line cannot carry the byte 0xE9") + monkeypatch.setenv("LC_ALL", listed_locale(codeset)) + for key, value in env.items(): + monkeypatch.setenv(key, value) + root = holder_copy(holder, tmp_path) + old = f"members/{FAMILY_MEMBER}".encode("ascii") + gitmodules = root / ".gitmodules" + gitmodules.write_bytes(gitmodules.read_bytes().replace( + b'[submodule "' + old + b'"]', b'[submodule "' + name + b'"]')) + subprocess.run(["git", "config", "--rename-section", + b"submodule." + old, b"submodule." + name], + cwd=str(root), capture_output=True, check=True) + modules = os.fsencode(root / ".git" / "modules") + b"/" + os.rename(modules + old, modules + name) + gitfile = root / os.fsdecode(old) / ".git" + gitfile.write_bytes(gitfile.read_bytes().replace( + b"/modules/" + old, b"/modules/" + name)) + commit_all(root, "Name the member's section otherwise") + return root + + +@pytest.mark.parametrize("name", BYTE_NAMES) +@pytest.mark.parametrize(("codeset", "env"), NAME_LOCALES) +def test_732_a_member_named_in_bytes_gets_its_keep_line( + standard, holder, tmp_path, monkeypatch, name, codeset, env): + """A commit on the detached HEAD of an emptied member whose section is + named `caf\\xe9` gets the keep line, under every locale, as the ASCII + one does (`test_413a_an_emptied_members_kept_repository_names_...`). + The name was looked up as `caf\\ufffd`, so `kept_gitdir` found no + repository and the commit was left in a reflog; and with Python's + filesystem encoding ASCII, either name raised `UnicodeEncodeError` + there, a traceback with no report.""" + root = member_named(holder, tmp_path, monkeypatch, name, codeset, env) + detached_member(root) + commit = emptied_with_its_repository_kept(root, f"members/{FAMILY_MEMBER}") + assert f"branch NAME {commit}" in why_of(row_of(standard, root, "members")) + + +@pytest.mark.parametrize("name", BYTE_NAMES) +@pytest.mark.parametrize(("codeset", "env"), NAME_LOCALES) +def test_732_a_member_named_in_bytes_whose_url_the_config_keeps_has_one( + standard, holder, tmp_path, monkeypatch, name, codeset, env): + """A member whose section kept its path and lost its url, named + `caf\\xe9`, has the url the holder's config keeps, under every locale, + as the ASCII one does (`test_414_3_a_member_url_the_holders_config_ + keeps_still_gets_the_line`). `configured_url` asked git for + `caf\\ufffd`'s url, and the member was called url-less.""" + root = member_named(holder, tmp_path, monkeypatch, name, codeset, env) + commit_in(detached_member(root)) + subprocess.run(["git", "config", "-f", ".gitmodules", "--unset", + b"submodule." + name + b".url"], + cwd=str(root), capture_output=True, check=True) + assert "no url" not in why_of(row_of(standard, root, "members")) + + def test_a_leg_that_carries_submodules_says_the_line_moves_them_too( standard, project): """The `legs` line's `--recursive` checks out the submodules a leg