git@cosmocr.at/gitweb.git · summary refs
githttp: acknowledge the common base, and drop haves we do not hold
commit 7f24700a165b05f8253af2e57029e370fc9f00b3
Patrick Gallagher <rpg137@gmail.com> · 2026-09-20 21:42 -0400
parent 3bed207d
Two follow-ups to the negotiation fix.

Filter the haves. The session walks req.Haves to build its exclusion set
and returns ErrObjectNotFound for anything it cannot resolve, so a client
holding commits it has never pushed -- ordinary local work -- turned an
incremental fetch into a 500. Only the haves we actually hold are passed
on. A deep client happened to escape this because its unknown haves arrive
in a NAK round that never reaches the session; a client with fewer than
git's first batch of 16 haves sends them alongside "done" and hit it
directly.

ACK the common base on the "done" round. Without it git's find_common
returns "no common commits", warns on every incremental fetch, and treats
the histories as unrelated.

The ACK goes on that round and no earlier, which is worth spelling out
because the reverse looks like the obvious optimisation. We advertise no
multi_ack, and in that mode a plain ACK does not mean "noted": it means
negotiation is over and the packfile follows. git acts on it at once,
sending a bare "done" request carrying no haves at all and reading that
response as a raw packfile -- so ACKing mid-negotiation discards every
have already offered and breaks the fetch with "protocol error: bad pack
header". Both halves are pinned by tests.
internal/githttp/githttp.go
// Package githttp implements the git "smart HTTP" wire protocol on top of
// go-git's server transport sessions. go-git ships the upload-pack/receive-pack
// session logic and the pkt-line/packp encoders, but not a net/http handler, so
// this package supplies the framing that stock `git` expects:
//
// GET <repo>/info/refs?service=git-upload-pack -> ref advertisement
// GET <repo>/info/refs?service=git-receive-pack -> ref advertisement
// POST <repo>/git-upload-pack -> pack negotiation (clone/fetch)
// POST <repo>/git-receive-pack -> ref updates (push)
//
// Only protocol v0 is implemented. A client that sends "Git-Protocol: version=2"
// gets no v2 response and transparently falls back to v0. go-git's server does
// not support shallow (--depth) clones.
package githttp
import (
"bytes"
"compress/gzip"
"context"
"fmt"
"io"
"net/http"
"github.com/go-git/go-git/v5/plumbing"
"github.com/go-git/go-git/v5/plumbing/format/pktline"
"github.com/go-git/go-git/v5/plumbing/protocol/packp"
"github.com/go-git/go-git/v5/plumbing/storer"
"github.com/go-git/go-git/v5/plumbing/transport"
"github.com/go-git/go-git/v5/plumbing/transport/server"
)
// maxBody bounds a POST body (the packfile for push, or the negotiation for
// fetch) to guard against a malicious or runaway client.
const maxBody = 256 << 20 // 256 MiB
// Service identifies a git smart-HTTP service.
type Service string
const (
UploadPack Service = "git-upload-pack" // clone / fetch
ReceivePack Service = "git-receive-pack" // push
)
// fixedLoader is a go-git transport loader that always serves one storer,
// ignoring the requested endpoint. gitweb resolves the repo before dispatch, so
// each request already has its storer.
type fixedLoader struct{ s storer.Storer }
func (l fixedLoader) Load(*transport.Endpoint) (storer.Storer, error) { return l.s, nil }
func newUploadSession(s storer.Storer) (transport.UploadPackSession, error) {
return server.NewServer(fixedLoader{s}).NewUploadPackSession(&transport.Endpoint{}, nil)
}
// NewReceivePackSession returns a receive-pack session bound to s. It is
// exported so the push write-back path (which must run the session against a
// private, writable storer) can reuse the same construction.
func NewReceivePackSession(s storer.Storer) (transport.ReceivePackSession, error) {
return server.NewServer(fixedLoader{s}).NewReceivePackSession(&transport.Endpoint{}, nil)
}
// InfoRefs writes the ref advertisement for svc over s, in smart-HTTP form:
// the "# service=<svc>" banner and a flush, followed by the pkt-line ref list
// (with capabilities). The caller must not have written a body yet.
func InfoRefs(w http.ResponseWriter, s storer.Storer, svc Service) error {
var ar *packp.AdvRefs
var err error
switch svc {
case UploadPack:
sess, e := newUploadSession(s)
if e != nil {
return e
}
defer sess.Close()
ar, err = sess.AdvertisedReferencesContext(context.Background())
case ReceivePack:
sess, e := NewReceivePackSession(s)
if e != nil {
return e
}
defer sess.Close()
ar, err = sess.AdvertisedReferencesContext(context.Background())
default:
return fmt.Errorf("githttp: unknown service %q", svc)
}
if err != nil {
return err
}
w.Header().Set("Content-Type", fmt.Sprintf("application/x-%s-advertisement", svc))
w.Header().Set("Cache-Control", "no-cache")
enc := pktline.NewEncoder(w)
if err := enc.Encodef("# service=%s\n", svc); err != nil {
return err
}
if err := enc.Flush(); err != nil {
return err
}
return ar.Encode(w)
}
// negotiation is the tail of an upload-pack request: the "have" lines the
// client is offering, and whether it closed the request with "done".
type negotiation struct {
haves []plumbing.Hash
done bool
}
// decodeNegotiation reads the have/done section that follows the upload
// request. UploadRequest.Decode stops at the flush-pkt ending the want section,
// so this part of the body is still unread; without it req.Haves stays empty
// and every fetch re-sends the full history.
func decodeNegotiation(r io.Reader) (negotiation, error) {
var n negotiation
sc := pktline.NewScanner(r)
for sc.Scan() {
line := bytes.TrimSuffix(sc.Bytes(), []byte("\n"))
switch {
case len(line) == 0:
// flush-pkt: end of this batch of haves.
case bytes.Equal(line, []byte("done")):
n.done = true
return n, nil
case bytes.HasPrefix(line, []byte("have ")):
text := string(bytes.TrimPrefix(line, []byte("have ")))
h := plumbing.NewHash(text)
if h.IsZero() {
return n, fmt.Errorf("malformed have line %q", line)
}
n.haves = append(n.haves, h)
}
}
return n, sc.Err()
}
// ServeUploadPack handles a POST git-upload-pack: it decodes the client's
// negotiation from the (possibly gzip-compressed) body, runs upload-pack over
// s, and streams the packfile response.
//
// Over stateless HTTP the client negotiates across several requests. It offers
// its haves in batches — the first holds 16 — and ends a batch that is not its
// last with a flush-pkt and no "done". In that round it is reading ACK/NAK
// pkt-lines to decide what to offer next, so a packfile must not be sent: the
// client would read the literal "PACK" where a pkt-line length belongs and fail
// with "protocol error: bad line length character: PACK". We answer such a
-// round with a bare NAK. Since we never ACK, the client keeps offering haves
-// until it runs out, then sends "done" and gets the pack.
+// round with a bare NAK; the commit we share is acknowledged only once the
+// client sends "done", which is also when the pack goes out.
func ServeUploadPack(w http.ResponseWriter, r *http.Request, s storer.Storer) error {
body, err := requestBody(r)
if err != nil {
return err
}
defer body.Close()
req := packp.NewUploadPackRequest()
if err := req.Decode(body); err != nil {
return err
}
neg, err := decodeNegotiation(body)
if err != nil {
return err
}
- // The session excludes everything reachable from the haves, so passing them
- // on is what keeps a fetch from re-sending history the client already has.
- req.Haves = neg.haves
+ // Only the haves we actually hold may be passed on. The session walks them
+ // to build the exclusion set and fails on any object it cannot find, and a
+ // client with unpushed local commits legitimately offers some we lack.
+ // Excluding everything reachable from the rest is what keeps a fetch from
+ // re-sending history the client already has.
+ common := commonHaves(s, neg.haves)
+ req.Haves = common
if !neg.done {
+ // The client is still negotiating, so no packfile yet -- but the answer
+ // must be NAK even when we do share a commit. We advertise no multi_ack,
+ // and in that mode a plain ACK does not mean "noted": it means
+ // negotiation is over and the pack follows. git acts on it immediately,
+ // sending a bare "done" request carrying no haves and reading that
+ // response as a raw packfile, so an ACK here costs both the negotiated
+ // haves and the fetch, which fails with "protocol error: bad pack
+ // header". The common base is acknowledged on the "done" round instead.
setUploadPackHeaders(w)
return pktline.NewEncoder(w).Encode([]byte("NAK\n"))
}
sess, err := newUploadSession(s)
if err != nil {
return err
}
defer sess.Close()
resp, err := sess.UploadPack(r.Context(), req)
if err != nil {
return err
}
+ // The client has finished negotiating, so this ACK names the base the pack
+ // was cut against rather than cutting negotiation short. Without it
+ // find_common returns "no common commits" and git warns on every fetch.
+ if len(common) > 0 {
+ resp.ACKs = []plumbing.Hash{common[0]}
+ }
setUploadPackHeaders(w)
return resp.Encode(w)
+}
+
+// commonHaves returns the haves present in s, in the order the client offered
+// them — git sends its newest commits first, so the first entry is the most
+// recent commit the two sides share.
+func commonHaves(s storer.Storer, haves []plumbing.Hash) []plumbing.Hash {
+ var common []plumbing.Hash
+ for _, h := range haves {
+ if _, err := s.EncodedObject(plumbing.AnyObject, h); err == nil {
+ common = append(common, h)
+ }
+ }
+ return common
}
func setUploadPackHeaders(w http.ResponseWriter) {
w.Header().Set("Content-Type", fmt.Sprintf("application/x-%s-result", UploadPack))
w.Header().Set("Cache-Control", "no-cache")
}
// DecodeReceivePack parses a POST git-receive-pack body into a reference-update
// request. The returned request's Packfile is a streaming reader over the rest
// of the body; the caller runs the receive-pack session (see the push package)
// and then calls WriteReceivePackResult.
func DecodeReceivePack(r *http.Request) (*packp.ReferenceUpdateRequest, error) {
body, err := requestBody(r)
if err != nil {
return nil, err
}
// Note: body is not closed here; req.Packfile streams from it and is closed
// by the receive-pack session.
req := packp.NewReferenceUpdateRequest()
if err := req.Decode(body); err != nil {
body.Close()
return nil, err
}
// A pack follows the commands only when at least one ref is created or
// updated; a delete-only push sends none (same rule as stock
// receive-pack). Decode leaves the body reader in req.Packfile regardless,
// and go-git's session unpacks whenever Packfile is non-nil, failing on
// the packless stream ("empty packfile") — so nil it out. Detection must
// come from the commands, not from probing the body: a read would block,
// because the client transmits nothing further until it sees our response.
needPack := false
for _, cmd := range req.Commands {
if cmd.Action() != packp.Delete {
needPack = true
break
}
}
if !needPack {
body.Close()
req.Packfile = nil
}
return req, nil
}
// WriteReceivePackResult writes the report-status for a completed push. A nil
// status (client did not request report-status) writes nothing.
func WriteReceivePackResult(w http.ResponseWriter, rs *packp.ReportStatus) error {
w.Header().Set("Content-Type", fmt.Sprintf("application/x-%s-result", ReceivePack))
w.Header().Set("Cache-Control", "no-cache")
if rs == nil {
return nil
}
return rs.Encode(w)
}
// requestBody returns the request body, transparently decompressing it when the
// client sent Content-Encoding: gzip (stock git gzips larger request bodies),
// and capping the amount read.
func requestBody(r *http.Request) (io.ReadCloser, error) {
var body io.ReadCloser = http.MaxBytesReader(nil, r.Body, maxBody)
if r.Header.Get("Content-Encoding") == "gzip" {
zr, err := gzip.NewReader(body)
if err != nil {
body.Close()
return nil, err
}
return gzipBody{zr: zr, under: body}, nil
}
return body, nil
}
// gzipBody couples a gzip reader with the underlying capped body so both are
// closed together.
type gzipBody struct {
zr *gzip.Reader
under io.ReadCloser
}
func (g gzipBody) Read(p []byte) (int, error) { return g.zr.Read(p) }
func (g gzipBody) Close() error {
err := g.zr.Close()
if cerr := g.under.Close(); err == nil {
err = cerr
}
return err
}
internal/githttp/uploadpack_negotiation_test.go
package githttp
import (
"bytes"
"encoding/binary"
"fmt"
"net/http/httptest"
+ "strings"
"testing"
"time"
"github.com/go-git/go-billy/v5/memfs"
"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/plumbing"
"github.com/go-git/go-git/v5/plumbing/object"
"github.com/go-git/go-git/v5/storage/memory"
)
// pktLine encodes s as a pkt-line.
func pktLine(s string) string { return fmt.Sprintf("%04x%s", len(s)+4, s) }
// twoCommitRepo builds an in-memory repo with two commits and returns its
// storer plus the two hashes, oldest first.
func twoCommitRepo(t *testing.T) (*memory.Storage, plumbing.Hash, plumbing.Hash) {
t.Helper()
st := memory.NewStorage()
repo, err := git.Init(st, memfs.New())
if err != nil {
t.Fatal(err)
}
wt, err := repo.Worktree()
if err != nil {
t.Fatal(err)
}
var hashes []plumbing.Hash
for i := range 2 {
f, err := wt.Filesystem.Create("f.txt")
if err != nil {
t.Fatal(err)
}
fmt.Fprintf(f, "v%d\n", i)
f.Close()
if _, err := wt.Add("f.txt"); err != nil {
t.Fatal(err)
}
h, err := wt.Commit(fmt.Sprintf("commit %d", i), &git.CommitOptions{
Author: &object.Signature{Name: "T", Email: "t@e.com", When: time.Unix(int64(1700000000+i), 0)},
})
if err != nil {
t.Fatal(err)
}
hashes = append(hashes, h)
}
return st, hashes[0], hashes[1]
}
// TestServeUploadPackNegotiationRoundSendsNoPackfile pins the pkt-line contract
// for a negotiation round that the client has not finished.
//
// Over stateless HTTP, git offers its haves in batches: the first batch ends
// with a flush-pkt and no "done", and the client then reads only ACK/NAK
// pkt-lines while it decides what to offer next. Emitting the packfile in that
// round desynchronises the client, which reads the literal bytes "PACK" where a
// 4-hex pkt-line length belongs and aborts with
//
// fatal: protocol error: bad line length character: PACK
//
// The packfile may only follow once the client has sent "done".
func TestServeUploadPackNegotiationRoundSendsNoPackfile(t *testing.T) {
st, first, tip := twoCommitRepo(t)
// want <tip>, flush, have <first>, flush -- deliberately no "done".
body := pktLine("want "+tip.String()+" ofs-delta agent=git/test\n") +
"0000" +
pktLine("have "+first.String()+"\n") +
"0000"
r := httptest.NewRequest("POST", "/x/git-upload-pack", bytes.NewReader([]byte(body)))
rec := httptest.NewRecorder()
if err := ServeUploadPack(rec, r, st); err != nil {
t.Fatalf("ServeUploadPack: %v", err)
}
got := rec.Body.Bytes()
if i := bytes.Index(got, []byte("PACK")); i >= 0 {
t.Errorf("response to an unfinished negotiation round contains a packfile at byte %d; "+
"only ACK/NAK pkt-lines are allowed until the client sends \"done\"\nresponse starts: %q",
i, got[:min(len(got), 32)])
}
}
// TestServeUploadPackDoneSendsPackfile is the companion: once the client says
// "done", the packfile is expected in the same response.
func TestServeUploadPackDoneSendsPackfile(t *testing.T) {
st, _, tip := twoCommitRepo(t)
body := pktLine("want "+tip.String()+" ofs-delta agent=git/test\n") +
"0000" +
pktLine("done\n")
r := httptest.NewRequest("POST", "/x/git-upload-pack", bytes.NewReader([]byte(body)))
rec := httptest.NewRecorder()
if err := ServeUploadPack(rec, r, st); err != nil {
t.Fatalf("ServeUploadPack: %v", err)
}
if !bytes.Contains(rec.Body.Bytes(), []byte("PACK")) {
t.Errorf("response to a finished negotiation has no packfile: %q",
rec.Body.Bytes()[:min(rec.Body.Len(), 32)])
}
}
// packObjectCount returns the object count from the header of the packfile
// embedded in an upload-pack response: "PACK", a 4-byte version, then a 4-byte
// big-endian count.
func packObjectCount(t *testing.T, resp []byte) int {
t.Helper()
i := bytes.Index(resp, []byte("PACK"))
if i < 0 {
t.Fatalf("response has no packfile: %q", resp[:min(len(resp), 32)])
}
if len(resp) < i+12 {
t.Fatalf("truncated packfile header")
}
return int(binary.BigEndian.Uint32(resp[i+8 : i+12]))
}
// TestServeUploadPackHonoursHaves checks that a "have" the server also holds
// trims the packfile. The haves arrive after the flush-pkt that ends the want
// section, which UploadRequest.Decode does not read past; left unparsed they
// never reach the session and every fetch re-sends the whole history.
func TestServeUploadPackHonoursHaves(t *testing.T) {
st, first, tip := twoCommitRepo(t)
post := func(body string) []byte {
t.Helper()
r := httptest.NewRequest("POST", "/x/git-upload-pack", bytes.NewReader([]byte(body)))
rec := httptest.NewRecorder()
if err := ServeUploadPack(rec, r, st); err != nil {
t.Fatalf("ServeUploadPack: %v", err)
}
return rec.Body.Bytes()
}
want := pktLine("want "+tip.String()+" ofs-delta agent=git/test\n") + "0000"
// A client with nothing gets the full history: 2 commits, 2 trees, 2 blobs.
full := packObjectCount(t, post(want+pktLine("done\n")))
if full != 6 {
t.Fatalf("full clone pack = %d objects, want 6", full)
}
// A client that already has the first commit gets only the second's objects.
delta := packObjectCount(t, post(want+pktLine("have "+first.String()+"\n")+pktLine("done\n")))
if delta >= full {
t.Errorf("pack with have %s = %d objects, full = %d: the have was ignored",
first.String()[:8], delta, full)
}
if delta != 3 {
t.Errorf("pack with have = %d objects, want 3 (one commit, tree, blob)", delta)
}
}
+
+// TestServeUploadPackNegotiationRoundNeverAcks pins a subtlety that looks like
+// a missed optimisation. We share the commit the client offers here, but the
+// round must still answer NAK: we advertise no multi_ack, and in that mode a
+// plain ACK means "negotiation over, pack follows". git acts on it at once,
+// sending a bare "done" request with no haves and reading that response as a
+// raw packfile — so ACKing here throws away every negotiated have and breaks
+// the fetch with "protocol error: bad pack header". The ACK belongs on the
+// "done" round, which TestServeUploadPackAcksCommonHaveOnDone covers.
+func TestServeUploadPackNegotiationRoundNeverAcks(t *testing.T) {
+ st, first, tip := twoCommitRepo(t)
+
+ body := pktLine("want "+tip.String()+" ofs-delta agent=git/test\n") +
+ "0000" +
+ pktLine("have "+first.String()+"\n") +
+ "0000" // no "done"
+
+ r := httptest.NewRequest("POST", "/x/git-upload-pack", bytes.NewReader([]byte(body)))
+ rec := httptest.NewRecorder()
+ if err := ServeUploadPack(rec, r, st); err != nil {
+ t.Fatalf("ServeUploadPack: %v", err)
+ }
+ if got, want := rec.Body.String(), pktLine("NAK\n"); got != want {
+ t.Errorf("response = %q, want %q", got, want)
+ }
+}
+
+// TestServeUploadPackAcksCommonHaveOnDone checks the final ACK. git's
+// find_common reports "no common commits" — and warns — unless the response to
+// the "done" round acknowledges a shared commit.
+func TestServeUploadPackAcksCommonHaveOnDone(t *testing.T) {
+ st, first, tip := twoCommitRepo(t)
+
+ body := pktLine("want "+tip.String()+" ofs-delta agent=git/test\n") +
+ "0000" +
+ pktLine("have "+first.String()+"\n") +
+ pktLine("done\n")
+
+ r := httptest.NewRequest("POST", "/x/git-upload-pack", bytes.NewReader([]byte(body)))
+ rec := httptest.NewRecorder()
+ if err := ServeUploadPack(rec, r, st); err != nil {
+ t.Fatalf("ServeUploadPack: %v", err)
+ }
+ if got, want := rec.Body.String(), pktLine("ACK "+first.String()+"\n"); !strings.HasPrefix(got, want) {
+ t.Errorf("response = %q..., want ACK prefix %q", got[:min(len(got), 60)], want)
+ }
+}
+
+// TestServeUploadPackUnknownHaveIsIgnored guards the have-filtering. A client
+// with local commits it has never pushed offers them as haves; the session
+// walks req.Haves to build its exclusion set and returns ErrObjectNotFound for
+// any it cannot resolve, which would turn an ordinary fetch into a 500.
+func TestServeUploadPackUnknownHaveIsIgnored(t *testing.T) {
+ st, first, tip := twoCommitRepo(t)
+ unknown := plumbing.NewHash("1111111111111111111111111111111111111111")
+
+ body := pktLine("want "+tip.String()+" ofs-delta agent=git/test\n") +
+ "0000" +
+ pktLine("have "+unknown.String()+"\n") +
+ pktLine("have "+first.String()+"\n") +
+ pktLine("done\n")
+
+ r := httptest.NewRequest("POST", "/x/git-upload-pack", bytes.NewReader([]byte(body)))
+ rec := httptest.NewRecorder()
+ if err := ServeUploadPack(rec, r, st); err != nil {
+ t.Fatalf("ServeUploadPack with an unknown have: %v", err)
+ }
+ // The known have must still trim the pack, and be acknowledged.
+ if n := packObjectCount(t, rec.Body.Bytes()); n != 3 {
+ t.Errorf("pack = %d objects, want 3: the known have was dropped along with the unknown one", n)
+ }
+ if got := rec.Body.String(); !strings.HasPrefix(got, pktLine("ACK "+first.String()+"\n")) {
+ t.Errorf("response does not start with ACK %s: %q", first, got[:min(len(got), 60)])
+ }
+}
internal/server/fetch_test.go
package server
import (
"fmt"
"net/http"
"net/http/httptest"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
"gitweb/internal/store"
)
// TestIncrementalFetch covers a fetch by a client that already holds most of
// the history. Clone never exercises negotiation: it sends "want" lines and
// "done" in one request, with no "have" lines at all. A fetch does, and git
// splits negotiation across requests — it offers its first batch of haves
// (INITIAL_FLUSH, 16) terminated by a flush-pkt and *no* "done", then waits for
// the server's ACK/NAK before deciding what to ask for next. A server that
// answers that round with a packfile breaks the client, which is still reading
// pkt-lines: it reports
//
// fatal: protocol error: bad line length character: PACK
//
// The history therefore has to be deep enough that git does not exhaust its
// haves (and so append "done") in the first round.
func TestIncrementalFetch(t *testing.T) {
if _, err := exec.LookPath("git"); err != nil {
t.Skip("git not available")
}
env := gitEnv(t)
ns := t.TempDir()
const owner = "u@d.com"
const repoPath = owner + "/deep.git"
bare := filepath.Join(ns, filepath.FromSlash(repoPath))
// A history comfortably deeper than git's 16-have first batch.
const depth = 24
work := t.TempDir()
mustGit(t, env, work, "init", "-q", "-b", "main")
for i := range depth {
writeTestFile(t, work, "f.txt", fmt.Sprintf("v%d\n", i))
mustGit(t, env, work, "add", "-A")
mustGit(t, env, work, "commit", "-qm", fmt.Sprintf("commit %d", i))
}
if err := os.MkdirAll(filepath.Dir(bare), 0o755); err != nil {
t.Fatal(err)
}
mustGit(t, env, work, "clone", "-q", "--bare", work, bare)
if err := os.WriteFile(filepath.Join(bare, "Access"), []byte("read,list: all\n*: "+owner+"\n"), 0o644); err != nil {
t.Fatal(err)
}
srv, err := New(Config{
Store: store.NewLocalStore(ns),
Web: http.FS(os.DirFS("../../web")),
CacheDir: t.TempDir(),
})
if err != nil {
t.Fatal(err)
}
ts := httptest.NewServer(srv.Handler())
defer ts.Close()
// The client clones the full history, so it has every commit to offer.
dst := filepath.Join(t.TempDir(), "client")
mustGit(t, env, ".", "clone", "-q", cloneURL(ts.URL, repoPath, "", ""), dst)
// One more commit lands in the bare repo, out of band.
writeTestFile(t, work, "f.txt", "tip\n")
mustGit(t, env, work, "add", "-A")
mustGit(t, env, work, "commit", "-qm", "tip")
mustGit(t, env, work, "push", "-q", bare, "main")
wantTip := strings.TrimSpace(mustGit(t, env, work, "rev-parse", "main"))
out, err := runGit(t, env, dst, "fetch", "origin")
if err != nil {
t.Fatalf("incremental fetch failed: %v\n%s", err, out)
}
if got := strings.TrimSpace(mustGit(t, env, dst, "rev-parse", "origin/main")); got != wantTip {
t.Errorf("origin/main = %s, want %s", got, wantTip)
}
+ // Acknowledging the common base is what keeps this warning away; without an
+ // ACK git assumes the histories are unrelated and takes far more than the
+ // delta.
+ if strings.Contains(out, "no common commits") {
+ t.Errorf("fetch reported no common commits, so the shared base went unacknowledged:\n%s", out)
+ }
+
+ // A deep client reaches the session only after a NAK round, so the haves it
+ // cannot share are filtered out before that. See
+ // TestFetchWithUnpushedLocalCommits for the shallow case, where they arrive
+ // alongside "done" and reach the session directly.
+ t.Run("unpushed local commits", func(t *testing.T) {
+ writeTestFile(t, dst, "local.txt", "mine\n")
+ mustGit(t, env, dst, "add", "-A")
+ mustGit(t, env, dst, "commit", "-qm", "local only")
+
+ writeTestFile(t, work, "f.txt", "newer\n")
+ mustGit(t, env, work, "add", "-A")
+ mustGit(t, env, work, "commit", "-qm", "newer")
+ mustGit(t, env, work, "push", "-q", bare, "main")
+ newTip := strings.TrimSpace(mustGit(t, env, work, "rev-parse", "main"))
+
+ out, err := runGit(t, env, dst, "fetch", "origin")
+ if err != nil {
+ t.Fatalf("fetch with unpushed local commits failed: %v\n%s", err, out)
+ }
+ if got := strings.TrimSpace(mustGit(t, env, dst, "rev-parse", "origin/main")); got != newTip {
+ t.Errorf("origin/main = %s, want %s", got, newTip)
+ }
+ })
}
func writeTestFile(t *testing.T, dir, name, content string) {
t.Helper()
if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil {
t.Fatal(err)
}
}
+
+// TestFetchWithUnpushedLocalCommits covers a client holding commits the server
+// has never seen — the ordinary "some local work, now fetch" case.
+//
+// The history is deliberately shallow: with fewer than git's first batch of 16
+// haves to offer, the client puts its haves and "done" in a single request, so
+// they reach the upload-pack session instead of being answered by a NAK round.
+// The session walks req.Haves to build its exclusion set and returns
+// ErrObjectNotFound for anything it cannot resolve, which turns the fetch into
+// a 500 unless the unknown haves are dropped first.
+func TestFetchWithUnpushedLocalCommits(t *testing.T) {
+ if _, err := exec.LookPath("git"); err != nil {
+ t.Skip("git not available")
+ }
+ env := gitEnv(t)
+ ns := t.TempDir()
+
+ const owner = "u@d.com"
+ const repoPath = owner + "/shallow.git"
+ bare := filepath.Join(ns, filepath.FromSlash(repoPath))
+
+ work := t.TempDir()
+ mustGit(t, env, work, "init", "-q", "-b", "main")
+ for i := range 3 {
+ writeTestFile(t, work, "f.txt", fmt.Sprintf("v%d\n", i))
+ mustGit(t, env, work, "add", "-A")
+ mustGit(t, env, work, "commit", "-qm", fmt.Sprintf("commit %d", i))
+ }
+ if err := os.MkdirAll(filepath.Dir(bare), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ mustGit(t, env, work, "clone", "-q", "--bare", work, bare)
+ if err := os.WriteFile(filepath.Join(bare, "Access"), []byte("read,list: all\n*: "+owner+"\n"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+
+ srv, err := New(Config{
+ Store: store.NewLocalStore(ns),
+ Web: http.FS(os.DirFS("../../web")),
+ CacheDir: t.TempDir(),
+ })
+ if err != nil {
+ t.Fatal(err)
+ }
+ ts := httptest.NewServer(srv.Handler())
+ defer ts.Close()
+
+ dst := filepath.Join(t.TempDir(), "client")
+ mustGit(t, env, ".", "clone", "-q", cloneURL(ts.URL, repoPath, "", ""), dst)
+
+ // Local work the server will never have.
+ writeTestFile(t, dst, "local.txt", "mine\n")
+ mustGit(t, env, dst, "add", "-A")
+ mustGit(t, env, dst, "commit", "-qm", "local only")
+
+ // Something new to fetch.
+ writeTestFile(t, work, "f.txt", "newer\n")
+ mustGit(t, env, work, "add", "-A")
+ mustGit(t, env, work, "commit", "-qm", "newer")
+ mustGit(t, env, work, "push", "-q", bare, "main")
+ wantTip := strings.TrimSpace(mustGit(t, env, work, "rev-parse", "main"))
+
+ out, err := runGit(t, env, dst, "fetch", "origin")
+ if err != nil {
+ t.Fatalf("fetch with unpushed local commits failed: %v\n%s", err, out)
+ }
+ if got := strings.TrimSpace(mustGit(t, env, dst, "rev-parse", "origin/main")); got != wantTip {
+ t.Errorf("origin/main = %s, want %s", got, wantTip)
+ }
+}