Re: [PATCH 13/18] t2017: mark --orphan/logAllRefUpdates=false test as REFFILES

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Apr 19 2021, Han-Wen Nienhuys via GitGitGadget wrote:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> In reftable, there is no notion of a per-ref 'existence' of a reflog. Each
> reflog entry has its own key, so it is not possible to distinguish between
> {reflog doesn't exist,reflog exists but is empty}. This makes the logic
> in log_ref_setup() (file refs/files-backend.c), which depends on the existence
> of the reflog file infeasible.

Okey, so I'd follow this if the test was doing something like "test -e
.git/logs" to test whether we didn't have reflogs for a specific branch
or something....

> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  t/t2017-checkout-orphan.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
> index c7adbdd39ab9..88d6992a5e1f 100755
> --- a/t/t2017-checkout-orphan.sh
> +++ b/t/t2017-checkout-orphan.sh
> @@ -76,7 +76,7 @@ test_expect_success '--orphan makes reflog by default' '
>  	git rev-parse --verify delta@{0}
>  '
>  
> -test_expect_success '--orphan does not make reflog when core.logAllRefUpdates = false' '
> +test_expect_success REFFILES '--orphan does not make reflog when core.logAllRefUpdates = false' '
>  	git checkout main &&
>  	git config core.logAllRefUpdates false &&
>  	git checkout --orphan epsilon &&

... but in this test we don't end up doing anything that obviously
depends on ref files, right after this context we do (and this is the
entire test):

    test_must_fail git rev-parse --verify epsilon@{0} &&
    git commit -m Epsilon &&
    test_must_fail git rev-parse --verify epsilon@{0}

So either this is over-skipping of a test, or a summary like this would
be more inaccurate (I'm not suggesting to include it, just writing it
out to state/check my assumptions):

    [...]the reflog implementation leaks the implementation detail that
    it has no per-ref instance of the reflog all the way up to syntax
    like "rev-parse --verify branch@{0}", which is just asking for the
    Nth reflog entry for a branch[...]

But maybe I'm wrong about that, "man git-rev-parse" says, "This" is
reference to the @{<n>} syntax:

    [...]This suffix may only be used immediately following a ref name
    and the ref must have an existing log ($GIT_DIR/logs/<refname>).

In your v7[1] of the reftable series there's no patch to
Documentation/revisions.txt altering that blurb.

So it seems to me that between this & that series there's some closing
of the gap needed with how this "must have an existing log" even works
under reftable.

Per [2] I had assumed that this worked under reftable by abstracting
away the syntax to some query for the ref name, and faking up "file does
not exist" as "there were no records" to anything like rev-parse, but it
doesn't work like that?

1. https://lore.kernel.org/git/pull.847.v7.git.git.1618832276.gitgitgadget@gmail.com/
2. https://eclipse.googlesource.com/jgit/jgit/+/master/Documentation/technical/reftable.md#log-record
3. https://lore.kernel.org/git/87sg3k40mc.fsf@evledraar.gmail.com/