Re: partial clone: promisor fetch during push (pack-objects)

From: Robert Coup <robert.coup@koordinates.com>
Date: Mar 1, 2021, 11:57 UTC
Message-ID: <CAFLLRpLoy6c_WSt1RR2-SPMzKK-MaBpkrCPuGqGGf8SwpCiSEA@mail.gmail.com>
In-Reply-To: <20210226205458.2909811-1-jonathantanmy@google.com>

Hi Jonathan,

On Fri, 26 Feb 2021 at 20:55, Jonathan Tan <jonathantanmy@google.com> wrote:

> So the cause is not that we are unnecessarily
> pushing the missing objects, but because they appear in the window when
> we do delta compression (just like you said at the start).

Thanks for persevering :)

Show 8 quoted lines

>
> > 2. Seems like a bug that check_object() doesn't honour
> > fetch_if_missing and skip the call to prefetch_to_pack().
>
> We do need to fetch if we're trying to pack a missing object. I think
> the real bug here is that we shouldn't prefetch if the to_pack entry has
> preferred_base set (which means that it will not go into the final
> packfile and is just available for delta compression).

That makes sense, I had a feeling it wouldn't be quite as simple as my suggestion.

Show 6 quoted lines

>
> > 3. But push doesn't pass --missing= to pack-objects anyway, so that
> > wouldn't actually solve the original issue. Should it?
>
> If we fix the bug I described above, I think it's still OK if push
> doesn't pass --missing=.

I'll have a go at a patch. Any suggestions on how to approach writing/expanding a test to cover this?

Thanks, Rob :)