Skip to content

change!: accept :3:<path> for the 'theirs' index stage, like Git does - #2892

Merged
Sebastian Thiel (Byron) merged 3 commits into
GitoxideLabs:mainfrom
ameyypawar:rev-conformity
Aug 6, 2026
Merged

change!: accept :3:<path> for the 'theirs' index stage, like Git does#2892
Sebastian Thiel (Byron) merged 3 commits into
GitoxideLabs:mainfrom
ameyypawar:rev-conformity

Conversation

@ameyypawar

@ameyypawar Amey Pawar (ameyypawar) commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Created by Claude Code on behalf of Amey, who reviewed it before submitting. Everything below this line is the agent's writing, not his.


Summary

  • :3:<path> now resolves to the 'theirs' side of a conflict instead of being read as a path.
  • The Navigate docs said stages run from 0 to 2 and named them base, ours and theirs, which is off by one against Git in both the range and the labels.
  • gix revision explain gets the same correction, and no longer panics when handed stage 3.

Commits

  • change!: accept :3:<path> for the 'theirs' index stage, like Git does. — the parser arm and the trait docs.
  • adapt to changes in gix-revision.gix revision explain, split out as DEVELOPMENT.md asks.

Git baseline

gitrevisions(7):

A colon, optionally followed by a stage number (0 to 3) and a colon, followed by a path, names a blob object in the index at the given path. A missing stage number (and the colon that follows it) names a stage 0 entry. During a merge, stage 1 is the common ancestor, stage 2 is the target branch's version (typically the current branch), and stage 3 is the version from the branch which is being merged.

The parser had arms for :0:, :1: and :2:, so :3:file fell through to the catch-all and became a lookup for a path literally named 3:file at stage 0. Measured in a repository left in a conflicted merge, with git 2.52.0:

revspec git rev-parse gix revision resolve, before
:1:conflict.txt df967b96 df967b96
:2:conflict.txt b19a1e93 b19a1e93
:3:conflict.txt 950b81b7 err: Couldn't find index '3:conflict.txt' stage 0 (implicit)
:4:conflict.txt no object no object

:4:, :5:, :01: and :10: stay part of the path on both sides, which the existing invalid_index_stage_is_part_of_path already pins.

The docs are the reason the arms stop at 2. They came first, in cee04e1, and the arms written twenty minutes later in ea22d3e matched them — both in #427.

Why this is marked breaking

A Navigate implementation written against the documented 0 to 2 range can now be handed a stage it does not expect. That is not hypothetical: the implementation behind gix revision explain had

match stage {
    0 => "base",
    1 => "ours",
    2 => "theirs",
    _ => unreachable!("BUG: parser assures of that"),
}

so gix revision explain :3:conflict.txt panicked as soon as the parser could emit stage 3. Its labels also carried the same off-by-one, printing stage 1 (ours) where stage 1 is the common ancestor. Both are fixed in the second commit, which is split out as DEVELOPMENT.md asks.

The implementation in gix needed nothing — it already maps 3 to Stage::Theirs, and its own unreachable!() notes that the driver is what guarantees valid stages.

Validation

A differential sweep of 141 revspecs, taking Git's answer via git rev-parse on the same fixture rather than an expectation, goes from 132 to 133 agreeing; :3:conflict.txt is the row that disappears. Six set-valued specs such as ^! and ^@ are excluded and listed by name, because gix revision resolve prints the spec form back rather than expanding the range, which is a difference in the command rather than in parsing.

Two harness faults were corrected before the numbers above were trusted: every fixture commit initially shared one timestamp, which made :/!-first disagree until the commits were given distinct times, and git rev-parse exits 0 while echoing an argument it cannot resolve, so acceptance had to mean "produced an object id" rather than a zero exit code.

Removing only the new arm makes various_valid_index_lookups_by_path_and_stage fail with [("3:dir/path", 0)] against [("dir/path", 3)].

  • cargo test — gix-revision 112, gix-refspec 77, gix 417, gitoxide-core 4
  • cargo fmt --check — gix-revision, gitoxide-core
  • cargo clippy -p gix-revision -p gitoxide-core --all-targets — clean for the changed files; the warnings it prints are in gitoxide-core/src/repository/index/entries.rs and the three unfulfilled_lint_expectations in gix-ref, all present on main

Not addressed here

The same sweep turned up two differences in the relative dates accepted after @{, both of which belong to gix-date rather than to this parser, and one of them touches a function with an open PR against it:

  • @{1.hour.ago} is rejected while @{1 hour ago} is accepted. approxidate_str() skips every byte that is neither a digit nor a letter, so ., -, _ and anything else all read the same as a space to Git.
  • Git's special[] table in date.c holds yesterday, noon, midnight, tea, PM, AM, never and now in 2.52.0, while this crate understands now, today and yesterday. Measured against a reflog with ordinary dates, @{noon}, @{midnight}, @{tea}, @{AM} and @{PM} resolve in Git and are rejected here. @{today} is not a gap of the same kind: 2.52.0 has no today in that table, so approxidate_str() leaves touched unset and reports an error, while master has since added { "today", date_today } — which is the behaviour this crate already has.

Happy to follow up on those separately if they are worth having.

The parser had arms for stages 0, 1 and 2 and let everything else fall through
to the catch-all, so `:3:file` was looked up as a path literally named
`3:file` at stage 0. `gitrevisions(7)` documents a stage number of 0 to 3, and
Git resolves `:3:file` to the blob from the branch being merged.

The trait's own docs said stages range from 0 to 2 and labelled them base, ours
and theirs. That is off by one: 0 is unconflicted, and 1, 2 and 3 are the common
ancestor, the target branch and the branch being merged. The docs came first, in
cee04e1, and the arms written twenty minutes later in ea22d3e matched them.

This is marked breaking because a `Navigate` implementation written against the
old documented range can now be handed a stage it does not expect. The one in
`gix` already maps 3 to `Stage::Theirs`, but the one behind `gix revision
explain` did not, which the next commit addresses.

`:4:` and other numbers stay part of the path, which Git agrees with.
`gix revision explain` panicked on `:3:<path>` through its `unreachable!()` arm
now that the parser can emit stage 3.

Its stage labels carried the same off-by-one as the trait docs, printing
"stage 1 (ours)" where `gitrevisions(7)` has stage 1 as the common ancestor.
@ameyypawar
Amey Pawar (ameyypawar) marked this pull request as draft August 6, 2026 17:12
@Byron
Sebastian Thiel (Byron) marked this pull request as ready for review August 6, 2026 17:53
@Byron

Copy link
Copy Markdown
Member

Thanks a lot, great catch!

It seems GH CI is currently down, so let's see when this merges. But in any case, there is a lot of work left to be done in gix-date if you want full compatibility, and that would definitely be appreciated. The crate has everything you'd need, I think, including baseline tests and fuzzing.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5a4e97bf27

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Comment thread gitoxide-core/src/repository/revision/explain.rs
@Byron
Sebastian Thiel (Byron) merged commit 453c17c into GitoxideLabs:main Aug 6, 2026
1 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants