fix(ci): Deploy detects changed images from the pushed diff, not head_commit.modified
Some checks failed
CI / lint (pull_request) Successful in 34s
CI / notebooks-smoke (pull_request) Successful in 1m29s
CI / test (pull_request) Failing after 2m8s
CI / lint (push) Successful in 34s
CI / test (push) Failing after 2m13s
CI / notebooks-smoke (push) Successful in 1m41s
Some checks failed
CI / lint (pull_request) Successful in 34s
CI / notebooks-smoke (pull_request) Successful in 1m29s
CI / test (pull_request) Failing after 2m8s
CI / lint (push) Successful in 34s
CI / test (push) Failing after 2m13s
CI / notebooks-smoke (push) Successful in 1m41s
The per-image jobs in deploy.yml were gated on contains(github.event.head_commit.modified, …). That list only describes the tip commit of a push and Gitea leaves it empty for merge commits, so every PR landed with a merge commit skipped all six image builds; the llm image for #1120 had to be built and deployed by hand. A new 'changes' job fetches the push's previous tip (github.event.before) into the shallow checkout and diffs it against the new tip, so the whole pushed range counts: a merge commit's branch, or all commits of a direct multi-commit push. Each image job now needs that job and gates on its boolean output; an image without path filters follows every push. When the previous tip is unknown or unfetchable (first push, history rewrite) every image builds rather than none. Tests cover the generated shape and run the detector script in a scratch repository for the merge-commit, multi-commit and unknown-tip cases.
This commit is contained in:
@@ -8,12 +8,64 @@ on:
|
||||
branches: [main]
|
||||
|
||||
jobs:
|
||||
changes:
|
||||
runs-on: ubuntu-latest
|
||||
outputs:
|
||||
notebooks: ${{ steps.filter.outputs.notebooks }}
|
||||
zotero: ${{ steps.filter.outputs.zotero }}
|
||||
docs: ${{ steps.filter.outputs.docs }}
|
||||
api: ${{ steps.filter.outputs.api }}
|
||||
llm: ${{ steps.filter.outputs.llm }}
|
||||
mc: ${{ steps.filter.outputs.mc }}
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: https://github.com/actions/checkout@v4
|
||||
|
||||
- name: Detect changed paths
|
||||
id: filter
|
||||
env:
|
||||
BEFORE: ${{ github.event.before }}
|
||||
run: |
|
||||
# `github.event.head_commit.modified` only describes the tip commit
|
||||
# and is empty for merge commits, so a PR merged with a merge commit
|
||||
# never matched any path filter. Diff the pushed range instead:
|
||||
# the branch tip before the push against the new one. When that
|
||||
# previous tip is unknown or unfetchable (first push, history
|
||||
# rewrite) build everything rather than nothing.
|
||||
changed=__all__
|
||||
if [ -n "$BEFORE" ] && [ "$BEFORE" != "0000000000000000000000000000000000000000" ] \
|
||||
&& git fetch --quiet --depth=1 origin "$BEFORE"; then
|
||||
changed="$(git diff --name-only "$BEFORE" "$GITHUB_SHA")" || changed=__all__
|
||||
fi
|
||||
echo "changed since $BEFORE:"
|
||||
printf '%s\n' "$changed"
|
||||
match() {
|
||||
name="$1"; shift
|
||||
hit=false
|
||||
if [ "$changed" = __all__ ]; then
|
||||
hit=true
|
||||
else
|
||||
while IFS= read -r f; do
|
||||
[ -n "$f" ] || continue
|
||||
for g in "$@"; do
|
||||
case "$f" in $g) hit=true ;; esac
|
||||
done
|
||||
done <<< "$changed"
|
||||
fi
|
||||
echo "$name=$hit" >> "$GITHUB_OUTPUT"
|
||||
echo "$name: $hit"
|
||||
}
|
||||
match notebooks 'notebooks/*' 'infra/marimo/*' 'infra/images/notebooks.Dockerfile'
|
||||
match zotero 'infra/images/zotero.Dockerfile' 'infra/images/zotero/*'
|
||||
match docs 'docs/*' 'infra/images/docs.Dockerfile'
|
||||
match api 'infra/images/api.Dockerfile' 'src/*' 'pyproject.toml'
|
||||
match llm 'infra/images/llm.Dockerfile' 'src/*' 'pyproject.toml'
|
||||
match mc 'infra/rustfs/*' 'infra/images/mc.Dockerfile'
|
||||
|
||||
notebooks:
|
||||
runs-on: ubuntu-latest
|
||||
if: >-
|
||||
contains(github.event.head_commit.modified, 'notebooks') ||
|
||||
contains(github.event.head_commit.modified, 'infra/marimo') ||
|
||||
contains(github.event.head_commit.modified, 'infra/images/notebooks.Dockerfile')
|
||||
needs: [changes]
|
||||
if: needs.changes.outputs.notebooks == 'true'
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: https://github.com/actions/checkout@v4
|
||||
@@ -64,9 +116,8 @@ jobs:
|
||||
|
||||
zotero:
|
||||
runs-on: ubuntu-latest
|
||||
if: >-
|
||||
contains(github.event.head_commit.modified, 'infra/images/zotero.Dockerfile') ||
|
||||
contains(github.event.head_commit.modified, 'infra/images/zotero')
|
||||
needs: [changes]
|
||||
if: needs.changes.outputs.zotero == 'true'
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: https://github.com/actions/checkout@v4
|
||||
@@ -117,9 +168,8 @@ jobs:
|
||||
|
||||
docs:
|
||||
runs-on: ubuntu-latest
|
||||
if: >-
|
||||
contains(github.event.head_commit.modified, 'docs') ||
|
||||
contains(github.event.head_commit.modified, 'infra/images/docs.Dockerfile')
|
||||
needs: [changes]
|
||||
if: needs.changes.outputs.docs == 'true'
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: https://github.com/actions/checkout@v4
|
||||
@@ -170,10 +220,8 @@ jobs:
|
||||
|
||||
api:
|
||||
runs-on: ubuntu-latest
|
||||
if: >-
|
||||
contains(github.event.head_commit.modified, 'infra/images/api.Dockerfile') ||
|
||||
contains(github.event.head_commit.modified, 'src') ||
|
||||
contains(github.event.head_commit.modified, 'pyproject.toml')
|
||||
needs: [changes]
|
||||
if: needs.changes.outputs.api == 'true'
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: https://github.com/actions/checkout@v4
|
||||
@@ -224,10 +272,8 @@ jobs:
|
||||
|
||||
llm:
|
||||
runs-on: ubuntu-latest
|
||||
if: >-
|
||||
contains(github.event.head_commit.modified, 'infra/images/llm.Dockerfile') ||
|
||||
contains(github.event.head_commit.modified, 'src') ||
|
||||
contains(github.event.head_commit.modified, 'pyproject.toml')
|
||||
needs: [changes]
|
||||
if: needs.changes.outputs.llm == 'true'
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: https://github.com/actions/checkout@v4
|
||||
@@ -278,9 +324,8 @@ jobs:
|
||||
|
||||
mc:
|
||||
runs-on: ubuntu-latest
|
||||
if: >-
|
||||
contains(github.event.head_commit.modified, 'infra/rustfs') ||
|
||||
contains(github.event.head_commit.modified, 'infra/images/mc.Dockerfile')
|
||||
needs: [changes]
|
||||
if: needs.changes.outputs.mc == 'true'
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: https://github.com/actions/checkout@v4
|
||||
@@ -325,7 +370,7 @@ jobs:
|
||||
|
||||
report:
|
||||
runs-on: ubuntu-latest
|
||||
needs: [notebooks, zotero, docs, api, llm, mc]
|
||||
needs: [changes, notebooks, zotero, docs, api, llm, mc]
|
||||
if: always() && !cancelled()
|
||||
steps:
|
||||
- name: Checkout
|
||||
|
||||
@@ -230,6 +230,89 @@ jobs:
|
||||
return (".gitea/workflows/ci.yml", content)
|
||||
|
||||
|
||||
_CHANGES_JOB = "changes"
|
||||
_CHANGES_STEP_ID = "filter"
|
||||
_ALL_CHANGED = "__all__"
|
||||
_NULL_SHA = "0" * 40
|
||||
|
||||
# Shell body of the change-detection step; the braces of the `match`
|
||||
# function are doubled so they survive the f-string.
|
||||
_CHANGES_SCRIPT = f"""\
|
||||
# `github.event.head_commit.modified` only describes the tip commit
|
||||
# and is empty for merge commits, so a PR merged with a merge commit
|
||||
# never matched any path filter. Diff the pushed range instead:
|
||||
# the branch tip before the push against the new one. When that
|
||||
# previous tip is unknown or unfetchable (first push, history
|
||||
# rewrite) build everything rather than nothing.
|
||||
changed={_ALL_CHANGED}
|
||||
if [ -n "$BEFORE" ] && [ "$BEFORE" != "{_NULL_SHA}" ] \\
|
||||
&& git fetch --quiet --depth=1 origin "$BEFORE"; then
|
||||
changed="$(git diff --name-only "$BEFORE" "$GITHUB_SHA")" || changed={_ALL_CHANGED}
|
||||
fi
|
||||
echo "changed since $BEFORE:"
|
||||
printf '%s\\n' "$changed"
|
||||
match() {{
|
||||
name="$1"; shift
|
||||
hit=false
|
||||
if [ "$changed" = {_ALL_CHANGED} ]; then
|
||||
hit=true
|
||||
else
|
||||
while IFS= read -r f; do
|
||||
[ -n "$f" ] || continue
|
||||
for g in "$@"; do
|
||||
case "$f" in $g) hit=true ;; esac
|
||||
done
|
||||
done <<< "$changed"
|
||||
fi
|
||||
echo "$name=$hit" >> "$GITHUB_OUTPUT"
|
||||
echo "$name: $hit"
|
||||
}}"""
|
||||
|
||||
|
||||
def _case_glob(pattern: str) -> str:
|
||||
"""Translate a ``path_filter`` glob into a bash ``case`` pattern.
|
||||
|
||||
``*`` already crosses ``/`` in ``case``, so ``src/**`` becomes ``src/*``.
|
||||
"""
|
||||
while "**" in pattern:
|
||||
pattern = pattern.replace("**", "*")
|
||||
return pattern
|
||||
|
||||
|
||||
def _changes_job(images: list[dict], runner: str) -> str:
|
||||
"""Emit the job that turns the pushed diff into one boolean per image."""
|
||||
outputs = "\n".join(
|
||||
f" {img['name']}: "
|
||||
f"${{{{ steps.{_CHANGES_STEP_ID}.outputs.{img['name']} }}}}"
|
||||
for img in images
|
||||
)
|
||||
matches = []
|
||||
for img in images:
|
||||
globs = [_case_glob(p) for p in img.get("path_filter", [])]
|
||||
if globs:
|
||||
quoted = " ".join(f"'{g}'" for g in globs)
|
||||
matches.append(f" match {img['name']} {quoted}")
|
||||
else:
|
||||
# No filter means the image follows every push.
|
||||
matches.append(f' echo "{img["name"]}=true" >> "$GITHUB_OUTPUT"')
|
||||
match_block = "\n".join(matches)
|
||||
return f"""\
|
||||
{_CHANGES_JOB}:
|
||||
runs-on: {runner}
|
||||
outputs:
|
||||
{outputs}
|
||||
steps:
|
||||
{_checkout_step()}
|
||||
|
||||
- name: Detect changed paths
|
||||
id: {_CHANGES_STEP_ID}
|
||||
env:
|
||||
BEFORE: ${{{{ github.event.before }}}}
|
||||
run: |
|
||||
{_CHANGES_SCRIPT}
|
||||
{match_block}"""
|
||||
|
||||
|
||||
def _gen_deploy(
|
||||
images: list[dict],
|
||||
scans: list[dict],
|
||||
@@ -241,23 +324,14 @@ def _gen_deploy(
|
||||
) -> tuple[str, str]:
|
||||
scan_set = {s["name"] for s in scans}
|
||||
|
||||
# Generate one job per image with path filters
|
||||
job_blocks = []
|
||||
job_names = []
|
||||
# Gitea Actions doesn't support `paths:` filters on push, so one job
|
||||
# computes which images the pushed range touches and the per-image
|
||||
# jobs gate on its outputs.
|
||||
job_blocks = [_changes_job(images, runner)]
|
||||
job_names = [_CHANGES_JOB]
|
||||
for img in images:
|
||||
name = img["name"]
|
||||
job_names.append(name)
|
||||
paths = img.get("path_filter", [])
|
||||
|
||||
# Gitea Actions doesn't support `paths:` on push without branches,
|
||||
# so use `if: contains(...)` on the modified files list.
|
||||
def _clean_path(p: str) -> str:
|
||||
return p.rstrip("*").rstrip("/")
|
||||
|
||||
path_checks = " ||\n ".join(
|
||||
f"contains(github.event.head_commit.modified, '{_clean_path(p)}')"
|
||||
for p in paths
|
||||
)
|
||||
tags = (
|
||||
f"{registry}/{owner_repo}/{name}:${{{{ env.SHORT_SHA }}}},"
|
||||
f"{registry}/{owner_repo}/{name}:latest"
|
||||
@@ -273,8 +347,8 @@ def _gen_deploy(
|
||||
job_blocks.append(f"""\
|
||||
{name}:
|
||||
runs-on: {runner}
|
||||
if: >-
|
||||
{path_checks}
|
||||
needs: [{_CHANGES_JOB}]
|
||||
if: needs.{_CHANGES_JOB}.outputs.{name} == 'true'
|
||||
steps:
|
||||
{_checkout_step()}
|
||||
|
||||
|
||||
211
tests/dev/test_gen_config_deploy.py
Normal file
211
tests/dev/test_gen_config_deploy.py
Normal file
@@ -0,0 +1,211 @@
|
||||
"""Tests for the generated Deploy workflow's change detection.
|
||||
|
||||
``dev/scripts/backends/gitea.py::_gen_deploy`` used to gate every image
|
||||
job on ``contains(github.event.head_commit.modified, ...)``. That list
|
||||
only describes the tip commit of a push and is empty for merge commits,
|
||||
so a PR landed with a merge commit never rebuilt any image. The workflow
|
||||
now has a ``changes`` job that diffs the pushed range (``before`` → the
|
||||
new tip) and the image jobs gate on its outputs.
|
||||
|
||||
The shape tests guard the generated YAML; the behavioural tests run the
|
||||
generated detector script in a scratch git repository so the shell logic
|
||||
is exercised for real (merge commit, direct push, unknown previous tip).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
|
||||
_DEV_SCRIPTS = Path(__file__).resolve().parents[2] / "dev" / "scripts"
|
||||
if str(_DEV_SCRIPTS) not in sys.path:
|
||||
sys.path.insert(0, str(_DEV_SCRIPTS))
|
||||
|
||||
from backends import gitea # noqa: E402
|
||||
|
||||
_IMAGES = [
|
||||
{
|
||||
"name": "api",
|
||||
"dockerfile": "infra/images/api.Dockerfile",
|
||||
"context": ".",
|
||||
"path_filter": ["infra/images/api.Dockerfile", "src/**", "pyproject.toml"],
|
||||
},
|
||||
{
|
||||
"name": "mc",
|
||||
"dockerfile": "infra/images/mc.Dockerfile",
|
||||
"context": "infra/rustfs/",
|
||||
"path_filter": ["infra/rustfs/**", "infra/images/mc.Dockerfile"],
|
||||
},
|
||||
{
|
||||
"name": "always",
|
||||
"dockerfile": "infra/images/always.Dockerfile",
|
||||
"context": ".",
|
||||
},
|
||||
]
|
||||
|
||||
|
||||
def _gen() -> tuple[str, dict]:
|
||||
path, content = gitea._gen_deploy(
|
||||
_IMAGES,
|
||||
[_IMAGES[0]],
|
||||
"git:3000",
|
||||
"homelab/stack",
|
||||
"ubuntu-latest",
|
||||
"latest",
|
||||
)
|
||||
return path, yaml.safe_load(content)
|
||||
|
||||
|
||||
def test_case_glob_collapses_double_star():
|
||||
assert gitea._case_glob("src/**") == "src/*"
|
||||
assert gitea._case_glob("infra/rustfs/**") == "infra/rustfs/*"
|
||||
assert gitea._case_glob("pyproject.toml") == "pyproject.toml"
|
||||
assert gitea._case_glob("a/**/**/b") == "a/*/*/b"
|
||||
|
||||
|
||||
def test_changes_job_exposes_one_output_per_image():
|
||||
path, wf = _gen()
|
||||
assert path == ".gitea/workflows/deploy.yml"
|
||||
changes = wf["jobs"]["changes"]
|
||||
assert changes["outputs"] == {
|
||||
"api": "${{ steps.filter.outputs.api }}",
|
||||
"mc": "${{ steps.filter.outputs.mc }}",
|
||||
"always": "${{ steps.filter.outputs.always }}",
|
||||
}
|
||||
step = next(s for s in changes["steps"] if s.get("id") == "filter")
|
||||
assert step["env"] == {"BEFORE": "${{ github.event.before }}"}
|
||||
assert (
|
||||
"match api 'infra/images/api.Dockerfile' 'src/*' 'pyproject.toml'"
|
||||
in (step["run"])
|
||||
)
|
||||
assert "match mc 'infra/rustfs/*' 'infra/images/mc.Dockerfile'" in step["run"]
|
||||
# An image without path filters follows every push.
|
||||
assert 'echo "always=true" >> "$GITHUB_OUTPUT"' in step["run"]
|
||||
|
||||
|
||||
def test_image_jobs_gate_on_changes_outputs_not_head_commit():
|
||||
_, wf = _gen()
|
||||
for name in ("api", "mc", "always"):
|
||||
job = wf["jobs"][name]
|
||||
assert job["needs"] == ["changes"]
|
||||
assert job["if"] == f"needs.changes.outputs.{name} == 'true'"
|
||||
assert wf["jobs"]["report"]["needs"] == ["changes", "api", "mc", "always"]
|
||||
_, content = gitea._gen_deploy(
|
||||
_IMAGES, [], "git:3000", "homelab/stack", "ubuntu-latest", "latest"
|
||||
)
|
||||
assert "contains(github.event.head_commit" not in content
|
||||
|
||||
|
||||
# ── Behavioural: run the detector script against real git history ──────
|
||||
|
||||
|
||||
def _git(*args: str, cwd: Path) -> str:
|
||||
env = {
|
||||
**os.environ,
|
||||
"GIT_AUTHOR_NAME": "t",
|
||||
"GIT_AUTHOR_EMAIL": "t@example.invalid",
|
||||
"GIT_COMMITTER_NAME": "t",
|
||||
"GIT_COMMITTER_EMAIL": "t@example.invalid",
|
||||
}
|
||||
return subprocess.run(
|
||||
["git", *args], cwd=cwd, env=env, check=True, capture_output=True, text=True
|
||||
).stdout.strip()
|
||||
|
||||
|
||||
def _commit_file(repo: Path, rel: str, msg: str) -> str:
|
||||
target = repo / rel
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
target.write_text(f"{msg}\n")
|
||||
_git("add", "-A", cwd=repo)
|
||||
_git("commit", "-q", "-m", msg, cwd=repo)
|
||||
return _git("rev-parse", "HEAD", cwd=repo)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def history(tmp_path: Path) -> dict[str, object]:
|
||||
"""A bare 'origin' whose main has: base → (direct push) → merge commit.
|
||||
|
||||
Returns the SHAs a push event would carry for each push plus a
|
||||
shallow clone of the final main, as the runner's checkout leaves it.
|
||||
"""
|
||||
origin = tmp_path / "origin.git"
|
||||
_git("init", "-q", "--bare", "-b", "main", str(origin), cwd=tmp_path)
|
||||
# Shallow fetches by raw SHA need the server's consent; Gitea grants
|
||||
# it via protocol v2, the local file transport must be told.
|
||||
_git("config", "uploadpack.allowAnySHA1InWant", "true", cwd=origin)
|
||||
|
||||
work = tmp_path / "work"
|
||||
_git("clone", "-q", str(origin), str(work), cwd=tmp_path)
|
||||
base = _commit_file(work, "README.md", "base")
|
||||
_commit_file(work, "docs/index.md", "docs only")
|
||||
direct = _commit_file(work, "infra/rustfs/policy.json", "mc config")
|
||||
_git("push", "-q", "origin", "main", cwd=work)
|
||||
|
||||
_git("checkout", "-q", "-b", "feature", cwd=work)
|
||||
_commit_file(work, "src/llm/rag.py", "feature: rag")
|
||||
_commit_file(work, "tests/llm/test_rag.py", "feature: test")
|
||||
_git("checkout", "-q", "main", cwd=work)
|
||||
_git("merge", "-q", "--no-ff", "-m", "merge feature", "feature", cwd=work)
|
||||
merge = _git("rev-parse", "HEAD", cwd=work)
|
||||
_git("push", "-q", "origin", "main", cwd=work)
|
||||
|
||||
shallow = tmp_path / "shallow"
|
||||
_git("clone", "-q", "--depth", "1", str(origin), str(shallow), cwd=tmp_path)
|
||||
assert _git("rev-parse", "HEAD", cwd=shallow) == merge
|
||||
|
||||
return {"base": base, "direct": direct, "merge": merge, "shallow": shallow}
|
||||
|
||||
|
||||
def _run_detector(checkout: Path, before: str, sha: str, tmp_path: Path) -> dict:
|
||||
_, wf = _gen()
|
||||
step = next(s for s in wf["jobs"]["changes"]["steps"] if s.get("id") == "filter")
|
||||
out_file = tmp_path / "github_output"
|
||||
out_file.write_text("")
|
||||
proc = subprocess.run(
|
||||
["bash", "-e", "-c", step["run"]],
|
||||
cwd=checkout,
|
||||
env={
|
||||
**os.environ,
|
||||
"BEFORE": before,
|
||||
"GITHUB_SHA": sha,
|
||||
"GITHUB_OUTPUT": str(out_file),
|
||||
},
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
assert proc.returncode == 0, proc.stdout + proc.stderr
|
||||
outputs = dict(
|
||||
line.split("=", 1) for line in out_file.read_text().splitlines() if line
|
||||
)
|
||||
return {"outputs": outputs, "log": proc.stdout}
|
||||
|
||||
|
||||
def test_merge_commit_push_rebuilds_images_the_branch_touched(history, tmp_path):
|
||||
"""The push that lands a merge commit must see every file the branch
|
||||
brought in, which `head_commit.modified` never did."""
|
||||
res = _run_detector(
|
||||
history["shallow"], history["direct"], history["merge"], tmp_path
|
||||
)
|
||||
assert res["outputs"] == {"api": "true", "mc": "false", "always": "true"}
|
||||
assert "src/llm/rag.py" in res["log"]
|
||||
assert "tests/llm/test_rag.py" in res["log"]
|
||||
|
||||
|
||||
def test_multi_commit_direct_push_covers_the_whole_range(history, tmp_path):
|
||||
"""A push of several plain commits is judged on all of them, not
|
||||
only the tip (the tip here touched infra/rustfs, an earlier one docs)."""
|
||||
shallow = history["shallow"]
|
||||
res = _run_detector(shallow, history["base"], history["direct"], tmp_path)
|
||||
assert res["outputs"] == {"api": "false", "mc": "true", "always": "true"}
|
||||
assert "docs/index.md" in res["log"]
|
||||
|
||||
|
||||
def test_unknown_previous_tip_builds_everything(history, tmp_path):
|
||||
for before in ("", "0" * 40, "f" * 40):
|
||||
res = _run_detector(history["shallow"], before, history["merge"], tmp_path)
|
||||
assert res["outputs"] == {"api": "true", "mc": "true", "always": "true"}, before
|
||||
Reference in New Issue
Block a user