git@cosmocr.at/gitweb.git · summary refs
push: fix thin packs, ref deletion, and error reporting
commit baba5e977b222d761f98a88a910c4a411d3a6231
Patrick Gallagher <rpg137@gmail.com> · 2026-07-04 23:06 -0400
parent 925041c5
Three go-git v5.12 server-side fixes for git push:

- Incremental pushes failed with "reference delta not found": go-git's
  filesystem.Storage advertises PackfileWriter, which makes
  UpdateObjectStorage copy the client's pack verbatim — a path that
  cannot resolve the REF_DELTA bases of the thin packs git always sends
  when histories overlap. Hide the interface (noPackfileWriter) so the
  pack is parsed against the storer, resolving thin deltas.

- Delete-only pushes failed with "empty packfile": no pack follows the
  commands, but packp leaves the body reader in req.Packfile and the
  session unpacks whenever it is non-nil. Nil it out when every command
  is a delete. Detection must come from the commands, not from probing
  the body — the client sends nothing further until it sees our
  response, so any body read (or Close, which drains) deadlocks.

- Ref deletion then hung forever: billy memfs answers every ModTime()
  with time.Now(), trapping go-git's packed-refs lock loop (which
  compares mtimes to detect concurrent changes) in an infinite spin.
  Wrap the materialized fs so Stat reports a constant ModTime
  (stableStatFS); the private copy has no concurrent writers.

Also stop reporting "ok" to the client when write-back to the store
fails after a successful unpack (failReport): the refs were never
persisted, so the client must see them rejected, not confirmed.

