Re: [PATCH] commit-graph: disable GIT_COMMIT_GRAPH_PARANOIA by default

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

On Thu, Nov 16, 2023 at 09:07:01AM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> >> Yeah. Just like we auto-enabled GIT_REF_PARANOIA for git-gc, etc, I
> >> think we should do the same here.
> >
> > I'm honestly still torn on this one. There are two cases that I can
> > think of where missing objects would be benign and where one wants to
> > use `git rev-list --missing`:
> >
> >     - Repositories with promisor remotes, to find the boundary of where
> >       we need to fetch new objects.
> >
> >     - Quarantine directories where you only intend to list new objects
> >       or find the boundary.
> >
> > And in neither of those cases I can see a path for how the commit-graph
> > would contain such missing commits when using regular tooling to perform
> > repository maintenance.
> 
> I can buy the promisor remotes use case---we may expect boundary
> objects missing without any repository corruption.  I do not know
> about the other one---where does our "rev-list --missing" start
> traversing in a quarantine directory is unclear. Objects that are
> still in quarantine are not (yet) made reachable from any refs, so
> even "rev-list --missing --all" would not make a useful traversal,
> no?

You typically know about the new tips when having a quarantine
directory. So you can discover the boundary between objects in your
quarantine directory and your main object database by executing `git
rev-list $NEWREVS --missing=print` and execute that command with
`GIT_OBJECT_DIRECTORY=$quarantine`. The benefit is that this scales with
the number of objects in the quarantine, and not with the size of the
overall repository.

As mention, this is really niche, but we do plan to use this in Gitaly
eventually.

> In any case, it sounds like you are not torn but are convinced that
> we should leave this loose by default ;-) and after thinking it over
> again, I tend to agree that it would be a better choice, as long as
> the feature "rev-list --missing" has any good use case other than
> corruption in repository.
> 
> So,... thanks for pushing back.

Okay, glad to hear that I'm not totally bonkers then. I'm going to wait
another few days for additional feedback before sending a v2. But if
what I'm saying also makes sense to others then v2 would only squash in
the diff I sent to run the subset of tests that is now failing  with
`GIT_COMMIT_GRAPH_PARANOIA=true`.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]