Peel a tag whichever field the git backend put the commit in

efc4874 · Gabriel Morell · 2026-09-16 19:14

7 files +232 -7
Message
{commit_body(@commit)}

Files changed

modified lib/git_gud/workflows.ex
+18 −7
@@ -79,21 +79,32 @@ defmodule GitGud.Workflows do
79 79 # peeling is a lookup rather than another object read. A lightweight tag has
80 80 # no peeled target and needs none: its sha is already the commit.
81 81 defp peel(repo, ref, sha) do
82 − case Repositories.get_commit(repo, sha) do
83 − {:ok, _commit} -> sha
84 − _not_a_commit -> peeled_target(repo, ref) || sha
85 − end
82 + if commit?(repo, sha), do: sha, else: peeled_target(repo, ref) || sha
86 83 end
87 84
88 − defp peeled_target(%Repository{disk_path: path}, ref) do
85 + # The two git backends disagree about which field of a tag ref carries the
86 + # commit, so ask for both and take whichever one *is* a commit:
87 + #
88 + # CLI target_sha = the tag object, peeled_sha = the commit
89 + # gix target_sha = the commit (it peels), peeled_sha = nil (hardcoded,
90 + # native/gitgud_gix/src/lib.rs)
91 + #
92 + # Reading only peeled_sha worked in a CLI-backed test and did nothing on a
93 + # deployment running the NIF, which is a difference no caller should have to
94 + # know about and none of them should encode twice.
95 + defp peeled_target(%Repository{disk_path: path} = repo, ref) do
89 96 with {:ok, refs} <- Git.list_refs(path),
90 − %{peeled_sha: peeled} when is_binary(peeled) <- Enum.find(refs, &(&1.name == ref)) do
91 − peeled
97 + %{} = row <- Enum.find(refs, &(&1.name == ref)) do
98 + Enum.find_value([Map.get(row, :peeled_sha), Map.get(row, :target_sha)], fn candidate ->
99 + is_binary(candidate) and commit?(repo, candidate) and candidate
100 + end)
92 101 else
93 102 _ -> nil
94 103 end
95 104 end
96 105
106 + defp commit?(repo, sha), do: match?({:ok, _commit}, Repositories.get_commit(repo, sha))
107 +
97 108 @doc """
98 109 Walk the tree at `head_sha`, returning `[{workflow_path, parsed_doc}]`.
99 110 Workflows that fail to parse are dropped silently for now; in a later
added test/git_gud/workflows/tag_dispatch_test.exs
+214 −0
@@ -0,0 +1,214 @@
1 +defmodule GitGud.Workflows.TagDispatchTest do
2 + @moduledoc """
3 + Pushing a tag starts the workflows that asked for tags.
4 +
5 + This uses a repository on disk with a real annotated tag in it, because
6 + that is the only shape that reproduces what happened: `git push` of an
7 + annotated `v0.1.0` created no run at all, so a release published nothing
8 + and said nothing. A lightweight tag on the same commit worked, which is
9 + what made it look like a workflow problem rather than a forge one.
10 +
11 + Two layers had to be right and a unit test of either would have missed the
12 + other — the filter (`tags:` must be consulted, and `branches:` alone must
13 + not claim a tag) and the peel (an annotated tag's ref points at a tag
14 + object, and everything downstream wants the commit).
15 + """
16 + use GitGud.DataCase, async: false
17 +
18 + alias GitGud.Repositories.Repository
19 + alias GitGud.Workflows
20 + alias GitGud.Workflows.WorkflowRun
21 +
22 + import GitGud.ForgeFixtures
23 +
24 + @workflow """
25 + name: publish
26 + on:
27 + push:
28 + branches: [master]
29 + tags: ['v*']
30 + jobs:
31 + publish:
32 + runs-on: self-hosted
33 + steps:
34 + - run: echo hello
35 + """
36 +
37 + setup do
38 + {_user, repo} = repository_fixture()
39 + {:ok, repo: repo}
40 + end
41 +
42 + # Plumbing rather than a work tree: the repository is bare, and blob ->
43 + # tree -> commit is less machinery than cloning it somewhere to make one
44 + # commit.
45 + defp commit_workflow(%Repository{disk_path: path}, yaml) do
46 + blob = git!(path, ["hash-object", "-w", "--stdin"], yaml)
47 + workflows = mktree(path, [{"100644", "blob", blob, "ci.yml"}])
48 + forgejo = mktree(path, [{"040000", "tree", workflows, "workflows"}])
49 + root = mktree(path, [{"040000", "tree", forgejo, ".forgejo"}])
50 +
51 + git!(path, ["commit-tree", root, "-m", "add a workflow"])
52 + end
53 +
54 + defp mktree(path, entries) do
55 + input =
56 + Enum.map_join(entries, "", fn {mode, type, sha, name} ->
57 + "#{mode} #{type} #{sha}\t#{name}\n"
58 + end)
59 +
60 + git!(path, ["mktree"], input)
61 + end
62 +
63 + # System.cmd/3 has no stdin, and the plumbing above needs it, so the two
64 + # commands that read stdin get a temp file and a shell redirect.
65 + defp git!(path, args, input \\ nil) do
66 + {out, 0} =
67 + case input do
68 + nil ->
69 + System.cmd("git", args, cd: path, stderr_to_stdout: true)
70 +
71 + _ ->
72 + file = Path.join(System.tmp_dir!(), "gitin-#{System.unique_integer([:positive])}")
73 + File.write!(file, input)
74 +
75 + try do
76 + command = Enum.map_join(["git" | args] ++ ["<", file], " ", &quote_arg/1)
77 + System.cmd("sh", ["-c", command], cd: path, stderr_to_stdout: true)
78 + after
79 + File.rm(file)
80 + end
81 + end
82 +
83 + String.trim(out)
84 + end
85 +
86 + defp quote_arg("<"), do: "<"
87 + defp quote_arg(arg), do: "'" <> String.replace(arg, "'", "'\\''") <> "'"
88 +
89 + defp runs(%Repository{id: rid}) do
90 + from(r in WorkflowRun, where: r.repository_id == ^rid) |> Repo.all()
91 + end
92 +
93 + defp sha!(hex), do: elem(GitGud.Git.from_hex(hex), 1)
94 +
95 + describe "an annotated tag" do
96 + setup %{repo: repo} do
97 + commit = commit_workflow(repo, @workflow)
98 + git!(repo.disk_path, ["update-ref", "refs/heads/master", commit])
99 + git!(repo.disk_path, ["tag", "-a", "v0.1.0", "-m", "release 0.1.0", commit])
100 +
101 + tag_object = git!(repo.disk_path, ["rev-parse", "refs/tags/v0.1.0"])
102 +
103 + # The thing that makes this case special, asserted rather than assumed:
104 + # the ref points at a tag object, not at the commit.
105 + assert git!(repo.disk_path, ["cat-file", "-t", tag_object]) == "tag"
106 + refute tag_object == commit
107 +
108 + {:ok, commit: commit, tag_object: tag_object}
109 + end
110 +
111 + test "dispatches, and records the commit rather than the tag object", %{
112 + repo: repo,
113 + commit: commit,
114 + tag_object: tag_object
115 + } do
116 + {:ok, _} =
117 + Workflows.dispatch(repo, %{
118 + trigger: "push",
119 + head_sha: sha!(tag_object),
120 + ref: "refs/tags/v0.1.0"
121 + })
122 +
123 + assert [run] = runs(repo)
124 + assert run.workflow_name == "publish"
125 + assert run.head_ref == "refs/tags/v0.1.0"
126 +
127 + # A run whose head_sha is a tag object is one whose checkout fails.
128 + assert run.head_sha == sha!(commit)
129 + end
130 +
131 + # The invariant `Workflows.peeled_target/2` leans on, asserted against
132 + # whichever backend is live — because the two disagree about which field
133 + # holds the commit, and reading only one of them is what shipped a fix
134 + # that worked in a CLI-backed test and did nothing on a NIF-backed
135 + # deployment. Whichever field it is, one of them must be the commit.
136 + test "list_refs exposes the commit in one of its two sha fields", %{
137 + repo: repo,
138 + commit: commit
139 + } do
140 + {:ok, refs} = GitGud.Git.list_refs(repo.disk_path)
141 + row = Enum.find(refs, &(&1.name == "refs/tags/v0.1.0"))
142 +
143 + assert sha!(commit) in [Map.get(row, :peeled_sha), Map.get(row, :target_sha)]
144 + end
145 +
146 + test "is not claimed by a workflow that only names branches", %{
147 + repo: repo,
148 + tag_object: _tag_object
149 + } do
150 + branches_only = String.replace(@workflow, " tags: ['v*']\n", "")
151 + commit = commit_workflow(repo, branches_only)
152 + git!(repo.disk_path, ["update-ref", "refs/heads/master", commit])
153 + git!(repo.disk_path, ["tag", "-a", "-f", "v0.1.0", "-m", "again", commit])
154 + tag_object = git!(repo.disk_path, ["rev-parse", "refs/tags/v0.1.0"])
155 +
156 + {:ok, _} =
157 + Workflows.dispatch(repo, %{
158 + trigger: "push",
159 + head_sha: sha!(tag_object),
160 + ref: "refs/tags/v0.1.0"
161 + })
162 +
163 + assert runs(repo) == []
164 + end
165 + end
166 +
167 + describe "a lightweight tag" do
168 + test "still dispatches — its sha is already the commit", %{repo: repo} do
169 + commit = commit_workflow(repo, @workflow)
170 + git!(repo.disk_path, ["update-ref", "refs/tags/v0.2.0", commit])
171 +
172 + {:ok, _} =
173 + Workflows.dispatch(repo, %{
174 + trigger: "push",
175 + head_sha: sha!(commit),
176 + ref: "refs/tags/v0.2.0"
177 + })
178 +
179 + assert [run] = runs(repo)
180 + assert run.head_sha == sha!(commit)
181 + end
182 + end
183 +
184 + describe "a branch push" do
185 + test "is unaffected", %{repo: repo} do
186 + commit = commit_workflow(repo, @workflow)
187 + git!(repo.disk_path, ["update-ref", "refs/heads/master", commit])
188 +
189 + {:ok, _} =
190 + Workflows.dispatch(repo, %{
191 + trigger: "push",
192 + head_sha: sha!(commit),
193 + ref: "refs/heads/master"
194 + })
195 +
196 + assert [run] = runs(repo)
197 + assert run.head_ref == "refs/heads/master"
198 + end
199 +
200 + test "to a branch the filter does not name creates nothing", %{repo: repo} do
201 + commit = commit_workflow(repo, @workflow)
202 + git!(repo.disk_path, ["update-ref", "refs/heads/topic", commit])
203 +
204 + {:ok, _} =
205 + Workflows.dispatch(repo, %{
206 + trigger: "push",
207 + head_sha: sha!(commit),
208 + ref: "refs/heads/topic"
209 + })
210 +
211 + assert runs(repo) == []
212 + end
213 + end
214 +end

Parents: f9f8d13