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

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

On Wed, Oct 13 2021, Derrick Stolee wrote:

> On 10/12/2021 4:34 PM, Ævar Arnfjörð Bjarmason wrote:...
>> 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.
>
> The commit-graph should be disabled if replace-objects are enabled. If
> there is a bug being introduced here it is because the commit-graph is
> being checked during fsck even though it would never be read when the
> replace-objects exist.
>
> Thanks,
> -Stolee

Thanks, isn't the obvious fix for this to extend your d6538246d3d
(commit-graph: not compatible with replace objects, 2018-08-20) to do
"read_replace_refs = 0;" in graph_verify()? That works for me on this
case.

I.e. if we ignore replace refs when writing the graph, and we don't use
it if there are any due to commit_graph_compatible(), why would we
consider them when verifying it?