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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 6, 2018, 21:29 UTC
Message-ID: <20180706212957.GD6195@aiede.svl.corp.google.com>
In-Reply-To: <xmqqk1q8jfak.fsf@gitster-ct.c.googlers.com>

Hi,

Junio C Hamano wrote:

>> Jonathan Tan <jonathantanmy@google.com> writes:

Show 6 quoted lines

>>> When cloning a repository with a tagged blob (like the Git repository)
>>> with --filter=blob:none, the following message appears:
>>>
>>>         error: missing object referenced by 'refs/tags/junio-gpg-pub'
>>>
>>> and the resulting repository also fails fsck.

[...]

Show 8 quoted lines

>                            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.
>
> Perhaps that line of consistent behaviour may be impossible due to
> the way the reference is stored inside a tree object and a ref?

Yes, exactly. I believe this is why we did not treat refs as promisors when we introduced promised objects.

We could revisit that decision and make refs into promisors, storing any extra metadata on the side. That would be a more invasive change than this series does, though. In the model used today, where refs are not promisors, this series is the logical thing to do.

Show 5 quoted lines

> E.g. a reference to an expected-to-be-missing blob still lets us
> know that the missing object is expected to be a blob, but a ref
> only stores the object name and not its type, so we can tell it is
> missing, but we cannot say it is OK to be missing because we expect
> it to be a blob, or something?

The main downside of saying that filters should not be applied to the objects named directly in a "want" is that such objects could be large.

The upsides are that

- it makes the protocol more consistent with the current invariants
  maintained by "git fsck"

- down the line, it should make operations like "fetch just this one
  tree" a little simpler, since you can use

   filter blob:none
   filter tree:none
   want <this tree>

Later we could introduce a separate kind of "want" for which filters apply to the object named directly, too, to provide more flexibility for the client. I can imagine cases where a person would want each of those two behaviors, so I don't think we are incurring unnecessary complexity by always sending the object named in a "want" without such an explicit filtering request. So I'm comfortable with the direction this series goes in, though I haven't looked at the patches in detail.

Thanks, Jonathan