Re: [PATCH v7 6/6] Reftable support for git-core

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux