Tag matrix latest-patch rows with minor-only tags#547
Conversation
Test Results1 591 tests 1 591 ✅ 8m 33s ⏱️ Results for commit ceba324. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull request overview
This PR adds “minor-only” floating tags for matrix builds by introducing a LATEST_PATCH tag-pattern filter that is applied only to the matrix row representing the latest patch within each (major.minor, …) grouping. This makes it easier for consumers to pull “latest patch for a minor combination” without knowing exact patch versions.
Changes:
- Add
stripPatchJinja filter and correspondingLATEST_PATCH-gated default matrix tag patterns to emit minor-only tags (e.g.,R4.3-python3.12). - Compute and persist an
isLatestPatchCombinationflag on generated matrixImageVersions to identify latest-patch rows per stripped-minor group. - Extend
ImageTargettag-pattern filtering and add unit tests covering filtering, rendering, and latest-patch row selection.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| posit-bakery/posit_bakery/config/templating/render.py | Adds strip_patch() and registers stripPatch filter in Jinja env. |
| posit-bakery/posit_bakery/config/tag.py | Introduces TagPatternFilter.LATEST_PATCH and adds stripPatch matrix tag patterns gated by it. |
| posit-bakery/posit_bakery/config/image/matrix.py | Computes latest-patch-per-minor-group signatures and sets isLatestPatchCombination on matrix ImageVersions. |
| posit-bakery/posit_bakery/config/image/version.py | Adds isLatestPatchCombination field to ImageVersion. |
| posit-bakery/posit_bakery/image/image_target.py | Filters out LATEST_PATCH tag patterns unless the target’s version is marked latest-patch. |
| posit-bakery/test/config/templating/test_render.py | Adds unit tests for the new stripPatch filter. |
| posit-bakery/test/config/test_tag.py | Updates collision test exclusions and adds coverage for stripPatch-rendered minor-only tags. |
| posit-bakery/test/config/image/test_matrix.py | Adds tests ensuring latest-patch selection logic behaves correctly across dependencies/values axes. |
| posit-bakery/test/image/test_image_target.py | Adds tests for is_latest_patch_combination and LATEST_PATCH tag-pattern filtering behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _STRIP_PATCH_RE = re.compile(r"(\d+\.\d+)\.\d+") | ||
|
|
||
|
|
||
| def strip_patch(s: str) -> str: | ||
| """Collapse ``MAJOR.MINOR.PATCH`` groups in a string to ``MAJOR.MINOR``. |
There was a problem hiding this comment.
This is an edge case, but I think it's right that we should consider 4+ part versions. Perhaps capture the first two segments instead and drop everything else?
ianpittwood
left a comment
There was a problem hiding this comment.
LGTM, but I am wondering if we could implement this in another way that houses more logic in the config.dependencies module?
Now that we have a ParsedVersion class, perhaps we should parse retrieved versions in the config.dependencies module into a ParsedVersion object. It may make it cleaner for us sort versions that way and transform them into MAJOR.MINOR versions.
| _STRIP_PATCH_RE = re.compile(r"(\d+\.\d+)\.\d+") | ||
|
|
||
|
|
||
| def strip_patch(s: str) -> str: | ||
| """Collapse ``MAJOR.MINOR.PATCH`` groups in a string to ``MAJOR.MINOR``. |
There was a problem hiding this comment.
This is an edge case, but I think it's right that we should consider 4+ part versions. Perhaps capture the first two segments instead and drop everything else?
Removes the duplicated _VERSION_SUBSTRING_RE constant and the private _extract_versions static method in favor of the equivalents in config.dependencies.version. _patch_sort_key now calls the shared extract_versions directly. Behavior is unchanged. Addresses ianpittwood's PR #547 review comment about housing version transforms in config.dependencies.
Matrix builds emit composite version tags like ``R4.3.3-python3.12.13``. To make it easy to pull "the latest patch for this minor combination," tag the latest-patch row in each ``(major.minor, ...)`` group with an additional minor-only tag like ``R4.3-python3.12``. - Add a ``stripPatch`` Jinja2 filter that drops the patch component from ``MAJOR.MINOR.PATCH`` groups in a composite tag string. - Add ``LATEST_PATCH`` to ``TagPatternFilter`` and gate the new ``stripPatch`` defaults on it so non-latest patches do not collide on the stripped tag. - Compute the latest-patch combinations in ``ImageMatrix`` by grouping cartesian rows by their dependency ``(major, minor)`` keys and the value-axis entries, then selecting the row with the highest versions in each group. - Expose the result on ``ImageVersion.isLatestPatchCombination`` and filter the new patterns out at ``ImageTarget.tag_patterns`` when the row is not the latest patch in its group.
Values-only matrices with version-like list values (e.g. ``go_version: [1.24.1, 1.24.2]``) flagged every row as the latest patch because grouping was on the raw string. ``stripPatch`` then collapsed both rows to ``go1.24``, producing colliding tags. Group value axes by ``(major, minor)`` when the value parses as a version with a minor component — matching what ``stripPatch`` collapses — and include them in the patch sort key so the highest-patch row wins. Non-version values still fall back to raw strings.
The previous fix grouped value axes by ``(major, minor)`` only when the whole value parsed as a ``DependencyVersion``. Prefix-bearing values like ``go1.24.1`` fail that parse and fell back to raw-string grouping, so ``go1.24.1`` and ``go1.24.2`` landed in separate groups and both rows were flagged latest-patch — yet ``stripPatch`` still collapsed both to ``go1.24`` on render, racing on push. Drive grouping from ``stripPatch`` itself. Anything that would render to the same stripped tag now lands in the same group, by construction. For ordering within a group, extract the first ``\\d+(\\.\\d+)+`` substring (e.g. ``1.24.10`` from ``go1.24.10``) and parse it as a ``DependencyVersion`` so prefix-bearing values participate in patch comparison alongside plain version strings. Extract ``strip_patch`` from the Jinja filter so both call sites share one implementation.
Drop the verbose phrases from the new ``test_to_image_versions_*`` tests so they read at a glance and line up with neighbors like ``test_to_image_versions_marks_latest_combination``.
After two rounds of fixes the latest-patch helper carried stale documentation and an unreachable exception handler that misled future readers. Cleanup only — no behaviour change. - ``_compute_latest_patch_signatures``: rewrite docstring to describe the ``strip_patch``-driven grouping that ``998de0a`` introduced; move dependency-version validation into a single explicit upfront pass and drop the second ``except InvalidVersion`` around ``max(...)``, which could never trigger because the same strings were already parsed in ``_minor_group_key``. - ``_minor_group_key``: drop the side-effect-only ``DependencyVersion(dep.versions[0])`` validation line now that the caller validates upfront. Function is now pure and the docstring says so. - Test ``test_to_image_versions_values_only_non_version_keeps_all``: rewrite the docstring and inline comment that still described the pre-``998de0a`` "raw-string fallback" behaviour. The grouping is now driven by ``strip_patch`` for every axis.
Constraint resolution never emits versions like ``3.12.3-rc1`` today (prereleases lose to clean patches in ``_filter_minor``), but an explicit ``dependencies:`` list can. Lock in that the patch numeric is the only segment ``stripPatch`` collapses, so prerelease rows group with their own siblings instead of clean-version rows. Doubles as a regression guard against future regex widenings — for instance ``(\\d+\\.\\d+)(?:\\.\\d+)+`` to handle 4-component calver strings would eat the ``-rc1`` too and silently collapse two distinct groups into one.
Values with multiple version-like substrings (e.g. ``go1.24-lib2.3.1`` vs ``go1.24-lib2.3.2``) collapse to the same ``stripPatch`` group, but the previous sort key extracted only the first segment. Both rows compared equal, so ``max()`` picked an arbitrary winner — the ``LATEST_PATCH`` tag could end up pointing at a non-highest patch for the group. Rename ``_extract_version`` to ``_extract_versions`` and have it return the tuple of every ``\\d+(\\.\\d+)+`` substring parsed as ``DependencyVersion``. Tuples compare element-wise, so the comparison cascades to the later segment when an earlier one ties. Within a single strip-patch group every row produces the same shape of tuple (same number of version-bearing positions), so mixed-type comparison during ``max()`` remains impossible.
The original stripPatch regex (\d+\.\d+)\.\d+ collapsed only the first three components, so 1.2.3.4 became 1.2.4 rather than 1.2. Tighten to (\d+\.\d+)(?:\.\d+)+ so 4+ component versions collapse fully to MAJOR.MINOR. Mirror _compute_latest_combination's defensive Exception hedge in _compute_latest_patch_signatures so an unexpected internal failure during version parsing skips latestPatch tag emission rather than killing the whole build. Add an end-to-end test pinning isLatestPatchCombination then filter retention then tag_suffixes emission. Coverage was split across three files, so a regression in any link could have slipped through.
Moves the version-string transforms next to DependencyVersion so callers in config.templating and config.image.matrix can share one definition instead of duplicating regex constants and parse loops. No call sites updated yet, so this commit only introduces the new functions.
Replaces the local definition in config.templating.render with an import. The Jinja filter registration in jinja2_env continues to resolve the same callable, so behavior is unchanged. Removes the inline regex constant since the imported function owns the pattern.
Removes the duplicated _VERSION_SUBSTRING_RE constant and the private _extract_versions static method in favor of the equivalents in config.dependencies.version. _patch_sort_key now calls the shared extract_versions directly. Behavior is unchanged. Addresses ianpittwood's PR #547 review comment about housing version transforms in config.dependencies.
The previous hedge only protected the dependency-pre-validation pass in _compute_latest_patch_signatures. The value-axis path goes through _patch_sort_key, which calls extract_versions and DependencyVersion directly without that pre-validation. A non-InvalidVersion exception there would still crash to_image_versions(). Wrap the max(...) call with a try/except Exception that logs once and returns None so the matrix degrades gracefully (skips latestPatch tags) instead of killing the whole build. extract_versions stays strict so other callers see real errors. Add a regression test that patches DependencyVersion for a values-only matrix, covering the path the dep-side test does not.
6b23dc1 to
ceba324
Compare
Matrix builds emit composite version tags like
R4.3.3-python3.12.13,which makes "pull the latest patch for this minor combination" hard —
callers need to know the exact patches. Add an additional minor-only
tag (
R4.3-python3.12) for the row that represents the latest patchin each
(major.minor, ...)group.Related: