Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ Does this change need to be included in patch version releases? By default, any
If yes, please explain why:
[Explain the criticality of this change and why it should be included in patch releases]

## Template Changes
A change to `template/` reaches future minor versions only if it is also made in the newest minor's template (the highest `template/vX/vX.Y/`). See "Deciding where to make your change" in CONTRIBUTING.md.
- [ ] N/A (no template changes)
- [ ] The change is also made in the newest minor's template
- [ ] The change is deliberately limited to older minors (a maintainer adds the `template-propagation-scoped` label)

## How Has This Been Tested?
[Describe the tests you ran]

Expand Down
29 changes: 29 additions & 0 deletions .github/workflows/check-template-propagation.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: Check template propagation
# Fails when a pull request changes an older minor's template without changing the newest minor's.
# Meant to be required on main through a ruleset. See "Deciding where to make your change" in CONTRIBUTING.md.
on:
pull_request:
branches: [main]
# labeled/unlabeled rerun the check when a maintainer adds or removes the template-propagation-scoped label.
# No paths filter: a required check that never reports leaves the pull request pending forever. The check
# passes straight away when no template files change.
types: [opened, synchronize, reopened, labeled, unlabeled]
jobs:
check:
name: Check template propagation
runs-on: ubuntu-latest
permissions:
contents: read
steps:
- uses: actions/checkout@v6
with:
# The check needs the base commit's history to diff against, but only file names, not contents.
fetch-depth: 0
filter: blob:none
- name: Check
env:
BASE_SHA: ${{ github.event.pull_request.base.sha }}
# For pull_request events, github.sha is the pull request merged into main.
HEAD_SHA: ${{ github.sha }}
SCOPED: ${{ contains(github.event.pull_request.labels.*.name, 'template-propagation-scoped') }}
run: python3 .github/workflows/utils/check_template_propagation.py
92 changes: 92 additions & 0 deletions .github/workflows/utils/check_template_propagation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
"""Fail a pull request that changes an older minor's template without changing the newest minor's.

Each minor version has its own template, template/vX/vX.Y/. A new minor's template is a one-time copy of
the previous minor's, and nothing carries later changes forward. So a change made only to an older minor's
template never reaches future minor versions. 4.6.0 missed #1343 this way.

Run by .github/workflows/check-template-propagation.yml, which passes its inputs as environment variables:
BASE_SHA the commit the pull request merges into
HEAD_SHA the pull request merged with BASE_SHA
SCOPED "true" when the pull request has the template-propagation-scoped label

See "Deciding where to make your change" in CONTRIBUTING.md.
"""

import os
import re
import subprocess
import sys

# template/v4/v4.6/dirs/etc/x.sh -> major 4, minor 6, path inside the template "dirs/etc/x.sh"
TEMPLATE_FILE = re.compile(r"^template/v(\d+)/v\1\.(\d+)/(.+)$")
SCOPED_LABEL = "template-propagation-scoped"


def parse_template_path(path):
"""Return (major, minor, path inside the template) for a per-minor template file, or None."""
match = TEMPLATE_FILE.match(path)
if not match:
return None
return int(match.group(1)), int(match.group(2)), match.group(3)


def find_missing_changes(changed_files, all_files):
"""Return (changed file, newest-minor counterpart) for each older-minor change the newest minor lacks.

changed_files: template files the pull request changes.
all_files: every template file after the pull request merges; used to find each major's newest minor.
"""
newest = {}
for path in all_files:
parsed = parse_template_path(path)
if parsed:
major, minor, _ = parsed
newest[major] = max(newest.get(major, minor), minor)

changed = set(changed_files)
missing = []
for path in sorted(changed):
parsed = parse_template_path(path)
if not parsed:
continue
major, minor, inner_path = parsed
newest_minor = newest.get(major, minor)
if minor == newest_minor:
continue
counterpart = f"template/v{major}/v{major}.{newest_minor}/{inner_path}"
if counterpart not in changed:
missing.append((path, counterpart))
return missing


def git_lines(*args):
return subprocess.run(["git", *args], check=True, capture_output=True, text=True).stdout.splitlines()


def main():
if os.environ.get("SCOPED") == "true":
print(f"Skipped: the pull request has the {SCOPED_LABEL} label.")
return 0

base, head = os.environ["BASE_SHA"], os.environ["HEAD_SHA"]
changed = git_lines("diff", "--name-only", f"{base}...{head}", "--", "template")
all_files = git_lines("ls-tree", "-r", "--name-only", head, "--", "template")
missing = find_missing_changes(changed, all_files)

if not missing:
print(f"OK: {len(changed)} template file(s) changed, none missing from the newest minor's template.")
return 0

for path, counterpart in missing:
print(f"::error file={path}::{path} changed, but {counterpart} did not.")
print(
"\nA change to an older minor's template only reaches future minor versions if it is also made in "
"the newest minor's template. Make the same change in the files listed above. If this change is "
f"deliberately only for older minors, ask a maintainer to add the {SCOPED_LABEL} label. "
"See 'Deciding where to make your change' in CONTRIBUTING.md."
)
return 1