Regression tests: "incremental push (thin pack)" and "push branch
create and delete" in clone_push_test.go both fail against the old
code (the delete case by hanging).
DEPLOY.md
# Deploying the gitweb browser
How to build and deploy the gitweb service to the upspin VM (`instance-2`). It is
a single Go binary (page templates + static assets embedded via `go:embed`) that
runs as a docker-compose service alongside the existing upspin stack, exactly
like [vault](../vault/DEPLOY.md) and [markdown](../markdown/DEPLOY.md).
## Architecture at a glance
- **Image:** `gcr.io/cosmocr-at-upspin/gitweb:latest`
- **Host:** `instance-2` (GCP, `us-central1-a`, COS 121), static IP `34.28.234.94`
- **TLS host:** `git.cosmocr.at` — the container terminates its own TLS with a
certbot cert; nginx SNI-routes `git.cosmocr.at → gitweb:8446`.
(DNS is a `*.cosmocr.at` wildcard → `34.28.234.94`, so no per-host record is
needed.)
-- **Runs as upspin user** `git@cosmocr.at`, **read-only**. It reads any bare repo
- whose `Access` file grants it `read,list`; it never writes to upspin. The demo
- repo is `git@cosmocr.at/demo.git` (`Access: read,list: all`); third parties
- expose a repo by granting `read,list: git@cosmocr.at`.
+- **Runs as upspin user** `git@cosmocr.at`. It reads any bare repo whose
+ `Access` file grants it `read,list`; push write-back additionally needs
+ `create,write,delete` for `git@cosmocr.at` on the repo. The demo repo is
+ `git@cosmocr.at/demo.git`; third parties expose a repo by granting
+ `read,list: git@cosmocr.at`.
- **Config & keys** live on the persistent data disk (not in the image):
- `/mnt/disks/data/gitweb/config` — upspin config (`secrets: /gitweb/secrets`)
- `/mnt/disks/data/gitweb/secrets/{public,secret}.upspinkey`
- `/mnt/disks/data/letsencrypt/live/git.cosmocr.at/` — TLS cert
- **Authoritative infra sources** (in the `control` repo):
- `upspin/vm/cloud-init-cos121.yaml` — the VM's cloud-init (embeds the
docker-compose + nginx.conf used on boot). **Edit here first, and it is only
live once pushed to the instance metadata `user-data` key** (see "If you
changed config" — cloud-init re-applies the metadata on every boot and
**overwrites** the `/var/upspin` config files, so anything not in the
metadata is reverted on the next reboot).
- `upspin/vm/docker-compose.vault.yml` — rendered from the cloud-init for local
reference; the live `/var/upspin/docker-compose.yml` is written from the
metadata on boot (all of vault, markdown, and gitweb live in it).
- `upspin/nginx/nginx.conf` — canonical nginx config (the SNI map).
- `upspin/vm/deploy-git.sh` — first-time end-to-end deploy script.
- `upspin/secrets/git/` — the browser's upspin config + keys (git-ignored).
Note the workstation-side secrets are under `secrets/git/`, but the on-VM
service + data dir are named `gitweb` (matching the compose service).
## Prerequisites
- `docker` locally (for `--platform linux/amd64` builds) and `gcloud` configured
for project `cosmocr-at-upspin`. (The Dockerfile is COPY-only over a
cross-compiled `CGO_ENABLED=0 GOOS=linux GOARCH=amd64` binary, so the amd64
image builds on an arm workstation with no qemu.)
- SSH to the VM. The deploy script defaults to `~/.ssh/google_compute_engine`;
override with `DEPLOY_SSH_KEY` to use a passphrase-less key:
```
DEPLOY_SSH_KEY=$HOME/.ssh/nopass_claude_code ./deploy-git.sh
```
- The `git@cosmocr.at` upspin user must exist on the keyserver and be in the
keyserver's Writers. (One-time; already done — see
`upspin/doc/provision_user.md`.)
## Routine redeploy (code change)
This is the common case. Because the templates + assets are embedded, **any**
change needs a rebuilt image.
1. **Build + push** the image from the gitweb repo:
```
make push
```
(= `CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build`, `docker build
--platform linux/amd64`, tag, `docker push`.)
2. **Pull + recreate** the container on the VM:
```
# authenticate the Docker daemon to gcr.io
sudo env HOME=/var/upspin /bin/sh /var/upspin/auth.sh
# pull the new image
sudo docker run --rm \
-v /var/run/docker.sock:/var/run/docker.sock \
-v /var/upspin/.docker:/root/.docker \
-v /var/upspin:/var/upspin -w /var/upspin \
docker/compose:1.29.2 -p patrick pull gitweb
# recreate just the gitweb container (rm + create; see "compose recreate" note)
sudo docker rm -f patrick_gitweb_1
sudo docker run --rm \
-v /var/run/docker.sock:/var/run/docker.sock \
-v /var/upspin/.docker:/root/.docker \
-v /var/upspin:/var/upspin -w /var/upspin \
docker/compose:1.29.2 -p patrick up -d --no-recreate gitweb
```
(Re-running `deploy-git.sh` also works and is idempotent; the cert step is a
no-op once the cert exists — but see the certbot gotcha below.)
3. **Verify:**
```
sudo docker logs --tail 5 patrick_gitweb_1 # "gitweb: serving on :8446"
curl -sI https://git.cosmocr.at/git@cosmocr.at/demo.git/ # 200
```
### If you changed config, the compose service, or nginx
This covers any change to the **compose services** (incl. `restart:` policies),
the **nginx routing/SNI map**, or the upspin config — i.e. anything that lives in
`cloud-init-cos121.yaml`.
> **⚠️ The instance metadata is the source of truth, not the files on the VM.**
> On every boot, cloud-init re-applies the VM's `user-data` metadata and
> **overwrites** `/var/upspin/docker-compose.yml` and
> `/var/upspin/nginx/nginx.conf`. A change you only `sudo cp` onto the VM (or
> `nginx -s reload`) works **until the next reboot**, then silently reverts. This
> is exactly how the vault service once vanished on its first reboot: it was added
> to the running VM and to the repo, but never pushed to the metadata. **Always
> push the metadata.**
**1. Edit + push the metadata (authoritative — survives reboots):**
```
# edit upspin/vm/cloud-init-cos121.yaml in the control repo, then from that repo:
gcloud compute instances add-metadata instance-2 --zone us-central1-a \
--metadata-from-file user-data=upspin/vm/cloud-init-cos121.yaml
```
Mirror the same edit into `upspin/vm/docker-compose.vault.yml` and
`upspin/nginx/nginx.conf` so the local reference copies stay identical. The push
takes effect on the **next reboot**; verify it landed (run on the VM):
```
curl -s -H "Metadata-Flavor: Google" \
http://metadata.google.internal/computeMetadata/v1/instance/attributes/user-data \
| grep -c 'gitweb:' # expect a non-zero count
```
**2. Apply now without waiting for a reboot (optional, for a live change):**
copy the rendered files up and reload/recreate only the affected pieces:
```
# control/upspin/vm/docker-compose.vault.yml -> /var/upspin/docker-compose.yml
# control/upspin/nginx/nginx.conf -> /var/upspin/nginx/nginx.conf
sudo docker exec patrick_nginx_1 nginx -t && sudo docker exec patrick_nginx_1 nginx -s reload
# recreate any container whose service/config changed (rm + up; see note below)
```
These hot-apply the change immediately; step 1 is what makes it durable. **Do
both** — they are not alternatives.
## First-time setup (new VM / from scratch)
Use `control/upspin/vm/deploy-git.sh`, which does it all idempotently: stages the
config + keys onto the data disk, issues the TLS cert (certbot webroot via the
running nginx), updates nginx + compose, pulls the image, recreates nginx (for the
SNI route) and starts gitweb, then verifies. Manual outline:
1. **DNS:** covered by the `*.cosmocr.at` wildcard → `34.28.234.94` (no per-host
record needed).
2. **Config + keys** onto the data disk: `/mnt/disks/data/gitweb/config` +
`/mnt/disks/data/gitweb/secrets/*.upspinkey` (keys `0600`, root-owned).
3. **TLS cert** (nginx must be serving `:80` for the ACME challenge):
```
sudo docker run --rm \
-v /mnt/disks/data/certbot-www:/var/www/certbot:rw \
-v /mnt/disks/data/letsencrypt:/etc/letsencrypt:rw \
certbot/certbot certonly --webroot --webroot-path=/var/www/certbot \
--register-unsafely-without-email --agree-tos --no-eff-email \
-d git.cosmocr.at
```
4. **nginx route** and the **gitweb compose service** (already in
`cloud-init-cos121.yaml`): SNI route `git.cosmocr.at gitweb:8446;`.
5. **Recreate nginx**, then **start gitweb** (commands as in "Routine redeploy").
Before the first run, make sure the infra edits are in `cloud-init-cos121.yaml`
**and pushed to the metadata** (see "If you changed config"): the `gitweb:`
compose service and the `git.cosmocr.at gitweb:8446;` line in the nginx `map`.
## Gotchas / troubleshooting
- **Config on the VM reverts on reboot if it isn't in the instance metadata.**
cloud-init re-applies `user-data` on every boot and overwrites the
`/var/upspin` config files. A `sudo cp` / `nginx -s reload` only holds until
the next reboot. Push every infra change with `gcloud compute instances
add-metadata … --metadata-from-file user-data=cloud-init-cos121.yaml` (see "If
you changed config"). Symptom of a miss: after a reboot the service is gone
from `docker-compose.yml` (compose logs `Found orphan containers …`) or nginx
returns `no host in upstream ""`.
- **certbot step must stay non-interactive.** The `certonly` invocation runs with
`--keep-until-expiring -n`, so on a re-run where the cert already exists it's a
silent no-op instead of prompting `1: Keep / 2: Renew`. Without those flags it
would block on stdin and die with `EOFError`, aborting the script (`set -e`)
before the pull/recreate steps — don't remove them. (`deploy-markdown.sh` and
`deploy-vault.sh` carry the same flags.)
- **Compose `command:` is flags only.** The image `ENTRYPOINT` is already
`/bin/gitweb`; do **not** repeat `/bin/gitweb` in the compose `command:`. If you
do, Go's flag parser stops at that leading positional and silently ignores all
flags (including `-tls_cert`), so it serves **plain HTTP** → clients get a TLS
"wrong version number" through nginx.
- **Compose recreate `KeyError: ContainerConfig`.** docker-compose 1.29.2 on
COS's Docker errors on in-place recreate. Always `docker rm -f <container>`
first, then `up -d --no-recreate`. (This is also why a redeploy with a new
image must `docker rm -f patrick_gitweb_1` — `up` alone with `--no-recreate`
keeps the old container/image.)
- **nginx network aliases (hairpin NAT).** GCP does not let a VM reach its own
external IP, so the gitweb container can't talk to `upspin.cosmocr.at` /
`key.cosmocr.at` via the public IP. The shared nginx service carries those as
network `aliases`, so the gitweb upspin client resolves them to nginx and
routes back in via SNI. Don't remove them.
- **Reading other users' `ee`-encrypted files.** A `read,list` grant lets gitweb
*list and fetch* a repo, but an `ee`-packed (encrypted) object is only
*readable* if a key was wrapped for `git@cosmocr.at` — the owner must run
`upspin share -fix -r <repo>` after granting. `plain`/`eeintegrity` objects
render with just the grant. Objects gitweb can't decrypt surface as `404`.
- **TLS cert.** nginx must be serving `:80` for the certbot webroot ACME
challenge; the cert lives at `/mnt/disks/data/letsencrypt/live/git.cosmocr.at/`
and the container hot-reloads it via the `certReloader` (no restart on renewal).
- **Restart policy is `unless-stopped`** (like every service in the stack). A
`docker.service` drop-in gates the daemon on the data-disk mount, so it can't
start against an unmounted disk. The store makes no upspin calls until the first
request, so there's no startup readiness race.
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, and it ignores the client's "have"
// lines, so an incremental fetch re-sends the full history — fine for clone,
// suboptimal for fetch.
package githttp
import (
"compress/gzip"
"context"
"fmt"
"io"
"net/http"
"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)
}
// 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.
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
}
sess, err := newUploadSession(s)
if err != nil {
return err
}
defer sess.Close()
resp, err := sess.UploadPack(r.Context(), req)
if err != nil {
return err
}
w.Header().Set("Content-Type", fmt.Sprintf("application/x-%s-result", UploadPack))
w.Header().Set("Cache-Control", "no-cache")
return resp.Encode(w)
}
// 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/gitrepo/push.go
package gitrepo
import (
"context"
"crypto/sha256"
"errors"
"fmt"
"os"
"path"
"sort"
"strings"
"sync"
+ "time"
"github.com/go-git/go-billy/v5"
"github.com/go-git/go-billy/v5/util"
"github.com/go-git/go-git/v5/plumbing/cache"
"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/storage/filesystem"
"gitweb/internal/githttp"
"gitweb/internal/store"
)
// ErrConcurrentModification is returned when a repo's store Sequence changed
// between materialization and write-back, meaning the tree was modified out of
// band during the push. The push is abandoned without writing anything.
var ErrConcurrentModification = errors.New("remote repository changed during push; please retry")
// Pusher applies a git receive-pack (push) to an upspin-stored repo. It
// materializes a private, writable copy of the repo, runs the push against it
// with go-git, and writes the changed files back to the store. Pushes to the
// same repo are serialized.
type Pusher struct {
st store.WriteStore
cache *Cache
mu sync.Mutex
locks map[string]*sync.Mutex
}
// NewPusher returns a Pusher writing through st and invalidating c on success.
func NewPusher(st store.WriteStore, c *Cache) *Pusher {
return &Pusher{st: st, cache: c, locks: map[string]*sync.Mutex{}}
}
func (p *Pusher) lockFor(repoPath string) *sync.Mutex {
p.mu.Lock()
defer p.mu.Unlock()
l := p.locks[repoPath]
if l == nil {
l = &sync.Mutex{}
p.locks[repoPath] = l
}
return l
}
+// stableStatFS wraps a billy filesystem so Stat reports a constant ModTime.
+// memfs answers every ModTime call with time.Now(), which traps go-git's
+// packed-refs locking loop — open, stat, lock, stat, retry until the mtimes
+// match to rule out a concurrent change — in an infinite spin whenever it
+// rewrites packed-refs (e.g. deleting a ref). The materialized copy is private
+// to one push, so a constant ModTime loses nothing.
+type stableStatFS struct{ billy.Filesystem }
+
+func (s stableStatFS) Stat(name string) (os.FileInfo, error) {
+ fi, err := s.Filesystem.Stat(name)
+ if err != nil {
+ return nil, err
+ }
+ return stableInfo{fi}, nil
+}
+
+// Chroot rewraps so the fix survives go-git chrooting into subdirectories.
+func (s stableStatFS) Chroot(path string) (billy.Filesystem, error) {
+ fs, err := s.Filesystem.Chroot(path)
+ if err != nil {
+ return nil, err
+ }
+ return stableStatFS{fs}, nil
+}
+
+type stableInfo struct{ os.FileInfo }
+
+func (stableInfo) ModTime() time.Time { return time.Time{} }
+
+// noPackfileWriter hides the storer's optional PackfileWriter capability from
+// the receive-pack session. With it visible, go-git copies the client's
+// packfile verbatim into objects/pack, and that path cannot resolve the
+// REF_DELTA entries of a thin pack — which git sends on every incremental
+// push, deltifying against objects that live in the repo, not in the pack —
+// so such pushes fail with "reference delta not found". Hidden, go-git parses
+// the pack against the storer, resolving thin deltas and storing the new
+// objects loose.
+type noPackfileWriter struct{ storer.Storer }
+
// ReceivePack runs the push described by req against repoPath and writes the
// result back to the store. The returned ReportStatus should be sent to the
// client even when err is non-nil (it carries per-ref status).
func (p *Pusher) ReceivePack(ctx context.Context, repoPath string, req *packp.ReferenceUpdateRequest) (*packp.ReportStatus, error) {
lock := p.lockFor(repoPath)
lock.Lock()
defer lock.Unlock()
seq0, err := p.st.Seq(repoPath)
if err != nil {
return nil, err
}
// A private copy: never the shared read cache, which serves concurrent
// readers and must not be mutated.
fs, before, err := materialize(p.st, repoPath)
if err != nil {
return nil, err
}
- storer := filesystem.NewStorage(fs, cache.NewObjectLRUDefault())
- sess, err := githttp.NewReceivePackSession(storer)
+ sto := filesystem.NewStorage(stableStatFS{fs}, cache.NewObjectLRUDefault())
+ sess, err := githttp.NewReceivePackSession(noPackfileWriter{sto})
if err != nil {
return nil, err
}
defer sess.Close()
rs, rpErr := sess.ReceivePack(ctx, req)
if rpErr != nil {
// Unpack or ref update failed; the storer may be in a partial state, so
// do not write anything back. The report status still goes to the client.
return rs, rpErr
}
after, err := walkManifest(fs)
if err != nil {
- return rs, err
+ return failReport(rs, err), err
}
// Guard against an out-of-band change to the tree since we materialized.
// The per-repo lock already excludes other pushes through this server.
seq1, err := p.st.Seq(repoPath)
if err != nil {
- return rs, err
+ return failReport(rs, err), err
}
if seq1 != seq0 {
- return rs, ErrConcurrentModification
+ return failReport(rs, ErrConcurrentModification), ErrConcurrentModification
}
if err := p.writeBack(fs, before, after, repoPath); err != nil {
- return rs, err
+ return failReport(rs, err), err
}
p.cache.Invalidate(repoPath)
return rs, nil
+}
+
+// failReport downgrades every "ok" per-ref status in rs to err's message. The
+// session updated only the private copy; when persisting to the store fails,
+// the client must see its refs rejected, not an "ok" report for updates that
+// were never written. The message is flattened to one line so it survives
+// pkt-line framing.
+func failReport(rs *packp.ReportStatus, err error) *packp.ReportStatus {
+ if rs == nil {
+ return nil
+ }
+ msg := strings.Join(strings.Fields(err.Error()), " ")
+ for _, cs := range rs.CommandStatuses {
+ if cs.Error() == nil { // "ok"
+ cs.Status = msg
+ }
+ }
+ return rs
}
// writeBack applies the difference between the before and after manifests to the
// store: new/changed files are written objects-first then refs, and files that
// disappeared are deleted last. This ordering keeps a concurrent reader's view
// consistent — it may briefly see extra unreachable objects, never a ref that
// points at a missing object.
func (p *Pusher) writeBack(fs billy.Filesystem, before, after Manifest, repoPath string) error {
var objectPuts, refPuts, dels []string
for name, info := range after {
if old, ok := before[name]; ok && old.Sum == info.Sum {
continue // unchanged
}
if strings.HasPrefix(name, "objects/") {
objectPuts = append(objectPuts, name)
} else {
refPuts = append(refPuts, name)
}
}
for name := range before {
if _, ok := after[name]; !ok {
dels = append(dels, name)
}
}
// Deterministic order (aids testing and reproducibility).
sort.Strings(objectPuts)
sort.Strings(refPuts)
sort.Strings(dels)
put := func(name string) error {
data, err := util.ReadFile(fs, name)
if err != nil {
return err
}
return p.st.Put(path.Join(repoPath, name), data)
}
for _, name := range objectPuts {
if err := put(name); err != nil {
return fmt.Errorf("push write object %s: %w", name, err)
}
}
for _, name := range refPuts {
if err := put(name); err != nil {
return fmt.Errorf("push write ref %s: %w", name, err)
}
}
for _, name := range dels {
if err := p.st.Delete(path.Join(repoPath, name)); err != nil && err != store.ErrNotFound {
return fmt.Errorf("push delete %s: %w", name, err)
}
}
return nil
}
// walkManifest builds a manifest of every regular file currently in fs,
// enforcing the size cap and skipping go-git's dotgit temp files.
func walkManifest(fs billy.Filesystem) (Manifest, error) {
man := Manifest{}
var total int64
err := util.Walk(fs, "/", func(p string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if info.IsDir() {
return nil
}
rel := strings.TrimPrefix(p, "/")
if strings.HasPrefix(path.Base(rel), "tmp_") {
return nil // go-git dotgit scratch file, not part of the repo
}
total += info.Size()
if total > maxRepoBytes {
return errRepoTooLarge
}
data, err := util.ReadFile(fs, rel)
if err != nil {
return err
}
man[rel] = FileInfo{Size: info.Size(), Sum: sha256.Sum256(data)}
return nil
})
if err != nil {
return nil, err
}
return man, nil
}
internal/gitrepo/push_test.go
package gitrepo
import (
+ "errors"
"strings"
"testing"
"github.com/go-git/go-billy/v5"
"github.com/go-git/go-billy/v5/memfs"
"github.com/go-git/go-billy/v5/util"
+ "github.com/go-git/go-git/v5/plumbing/protocol/packp"
"gitweb/internal/store"
)
// recordingStore records Put/Delete calls for assertions. Reads are unused by
// writeBack (it reads file bytes from the billy fs), so they are stubs.
type recordingStore struct {
puts []string
dels []string
}
func (r *recordingStore) Stat(string) (bool, error) { return false, store.ErrNotFound }
func (r *recordingStore) Get(string) ([]byte, error) { return nil, store.ErrNotFound }
func (r *recordingStore) List(string) ([]store.Entry, error) { return nil, store.ErrNotFound }
func (r *recordingStore) Seq(string) (int64, error) { return 0, nil }
func (r *recordingStore) Put(p string, _ []byte) error { r.puts = append(r.puts, p); return nil }
func (r *recordingStore) Delete(p string) error { r.dels = append(r.dels, p); return nil }
func writeFile(t *testing.T, fs billy.Filesystem, name, content string) {
t.Helper()
if err := util.WriteFile(fs, name, []byte(content), 0o644); err != nil {
t.Fatal(err)
}
}
func TestWriteBackDiffAndOrdering(t *testing.T) {
fs := memfs.New()
// Initial state.
writeFile(t, fs, "HEAD", "ref: refs/heads/main\n")
writeFile(t, fs, "objects/pack/pack-old.pack", "OLD")
writeFile(t, fs, "refs/heads/main", "aaaa\n")
writeFile(t, fs, "refs/heads/stale", "bbbb\n")
before, err := walkManifest(fs)
if err != nil {
t.Fatal(err)
}
// Simulate a push: new packfile, updated main ref, deleted stale ref.
writeFile(t, fs, "objects/pack/pack-new.pack", "NEW")
writeFile(t, fs, "refs/heads/main", "cccc\n")
if err := fs.Remove("refs/heads/stale"); err != nil {
t.Fatal(err)
}
after, err := walkManifest(fs)
if err != nil {
t.Fatal(err)
}
rec := &recordingStore{}
p := NewPusher(rec, NewCache(rec))
if err := p.writeBack(fs, before, after, "u@d.com/r.git"); err != nil {
t.Fatalf("writeBack: %v", err)
}
// Expected: the new packfile and the changed ref are Put; the removed ref is
// Deleted. HEAD and the old packfile are unchanged and must not be rewritten.
wantPuts := map[string]bool{
"u@d.com/r.git/objects/pack/pack-new.pack": true,
"u@d.com/r.git/refs/heads/main": true,
}
if len(rec.puts) != len(wantPuts) {
t.Fatalf("puts = %v, want keys %v", rec.puts, wantPuts)
}
for _, p := range rec.puts {
if !wantPuts[p] {
t.Errorf("unexpected put %q", p)
}
}
// Object writes must precede ref writes.
if !strings.Contains(rec.puts[0], "/objects/") {
t.Errorf("expected object put first, got order %v", rec.puts)
}
// The stale ref must be deleted.
if len(rec.dels) != 1 || rec.dels[0] != "u@d.com/r.git/refs/heads/stale" {
t.Fatalf("dels = %v, want [u@d.com/r.git/refs/heads/stale]", rec.dels)
+ }
+}
+
+func TestFailReport(t *testing.T) {
+ rs := packp.NewReportStatus()
+ rs.UnpackStatus = "ok"
+ rs.CommandStatuses = []*packp.CommandStatus{
+ {ReferenceName: "refs/heads/main", Status: "ok"},
+ {ReferenceName: "refs/heads/other", Status: "already failed"},
+ }
+ failReport(rs, errors.New("store broke:\n\tdetail"))
+
+ if got := rs.CommandStatuses[0].Status; got != "store broke: detail" {
+ t.Errorf("ok status = %q, want flattened error", got)
+ }
+ // A status that already carries a failure is not overwritten.
+ if got := rs.CommandStatuses[1].Status; got != "already failed" {
+ t.Errorf("failed status overwritten: %q", got)
+ }
+ if failReport(nil, errors.New("x")) != nil {
+ t.Error("failReport(nil) != nil")
}
}
func TestWalkManifestSkipsTmp(t *testing.T) {
fs := memfs.New()
writeFile(t, fs, "HEAD", "x")
writeFile(t, fs, "objects/tmp_pack_123", "scratch")
man, err := walkManifest(fs)
if err != nil {
t.Fatal(err)
}
if _, ok := man["objects/tmp_pack_123"]; ok {
t.Error("walkManifest included a tmp_ scratch file")
}
if _, ok := man["HEAD"]; !ok {
t.Error("walkManifest missing HEAD")
}
}
internal/server/clone_push_test.go
package server
import (
"crypto/sha256"
"encoding/hex"
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
"gitweb/internal/store"
)
// gitEnv returns a hermetic environment for invoking git in tests: no prompts,
// no system/global config, no credential helpers, and an isolated HOME.
func gitEnv(t *testing.T) []string {
return append(os.Environ(),
"HOME="+t.TempDir(),
"GIT_TERMINAL_PROMPT=0",
"GIT_CONFIG_NOSYSTEM=1",
"GIT_ASKPASS=true",
"GIT_AUTHOR_NAME=T", "GIT_AUTHOR_EMAIL=t@e.com",
"GIT_COMMITTER_NAME=T", "GIT_COMMITTER_EMAIL=t@e.com",
)
}
func runGit(t *testing.T, env []string, dir string, args ...string) (string, error) {
t.Helper()
c := exec.Command("git", args...)
c.Dir = dir
c.Env = env
out, err := c.CombinedOutput()
return string(out), err
}
func mustGit(t *testing.T, env []string, dir string, args ...string) string {
t.Helper()
out, err := runGit(t, env, dir, args...)
if err != nil {
t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out)
}
return out
}
// makeBareRepo builds a bare repo at ns/<repoPath> with one commit, and writes
// an Access file with the given content beside it.
func makeBareRepo(t *testing.T, env []string, ns, repoPath, access string) {
t.Helper()
work := t.TempDir()
mustGit(t, env, work, "init", "-q", "-b", "main")
if err := os.WriteFile(filepath.Join(work, "README.md"), []byte("# hello\n"), 0o644); err != nil {
t.Fatal(err)
}
mustGit(t, env, work, "add", "-A")
mustGit(t, env, work, "commit", "-qm", "initial")
dst := filepath.Join(ns, filepath.FromSlash(repoPath))
if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil {
t.Fatal(err)
}
mustGit(t, env, work, "clone", "-q", "--bare", work, dst)
if err := os.WriteFile(filepath.Join(dst, "Access"), []byte(access), 0o644); err != nil {
t.Fatal(err)
}
}
// writeTokens writes a gitweb tokens file granting the given plaintext tokens to
// user (by storing their SHA-256 hashes).
func writeTokens(t *testing.T, ns, user string, tokens ...string) {
t.Helper()
dir := filepath.Join(ns, user, "gitweb")
if err := os.MkdirAll(dir, 0o755); err != nil {
t.Fatal(err)
}
var b strings.Builder
for _, tok := range tokens {
h := sha256.Sum256([]byte(tok))
fmt.Fprintf(&b, "%s token\n", hex.EncodeToString(h[:]))
}
if err := os.WriteFile(filepath.Join(dir, "tokens"), []byte(b.String()), 0o644); err != nil {
t.Fatal(err)
}
}
// cloneURL builds an http clone URL, optionally embedding Basic-auth creds.
func cloneURL(base, repoPath, user, token string) string {
u, _ := url.Parse(base)
if user != "" {
u.User = url.UserPassword(user, token)
}
u.Path = "/" + repoPath
return u.String()
}
func TestCloneAndPush(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 writer = "w@d.com"
const reader = "r@d.com"
makeBareRepo(t, env, ns, owner+"/public.git", "read,list: all\n*: "+owner+"\n")
makeBareRepo(t, env, ns, owner+"/private.git",
"*: "+owner+"\nread,list,write,create,delete: "+writer+"\nread,list: "+reader+"\n")
writeTokens(t, ns, writer, "writer-token")
writeTokens(t, ns, reader, "reader-token")
web := os.DirFS("../../web")
srv, err := New(Config{Store: store.NewLocalStore(ns), Web: http.FS(web)})
if err != nil {
t.Fatal(err)
}
ts := httptest.NewServer(srv.Handler())
defer ts.Close()
t.Run("anon clone public", func(t *testing.T) {
dst := filepath.Join(t.TempDir(), "pub")
if out, err := runGit(t, env, ".", "clone", "-q", cloneURL(ts.URL, owner+"/public.git", "", ""), dst); err != nil {
t.Fatalf("clone public failed: %v\n%s", err, out)
}
if _, err := os.Stat(filepath.Join(dst, "README.md")); err != nil {
t.Fatalf("cloned repo missing README: %v", err)
}
})
t.Run("anon clone private denied", func(t *testing.T) {
dst := filepath.Join(t.TempDir(), "priv")
if out, err := runGit(t, env, ".", "clone", "-q", cloneURL(ts.URL, owner+"/private.git", "", ""), dst); err == nil {
t.Fatalf("anonymous clone of private repo succeeded, want failure\n%s", out)
}
})
t.Run("authed clone private", func(t *testing.T) {
dst := filepath.Join(t.TempDir(), "priv-ok")
u := cloneURL(ts.URL, owner+"/private.git", writer, "writer-token")
if out, err := runGit(t, env, ".", "clone", "-q", u, dst); err != nil {
t.Fatalf("authed clone failed: %v\n%s", err, out)
}
})
t.Run("wrong token denied", func(t *testing.T) {
dst := filepath.Join(t.TempDir(), "priv-bad")
u := cloneURL(ts.URL, owner+"/private.git", writer, "nope")
if out, err := runGit(t, env, ".", "clone", "-q", u, dst); err == nil {
t.Fatalf("clone with wrong token succeeded, want failure\n%s", out)
}
})
t.Run("push as writer", func(t *testing.T) {
dst := filepath.Join(t.TempDir(), "push")
u := cloneURL(ts.URL, owner+"/private.git", writer, "writer-token")
mustGit(t, env, ".", "clone", "-q", u, dst)
if err := os.WriteFile(filepath.Join(dst, "new.txt"), []byte("added\n"), 0o644); err != nil {
t.Fatal(err)
}
mustGit(t, env, dst, "add", "-A")
mustGit(t, env, dst, "commit", "-qm", "add new.txt")
if out, err := runGit(t, env, dst, "push", "-q", "origin", "main"); err != nil {
t.Fatalf("push failed: %v\n%s", err, out)
}
// A fresh clone must see the pushed commit.
fresh := filepath.Join(t.TempDir(), "fresh")
mustGit(t, env, ".", "clone", "-q", u, fresh)
if _, err := os.Stat(filepath.Join(fresh, "new.txt")); err != nil {
t.Fatalf("pushed file not visible after re-clone: %v", err)
}
})
+ t.Run("incremental push (thin pack)", func(t *testing.T) {
+ dst := filepath.Join(t.TempDir(), "thin")
+ u := cloneURL(ts.URL, owner+"/private.git", writer, "writer-token")
+ mustGit(t, env, ".", "clone", "-q", u, dst)
+ // A file large enough that git deltas the second version against the
+ // first. On the second push that base blob is not in the pack (the
+ // server already has it), so git sends a thin pack with a REF_DELTA —
+ // the case that used to fail with "reference delta not found".
+ lines := strings.Repeat("a line of text that compresses and deltas well\n", 200)
+ if err := os.WriteFile(filepath.Join(dst, "big.txt"), []byte(lines), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ mustGit(t, env, dst, "add", "-A")
+ mustGit(t, env, dst, "commit", "-qm", "add big.txt")
+ mustGit(t, env, dst, "push", "-q", "origin", "main")
+
+ if err := os.WriteFile(filepath.Join(dst, "big.txt"), []byte(lines+"one changed line\n"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ mustGit(t, env, dst, "add", "-A")
+ mustGit(t, env, dst, "commit", "-qm", "append to big.txt")
+ if out, err := runGit(t, env, dst, "push", "origin", "main"); err != nil {
+ t.Fatalf("incremental push failed: %v\n%s", err, out)
+ }
+
+ fresh := filepath.Join(t.TempDir(), "fresh-thin")
+ mustGit(t, env, ".", "clone", "-q", u, fresh)
+ got, err := os.ReadFile(filepath.Join(fresh, "big.txt"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if string(got) != lines+"one changed line\n" {
+ t.Error("re-clone does not have the incrementally pushed content")
+ }
+ })
+
+ t.Run("push branch create and delete", func(t *testing.T) {
+ dst := filepath.Join(t.TempDir(), "branch")
+ u := cloneURL(ts.URL, owner+"/private.git", writer, "writer-token")
+ mustGit(t, env, ".", "clone", "-q", u, dst)
+ mustGit(t, env, dst, "push", "-q", "origin", "main:refs/heads/scratch")
+ // Deleting a ref sends no packfile at all; the server must not choke
+ // on the empty pack stream.
+ if out, err := runGit(t, env, dst, "push", "origin", ":refs/heads/scratch"); err != nil {
+ t.Fatalf("delete push failed: %v\n%s", err, out)
+ }
+ refs := mustGit(t, env, dst, "ls-remote", "origin")
+ if strings.Contains(refs, "scratch") {
+ t.Errorf("scratch branch still on remote:\n%s", refs)
+ }
+ })
+
t.Run("push as reader denied", func(t *testing.T) {
dst := filepath.Join(t.TempDir(), "push-ro")
u := cloneURL(ts.URL, owner+"/private.git", reader, "reader-token")
mustGit(t, env, ".", "clone", "-q", u, dst)
if err := os.WriteFile(filepath.Join(dst, "sneaky.txt"), []byte("x\n"), 0o644); err != nil {
t.Fatal(err)
}
mustGit(t, env, dst, "add", "-A")
mustGit(t, env, dst, "commit", "-qm", "sneak")
if out, err := runGit(t, env, dst, "push", "-q", "origin", "main"); err == nil {
t.Fatalf("reader push succeeded, want 403 failure\n%s", out)
}
})
}