GITGUD

neiam neiam /gitgud

Git Gud

public · Issues · Pulls · Labels · Forks · Compare · Actions success · Badges · Packages

⭐ 1

Log in to mark this repository.

Run workflows for annotated tags, and honour the filter that names them

f9f8d13 · Gabriel Morell · 2026-09-16 19:00

✓ build ✓ hello
9 files +169 -21
Message 
{commit_body(@commit)}

Files changed

modified lib/git_gud/workflows.ex
+31 −4
@@ -39,8 +39,8 @@ defmodule GitGud.Workflows do
39 39 def dispatch(%Repository{} = repo, event) do
40 40 %{trigger: trigger, head_sha: head_sha, ref: ref} = event
41 41 triggered_by_id = Map.get(event, :triggered_by_id)
42 − branch = branch_from_ref(ref)
43 − ctx = %{branch: branch}
42 + ctx = ref_context(ref)
43 + head_sha = peel(repo, ref, head_sha)
44 44
45 45 workflows = discover_workflows(repo, head_sha)
46 46
@@ -64,8 +64,35 @@ defmodule GitGud.Workflows do
64 64 {:ok, runs}
65 65 end
66 66
67 − defp branch_from_ref("refs/heads/" <> b), do: b
68 − defp branch_from_ref(_), do: nil
67 + defp ref_context("refs/heads/" <> branch), do: %{branch: branch}
68 + defp ref_context("refs/tags/" <> tag), do: %{tag: tag}
69 + defp ref_context(_), do: %{}
70 +
71 + # An annotated tag's ref points at a *tag object*, not at a commit, and
72 + # everything downstream wants the commit: `discover_workflows` reads the
73 + # tree, the run records `head_sha`, and the runner checks it out. Before this,
74 + # pushing an annotated tag silently did nothing at all — `get_commit` cannot
75 + # read a tag object, so no workflows were discovered and no run was created,
76 + # while a lightweight tag on the same commit worked fine.
77 + #
78 + # `list_refs` carries the peeled target (`git for-each-ref` resolves it), so
79 + # peeling is a lookup rather than another object read. A lightweight tag has
80 + # no peeled target and needs none: its sha is already the commit.
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
86 + end
87 +
88 + defp peeled_target(%Repository{disk_path: path}, ref) do
89 + with {:ok, refs} <- Git.list_refs(path),
90 + %{peeled_sha: peeled} when is_binary(peeled) <- Enum.find(refs, &(&1.name == ref)) do
91 + peeled
92 + else
93 + _ -> nil
94 + end
95 + end
69 96
70 97 @doc """
71 98 Walk the tree at `head_sha`, returning `[{workflow_path, parsed_doc}]`.
modified lib/git_gud/workflows/yaml.ex
+30 −17
@@ -98,34 +98,47 @@ defmodule GitGud.Workflows.Yaml do
98 98
99 99 `event` is one of `"push"`, `"pull_request"`, `"manual"`. `ctx` is a
100 100 map with optional keys: `:branch`, `:tag`. Branch/tag filters
101 − (`branches:`, `tags:`, `paths:`) are honored loosely — wildcards via
102 − `:fnmatch`.
101 + (`branches:`, `tags:`) are honored with GitHub's semantics: naming one
102 + excludes the other kind of ref entirely. Wildcards via `:fnmatch`.
103 103 """
104 104 def matches?(%{triggers: triggers}, event, ctx \\ %{}) when is_map(triggers) do
105 − case Map.get(triggers, event) do
106 − nil -> false
107 − %{} = config -> branch_match?(config, ctx) and tag_match?(config, ctx)
108 − _ -> true
105 + # `Map.fetch`, not `Map.get`: a bare `on: push:` with nothing under it
106 + # parses to %{"push" => nil}, and reading that as "no such trigger" made a
107 + # workflow that asked for every push get none at all.
108 + case Map.fetch(triggers, event) do
109 + :error -> false
110 + {:ok, %{} = config} -> ref_match?(config, ctx)
111 + {:ok, _no_filters} -> true
109 112 end
110 113 end
111 114
112 − defp branch_match?(config, %{branch: branch}) when is_binary(branch) do
113 − case Map.get(config, "branches") do
114 − nil -> true
115 − patterns -> Enum.any?(List.wrap(patterns), &glob_match?(&1, branch))
115 + # A push is to a branch or to a tag, never both, so the two filters are one
116 + # decision rather than two independent ones.
117 + #
118 + # The rule GitHub uses, and the one people write workflows against: naming
119 + # *either* filter excludes everything the other kind of ref would match. So
120 + # `branches: [master]` alone does not fire on a tag, and `tags: ['v*']` alone
121 + # does not fire on a branch. Treating an absent `tags:` as "no tag filter,
122 + # therefore any tag" instead makes `branches: [master]` fire on every tag
123 + # push — which is what this did before, and it is never what was meant.
124 + defp ref_match?(config, %{tag: tag}) when is_binary(tag) do
125 + case {Map.get(config, "tags"), Map.get(config, "branches")} do
126 + {nil, nil} -> true
127 + {nil, _branches_only} -> false
128 + {patterns, _} -> Enum.any?(List.wrap(patterns), &glob_match?(&1, tag))
116 129 end
117 130 end
118 131
119 − defp branch_match?(_, _), do: true
120 −
121 − defp tag_match?(config, %{tag: tag}) when is_binary(tag) do
122 − case Map.get(config, "tags") do
123 − nil -> true
124 − patterns -> Enum.any?(List.wrap(patterns), &glob_match?(&1, tag))
132 + defp ref_match?(config, %{branch: branch}) when is_binary(branch) do
133 + case {Map.get(config, "branches"), Map.get(config, "tags")} do
134 + {nil, nil} -> true
135 + {nil, _tags_only} -> false
136 + {patterns, _} -> Enum.any?(List.wrap(patterns), &glob_match?(&1, branch))
125 137 end
126 138 end
127 139
128 − defp tag_match?(_, _), do: true
140 + # No ref in the context (a manual run, say): nothing to filter on.
141 + defp ref_match?(_config, _ctx), do: true
129 142
130 143 defp glob_match?(pattern, str) do
131 144 # Cheap fnmatch: treat * as .*, ? as ., everything else literal.
added test/git_gud/workflows/trigger_filters_test.exs
+108 −0
@@ -0,0 +1,108 @@
1 +defmodule GitGud.Workflows.TriggerFiltersTest do
2 + @moduledoc """
3 + Which pushes a workflow's `on:` actually claims.
4 +
5 + Written after a tag push published nothing: `git push gg v0.1.0` created no
6 + run at all, and the workflow it should have started was the one that
7 + publishes a crate. Two separate faults, one symptom — the peeling half lives
8 + in `GitGud.Workflows.dispatch/2` and needs a repository on disk; this covers
9 + the filter half, which is pure.
10 +
11 + The rule is GitHub's, and the reason it is not "each filter is independent"
12 + is the common workflow shape: `branches: [master]` with no `tags:` key must
13 + not fire on every tag push, which is what treating an absent `tags:` as "any
14 + tag" does.
15 + """
16 + use ExUnit.Case, async: true
17 +
18 + alias GitGud.Workflows.Yaml
19 +
20 + defp doc(on) do
21 + {:ok, doc} =
22 + Yaml.parse("""
23 + name: ci
24 + on:
25 + #{on}
26 + jobs:
27 + build:
28 + runs-on: self-hosted
29 + steps:
30 + - run: true
31 + """)
32 +
33 + doc
34 + end
35 +
36 + @both """
37 + push:
38 + branches: [master]
39 + tags: ['v*']
40 + """
41 +
42 + @branches_only """
43 + push:
44 + branches: [master]
45 + """
46 +
47 + @tags_only """
48 + push:
49 + tags: ['v*']
50 + """
51 +
52 + @bare """
53 + push:
54 + """
55 +
56 + describe "a branch push" do
57 + test "matches a branch pattern, and only that pattern" do
58 + assert Yaml.matches?(doc(@branches_only), "push", %{branch: "master"})
59 + refute Yaml.matches?(doc(@branches_only), "push", %{branch: "topic"})
60 + end
61 +
62 + test "matches when both filters are named" do
63 + assert Yaml.matches?(doc(@both), "push", %{branch: "master"})
64 + refute Yaml.matches?(doc(@both), "push", %{branch: "topic"})
65 + end
66 +
67 + test "does not match a workflow that only names tags" do
68 + refute Yaml.matches?(doc(@tags_only), "push", %{branch: "master"})
69 + end
70 +
71 + test "matches when no filter is named at all" do
72 + assert Yaml.matches?(doc(@bare), "push", %{branch: "anything"})
73 + end
74 + end
75 +
76 + describe "a tag push" do
77 + test "matches a tag pattern, and only that pattern" do
78 + assert Yaml.matches?(doc(@tags_only), "push", %{tag: "v0.1.0"})
79 + refute Yaml.matches?(doc(@tags_only), "push", %{tag: "nightly"})
80 + end
81 +
82 + test "matches when both filters are named" do
83 + assert Yaml.matches?(doc(@both), "push", %{tag: "v1.2.3"})
84 + refute Yaml.matches?(doc(@both), "push", %{tag: "release-1"})
85 + end
86 +
87 + # The regression this file exists for, from the other side: a workflow
88 + # that names only branches used to fire on every tag push, because an
89 + # absent `tags:` key read as "no tag filter, so any tag".
90 + test "does not match a workflow that only names branches" do
91 + refute Yaml.matches?(doc(@branches_only), "push", %{tag: "v0.1.0"})
92 + end
93 +
94 + test "matches when no filter is named at all" do
95 + assert Yaml.matches?(doc(@bare), "push", %{tag: "v0.1.0"})
96 + end
97 + end
98 +
99 + describe "other events" do
100 + test "an event the workflow does not name never matches" do
101 + refute Yaml.matches?(doc(@both), "pull_request", %{branch: "master"})
102 + end
103 +
104 + test "a context with no ref in it filters on nothing" do
105 + assert Yaml.matches?(doc(@branches_only), "push", %{})
106 + end
107 + end
108 +end

Parents: b0551cd