fix: expand attribute macros only when set#2641
Merged
Merged
Conversation
`gix_attributes::Search` expanded macro attributes whenever a matching pattern referenced a macro name, even if the match unset the macro, reset it to unspecified, or assigned a value. Match Git behavior by expanding a macro only when its matched assignment state is exactly `Set`. This follows the reference implementation in git.git `attr.c::fill_one`, which queues a macro only when `item->macro && item->value == ATTR__TRUE` (checked at git.git 7760f83b59). Co-authored-by: Sebastian Thiel <[email protected]>
Sebastian Thiel (Byron)
force-pushed
the
fix-2639
branch
from
June 16, 2026 01:33
d20eff7 to
4039075
Compare
Sebastian Thiel (Byron)
marked this pull request as ready for review
June 16, 2026 01:33
Sebastian Thiel (Byron)
enabled auto-merge
June 16, 2026 01:33
There was a problem hiding this comment.
Pull request overview
Adjusts gix-attributes macro expansion behavior to match git check-attr: macro sub-assignments are expanded only when the macro is set (not unset/unspecified/value).
Changes:
- Gate macro expansion in
Outcome::fill_attributes()onState::Set. - Add a new baseline fixture (
macro-expansion) coveringSet,Unset,Unspecified, andValuemacro states. - Add a regression test asserting parity with
git check-attr -afor the new fixture.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| gix-attributes/src/search/outcome.rs | Restricts macro expansion to State::Set to match git’s behavior. |
| gix-attributes/tests/search/mod.rs | Adds a regression test comparing results against a git-generated baseline. |
| gix-attributes/tests/fixtures/make_attributes_baseline.sh | Adds a new fixture repository and baseline generation for macro state cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sebastian Thiel (Byron)
disabled auto-merge
June 16, 2026 01:45
Sebastian Thiel (Byron)
enabled auto-merge
June 16, 2026 01:48
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
Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Summary
Fixes #2639.
gix-attributespreviously expanded a macro whenever a matching rule referenced the macro name, even when the matched assignment unset it, reset it to unspecified, or assigned a value. This now expands macro attributes only when the matched macro state is exactlySet.Git Baseline
Git gates macro expansion in
attr.c::fill_oneonitem->macro && item->value == ATTR__TRUE. I checked this in the local git.git reference checkout at7760f83b59.Validation
cargo test -p gix-attributes macro_attributes_expand_only_when_macro_is_setcargo test -p gix-attributescargo fmt --checkcodex review --commit d20eff7b07d681169efb4189fedea1b3d5457754