On Fri, May 24, 2019 at 01:58:03PM +0200, Ævar Arnfjörð Bjarmason wrote: > > 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? I don't understand what an "implicit *.keep flag" is. However, since a reachability bitmap is always associated with a pack, but the commit-graph is not, I don't think this is a valid comparison. > >> 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".
