From 0f184845dca823c5cb3fe3f13ef61fe977716d08 Mon Sep 17 00:00:00 2001 From: Mihir Srivastava Date: Fri, 23 Oct 2026 12:17:32 -0700 Subject: [PATCH] test: verify installed Git source reader protocols on Windows --- docs/research/windows-installed-git.md | 171 +++++++++++++++++++++++ tests/test_research_git_installed.py | 179 +++++++++++++++++++++++++ 2 files changed, 350 insertions(+) create mode 100644 docs/research/windows-installed-git.md create mode 100644 tests/test_research_git_installed.py diff --git a/docs/research/windows-installed-git.md b/docs/research/windows-installed-git.md new file mode 100644 index 0000000000..31fb59b6be --- /dev/null +++ b/docs/research/windows-installed-git.md @@ -0,0 +1,171 @@ +# Actual older Git for Windows reader checks + +The shared source reader passes the previously unrun older-Git Windows boundary +from [Discussion #4681](https://github.com/CrocSwap/integer-mult-bounds/discussions/4681). +The actual MinGit **2.35.3.windows.1** executable rejects `cat-file --batch -z` +with exit 129 and no stdout. The unchanged reader retries exactly once through +the line protocol and returns the expected object IDs and bytes. The installed +Git **2.47.1.windows.1** is the supported-protocol control and needs one batch +process. This is tooling compatibility evidence, not a mathematical replay. + +## New coverage + +[The four installed-Git tests](../../tests/test_research_git_installed.py) use +real blobs, NUL-delimited trees and commits. The temporary repositories contain +only `.git`; no working file has to carry a Windows-forbidden name. SHA-1 and +SHA-256 fixtures each check eight reads: empty, 262,148-byte binary, space and +Unicode names, literal LF/CR/CRLF names, and a duplicate empty object. +Independent `hashlib` calculations establish every expected blob ID. + +The launch observer delegates to the real `Popen` with its original arguments +and pipes. It does not replace Git or synthesize a protocol reply. On an +unsupported binary, the exact-byte tests wait for the actual probe to exit +before returning its handle to the reader, exercising the native closed-stdin +case as well. The early-close test uses ordinary launch scheduling. The tests +check the exact batch-process sequence and the three literal `rev-parse` +arguments used for line-separator names on the legacy path. + +Additional controls require missing objects, tree objects and NUL-bearing +requests to reject. Early closure must not consume the next request, every +observed process must be reaped with its pipes closed, and no watchdog thread +may remain at a blob yield. The existing simulated-failure tests still supply +controlled non-129 exits, failures after a valid reply and pipe error injection; +these new tests complement their coverage. + +The new module is discovered by the existing onboarding workflow's +`test_research_*.py` command. No workflow, production reader, source pin, frozen +package, scientific artifact or selected record is changed. + +## Sources and environment + +Canonical main base: `7c472563474e60185e6ca101a17622133d047e56`, priority revision 7. +The unchanged `scripts/research_package.py` has Git blob +`ef9b52c392323362621baf6ee188a26fc5122bf9` and SHA256 +`8e07ddee4d8e1c70a278538b2758d423a298b96d5d1a18233dc29925696fc96f`. + +The old executable came from the official +[Git for Windows 2.35.3 release](https://github.com/git-for-windows/git/releases/tag/v2.35.3.windows.1), +asset `MinGit-2.35.3-64-bit.zip` (26,052,150 bytes). Its downloaded SHA256 equals +the publisher's value: + +```text +9b88c81ad796acf9969825e8736b953cb0e0bfb4717be57fd59336ae94d521e4 +``` + +The archive was expanded in the task's scratch directory. Its `cmd` directory +was prepended only to the test process environment's `PATH`; it was not +installed as system Git and performed only operations on disposable local +fixtures. The tests do not download or install Git. + +Native evidence uses Windows 11 build 26300, CPython 3.12.0 x64 and the two Git +versions above. `PYTHONUTF8=1` is explicit for the complete existing reader suite. +An initial broader run with MinGit 2.40.1 and the default Windows encoding failed +the existing `test_unicode_spelling_is_not_normalized` diagnostic assertion: +the rejected decomposed filename was escaped on stderr. The planner still +rejected the alias. This is not counted as a suite pass or fixed by this change. +MinGit 2.40.1 also supports `-z`, so its initial probe is not fallback evidence; +the canonical claim retains that correction. + +## Reproduce + +With either Git version on `PATH`, run the new module normally and with Python +assertions disabled: + +```sh +python -W error -B -m unittest discover -s tests -p 'test_research_git_installed.py' -v +python -O -W error -B -m unittest discover -s tests -p 'test_research_git_installed.py' -v +``` + +For the full focused suite on Windows, save and restore the environment in the +same PowerShell process. Substitute the local extracted MinGit directory: + +```powershell +$savedPath = $env:PATH +$savedUtf8 = $env:PYTHONUTF8 +try { + $env:PATH = (Resolve-Path 'PATH_TO_MINGIT/cmd').Path + ';' + $savedPath + $env:PYTHONUTF8 = '1' + git --version + python -W error -B -m unittest discover -s tests -p 'test_research_git_*.py' -v + if ($LASTEXITCODE -ne 0) { throw 'Normal reader suite failed' } + python -O -W error -B -m unittest discover -s tests -p 'test_research_git_*.py' -v + if ($LASTEXITCODE -ne 0) { throw 'Optimized reader suite failed' } +} finally { + $env:PATH = $savedPath + $env:PYTHONUTF8 = $savedUtf8 +} +``` + +Local results on 2026-10-23, with `-W error -B` throughout: + +| Environment and scope | Normal Python | Optimized Python (`-O`) | +| --- | --- | --- | +| Windows, MinGit 2.35.3, full focused suite | 61 discovered: 53 passed, 8 skipped (86.099 s) | 61 discovered: 53 passed, 8 skipped (77.445 s) | +| Windows, Git 2.47.1, full focused suite | 61 discovered: 53 passed, 8 skipped (73.409 s) | 61 discovered: 53 passed, 8 skipped (71.214 s) | +| Windows, MinGit 2.35.3, final new module | 4 passed, none skipped (3.903 s) | 4 passed, none skipped (3.821 s) | +| WSL Ubuntu, Git 2.43.0, CPython 3.12.3, final new module | 4 passed, none skipped (0.466 s) | 4 passed, none skipped (0.190 s) | + +The eight Windows skips are six existing POSIX descendant tests and two existing +tests that create physical CR/LF filenames. The new object-tree fixtures cover +literal CR/LF names without those filesystem restrictions. The full older-Git +run preceded a final test-only failure-teardown guard and unused-import cleanup; +the final new module was then rerun with both Git versions and on Linux. The +production reader was identical throughout. + +Two separate disposable source copies supplied semantic failure controls with +MinGit 2.35.3. Adding an immediate `raise` after +`except _UnsupportedNulBatch as exc:` disables fallback; replacing the +`if not nul and ("\n" in spec or "\r" in spec):` condition with `if False:` +disables literal-name resolution. In each copy, running +`python -W error -B tests/test_research_git_installed.py InstalledGitTests.test_installed_git_exact_sha1_objects_and_literal_line_separators -v` +exited 1: respectively an unsupported-batch exception and rejection of +`HEAD:line\nname` as missing/non-blob. Neither mutation was retained. This shows +the new test detects these transport regressions, beyond passing the current +implementation. + +## Limits and next questions + +The native Windows direct-child deadline controls use real Python subprocesses +to stall headers, payloads, terminators and writes; they do not demonstrate a +production Git hang. Their success corroborates the composed #4686/#4738/#5185 +reader on this host. Windows descendant-held pipes, escaped processes, other +Git/Python versions and arbitrary package replay compatibility remain outside +scope. No selected cold replay, broad mathematical suite, proof-kernel review +or independent human acceptance is claimed. Hosted CI is separate from the local +evidence above. + +The concrete older-Git handoff is complete for this version pair. A useful next +step would be a native Windows descendant-held-pipe ownership fixture, as already +identified by #5185: first show that a ready descendant can keep the pipe open +after the leader exits, then test a bounded ownership mechanism without changing +source authentication. That needs a separate lifetime contract and is unclaimed +here. The Windows diagnostic-encoding mismatch above is a separate small test +portability question; begin with the single failing spelling test. + +## Coordination and credit + +This follows task `portable-replay`, refined to the explicit older-Git handoff +after checking #1166 and the source thread. The verifier-engineering chooser +seed was `1f8eb9e1a9a35f2f4e6152b366e68143`. The original card's canonical-payload +method is **changed** to an installed-binary and immutable-source transport check: +valid source pins must be readable before research can be independently replayed. +The stop rule is this version pair and the composed direct-child controls, +without a production-reader redesign. + +The [canonical claim](https://github.com/CrocSwap/integer-mult-bounds/discussions/4681#discussioncomment-19152845) +records the exact scope and correction. Initial remote overlap covered 304 open +PR bodies and 4,625 Discussion records with complete pagination. The submission +refresh at 19:13 UTC covered 305 open PR bodies and 4,637 Discussion records, +again with complete pagination; the specific older-Git search surfaced this +claim and its predecessor handoffs. Canonical main was unchanged on refresh. +The sparse local note inventory is incomplete; relevant Git compatibility/provenance +packages, evidence notes, the central review ledger and the live #4681 thread +were read. Searches do not cover every historical diff or unpublished result. + +Nick Petalas's #2752 supplies the fallback and original compatibility tests; +Mihir Srivastava's #4686 supplies Windows pipe handling; Brian / JoyboyBrian's +#4738/#5185 supply response deadlines and POSIX descendant ownership. Woklerx, +Arham Tahir and Douglas Colkitt retain the streaming, batching and package-contract +credit. The object-tree fixture follows Tekalign's exporter tests and their +Windows portability successors. Prepared for Mihir Srivastava with substantial +OpenAI Codex assistance, one agent. Apache-2.0. diff --git a/tests/test_research_git_installed.py b/tests/test_research_git_installed.py new file mode 100644 index 0000000000..5da77cfbe3 --- /dev/null +++ b/tests/test_research_git_installed.py @@ -0,0 +1,179 @@ +"""Exercise the installed Git's actual protocol, including Git-only path names. + +Complements Nick Petalas's simulated option-rejection tests and the Windows +pipe/deadline work by Mihir Srivastava and Brian / JoyboyBrian. Object-tree +fixtures follow the existing exporter tests. Prepared for Mihir Srivastava +with substantial OpenAI Codex assistance. Apache-2.0. +""" +from contextlib import closing, contextmanager +import hashlib +import importlib.util +from pathlib import Path +import subprocess +import tempfile +import threading +import unittest +from unittest.mock import patch + +ROOT = Path(__file__).resolve().parents[1] +SPEC = importlib.util.spec_from_file_location( + "installed_git_package", ROOT / "scripts/research_package.py") +PACKAGE = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(PACKAGE) + + +class InstalledGitTests(unittest.TestCase): + def setUp(self): + temporary = tempfile.TemporaryDirectory() + self.addCleanup(temporary.cleanup) + self.root = Path(temporary.name) + self.payloads = { + "empty": b"", + "binary": b"\x00\xff\r\n" * 65537, + "space name": b"spaces\n", + "unicode-\u03ba": b"unicode path\n", + "line\nname": b"literal LF\x00\xff", + "carriage\rname": b"literal CR\x00\xff", + "pair\r\nname": b"literal CRLF\x00\xff", + } + + def git(self, *args, data=None): + return subprocess.run(["git", *args], cwd=self.root, input=data, + capture_output=True, check=True, timeout=15, + env=PACKAGE.git_source_env()).stdout + + def fixture(self, algorithm="sha1"): + self.git("init", "-q", "--object-format=" + algorithm) + objects, entries = {}, [] + for name, data in sorted(self.payloads.items()): + oid = self.git("hash-object", "-w", "--stdin", data=data).decode().strip() + expected = hashlib.new(algorithm, b"blob " + str(len(data)).encode() + + b"\0" + data).hexdigest() + self.assertEqual(oid, expected) + objects[name] = oid + entries.append(f"100644 blob {oid}\t{name}\0".encode("utf-8")) + nested = self.git("mktree", "-z", data=( + f"100644 blob {objects['empty']}\tleaf\0".encode())).decode().strip() + entries.append(f"040000 tree {nested}\tdir\0".encode()) + tree = self.git("mktree", "-z", data=b"".join(entries)).decode().strip() + commit = self.git("-c", "user.name=Fixture", "-c", + "user.email=fixture@example.invalid", "commit-tree", + tree, "-m", "Git-only names").decode().strip() + self.git("update-ref", "HEAD", commit) + # No checkout or OS filename translation participates in this fixture. + self.assertEqual({path.name for path in self.root.iterdir()}, {".git"}) + names = sorted([*self.payloads, "dir/leaf"]) + self.assertEqual(self.git("ls-tree", "-r", "-z", "--name-only", "HEAD"), + "".join(name + "\0" for name in names).encode("utf-8")) + return objects + + def installed_protocol(self): + probe = subprocess.run(["git", "cat-file", "--batch", "-z"], + cwd=self.root, input=b"", capture_output=True, + timeout=15, env=PACKAGE.git_source_env()) + self.assertIn(probe.returncode, (0, 129), probe.stderr) + self.assertEqual(probe.stdout, b"") + return probe.returncode == 0 + + @contextmanager + def observe(self, wait_for_probe_exit=False): + """Record launches without replacing an executable, reply or pipe.""" + real_popen = subprocess.Popen + calls, children = [], [] + + def start(argv, **kwargs): + calls.append(list(argv)) + child = real_popen(argv, **kwargs) + children.append(child) + if wait_for_probe_exit and argv == ["git", "cat-file", "--batch", "-z"]: + # The actual old binary rejects the option. Delay returning its + # handle until it exits to exercise the native closed pipe too. + self.assertEqual(child.wait(timeout=15), 129) + return child + + try: + with patch.object(PACKAGE.subprocess, "Popen", side_effect=start): + yield calls + finally: + try: + self.assertTrue(all(child.poll() is not None for child in children)) + self.assertTrue(all(stream is None or stream.closed for child in children + for stream in (child.stdin, child.stdout, child.stderr))) + finally: + # Check ownership before fallback teardown: teardown must not + # manufacture a pass, but a regression must not leak children. + for child in children: + if child.poll() is None: + child.kill() + child.wait(timeout=5) + for stream in (child.stdin, child.stdout, child.stderr): + if stream is not None and not stream.closed: + try: + stream.close() + except OSError: + pass + + def assert_protocol(self, calls, supports_nul): + batches = [args for args in calls if args[:3] == ["git", "cat-file", "--batch"]] + expected = [["git", "cat-file", "--batch", "-z"]] + if not supports_nul: + expected.append(["git", "cat-file", "--batch"]) + self.assertEqual(batches, expected) + + def check_exact_objects(self, algorithm): + objects = self.fixture(algorithm) + supports_nul = self.installed_protocol() + names = [*self.payloads, "empty"] + expected = [(objects[name], self.payloads[name]) for name in names] + before = set(threading.enumerate()) + with self.observe(wait_for_probe_exit=not supports_nul) as calls, closing(PACKAGE.git_blobs( + self.root, ("HEAD:" + name for name in names))) as rows: + actual = [] + for row in rows: + actual.append(row) + self.assertEqual(set(threading.enumerate()), before) + self.assertEqual(actual, expected) + self.assert_protocol(calls, supports_nul) + resolutions = [args for args in calls if args[:2] == ["git", "rev-parse"]] + expected_resolutions = [] if supports_nul else [ + ["git", "rev-parse", "--verify", "--end-of-options", "HEAD:" + name] + for name in names if "\r" in name or "\n" in name] + self.assertEqual(resolutions, expected_resolutions) + self.assertEqual(set(threading.enumerate()), before) + + def test_installed_git_exact_sha1_objects_and_literal_line_separators(self): + self.check_exact_objects("sha1") + + def test_installed_git_exact_sha256_objects_and_literal_line_separators(self): + self.check_exact_objects("sha256") + + def test_installed_git_early_close_does_not_consume_later_requests(self): + objects = self.fixture() + supports_nul = self.installed_protocol() + consumed = [] + + def requests(): + for name in ("empty", "absent"): + consumed.append(name) + yield "HEAD:" + name + + with self.observe() as calls, closing(PACKAGE.git_blobs(self.root, requests())) as rows: + self.assertEqual(next(rows), (objects["empty"], b"")) + self.assertEqual(consumed, ["empty"]) + self.assert_protocol(calls, supports_nul) + + def test_installed_git_rejects_missing_tree_and_nul_requests(self): + self.fixture() + supports_nul = self.installed_protocol() + for spec in ("HEAD:absent", "HEAD:dir", "HEAD:empty\0HEAD:binary"): + with self.subTest(spec=spec), self.observe() as calls: + with self.assertRaisesRegex(ValueError, "missing or non-blob|invalid Git object"): + list(PACKAGE.git_blobs(self.root, [spec])) + if "\0" in spec: + self.assertEqual(calls, [["git", "cat-file", "--batch", "-z"]]) + else: + self.assert_protocol(calls, supports_nul) + + +if __name__ == "__main__": + unittest.main()