Re: [PATCH 0/2] Avoiding errors when partial cloning a tagged blob

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jul 6, 2018, 21:25 UTC
Message-ID: <20180706212546.178115-1-jonathantanmy@google.com>
In-Reply-To: <xmqqk1q8jfak.fsf@gitster-ct.c.googlers.com>

Show 10 quoted lines

> Hmph, the approach taken by these two patches smells bad.
> 
> When a blob is deliberately omitted with --fitler=blob:none, the
> fsck that encounters an entry in a tree object that is about that
> expected-to-be-and-actually-is-missing blob does *not* complain and
> that is by design, right?  Naïvely, I may expect fsck to follow the
> same principle--when it encounters a reference to an object that is
> deliberately missing, refrain from complaining, regardless of the
> place the offending reference was found, be it inside a tree object
> or a ref.

The difference between the two is that this tree object is from a packfile with an associated ".promisor" file, indicating that the destinations of any "dangling" links are promisor objects (and thus, fsck and others will not complain about those). We don't have such a mechanism for refs.

This was a tradeoff between the desire to still detect repository corruption whenever we can, and to not impact performance greatly when operating on a partial repository.

Besides the solution in this patch set, we could (1) allow any ref to point to a missing object, (2) allow a subset of refs to point to a missing object, or (3) have another place in $GIT_DIR that can record the hashes of promisor objects. (1) seems contrary to the repository corruption detection goal that we have. (2) is plausible, but we would have to figure out what to record - it's fine to record the RHS of the default refspec when we clone, but I don't know what we should record when the user subsequently runs "git fetch origin refs/heads/master:refs/heads/master". As for (3), that would require another repository extension since we wouldn't want an old Git to fetch into a new Git repository without updating the promisor-hashes file - but at this point, it seems easier to update the protocol instead. Any comments are welcome, and I'll think about this some more.