On Wed, Oct 11, 2023 at 6:54 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > > > Seems like this is because of commit-graph being enabled, I think > > the best thing to do here would be to disable the commit graph of > > these tests. > > If the CI uncovered that the new code is broken and does not work > with commit-graph, wouldn't the above a totally wrong approach to > correct it? If the updated logic cannot work correctly when > commit-graph covers the history you intentionally break, shouldn't > the code, when the feature that is incompatible with commit-graph is > triggered, disable the commit-graph? I am assuming that the new > feature is meant to be used to recover from a corrupt repository, > and if it does not work well when commit-graph knows (now stale > after repository corruption) more about the objects that are corrupt > in the object store, we do want to disable commit-graph. After all, > commit-graph is a secondary information that is supposed to be > recoverable from the primary data that is what is in the object > store. Disabling commit-graph in the test means you are telling the > end-users "do not use commit-graph if you want to use this feature", > which sounds like a wrong thing to do. I agree with what you're saying. Disabling writing the commit-graph for only the test doesn't serve the real usage. To ensure that the feature should work with corrupt repositories with stale commit-graph, I'm thinking of disabling the commit-graph whenever the `--missing` option is used. The commit graph already provides an API for this, so it should be simple to do. Thanks!
