Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES

From: Han-Wen Nienhuys <hanwen@google.com>
Date: Feb 1, 2022, 20:06 UTC
Message-ID: <CAFQ2z_OFRJh9cwxnbDzrshYPGOvJC6Rz1eHTF-aKURno+41Cvw@mail.gmail.com>
In-Reply-To: <xmqqzgnbh7rv.fsf@gitster.g>

On Mon, Jan 31, 2022 at 11:15 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 29 quoted lines

>
> Taylor Blau <me@ttaylorr.com> writes:
>
> > On Mon, Jan 31, 2022 at 05:50:19PM +0000, Han-Wen Nienhuys via GitGitGadget wrote:
> >> From: Han-Wen Nienhuys <hanwen@google.com>
> >>
> >> The reftable backend doesn't support mere existence of reflogs.
> >
> > Perhaps I'm missing something obvious, but this and the previous patch
> > seem to be conflicting each other.
> >
> > My understanding of the previous change is that you wanted a reflog
> > entry when the REFFILES prerequisite isn't met. But this patch says what
> > matches my understanding is that reftable and reflogs do not play
> > together.
> >
> > If reflogs do not interact with the reftable backend, then what does
> > this patch do?
>
> One difference between the files and the reftable backend is that
> with the files backend, you can say "I am not adding any entry yet,
> but remember that reflog is enabled for this ref, while all other
> refs reflog is not enabled", and the way to do so is to touch the
> "$GIT_DIR/logs/refs/heads/frotz" file---this enables reflog for the
> "frotz" branch, even if core.logAllRefUpdates is not set.
>
> Because there is no generic reflog API that says "enable log for
> this ref", a test that checks this feature with files backend would
> do "touch .git/refs/heads/frotz".

There is refs_create_reflog(), so the generic reflog API exists. The problem is that there is no sensible way to implement it in reftable.

One option is (reflog exists == there exists at least one reflog entry for the ref). This messes up the test from this patch, because it creates a reflog, but because it doesn't populate the reflog, so we return false for git-reflog-exists.

It also turns out to mess up the tests in t3420, as follows:

++ git stash show -p error: refs/stash@{0} is not a valid reference

I get

  reflog_exists: refs/stash: 0

and "git stash show -p" aborts with "error: refs/stash@{0} is not a valid reference".

So I now went with the other option, ie. (reflog exists == true), ie. every conceivable ref has a reflog (but most are empty). This makes t3420 pass.

This behavior also confuses t1405, because in

  $RUN delete-reflog HEAD &&
  test_must_fail git reflog exists HEAD

the last command now always returns true.

-- 
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