Subject: Re: [PATCH 0/4] Check .gitmodules when using packfile URIs

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 19 Feb 2021 02:08:47 +0100

On Fri, Feb 19 2021, Junio C Hamano wrote:

> Jonathan Tan <[email-masked]> writes:
>
>> This patch set resolves the .gitmodules-and-tree-in-separate-packfiles
>> issue I mentioned in [1] by having index-pack print out all dangling
>> .gitmodules (instead of returning with an error code) and then teaching
>> fetch-pack to read those and run its own fsck checks after all
>> index-pack invocations are complete.
>>
>> As part of this, index-pack has to output (1) the hash that goes into
>> the name of the .pack/.idx file and (2) the hashes of all dangling
>> .gitmodules. I just had (2) come after (1). If anyone has a better idea,
>> I'm interested.
>>
>> I also discovered a bug in that different index-pack arguments were used
>> when processing the inline packfile and when processing the ones
>> referenced by URIs. Patch 1-3 fixes that bug by passing the arguments to
>> use as a space-separated URL-encoded list. (URL-encoded so that we can
>> have spaces in the arguments.) Again, if anyone has a better idea, I'm
>> interested. It is only in patch 4 that we have the dangling .gitmodules
>> fix.
>
> This seems to have been stalled but I think it would be a better
> approach to use a custom callback for error reporting, suggested by
> Ævar, which would be where his fsck API clean-up topic would lead
> to.
>
> If it is not ultra-urgent, perhaps you can retract the ones that are
> queued right now, work with Ævar to finish the error-callback work
> and rebuild this topic on top of it?  Thanks.

If my vote counts for something I think it makes sense to have
Jonathan's series go first and just ignore my fsck API improvement
patches (well, the part of my v1[1] which conflicts with his work).

I'm also happy to help him queue his on top of a v1 version of my
series.

But the end result of doing so (shown after the "--" in [1]) is just a
small re-arrangement of code to get a cleaner fsck API use, it doesn't
actually matter to anyone using git.

Whereas his patches actually do, we have in-the-wild server/repo/clone
setups that are getting on-clone errors, and the window for 2.31 is
getting closer.

We can always do the small API use refactoring later. My interest in
barking up that tree was just that I've been poking at that part of the
fsck API and have some follow-up work that hasn't made it onto the list
yet that makes other use of the fsck API.

So in the longer term I wanted us to think about not needing N special
cases like "print_dangling_gitmodules" if we could help it, but in the
shorter term having it is a non-issue.

1. https://lore.kernel.org/git/[email-masked]/