Re: [PATCH v4 0/3] Use default values from settings instead of config

Ævar Arnfjörð Bjarmason [off-list ref] writes:

...how isn't disabling those t3800-mktag.sh tests just plasting over
corruption that we're noticing because of your changes to (rightly) fix
the bug where "fsck" wasn't checking the graph at all?

IOW haven't we just found exactly the sort of bug that
"GIT_TEST_COMMIT_GRAPH" is put in place to find for us, but now instead
of fixing it we're hiding it?

If I comment yout your addition of GIT_TEST_COMMIT_GRAPH=0 in that file
I see that we fail N number of tests, but all of them are actually
fallout of just this test:

        git replace $head_parent $head && 
        git replace -f $tree $blob 

I.e. we've created a replacement object replacing a tree with a blob, as
part of tests I added to test how mktag handles those sorts of weird
edge cases.

This then causes the graph verify code to throw a hissy fit with:

    root tree OID for commit 0ddfaf193ff13d6ab39b7cbd9eed645e3ee2f050 in
    commit-graph is da5497437fd67ca928333aab79c4b4b55036ea66 !=
    0fbca9850869684085d654f9e1380c9780802570

I.e. when we wrote the graph we somehow didn't notice that the root tree
node we wrote is to an object that's not actually a tree? Isn't this a
bug where some part of the commit graph writing should be doing its own
extended OID lookup that's replacement-object aware, it didn't, and we
wrote a corrupt graph as a result?

If there is a legitimate reason why we're not just hiding a bug we've
turned up with these fixes let's disable that one test, not the entire
test file.

If you don't run the one test that fails (which is split up into 3
individual pieces) there's still 143 other tests that are run, all of
those presumably benefit from finding future bugs with
GIT_TEST_COMMIT_GRAPH=true, particularly since the test file seems to
just have turned up one just now...

I think this falls on my shoulders. I assumed that the failures were
expected behavior, not bugs. You are right that we shouldn't be
plastering over bugs.

I'll have to ask for help here because I don't know enough about mktag
to distinguish between 'expected' and 'unexpected' failures. The best I
can do is to add GIT_TEST_COMMIT_GRAPH=0 + NEEDSWORK for the failing
tests. But if that's good enough for now, I'll just do that :)