Subject: Re: [PATCH v6 18/20] Reftable support for git-core

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 13 Apr 2021 09:18:15 +0200

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

> It can be activated by passing --ref-storage=reftable to "git init", or setting
> GIT_TEST_REFTABLE in the environment.
> [...]
> +const char *default_ref_storage(void)
> +{
> +	const char *test = getenv("GIT_TEST_REFTABLE");
> +	return test ? "reftable" : "files";
> +}

Nit: use git_env_bool() here. So GIT_TEST_REFTABLE=true is true, but
GIT_TEST_REFTABLE= is not.

Not a nit: are these tests supposed to work / do they work for you? For
me there's a *lot* of failures, e.g. t6120-describe.sh fails for reasons
you might expect, manually tweaking things in the FS-refstore, but
e.g. t5510-fetch.sh segfaults.

t0001-init.sh fails with:

    git: refs/reftable-backend.c:194: git_reftable_init_db: Assertion `refs->worktree_reftable_dir == NULL' failed.

This is on an amd64 Debian w/Linux 4.19, so nothing exotic.

> [...re-arranged a bit...]
>
> +if test -n "$GIT_TEST_REFTABLE"
> +then
> +  test_set_prereq REFTABLE
> +fi
> +
>  ( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1
>  test -z "$NO_PERL" && test_set_prereq PERL
>  test -z "$NO_PTHREADS" && test_set_prereq PTHREADS

Ah, so here's the (seemingly underused) prereq, but:

> --- a/t/t1450-fsck.sh
> +++ b/t/t1450-fsck.sh
> @@ -8,6 +8,12 @@ test_description='git fsck random collection of tests
>  
>  . ./test-lib.sh
>  
> +if test_have_prereq REFTABLE
> +then
> +  skip_all='skipping tests; incompatible with reftable'
> +  test_done
> +fi
> +

I understand that getting this series to land has been a pain, but this
sort of thing doesn't inspire confidence.

Manually running it under reftable anyway I have 17 out of 81 tests
failing. A lot of tests won't run under reftable because they're of the
form:

	test_when_finished "remove_object $tag" &&
	echo $tag >.git/refs/tags/wrong &&
	test_when_finished "git update-ref -d refs/tags/wrong" &&

I.e. we have some hack to get around update-ref guarding things for us.

I would think that a better approach here would be to start with some
(per-se unrelated) series to teach update-ref some mode like
hash-object's --literally, i.e. "YOLO this ref update".

Then adjust our various tests that move, clobber, or intentionally
corrupt things in the refstore to use that helper/option.

At that point we can actually run things like t1450-fsck.sh, maybe we
simply have to skip some of them because we think the sort of corruption
we're testing/worried about is impossible with reftable, but that in
itself is *very* interesting.

I.e. do we not have to handle certain edge cases at all in fsck and
friends because of inherent properties of the reftable backend (e.g. I'd
assume, but don't know, that we're not likely to get one corrupt ref
entry with it), but of course we can have a ref pointing to a loose
object that then gets removed, and is thus corrupt.

Anyway. I'm speculating, but that's also my point, that I'm having to
speculate. That comes down to us having a new GIT_TEST_* mode that
doesn't pass the test suite._