On Fri, May 24 2019, SZEDER Gábor wrote: > On Fri, May 24, 2019 at 01:17:06PM +0200, Ævar Arnfjörð Bjarmason wrote: >> >> On Fri, May 24 2019, SZEDER Gábor wrote: >> >> > On Fri, May 24, 2019 at 11:01:39AM +0200, Ævar Arnfjörð Bjarmason wrote: >> >> I don't think it's a performance problem to have an old commit-graph >> >> lying around. But if you turn on the commit-graph, run gc a bunch, then >> >> turn it off in config we'll have it lying around forever, even if you do >> >> subsequent gc's. >> >> >> >> So I think we should delete such things on the general principle that >> >> the end-state of a gc's shouldn't be the accumulation of the values of >> >> past configuration options if we can help it. >> >> >> >> Maybe that screws over other users who did a "commit-graph write" >> >> without setting gc.writeCommitGraph, but I think the only sane thing to >> >> do is to make "gc" fully 'own' such things if its turned on at all. >> > >> > Note that there is 'core.commitGraph' as well; as long as it's >> > enabled, no commit-graph files should be deleted. >> >> Why? If we won't update it or write it if it's not there, why keep it >> around? > > To read it, if 'core.commitGraph' says that is should be read. > >> It means the commit-graph code and anything else (like bitmaps) needs to >> deal with stale data for the common and default gc --auto case. >> >> You also can't have e.g. a global core.commitGraph=true config along >> with a per-repo gc.writeCommitGraph=true config do what you expect. >> >> Now just because you wanted to write it for some you'll end up keeping >> it around forever because you'd also want to optimistically always use >> it if it's there. > > This is exactly what I expect it to do. Do you also expect base packs with an associated bitmap to have an implicit *.keep flag under gc with pack.writeBitmaps=false and pack.useBitmaps=true? >> Note that I'm talking about the *default* gc semantics, they don't have >> to cover all advanced use-cases, just be good enough for most, and it's >> also important that they're as simple as possible, and don't result in >> stuff like "my performance sucks because I turned this config option on >> once a year ago for 2 days".