On Thu, Apr 30, 2020 at 11:17 PM Emily Shaffer <emilyshaffer@xxxxxxxxxx> wrote: > > On Mon, Apr 27, 2020 at 08:13:28PM +0000, Han-Wen Nienhuys via GitGitGadget wrote: > > > > > > This happens implicitly in the files/packed ref backend; making it > > explicit simplifies adding alternate ref storage backends, such as > > reftable. > > As an outsider to this part of the codebase, a little more explanation > could be handy in the commit message. I found the backends you mentioned > in refs, and it seems like they're the only two, but it's not obvious > how this delta is related to those backends. Furthermore, grepping looks > like this function whose behavior is changing is being called from > somewhere else, with no change to that function (and it looks like the > callsite's callback doesn't check whether a ref begins with refs/ or > not). > > All this to say - it's hard to convince myself this is a safe change, > and I'd really like to read more to understand why you made it. I'll be the first to admit that I'm on shaky ground here. However, given that the test suite passes, if this is breaking some behavior, it's probably not very well tested behavior. Here is what I know: Git stores refs in multiple places: - normal refs, packed: .git/packed-refs - normal refs, loose: under refs/*/ - special refs: HEAD, ORIG_HEAD, REBASE_HEAD. Currently, the special refs can only be read with refs_read_raw_ref(). If you iterate over the refs in the files/packed backend, you can never find HEAD, ORIG_HEAD etc. Reftable does not have different classes of ref storage, so this means that the whole space of refs (including HEAD) is managed by the reftable backend, and if you iterate over the refs, reftable will also produce HEAD. This is not spelled out in the spec, but JGit actually stores HEAD in reftable too, and we want to be interoperable. Without this patch, commands like "git show-ref" will produce an entry "HEAD", which is a regression. With this patch, the default iteration is limited to the "refs/" prefix, so we don't produce HEAD in the reftable backend by default. Now, I have several questions: * how does this interact with worktrees? It seems that there is a special worktree namespace? * if you are doing a rebase, and have unreachable objects in ORIG_HEAD, REBASE_HEAD, how does git-gc ensure that these objects stay alive? I'd think that you need to iterate over all entries (including REBASE_HEAD and friends), but I haven't been able to understand how that works. -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado