ref:8de061c4ca174f71cee22b04293470bc64ce4f6c

feat: ReceivePack supports atomic push

Advertises `atomic` alongside `report-status` and `delete-refs`. When the client indicates atomic in its capabilities list, ReceivePack validates every ref-update command before touching storage; if any command fails validation (bad ref name, stale old_sha, non-ff, update_hook rejection), the whole batch is refused with `ng` per ref and no ref moves. Two-phase flow: 1. Validate all commands. Per-command validation checks: * Ref.validate_ref_name/1 * old_sha state (must be zero for create, current for update, present for delete) * update_hook's return value 2. Commit all. Snapshot every target ref's current value, then apply commands sequentially. If any apply fails mid-way, restore every touched ref from its snapshot (and delete refs that didn't exist pre-flight). Note on transactionality: ref storage backends (filesystem, S3) are not multi-key atomic. A rollback can itself fail, leaving refs in a partially-applied state. This implementation is as atomic as the storage layer allows — the doc comment on do_atomic_ref_updates/1 calls this out explicitly. Test rewritten to actually exercise the atomic path: a pre-registered update_hook rejects any update to refs/heads/rejected. The client pushes `main:refs/heads/accepted` and `main:refs/heads/rejected` in a single --atomic push; the test asserts BOTH refs are absent on the server afterwards (not just that the push exit code is non-zero). Non-atomic behaviour is unchanged: per-ref failures still produce per-ref `ng` entries without affecting other refs.
SHA: 8de061c4ca174f71cee22b04293470bc64ce4f6c
Author: Cole Christensen <cole.christensen@macmillan.com>
Date: 2026-04-19 00:51
Parents: dfd72ff
4 files changed +184 -56
Type
lib/ex_git_objectstore/pack/filter.ex +4 −6
@@ -195,12 +195,10 @@
defp pattern_match?(pattern, path) do
pattern = String.trim_leading(pattern, "/")
cond do
String.ends_with?(pattern, "/") ->
String.starts_with?(path, pattern) or String.starts_with?(path <> "/", pattern)
true ->
path == pattern or String.starts_with?(path, pattern <> "/")
if String.ends_with?(pattern, "/") do
String.starts_with?(path, pattern) or String.starts_with?(path <> "/", pattern)
else
path == pattern or String.starts_with?(path, pattern <> "/")
end
end
end
lib/ex_git_objectstore/protocol/receive_pack.ex +133 −6
@@ -40,7 +40,7 @@
# Capabilities we actually implement. Others are scaffolded but not yet functional:
# Future work: ofs-delta — Writer doesn't generate OFS_DELTA yet
# Future work: side-band-64k — not yet used in report responses
@capabilities "report-status delete-refs"
@capabilities "report-status delete-refs atomic"
@type command :: %{
ref: String.t(),
@@ -498,23 +498,151 @@
end
defp do_process_ref_updates(state) do
if MapSet.member?(state.client_caps, "atomic") do
do_atomic_ref_updates(state)
else
do_sequential_ref_updates(state)
end
end
# Client did NOT ask for `atomic`: each ref-update is independent.
# Per-command failures (hook reject, stale old_sha, …) show up as
# `ng` lines in report-status but don't affect the other commands.
defp do_sequential_ref_updates(state) do
results =
Enum.map(state.commands, fn cmd ->
case run_update_hook(state.update_hook, cmd) do
:ok ->
{cmd.ref, apply_ref_command(state.repo, cmd)}
result = apply_ref_command(state.repo, cmd)
{cmd.ref, result}
{:error, _} = err ->
{cmd.ref, err}
end
end)
finalize_ref_updates(state, results)
end
# Client asked for `atomic`: either every ref update lands, or none
# do. Phase 1 validates every command (update_hook + old_sha check)
# without touching storage. Phase 2 applies them all; if any apply
# fails mid-way, we restore every ref we already changed from a
# pre-flight snapshot.
#
# Note on transactionality: the filesystem / S3 ref backends aren't
# multi-key atomic. A rollback can itself fail (e.g. the VM dies
# between steps), leaving refs in a partially-applied state. This
# implementation is as atomic as the storage layer allows.
defp do_atomic_ref_updates(state) do
validations =
Enum.map(state.commands, fn cmd ->
{cmd, validate_ref_command(state.repo, state.update_hook, cmd)}
end)
case Enum.find(validations, fn {_cmd, v} -> match?({:error, _}, v) end) do
nil -> atomic_commit_phase(state)
{_failing_cmd, {:error, reason}} -> atomic_reject_all(state, validations, reason)
end
end
defp atomic_reject_all(state, validations, reason) do
results =
Enum.map(validations, fn
{cmd, :ok} -> {cmd.ref, {:error, {:atomic_rejected, reason}}}
{cmd, {:error, r}} -> {cmd.ref, {:error, r}}
end)
finalize_ref_updates(state, results)
end
defp atomic_commit_phase(state) do
snapshots =
Enum.map(state.commands, fn cmd -> {cmd.ref, Ref.get(state.repo, cmd.ref)} end)
{results, any_failure} = apply_atomic_commands(state.repo, state.commands)
if any_failure do
rollback_refs(state.repo, snapshots)
finalize_ref_updates(state, mark_all_rolled_back(results, any_failure))
else
finalize_ref_updates(state, results)
end
end
defp apply_atomic_commands(repo, commands) do
{acc, failure} =
Enum.reduce(commands, {[], nil}, fn cmd, state ->
apply_atomic_command(repo, cmd, state)
end)
{Enum.reverse(acc), failure}
end
defp apply_atomic_command(_repo, cmd, {acc, failure}) when not is_nil(failure) do
{[{cmd.ref, {:error, {:atomic_rolled_back, failure}}} | acc], failure}
end
defp apply_atomic_command(repo, cmd, {acc, nil}) do
case apply_ref_command(repo, cmd) do
:ok -> {[{cmd.ref, :ok} | acc], nil}
{:error, reason} -> {[{cmd.ref, {:error, reason}} | acc], reason}
end
end
defp mark_all_rolled_back(results, reason) do
Enum.map(results, fn
{ref, :ok} -> {ref, {:error, {:atomic_rolled_back, reason}}}
other -> other
end)
end
defp rollback_refs(repo, snapshots) do
Enum.each(snapshots, fn {ref, snapshot} ->
case snapshot do
# Ref existed before — restore its prior value, bypassing CAS.
{:ok, sha} -> Ref.put(repo, ref, sha, nil)
# Ref didn't exist before — if we created it, remove it.
{:error, _} -> Ref.delete(repo, ref)
end
end)
end
defp validate_ref_command(repo, update_hook, cmd) do
with :ok <- Ref.validate_ref_name(cmd.ref),
# Track whether any ref updates failed
:ok <- validate_ref_state(repo, cmd) do
run_update_hook(update_hook, cmd)
end
end
# Create: old_sha must be zero; ref must not yet exist.
defp validate_ref_state(repo, %{old_sha: @zero_sha, ref: ref}) do
case Ref.get(repo, ref) do
{:ok, _} -> {:error, :ref_already_exists}
{:error, _} -> :ok
end
end
# Delete: ref must exist.
defp validate_ref_state(repo, %{new_sha: @zero_sha, ref: ref}) do
case Ref.get(repo, ref) do
{:ok, _} -> :ok
{:error, _} -> {:error, :ref_not_found}
end
end
# Update: old_sha must match current.
defp validate_ref_state(repo, %{old_sha: old_sha, ref: ref}) do
case Ref.get(repo, ref) do
{:ok, ^old_sha} -> :ok
{:ok, _} -> {:error, :non_fast_forward}
{:error, _} -> {:error, :ref_not_found}
end
end
defp finalize_ref_updates(state, results) do
any_errors? = Enum.any?(results, fn {_ref, r} -> match?({:error, _}, r) end)
overall_result = if any_errors?, do: {:error, :some_refs_failed}, else: :ok
# Call post_receive hook with change list (failures logged, don't affect result)
changes =
Enum.zip(state.commands, results)
|> Enum.map(fn {cmd, {_ref, status}} ->
@@ -523,7 +651,6 @@
run_post_receive_hook(state.post_receive_hook, changes)
# Only send report-status if client requested it (or if no commands/caps parsed)
if MapSet.member?(state.client_caps, "report-status") or MapSet.size(state.client_caps) == 0 do
report = build_report(results)
{report, %{state | phase: :done, result: overall_result}}
lib/ex_git_objectstore/protocol/upload_pack_v2.ex +21 −23
@@ -701,27 +701,27 @@
defp walk_tree_metadata(repo, sha, depth, path, acc) do
existing = Map.get(acc.depths, sha, :infinity)
cond do
depth >= existing -> acc
if depth >= existing do
acc
else
descend_tree_metadata(repo, sha, depth, path, put_depth(acc, sha, depth))
true -> descend_tree_metadata(repo, sha, depth, path, put_depth(acc, sha, depth))
end
end
defp descend_tree_metadata(repo, sha, depth, path, acc) do
case ObjectResolver.read(repo, sha) do
{:ok, %Tree{entries: entries}} ->
Enum.reduce(entries, acc, fn entry, inner ->
child_path = if path == "", do: entry.name, else: path <> "/" <> entry.name
walk_tree_metadata(repo, entry.sha, depth + 1, child_path, inner)
end)
{:ok, %Blob{}} ->
{:ok, %Tree{entries: entries}} -> descend_tree_entries(repo, entries, depth, path, acc)
{:ok, %Blob{}} -> put_path(acc, sha, path)
_ -> acc
put_path(acc, sha, path)
_ ->
acc
end
end
defp descend_tree_entries(repo, entries, depth, path, acc) do
Enum.reduce(entries, acc, fn entry, inner ->
child_path = if path == "", do: entry.name, else: path <> "/" <> entry.name
walk_tree_metadata(repo, entry.sha, depth + 1, child_path, inner)
end)
end
defp put_depth(acc, sha, depth), do: %{acc | depths: Map.put(acc.depths, sha, depth)}
defp put_path(acc, sha, path), do: %{acc | paths: Map.put_new(acc.paths, sha, path)}
@@ -835,15 +835,13 @@
defp process_shallow_parents(sha, %Commit{parents: parents} = commit, budget, rest, state) do
since_ok? = state.opts.since == nil or commit_time(commit) >= state.opts.since
walk_parents? = parent_walk_allowed?(budget) and since_ok?
cond do
not walk_parents? ->
# Boundary commit: we stop here, parents are excluded from the pack.
walk_shallow_loop(rest, %{state | new_shallow: MapSet.put(state.new_shallow, sha)})
true ->
state = maybe_unshallow(state, sha)
parent_items = Enum.map(parents, fn p -> {p, next_budget(sha, budget, state.opts)} end)
walk_shallow_loop(rest ++ parent_items, state)
if walk_parents? do
state = maybe_unshallow(state, sha)
parent_items = Enum.map(parents, fn p -> {p, next_budget(sha, budget, state.opts)} end)
walk_shallow_loop(rest ++ parent_items, state)
else
# Boundary commit: we stop here, parents are excluded from the pack.
walk_shallow_loop(rest, %{state | new_shallow: MapSet.put(state.new_shallow, sha)})
end
end
test/ex_git_objectstore/integration/receive_pack_git_client_test.exs +26 −21
@@ -191,42 +191,47 @@
# ref updates or none of them. ReceivePack must refuse the whole
# batch if any one update would fail (invalid ref name, hook
# rejection, non-ff without --force, etc.).
#
# We use an update_hook that rejects any update to
# `refs/heads/rejected` so the atomic branch is exercised without
# relying on git client quirks (e.g. whether the CLI allows sending
# two refspecs in one push). The two commands are sent via a
# single push so the server sees them in one receive-pack session.
@tag :tmp_dir
test "atomic push rejects the whole batch when one ref is bad",
test "one hook-rejected command rolls back the whole batch",
%{tmp_dir: tmp_dir} do
{port, stop, repo} = seeded_receive_pack("atomic", [{"a.txt", "v1\n"}])
repo = RepoHelper.memory_repo("atomic-hook")
ExGitObjectstore.init(repo)
try do
{upload_port, stop_upload} = GitDaemon.start_upload_pack(repo)
client = Path.join(tmp_dir, "client")
hook = fn ref, _old, _new ->
if ref == "refs/heads/rejected",
do: {:error, :hook_rejects_this_ref},
else: :ok
end
try do
GitDaemon.seed_client_clone("git://127.0.0.1:#{upload_port}/repo", client)
after
stop_upload.()
{port, stop} = GitDaemon.start_receive_pack(repo, update_hook: hook)
end
try do
File.write!(Path.join(client, "a.txt"), "v2\n")
client = GitDaemon.init_client_dir(tmp_dir)
File.write!(Path.join(client, "a.txt"), "hi\n")
GitDaemon.git!(client, ["add", "a.txt"])
GitDaemon.git!(client, ["commit", "-m", "second"])
server_before = Ref.get(repo, "refs/heads/main")
GitDaemon.git!(client, ["commit", "-m", "first"])
# Push a good ref and an impossible ref name together. The
# "bad" ref update should take down the whole atomic batch.
{out, code} =
GitDaemon.git_at(client, [
"push",
"--atomic",
"git://127.0.0.1:#{port}/repo",
"main:refs/heads/accepted",
"main:refs/heads/main",
"main:refs/not-a-valid-prefix/xyz"
"main:refs/heads/rejected"
])
refute code == 0, "atomic push with a rejected ref should fail:\n#{out}"
assert {:error, _} = Ref.get(repo, "refs/heads/accepted"),
refute code == 0, "atomic push with bad ref should fail:\n#{out}"
"atomic rollback failed — `accepted` was applied despite batch failure"
assert {:error, _} = Ref.get(repo, "refs/heads/rejected")
assert Ref.get(repo, "refs/heads/main") == server_before,
"atomic batch was partially applied"
after
stop.()
end