if __name__ == "__main__":
sys.exit(main())
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ Include every template directory you created or edited in your PR, and open the

> **Why this rule exists:** templates are copied forward only **once** — at the moment a new minor version is first created (see "How new minor version templates are auto-created at build time" below). After that, each minor's template is an independent, frozen snapshot. So editing an *already-released* minor's template does not reach any newer minor. A common past mistake was editing v4.5 while v4.6 was already in flight: the change landed only in the v4.5 line and never reached v4.6+. Following the rule above avoids this — you always land the change on the newest minor template, creating it first if the current newest is already released.

A CI check (`check_template_propagation`) backs this up: if your PR edits an older minor's template file while a newer minor template exists, it fails unless the same file is also changed in the newest minor template. For a change that is intentionally scoped to older minor lines only (e.g. a targeted backport), add the line `template-propagation: scoped` to your PR description to opt out.
The "Check template propagation" check backs this up on every pull request to `main`: if your PR changes a file in an older minor's template, it fails unless the same file also changes in the newest minor's template. The failure lists each file that is missing the change. For a change that is intentionally scoped to older minor lines only (e.g. a targeted backport), ask a maintainer to add the `template-propagation-scoped` label to your PR; the check reruns and passes. The check only confirms that the newest minor's file changed too, so reviewers still need to confirm it changed the same way.

#### Applying a security fix or infrastructure change (applies to all supported minor versions)

Expand Down
67 changes: 67 additions & 0 deletions test/test_check_template_propagation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import subprocess

import pytest
from check_template_propagation import find_missing_changes, main, parse_template_path

pytestmark = pytest.mark.unit

ALL_FILES = [
"template/v3/v3.9/Dockerfile",
"template/v4/v4.5/Dockerfile",
"template/v4/v4.5/dirs/a.sh",
"template/v4/v4.6/Dockerfile",
"template/v4/v4.6/dirs/a.sh",
]


def test_parse_template_path():
assert parse_template_path("template/v4/v4.6/dirs/etc/x.sh") == (4, 6, "dirs/etc/x.sh")
assert parse_template_path("template/v4/v3.9/Dockerfile") is None
assert parse_template_path("template/v4/dirs/etc/x.sh") is None
assert parse_template_path("build_artifacts/v4/v4.6/v4.6.0/Dockerfile") is None


def test_older_minor_change_must_also_change_the_newest_minor():
assert find_missing_changes(["template/v4/v4.5/dirs/a.sh"], ALL_FILES) == [
("template/v4/v4.5/dirs/a.sh", "template/v4/v4.6/dirs/a.sh")
]
both = ["template/v4/v4.5/dirs/a.sh", "template/v4/v4.6/dirs/a.sh"]
assert find_missing_changes(both, ALL_FILES) == []


def test_changes_to_the_newest_minor_or_other_files_need_nothing_else():
assert find_missing_changes(["template/v4/v4.6/dirs/a.sh"], ALL_FILES) == []
# Each major has its own newest minor: v3.9 is the newest v3 template.
assert find_missing_changes(["template/v3/v3.9/Dockerfile"], ALL_FILES) == []
assert find_missing_changes(["template/v4/dirs/a.sh", "src/main.py"], ALL_FILES) == []


def test_main_reads_the_git_trees_and_honours_the_label(tmp_path, monkeypatch, capsys):
def git(*args):
command = ["git", "-c", "user.name=test", "-c", "user.email=test@example.com", *args]
return subprocess.run(command, cwd=tmp_path, check=True, capture_output=True, text=True).stdout.strip()

git("init", "-q")
for minor in (5, 6):
(tmp_path / f"template/v4/v4.{minor}").mkdir(parents=True)
(tmp_path / f"template/v4/v4.{minor}/a.sh").write_text("old\n")
git("add", ".")
git("commit", "-q", "-m", "base")
monkeypatch.chdir(tmp_path)
monkeypatch.setenv("BASE_SHA", git("rev-parse", "HEAD"))

(tmp_path / "template/v4/v4.5/a.sh").write_text("new\n")
git("commit", "-q", "-a", "-m", "4.5 only")
monkeypatch.setenv("HEAD_SHA", git("rev-parse", "HEAD"))
monkeypatch.setenv("SCOPED", "false")
assert main() == 1
assert "template/v4/v4.6/a.sh did not" in capsys.readouterr().out

monkeypatch.setenv("SCOPED", "true")
assert main() == 0

(tmp_path / "template/v4/v4.6/a.sh").write_text("new\n")
git("commit", "-q", "-a", "-m", "4.6 too")
monkeypatch.setenv("HEAD_SHA", git("rev-parse", "HEAD"))
monkeypatch.setenv("SCOPED", "false")
assert main() == 0
Loading