All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Coup <robert.coup@koordinates.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: partial clone: promisor fetch during push (pack-objects)
Date: Mon, 1 Mar 2021 11:57:43 +0000	[thread overview]
Message-ID: <CAFLLRpLoy6c_WSt1RR2-SPMzKK-MaBpkrCPuGqGGf8SwpCiSEA@mail.gmail.com> (raw)
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 :)

>
> > 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.

>
> > 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 :)

  reply	other threads:[~2021-03-01 11:58 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-26 12:01 partial clone: promisor fetch during push (pack-objects) Robert Coup
2021-02-26 20:54 ` Jonathan Tan
2021-03-01 11:57   ` Robert Coup [this message]
2021-03-01 20:55     ` Jonathan Tan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFLLRpLoy6c_WSt1RR2-SPMzKK-MaBpkrCPuGqGGf8SwpCiSEA@mail.gmail.com \
    --to=robert.coup@koordinates.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.