From: Ævar Arnfjörð Bjarmason Date: Thu, 14 Oct 2021 23:38:02 GMT Subject: Re: [PATCH v4 0/3] Use default values from settings instead of config Message-ID: <87ilxzfa7s.fsf@evledraar.gmail.com> In-Reply-To: On Thu, Oct 14 2021, Glen Choo wrote: > Derrick Stolee writes: > >>>> 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. >> >> Ignoring the replace refs while verifying will allow you to verify the >> on-disk commit-graph file without issue. > > It seems like we've converged on doing read_replace_refs = 0 \o/ > > If we are going to do this twice in graph_verify() and graph_write(), is > there any reason why I shouldn't just do "read_replace_refs = 0" once in > cmd_commit_graph()? IOW any time we do anything with commit-graphs, we > should just ignore replace refs because they're incompatible. No reason, I think that's the best way to do this. I've submitted a series to fix that verify issue, as noted in the CL these patches on top of it without that disabling of the mktag tests will pass with GIT_TEST_COMMIT_GRAPH=true): https://lore.kernel.org/git/cover-0.3-00000000000-20211014T233343Z-avarab@gmail.com