Han-Wen Nienhuys <hanwen@xxxxxxxxxx> writes: > On Wed, Feb 26, 2020 at 7:13 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: >> > diff --git a/t/t0031-reftable.sh b/t/t0031-reftable.sh >> > new file mode 100755 >> > index 00000000000..d35211c9db2 >> > --- /dev/null >> > +++ b/t/t0031-reftable.sh > > Thanks for your careful review. Should we not start with the C code, > though? I've written the reftable library to my best ability, but it > hasn't been peer reviewed yet. My intention was to let others, especially those from outside Google, to comment before I do ;-) >> test_write_lines HEAD refs/heads/master >expect && >> >> > + git show-ref | cut -f2 -d" " > got && >> >> Is the order of "show-ref" output guaranteed in some way? Otherwise >> we may need to make sure that we sort it to keep it stable. > > The ref iteration result is lexicographically ordered. > >> Without --show-head, HEAD should not appear in "git show-ref" >> output, but the expected output has HEAD, which is puzzling. > > The HEAD symref is stored in reftable too, and to the reftable code > it's just another ref. How should we refactor this? Shouldn't the > files ref backend produce "HEAD" and then it's up to the show-ref > builtin to filter it out? After looking at show-ref.c, the answer is obvious, I would think. for_each_ref() MUST NOT include HEAD in its enumeration, and that is why the caller of for_each_ref() makes a separate call to head_ref() when it wants to talk about HEAD. As a backend-agnostic abstraction layer, behaviour of functions like for_each_ref() should not change depending on what ref backend is in use. > The reflog entries are only removed if someone calls into the ref > backend to expire them. Does that happen automatically? "gc auto" may kick in in theory but not likely in these tests, so it would be safe and sensible to know and check the expected number of reflog entries in this test, I guess. Thanks.