Wrong refs returned by repo.references()?.local_branches()? sometimes in non-garbage-collected repos

[ilyagr]

Current behavior 😯

I'm not sure how to have a minimal example, but I'll attach the repo where I saw this problem.

TLDR, I have these two branches:

$  git branch -v | rg pr4021
  ig-pr4021                      4dec14596 minor fixups (clippy, UI)
  old-ig-pr4021                  21b572308 minor fixups (clippy, UI)

However, when I use gix to get the list of references, it lists "refs/heads/ig-pr4021" twice, with different ids:

[src/main.rs:10:13] b = Reference {
    name: FullName(
        "refs/heads/ig-pr4021",
    ),
    target: Object(
        Sha1(21b57230833a1733f6685e14eabe936a09689a1b),
    ),
    peeled: None,
}
[src/main.rs:10:13] b = Reference {
    name: FullName(
        "refs/heads/ig-pr4021",
    ),
    target: Object(
        Sha1(4dec145966c546402c5a9e28b932e7c8c939e01e),
    ),
    peeled: None,
}
[src/main.rs:10:13] b = Reference {
    name: FullName(
        "refs/heads/old-ig-pr4021",
    ),
    target: Object(
        Sha1(21b57230833a1733f6685e14eabe936a09689a1b),
    ),
    peeled: None,
}

As the names suggest, ig-pr4021 used to point to 21b57, but that was a long time ago.

The program that generated the above output is:

use std::path::Path;

use gix::bstr::{B, BString, ByteSlice};

fn main() -> anyhow::Result<()> {
    let repo = gix::discover(Path::new("/path/to/repo"))?;
    for b in repo.references()?.local_branches()? {
        let b = b.unwrap();
        if b.name().as_bstr().find(b"pr4021").is_some() {
            dbg!(b);
        }
    }
    Ok(())
}

I'm using gix v0.70.0 on ARM Mac OS. I can send you my lockfile, but I just populated it today; I'm hoping you can easily reproduce this (if you're OK with downloading my repo).

Workaround

Running git gc fixed this.

Expected behavior 🤔

The program should only print

[src/main.rs:10:13] b = Reference {
    name: FullName(
        "refs/heads/ig-pr4021",
    ),
    target: Object(
        Sha1(4dec145966c546402c5a9e28b932e7c8c939e01e),
    ),
    peeled: None,
}
[src/main.rs:10:13] b = Reference {
    name: FullName(
        "refs/heads/old-ig-pr4021",
    ),
    target: Object(
        Sha1(21b57230833a1733f6685e14eabe936a09689a1b),
    ),
    peeled: None,
}

Git behavior

See above

Steps to reproduce 🕹

See above. (I will upload the repo in a bit and edit this message) (done).

The repo is at https://drive.google.com/file/d/14pheDpbYL8u9p8VWKWIIO4gbv3xjy35t/view?usp=sharing (150MB). Sorry for the size but, again, running git gc makes the bug go away.

[Byron]

Thanks a lot for reporting and the instructions for reproduction!

The problem here lies within the overlay iterator which is supposed to list loose references over packed ones if there is a choice.

It seems that this fails here as there is a .git/refs/ig/ subdirectory with three refs in it.

[Byron]

I have create a PR with the reproduction, one that makes pretty clear what's happening. The iterators for packed-refs and loose refs are supposed to be sorted to stay in sync, but apparently that's not the case anymore due to the subdirectory.

It appears that the order in packed-refs is files-first, but the order of the iterator is directory-first. A fix is on the way, I hope to finish it till the weekend.

[ilyagr]

Thank you! No rush on my account; while I think this definitely needs to be fixed in the medium term, few people will probably encounter it in the wild until gix conquers the world. Looking at your reproduction, the bug is more subtle than I expected.

I'm a bit confused by the reproduction in a5c7adb since I see no evidence of the buggy behavior in the insta snapshot; I'd expect refs/heads/ig-pr4021 to appear twice in there. Most likely, it's fine (I'm guessing the test in that commit intentionally fails until you develop a fix, or perhaps I'm confused somehow about what the test demonstrates) but I thought I'd let you know just in case.

[Byron]

I'm a bit confused by the reproduction in a5c7adb since I see no evidence of the buggy behavior in the insta snapshot; I'd expect refs/heads/ig-pr4021 to appear twice in there. Most likely, it's fine (I'm guessing the test in that commit intentionally fails until you develop a fix, or perhaps I'm confused somehow about what the test demonstrates) but I thought I'd let you know just in case.

Indeed, it's the desired good state, and the test fails in this commit as 'the one' reference appears twice.
The fix is also already available as a PoC, but I'd think tomorrow morning I will managed to finish it, finally.