Featured image of post Four lines cost a whole website

Four lines cost a whole website

To append four lines to one YAML file, the release job was cloning this entire website into memory. The first fix for it failed without making a sound.

Each time a project in the estate cuts a release, colophon appends four lines to a YAML file on this blog, and the changelog page picks them up. Four lines. A date, a name, a tag and a URL.

To write them, the release job was cloning the entire website into memory.

What it was doing

The adapter clones the blog, reads the file, adds the release to it, commits and pushes. In go/repo terms the clone is already three options deep, because none of this wants the full history:

_, worktree, err := handle.OpenInMemory(ctx, remote, target,
    repo.WithSingleBranch(target), repo.WithNoTags(), repo.WithShallowClone(1),
)

Shallow, one branch, no tags, and still the entire working tree of a website with a decade of posts in it, materialised in memory so that four lines can go into one file under data/releases/.

Git has sparse checkout for exactly this, and go-git supports it. go/repo did not: its option list stopped at shallow, single-branch, no-tags and submodules, and the natural reading of that list is that sparse isn’t available.

The recipe in the ticket

So the session building the adapter did the right thing and raised a ticket on go/repo asking for it to be a first-class option. It also did a helpful thing: it had found a way to get sparse working through the abstraction, and it wrote that into the ticket as what it was doing meanwhile.

_, worktree, err := handle.OpenInMemory(ctx, remote, target,
    repo.WithSingleBranch(target), repo.WithNoTags(), repo.WithShallowClone(1),
    func(o *git.CloneOptions) { o.NoCheckout = true },   // reaching through
)

// ...then, by hand, the bit that makes it sparse:
err = worktree.ResetSparsely(
    &git.ResetOptions{Mode: git.MixedReset},
    []string{"data/releases"},
)

That reach-through was allowed and the docs taught it: go/repo’s option type was a bare callback over go-git’s own struct, so writing one by hand was how you reached any field the module had no option for. (That snippet compiles against go/repo v0.4.0 and against nothing later.)

It’s a plausible recipe, and the shape you’ll find on issue threads. It’s also wrong in a way that doesn’t show, because ResetSparsely under MixedReset rewrites the index and writes no files at all. The worktree after that second step is empty.

The adapter then does this with it:

current, err := read(worktree, name)       // not there: os.IsNotExist, so nil
body, err := req.Change(current)           // nil in, so a document of one entry
f, err := worktree.Filesystem.Create(name)
_, err = f.Write(body)
_, err = worktree.Add(name)
_, err = worktree.Commit(message, &git.CommitOptions{Author: &author})

Each of those lines is correct. It’s a read-modify-write over a structured document, which is what you want for YAML, because you can’t append to YAML by bolting bytes onto the end. read returning nil for a missing file is deliberate and right: a project cutting its first release has no file yet, and that’s an ordinary answer, not an error.

Put an empty worktree underneath it and those two correct behaviours combine into a commit that replaces the file with a single entry. Nothing fails. Nothing warns. The release goes green.

The spike that killed it

The version of this story where it ate live data is a better story, and it isn’t the one I’ve got.

The session that picked up the ticket didn’t implement the recipe that came with it. It built a fixture first, four files across three directories, cloned it into memory, ran each of the three possible second steps, then did the append and the commit and looked at what came out.

second stepworktree afterthe file in the commit
ResetSparsely, MixedReset (the recipe in the ticket)emptyreplaced by the one new entry
ResetSparsely, HardResetthe one directoryoriginal plus the new entry
Checkout with SparseCheckoutDirectoriesthe one directoryoriginal plus the new entry

Three rows, and the recipe in the ticket is the one that eats your file. The finding went back across to the session that raised it three minutes after the ticket was filed, before the adapter merged: the recipe leaves the worktree empty, the append truncates, go and look at the spec and the adapter before this goes anywhere.

So the file is fine. Every release this blog has ever listed is still in it. What nearly happened is the gap between a few minutes of measuring and a bug that reports success for as long as you leave it be.

The recipe being wrong doesn’t bother me much. That trap is sat on public issue threads for anyone to copy, it needs the right reset mode and the wrong one fails silently, and I’d have copied it myself… it looks like the answer. What gets me is that it went into a ticket as a fact before anyone had run it, and a throwaway fixture found it out before anybody built on it. If the decision turns on behaviour, go and watch the behaviour.

One option, both steps

The fix doesn’t defend against that recipe. It removes the ability to write it.

_, worktree, err := handle.OpenInMemory(ctx, remote, target,
    repo.WithSingleBranch(target), repo.WithNoTags(), repo.WithShallowClone(1),
    repo.WithSparseCheckout("data/releases/"),
)

That last option sets NoCheckout on the clone and does the sparse checkout afterwards, and the step it runs is Checkout with the sparse directories rather than ResetSparsely. go-git’s Checkout composes the reference to move to with the sparse list and picks the reset mode itself, so the module never chooses MixedReset and neither does anybody calling it. One name, no ordering, no mode, no go-git import reaching the consumer.

The half-measure was tempting and I’m glad it didn’t happen: an option called WithNoCheckout() that sets the flag and leaves the second step to the caller keeps the whole trap and gives it a friendlier name.

Owning that second step cost a type change, because CloneOption had been func(*git.CloneOptions) and go-git’s struct has nowhere to carry a post-clone instruction, nor any way to reach the handle that would run one. go/repo owns a CloneOptions struct now, embedding go-git’s, so a hand-written option still reaches every field it ever did. Below 1.0, so it went out with a Release-Note: trailer instead of a breaking-change footer, and the migration cost to the one real consumer came out at nothing, measured, not assumed: colophon had no hand-written clone options in committed code at all.

The fixture was still up, so I had it measure the saving I’d been assuming was large. Three hundred incompressible 20 KiB files: 12.6 MiB of live heap after a full in-memory open, 5.4 MiB after a sparse one. Real, and less than half what I’d have guessed, because the packfile gets fetched and held either way. Sparse saves the working-tree copy, not the objects. Keeping the objects small is the job of the shallow, single-branch, tagless options, and they’re separate options for a reason.

Two ways to get it wrong

The option refuses an empty directory list, and it refuses a list that matches nothing, and I’d argue for those being two different errors rather than one.

ErrNoSparseDirectories is WithSparseCheckout() with nothing in it, which compiles, and is a plausible bug the day the directory comes from a config value that was never set. Applying that as a full checkout silently spends the memory the caller asked to save; applying it as an empty sparse set hands back an empty worktree and says nothing.

ErrNoSparseMatch is the typo, a directory the tree doesn’t have, and it’s in there because the colophon session went and measured what happens without it. The checkout succeeds, the worktree is empty, and the first sign of anything wrong is cannot create empty commit: clean working tree from a commit three calls later. An idempotent writer can’t tell that from “there was nothing to do this time”, so a mistyped path reports success indefinitely.

An empty list is a value never set. A list matching nothing is a value set wrong. Different mistakes, different fixes, and both land where the mistake was made instead of three frames downstream in a disguise.

The ticket was right about the need, right about the cost, and right that the module should own this. It was wrong about one reset mode, in a way that comes out green.

Built with Hugo · Theme Stack designed by Jimmy