build: one definition per artifact of what it ships (milestone 313 step 1)
Build images / sign-extension (push) Successful in 4s
CI / lint (push) Successful in 4s
CI / extension-version (push) Successful in 5s
CI / frontend-build (push) Successful in 41s
CI / backend-lint-and-test (push) Successful in 2m4s
CI / integration (push) Successful in 4m15s
Build images / build-web (push) Successful in 4m50s
Build images / build-ml (push) Successful in 5m43s
Build images / build-agent (push) Successful in 10m45s
Build images / sign-extension (push) Successful in 4s
CI / lint (push) Successful in 4s
CI / extension-version (push) Successful in 5s
CI / frontend-build (push) Successful in 41s
CI / backend-lint-and-test (push) Successful in 2m4s
CI / integration (push) Successful in 4m15s
Build images / build-web (push) Successful in 4m50s
Build images / build-ml (push) Successful in 5m43s
Build images / build-agent (push) Successful in 10m45s
scripts/artifacts.sh generalises what packaging.sh established for the
extension: four published artifacts, four path sets, four independent
versions derived from the newest commit touching each set.
Measured on this commit, and this is the point of the whole thing:
web tag=2026.8.27 version=2026.8.27.1547 rev=a7e626a
ml tag=2026.8.27 version=2026.8.27.1547 rev=a7e626a
agent tag=2026.7.17 version=2026.7.17.1657 rev=57e5243
extension tag=2026.8.27 version=2026.8.27.1547 rev=a7e626a
The agent is six weeks behind because agent/fc_agent has not changed since
57e5243. Today it rebuilds and re-tags on every push regardless; from step
4 it will not.
Three outputs, because they answer different questions and conflating them
is how this goes wrong:
tag YYYY.M.D the published image tag. Day precision, per the
operator: same-day work is not worth pinning, so
a second build that day replaces the first.
version YYYY.M.D.HHMM the ordering key. The extension needs this and
cannot use `tag`: Firefox compares it to decide
whether an update exists, so two same-day builds
must be distinguishable or the second hits the
ext-<version> cache and ships stale bytes. That
is issue #2397's failure mode exactly.
revision <sha> content identity. Because `tag` is only
day-precise, "does this tag already exist" cannot
decide whether a build can be skipped — two
different builds legitimately share a tag. Step 4
keys on this instead.
Path sets read from the Dockerfiles rather than guessed. Notable calls:
- web includes the extension's packaged set, because build.yml bakes the
signed XPI into frontend/public/extension/ before the docker build. Miss
that and :latest serves a NEW extension under an unchanged web version.
- web excludes frontend/test: vite builds from src/, index.html and
public/, so a spec change lands in the builder layer but never in dist.
- agent is agent/Dockerfile + agent/requirements.txt + agent/fc_agent,
NOT agent/. README.md, ruff.toml and docker-compose.yml sit in that
directory and never reach the image.
- every set includes its own Dockerfile and requirements: a base-image
bump changes the artifact as surely as a source edit does.
- the extension's set is read from packaging.sh, not restated. One
definition, per #2397.
tests/test_artifact_paths.py guards both directions of being wrong, since
both are silent. Too narrow — a COPY'd file missing from the set — means the
version does not move when the content does, and a pin serves stale bytes.
Too wide means re-versioning for a change the artifact does not ship. The
test parses each Dockerfile's COPY lines and compares them against the
declaration, so adding a COPY without updating the set fails the lane.
No workflow reads any of this yet. Step 2 shadows it.
This commit is contained in:
@@ -0,0 +1,138 @@
|
||||
"""`scripts/artifacts.sh` path sets must match what the Dockerfiles copy.
|
||||
|
||||
Each published artifact's version derives from the newest commit touching its
|
||||
own shipped file set (milestone 313). The whole scheme rests on those sets
|
||||
being right, and both ways of being wrong are silent:
|
||||
|
||||
* **too narrow** — a file ships but is not in the set, so the version does not
|
||||
move when the content does, and a pin serves stale bytes. This is the
|
||||
dangerous direction and the one this module exists for.
|
||||
* **too wide** — a file is in the set but never reaches the image, so the
|
||||
artifact re-versions and rebuilds for a change it does not ship.
|
||||
|
||||
Nothing else notices either. The version still derives, CI still goes green,
|
||||
and the mismatch only surfaces as "I pinned that build and got the wrong
|
||||
bytes". So the Dockerfiles are read here and compared against the declaration.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
|
||||
# artifact -> (dockerfile, build context relative to the repo root)
|
||||
ARTIFACTS = {
|
||||
"web": ("Dockerfile", ""),
|
||||
"ml": ("Dockerfile.ml", ""),
|
||||
"agent": ("agent/Dockerfile", "agent"),
|
||||
}
|
||||
|
||||
# COPY --from=<stage> copies from an earlier build stage, not from the build
|
||||
# context, so its source is not a repo path and cannot be in a path set.
|
||||
_COPY = re.compile(r"^\s*COPY\s+(?!--from=)(?P<args>.+)$", re.MULTILINE)
|
||||
|
||||
|
||||
def declared_paths(artifact: str) -> list[str]:
|
||||
out = subprocess.run(
|
||||
["sh", str(ROOT / "scripts" / "artifacts.sh"), "paths", artifact],
|
||||
capture_output=True, text=True, check=True, cwd=ROOT,
|
||||
).stdout
|
||||
return out.split()
|
||||
|
||||
|
||||
def includes(artifact: str) -> list[str]:
|
||||
"""The set minus its `:(exclude)…` entries."""
|
||||
return [p for p in declared_paths(artifact) if not p.startswith(":(exclude)")]
|
||||
|
||||
|
||||
def copy_sources(dockerfile: str, context: str) -> list[str]:
|
||||
"""Repo-relative sources of every context COPY in a Dockerfile."""
|
||||
text = (ROOT / dockerfile).read_text()
|
||||
sources: list[str] = []
|
||||
for m in _COPY.finditer(text):
|
||||
args = m.group("args").split()
|
||||
# Last arg is the destination; everything before it is a source.
|
||||
for src in args[:-1]:
|
||||
# `frontend/package-lock.json*` — the glob is an optional-file
|
||||
# idiom; the directory it sits in is what matters for coverage.
|
||||
src = src.rstrip("*")
|
||||
sources.append(f"{context}/{src}" if context else src)
|
||||
return sources
|
||||
|
||||
|
||||
def covered_by(path: str, include: str) -> bool:
|
||||
"""`path` ships if an include names it or one of its ancestors."""
|
||||
path = path.rstrip("/").lstrip("./")
|
||||
include = include.rstrip("/")
|
||||
return path == include or path.startswith(include + "/")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("artifact", sorted(ARTIFACTS))
|
||||
def test_every_copied_path_is_in_the_artifacts_path_set(artifact):
|
||||
"""The too-narrow direction — the one that serves stale bytes on a pin."""
|
||||
dockerfile, context = ARTIFACTS[artifact]
|
||||
inc = includes(artifact)
|
||||
for src in copy_sources(dockerfile, context):
|
||||
assert any(covered_by(src, i) for i in inc), (
|
||||
f"{dockerfile} copies {src!r} into the {artifact} image, but no "
|
||||
f"include in scripts/artifacts.sh covers it. The {artifact} "
|
||||
f"version will not move when that file changes, so a pinned build "
|
||||
f"will serve stale bytes. Add it to the path set.\n"
|
||||
f" declared includes: {inc}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("artifact", sorted(ARTIFACTS))
|
||||
def test_the_dockerfile_itself_is_in_the_path_set(artifact):
|
||||
"""Changing a base image or a RUN changes the artifact as surely as
|
||||
changing a source file, so each set must include its own Dockerfile."""
|
||||
dockerfile, _ = ARTIFACTS[artifact]
|
||||
assert any(covered_by(dockerfile, i) for i in includes(artifact)), (
|
||||
f"{dockerfile} is not in the {artifact} path set — a base-image bump "
|
||||
f"would not move the version."
|
||||
)
|
||||
|
||||
|
||||
def test_the_web_image_versions_on_an_extension_change():
|
||||
"""The web image bundles the signed XPI, so the extension's packaged files
|
||||
are part of what it ships. Miss this and `:latest` serves a NEW extension
|
||||
under an unchanged web version — a pin that quietly disagrees with itself.
|
||||
"""
|
||||
inc = includes("web")
|
||||
assert any(covered_by("extension/background/background.js", i) for i in inc), (
|
||||
"the web path set does not cover the extension's packaged files, but "
|
||||
"build.yml downloads the signed XPI into frontend/public/extension/ "
|
||||
"before the docker build"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"artifact, path",
|
||||
[
|
||||
# Deliberate exclusions — the too-wide direction. Each of these lives
|
||||
# beside shipped code but never reaches an image, and including it
|
||||
# would re-version the artifact for a change it does not carry.
|
||||
("agent", "agent/README.md"),
|
||||
("agent", "agent/ruff.toml"),
|
||||
("agent", "agent/docker-compose.yml"),
|
||||
# vite builds from src/, index.html and public/; it never reads test/,
|
||||
# so a frontend test change cannot reach `dist`.
|
||||
("web", "frontend/test/gallery.spec.js"),
|
||||
],
|
||||
)
|
||||
def test_files_that_never_reach_an_image_do_not_version_it(artifact, path):
|
||||
paths = declared_paths(artifact)
|
||||
excluded = [p[len(":(exclude)"):] for p in paths if p.startswith(":(exclude)")]
|
||||
inc = [p for p in paths if not p.startswith(":(exclude)")]
|
||||
|
||||
included = any(covered_by(path, i) for i in inc)
|
||||
exempted = any(covered_by(path, e.rstrip("*").rstrip("/")) for e in excluded)
|
||||
assert not included or exempted, (
|
||||
f"{path} is in the {artifact} path set but is not copied into the "
|
||||
f"image — it would re-version and rebuild {artifact} for a change it "
|
||||
f"does not ship."
|
||||
)
|
||||
Reference in New Issue
Block a user