Re: [PATCH] send-pack: avoid sending the whole tree when pushing from a shallow clone

From: Elijah Newren <newren@gmail.com>
Date: Aug 25, 2026, 05:00 UTC
Message-ID: <CABPp-BHwa7QM=XDuO=9xqm-OL8dn8uGf1=rv+sgBRQ9hHKMFuQ@mail.gmail.com>
In-Reply-To: <aovW5bxu1F8jYKYl@pks.im>

On Sun, Aug 23, 2026 at 10:30 PM Patrick Steinhardt <ps@pks.im> wrote:

[...]

Show 7 quoted lines

> TIL, thanks. I don't think I was even aware of "push.negotiate", and I
> mostly went by the folklore of "just clone with --depth=2" that I saw
> repeated on many sites.
>
> But this and all of your other answers make me lean strongly into the
> direction that the fix is at the wrong level, and the proper fix really
> is to enable "push.negotiate" by default.

I don't think that fixes the problem, though:

  a) Users can do a shallow clone of a specific branch for a specific
pull-request/merge-request.  Then the pull-request/merge-request is
rebased, and sensitive data removed due to a leaked secret.  The
shallow graft is no longer common.  Pushing from the shallow clone
should fail, but it shouldn't have to send several gigabytes of data
in order to get the failure message.
  b) (Very similar to a) Users can do a shallow clone of one repo (a
local repository cache?) and then push to another; the shallow graft
thus may not be common.  An error is expected, but sending gigabytes
of data to get the error isn't.
  c) Users set push.negotiate=false explicitly.  In my opinion, they
shouldn't get this bug just for opting out of that kind-of-related
feature.
  d) push.negotiate=true silently fails for some setups

I think case (d) is particularly interesting: For push.negotiate to
work, fetch v2 must be working.  For http, that's not a big deal.
When using ssh, it requires the client to send GIT_PROTOCOL=version=2
environment variable and for the server to accept it:
  * Server side:
    * Some self-hosting forges may not automatically support receiving
the environment variable.  My searches suggest BitBucket always uses
v0 for ssh, and GitLab depends on the installation method -- either
the Linux package or self-compile installs requiring manual action
(only Helm and the all-in-one Docker image are preconfigured)
    * Some corporate setups may specifically want to disallow sending
any environment variables over ssh (perhaps through an "upstream stock
configuration only" policy?)
  * Client side:
    * git only requests v2 when it decides the client is OpenSSH (I
think that maps to the command being named ssh/ssh.exe, or an
auto-probe succeeds)
    * plink / putty / tortoiseplink appear to not allow sending this
environment variable, so many Windows users may be cut out
    * Some corporate setups might restrict sending environment
variables over ssh on the client side as well

When it's not supported, it falls back to v0 with a simple warning, does no negotiation, and runs into the old bug.

So, while I support the idea of moving towards push.negotiate=true or even adding push.negotiate=shallow, because they would provide other benefits, I don't think they fix the problem at hand and thus believe that this patch is still important.

[...]

Show 5 quoted lines

> > Since we've got another place where commit --amend can serve as a
> > foot-gun that I've long meant to fix up, I'll submit a separate series
> > that'll make it throw errors for both cases.
>
> That makes sense.

Turns out there's a bunch of additional stuff on the shallow side, so I think I'm going to split it into two series; a single patch for rebase/revert/am, and five or so patches for shallow graft handling across a variety of commands.

Show 10 quoted lines

> That's all fair, but it does dramatically help in the case of shallow
> clones. And the number of times I've seen this question come up hints
> that this is a very common scenario.
>
> We could be clever about it: if "push.negotiate" is very likely to help
> in shallow clones but mostly just adds latency in full clones, then why
> don't we introduce a new "push.negotiate=shallow" option that enables
> this feature automatically for shallow clones and make it the default?
> That to me sounds like a low-hanging fruit, and I would prefer such a
> fix compared to introducing new logic.

I kind of like the idea of somehow making push.negotiate default on for big repos in general (not just shallow), but while push.negotiate has lots of other benefits and has the side effect of solving most cases of this problem for some users, it falls short of actually fully solving the problem. This patch, or something like it, is still needed.