fix(gix-index): correctly decode untracked extension#2591
Conversation
6253514 to
85643fa
Compare
- Fix ctime/mtime flip in stat decoding - Fix incomplete decoding to mirror git, with associated ported git test Co-authored-by: GPT 5.4 <codex@openai.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
85643fa to
34b2f48
Compare
34b2f48 to
0e4bd3c
Compare
Tasks
|
0e4bd3c to
954c5c8
Compare
Add `scripted_fixture_read_only_needs_archive()` for fixtures whose generated output can vary in byte order across platforms or filesystems. The helper still validates archive identity, but bypasses GIX_TEST_IGNORE_ARCHIVES and re-extracts the checked-in archive so tests can opt into frozen fixture bytes when rerunning the producer is non-deterministic, for instance due to filesystem order. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
954c5c8 to
83ab103
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes gix-index’s decoding of the index UNTR (untracked cache) extension to match Git’s on-disk layout, and corrects a ctime/mtime swap in stat decoding. It also adds fixtures + snapshot-style tests to validate end-to-end decoding of the untracked cache directory graph.
Changes:
- Fix
decode::stat()to assign decodedctime/mtimeto the correctentry::Statfields. - Adjust
UNTRdecoding to read both stats, thendir_flags, then both hashes (matching Git), and add public accessors +Debugderives for snapshotting. - Add new archived fixtures + tests for empty/populated/nested untracked cache decoding; update testtools to support “must use archive” fixtures and isolate
XDG_CONFIG_HOME.
Reviewed changes
Copilot reviewed 11 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/tools/src/tests.rs | Adds a test to ensure configure_command() overrides XDG_CONFIG_HOME. |
| tests/tools/src/lib.rs | Adds “needs archive” fixture helper; ensures fixture commands run with a private XDG_CONFIG_HOME; threads needs_archive through archive extraction. |
| gix-index/tests/index/main.rs | Adds a fixture helper that forces using archived fixtures even when archives are otherwise ignored. |
| gix-index/tests/index/file/read.rs | Adds snapshot tests (via insta) for decoded UNTR extension shapes (empty/populated/nested) using archived fixtures. |
| gix-index/tests/fixtures/make_index/untracked_cache_empty.sh | New fixture generator for an index with an empty UNTR cache. |
| gix-index/tests/fixtures/make_index/untracked_cache_populated.sh | New fixture generator producing a populated UNTR cache via git status. |
| gix-index/tests/fixtures/make_index/untracked_cache_nested.sh | New fixture generator covering nested directories and per-dir ignore OIDs in UNTR. |
| gix-index/src/extension/untracked_cache.rs | Implements Git-aligned UNTR decode layout; adds Debug derives and accessor methods for snapshot-friendly inspection. |
| gix-index/src/extension/mod.rs | Derives Debug for UntrackedCache. |
| gix-index/src/decode/mod.rs | Fixes ctime/mtime field assignment in stat decoding. |
| gix-index/Cargo.toml | Adds insta as a dev-dependency for new snapshot tests. |
| Cargo.lock | Locks new dev-dependency additions (e.g., insta/transitives). |
Seed the untracked-cache fixture paths with a known mtime before Git populates the UNTR extension. Keeping that mtime visible in the snapshot covers the ctime/mtime decode ordering: if the fields are swapped, the expectation will show the unexpected timestamp. Leave ctime redacted because it is filesystem metadata-change time and varies by platform and run, while still preserving enough snapshot detail to catch the decode-order regression. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
2196d82 to
e8de738
Compare
This allows tests to rely on it more with insta, and not miss a thing. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
e8de738 to
e4bdd1f
Compare
|
Just as an FYI for Eliah Kagan (@EliahKagan), This pr also adds an extension to gix-testtools which allows to force the usage of an archive. This is useful if some data created by the underlying shell script is inherently unstable and not under our control. |
|
Oh, and now that I see it: Thanks a lot for making this happen Aaron Moat (@AaronMoat), it's much appreciated! It's interesting that in the age of agentic, you don't necessarily need less time to get into something, because you're still the puny human you were before, but instead you get more done in that time as you can just instruct the agent to do it real quick. While I look, play, investigate, watch and learn, there's always an agent doing work for me. And that then has to be reviewed again, so there's more work and more work being created on the fly, which just didn't happen before 😁. |
Split out from #2503 as requested. I thought including the untracked cache parsing end-to-end was small enough and more logical than just the ctime/mtime flip -- as that's the only spot it's used anyway. Hope that's okay.
The tests added are limited; most of git's tests are focused on its interactions with
git status. The tests added are attempting to replicate these.As for the ctime/mtime Sebastian Thiel (@Byron):
I'm not really sure what we should test: I don't see an obvious git test to port. As for correctness, it seems that the data is just read into this struct, which is ctime then mtime. The existing code was reading ctime then mtime variables correctly, then assigning the reverse into the rust struct.
Let me know if there's any test suggestions, happy to extend them. I don't know the git source code very well so may very well be missing some good tests there.