Conversation
Advancing a reference atomically (update only if it still points at an expected OID) was the one ref operation Rugged could not do in-process, forcing callers to shell out to `git update-ref <ref> <new> <old>` even when doing everything else through Rugged. libgit2 exposes git_reference_create_matching for exactly this; wire it up behind a :current_id option on #create. A mismatch on an existing ref returns nil (like merge_trees on conflict); an absent ref raises; a symbolic target or malformed current_id raises ArgumentError.
JonJagger
added a commit
to cyber-dojo/saver
that referenced
this pull request
Sep 20, 2026
* Move the ref advance in-process via a rugged fork A v2 save did everything in-process through libgit2 except one step: it shelled out to "git update-ref refs/heads/main <new> <base>" to advance main only when it still pointed at the commit the change was built on. That precondition is how a concurrent writer is detected, so it could not be dropped, and it left a ~20ms subprocess on every save of a path that was otherwise entirely in-process. The gap was in rugged, not in libgit2, which has always exposed git_reference_create_matching. cyber-dojo now carries a fork at github.com/cyber-dojo/rugged adding a :current_id option to ReferenceCollection#create, offered upstream as libgit2/rugged#1014. So the Dockerfile installs rugged from that fork, pinned to a commit and cloned with its vendored libgit2 submodule, instead of from rubygems. If the pull request is merged and released this goes back to rubygems. The shell call becomes External::Git#advance_main, which raises RefAdvanceFailed when the swap loses, preserving what the shell helper did on a non-zero exit. Its two call sites, commit_event and option_set, are the last v2 save users of External::Shell; download still shells out for git clone and tar, so Shell stays. Comments and docs that described the advance as a git CLI call are corrected, and they now spell out compare-and-swap rather than CAS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Move download in-process and delete External::Shell download was the last thing in saver spawning a subprocess: git clone, git remote remove origin, and tar -czf, all through External::Shell. So a whole external, and the shell-quoting that comes with it, survived only to build a tarball once per download. The clone becomes External::Git#clone_without_origin, over Rugged::Repository.clone_at and remotes.delete. clone_at clones non-bare, so the full history, the tags and a checkout of HEAD all come with it. rugged's delete leaves an inert empty [remote "origin"] section where the git CLI removes the section too; either way no url is recorded and git lists no remote. The tar becomes TGZ.of, which saver already had and nothing called, over the files the clone leaves on disk. Every file git tracks is mode 0644, which is the mode TGZ.of writes, so what a user extracts matches the index and reads as clean. With no callers left, External::Shell and its test are gone. The tests keep driving the real tar and the real git, now through backticks rather than an external. Reading a download back with the same libraries that wrote it would only show that saver agrees with itself, and the tarball has to open with the tools a user has. That also keeps rugged out of the tests, so every rugged call in the repo is one production makes. There was no client-side download test at all, so kata_download is uncommented on the client and Dn4Kp1 covers the endpoint end to end over HTTP. It reads .git/config rather than running git remote, as the client image has no git. Giving TGZ.of its first real caller showed the other half of that library has none: TGZ.files, TarFile::Reader and Gnu.unzip were reachable only from their own round-trip tests, which proved the reader against the writer and nothing about saver. They are removed, along with those tests. The writer and the zip stay, covered now by a download that real tar extracts. kL375s carried a docstring copy-pasted from kata_exists. It now says what the test asserts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed in #1013.
Why
Advancing a reference atomically -- update it to a new OID only if it still
points at an expected OID -- is the one ref operation (that I need) Rugged cannot not do
in-process. I have to shell out to
git update-ref <ref> <new> <old>,even when doing everything else (index build,
Rugged::Commit.create, reads)through Rugged. libgit2 already exposes
git_reference_create_matchingforexactly this; this wires it up.
What
Adds a
:current_idoption toRugged::ReferenceCollection#create. Whenpresent, it dispatches to
git_reference_create_matching(compare-and-swap)instead of
git_reference_create:Behaviour (all covered by tests):
current_id-> updated, returns the newRugged::Referencenil(matchesmerge_trees/rebase.commit, which returnnilonGIT_EMERGECONFLICT/GIT_EAPPLIED)Rugged::ReferenceError:current_idwith a symbolic target ->ArgumentError:current_id->ArgumentErrorNotes
:current_idis only consulted for direct (OID) references; existingbehaviour is unchanged when it is absent.
See #1013 for the API discussion -- happy to reshape (e.g. onto
#updateor adedicated method) if you'd prefer.