Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 63 additions & 12 deletions shape-doctor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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:
Expand All @@ -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
Expand All @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium · Configured URLs or keep lines go missing

When the doctor runs on Windows and a .gitmodules section name contains bytes that are not valid UTF-8, git_bytes returns surrogate-containing text, which Git for Windows converts to different bytes. configured_url and kept_gitdir then fail to find the member, so its URL or keep line is missing.

Have git_bytes preserve the original invalid UTF-8 bytes on Windows instead of returning surrogate-containing text.

Prompt for AI agents
In `shape-doctor.py` at line 5253:

**Configured URLs or keep lines go missing**

When the doctor runs on Windows and a `.gitmodules` section name contains bytes that are not valid UTF-8, `git_bytes` returns surrogate-containing text, which Git for Windows converts to different bytes. `configured_url` and `kept_gitdir` then fail to find the member, so its URL or keep line is missing.

Have `git_bytes` preserve the original invalid UTF-8 bytes on Windows instead of returning surrogate-containing 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
Expand Down Expand Up @@ -5246,11 +5286,21 @@ def kept_gitdir(root: Path, name: str) -> Path | None:

BY NAME, which is not always the path: `--git-path modules/<name>` 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():
Expand Down Expand Up @@ -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
Expand All @@ -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"])
Expand Down
5 changes: 3 additions & 2 deletions tests/test_doctor_gitmodules_not_utf8.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
77 changes: 77 additions & 0 deletions tests/test_doctor_legs_members_next.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
Loading