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

> On Fri, Aug 21, 2026 at 6:17 AM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Fri, Aug 21, 2026 at 06:55:51AM +0000, Elijah Newren via GitGitGadget wrote:
> > > From: Elijah Newren <newren@gmail.com>
> > >
> > > When pushing from a shallow clone, even if we only have made a small
> > > one-line change to a tiny file, we often push the entire toplevel tree
> > > of files.  For large repositories, this could be gigabytes instead of
> > > kilobytes.
> >
> > Oh yeah, that issue. It's a common foot gun indeed, and the common
> > advice here is to never clone with "--depth=1", but always with
> > "--depth=2" so that there is at least one non-grafted commit available
> > on the client so that they can indeed perform proper negotiation with a
> > server. But over the years I had to explain this again and again, so it
> > is clear that this common knowledge might only be commonly known to
> > people who have spent way too much time in the Git codebase.
> 
> I don't think --depth=2 actually helps here.  What enables real
> negotiation is push.negotiate, not the extra commit, and
> push.negotiate works just as well at --depth=1.
> 
> Without push.negotiate, send-pack's only negatives come from the refs
> the server advertised filtered by what we actually have.  In the
> foot-gun scenario -- clone shallow, server advances, then push, using
> depth of 2 just walks one commit further to the graft and then
> re-sends the whole tree anyway.  Running the four combinations (server
> advanced after clone, optimization disabled) in a small test repo:
> 
>     depth=1, push.negotiate=false:  Enumerating objects: 205
>     depth=2, push.negotiate=false:  Enumerating objects: 208
>     depth=1, push.negotiate=true:   Enumerating objects: 4
>     depth=2, push.negotiate=true:   Enumerating objects: 4
> 
> --depth=2 without negotiation is if anything a hair worse, while
> negotiation fixes it regardless of depth (the negotiator offers the
> shallow graft commit itself as a "have", and the server ACKs it).
> 
> --depth=2 can in rare cases help, but only in the lucky/accidental
> case where some advertised ref happens to point at the extra commit
> you now have.

> > It's a good question to ask. In theory though, can't it happen that the
> > client changes the commit in question locally, e.g. via `git commit
> > --amend`, and then pushes? If we now assume that the local commit exists
> > on the remote side then we'd be insufficient information to the server.
> 
> Oh, wow, I had never thought to amend a shallow graft.  As soon as you
> asked, I assumed it'd create a corrupt repo -- a commit that wasn't
> itself a shallow graft but had parents we didn't know about.  I got
> surprised in a different way, though: commit --amend treats a shallow
> graft as a parent-less commit, and thus creates a new root commit.
> That does avoid corruption, but only by providing a different kind of
> foot-gun.  (If users really wanted a new root commit, `git
> {switch,checkout} --orphan` is the tool to do that.)
> 
> 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.

> > [snip]
> > >     Users can work around the problem described in this patch with
> > >     push.negotiate=true, but while we can educate some users to set that,
> > >     trying to get them all to do so is quite unlikely. Let's help users by
> > >     providing sane default behavior.
> >
> > Makes me wonder whether the default is something that we should adjust
> > so that this defaults to enabled. Are there any downsides to doing so?
> 
> The only one I can think of is that it adds a round-trip to every
> push, which increases latency in order to sometimes reduce bandwidth
> and cpu.
> 
> It can dramatically reduce bandwidth and cpu, but not always (single
> person projects would probably never see a benefit, for example, nor
> would anyone interacting with a fetch v0 server), and it always
> increases latency.