Repository navigation
fix(gix): infer directory entries from trailing separators - #3190
Merged
Merged
Conversation
<!-- agent --> Path-component iteration discards trailing separators. Callers such as `gix::AttributeStack::at_entry()` need that information to distinguish absent directories from files, as reported in #3189. Add `ToNormalPathComponents::ends_with_separator()` to inspect the original input before component iteration. Native paths recognize the platform's directory separators, including `\` on Windows; Git strings and byte strings recognize only `/`. Implement the method for all five supported path types without changing their component iteration. The tests cover empty paths, trailing slashes, native Windows backslashes, and the distinction between native and Git paths. BREAKING CHANGE: custom implementations of `gix_fs::stack::ToNormalPathComponents` must implement the new required `ends_with_separator()` method. Assisted-by: GPT 6.0 Astra Co-authored-by: GPT 6.0 Astra <codex@openai.com>
<!-- agent --> The reproduction in #3189 queries an absent `absent/target/` under a `target/` ignore rule. `AttributeStack::at_entry(path, None)` reports it as included even though its documentation and `git check-ignore --no-index` identify the trailing slash as a directory hint. The wrapper forwards the unknown mode to `at_path()`, which assumes a file. Use `ToNormalPathComponents::ends_with_separator()` to infer `Mode::DIR` before passing the path to the underlying stack, only when the caller supplies no mode. This also recognizes native Windows backslashes while retaining all supported input types. Explicit modes take precedence, and `at_path()` keeps its existing behavior. Git reference: the local Git checkout identifies itself as `2.55.0.782.g1630431f32.dirty`; `builtin/check-ignore.c` calls `last_matching_pattern()` in `dir.c`, whose directory-only rules require `DT_DIR`. The executable baseline is Apple Git `2.54.0`, which ignores `absent/target/` and leaves `absent/target` included in the fixture. Assisted-by: GPT 6.0 Astra Co-authored-by: GPT 6.0 Astra <codex@openai.com>
Sebastian Thiel (Byron)
force-pushed
the
fix-attribute-query
branch
from
November 10, 2026 20:26
4202740 to
9bb7d88
Compare
Sebastian Thiel (Byron)
marked this pull request as ready for review
November 10, 2026 20:29
Contributor
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bb7d88f1b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by
Codex.Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Fixes #3189.
YevheniiKotyrlo's reproduction found that querying an absent
absent/target/under atarget/ignore rule disagreed with Git and withAttributeStack::at_entry()'s documented behavior. This matters to applications checking exclusion rules for paths that do not exist on disk.at_entry(path, None)now infers a directory from a trailing/, or a native\on Windows. Paths without a trailing separator retain file behavior, and an explicit mode still wins. Both public method signatures stay intact: the existing path-component trait gains a defaultedends_with_separator()method for its five built-in path types. Custom path types can override that method to provide the same inference.The read-only regression compares strings, byte strings, and native paths with
git check-ignore --no-index, checks explicit file/directory modes and unchangedat_path()behavior, and verifies that no directories are created. It failed before the fix and passes afterward. Commit review caught the native Windows backslash case; portable separator tests and Windows-specific exclusion assertions cover it.Validation passed on macOS:
sha1,excludes.gix-fsandgix-worktreetests, including their Git baselines, plus the new separator test.cargo clippy -p gix -p gix-fs --all-targets -- -D warnings.gixand experimental APIs enabled forgix-fs.Windows CI passed both regression tests, including the native backslash assertions. Strict default-feature docs encounter three existing links to networking-only clone methods; enabling
blocking-network-clientresolves them.Git baseline: Apple Git 2.54.0 ignores
absent/target/and leavesabsent/targetincluded. The local Git reference checkout (2.55.0.782.g1630431f32.dirty) confirms directory-only matching throughbuiltin/check-ignore.canddir.c::last_matching_pattern().CI completed for
4202740: the main QA workflow passed, including Linux, macOS, Windows, feature tests, lint, API checks, docs, and journey tests. CodeQL, fuzzing, and workflow security analysis passed too. No review comments or unresolved threads were present at the final inspection.Nine advisory downstream checks remain red on API incompatibilities already present in the base commit:
cargo,crates-index,rustsec,jj, andjj-testutilsuse older repository-option APIs;vergen-gixuses older shallow/path result APIs;starshipuses the older diff-resource signature; both GitButler variants use the former credentialsidentityfield. These checks are non-blocking, and none of their failing APIs changed in this PR.