On Tue, Apr 13, 2021 at 9:18 AM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> 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.
thanks, adapted this.
> 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.
I'll have a look; segfaults should not happen, of course.
> > +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".
I disagree. I think this would be a job better suited to a
test-helper. Then we don't put tools into users' hands that
potentially corrupt the repository. I don't understand why hash-object
--literally is not a test helper either.
> 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.
One reason for skipping some files wholesale, is that quite a few
tests are not isolated in their test functions, so skipping an
individual test functions affects follow-on tests. I can't remember if
t1450 is one of them, though.
> 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._
The speculation is warranted, but there are currently about 80 files
that have some sort of test failure, and some of these are for quite
common workflows. I would rather focus on those before looking at
tests like fsck. Functional failures may have far reaching
implications on the code structure (example: worktrees considerably
complicated both the reftable library and its git-core glue), while
corruption issues if they occur, will be due to localized bugs.
--
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
--
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
