Identify C++ CI matrix jobs with a generated runId - #660
Merged
Conversation
The `preset-name` step built a string per matrix job by reassembling the
four axes in shell. Its name no longer described it: the CMake preset is
the literal `ci`, and the string's only consumers are the Conan cache
key prefixes in cmake-project-setup and cmake-project-finalize. Now that
the generator emits the matrix, the same value can come from the entry
it already describes, so the step is redundant.
Add a `runId` field, joining every field of the entry so it stays unique
by construction even if the matrix grows another axis, and read it as
`conan-${{ matrix.runId }}` where the cache prefix is needed. The
prefixes must stay distinct per scenario: cmake-project-setup falls back
to restoring on the bare prefix, so a shared one would have every job
pull a sibling configuration's packages, miss on package_id and rebuild
from source anyway.
The vestigial `ci-` infix is dropped, since it described the preset name
this string no longer is. That changes every prefix, so the existing
Conan caches are orphaned and the first run rebuilds from source before
saving under the new keys.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the C++ CI workflow to use a generated, per-matrix-entry identifier (runId) for Conan cache key prefixes, removing the redundant shell step that previously reconstructed a preset-name-like string.
Changes:
- Remove the
preset-namestep from the workflow and switch Conan cache key prefixes toconan-${{ matrix.runId }}. - Extend the CI matrix generator to emit a
runIdfield derived from the matrix entry fields.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/cpp-build-test-run.yaml | Drops the preset-name step and uses matrix.runId for Conan cache key prefixes in setup/finalize steps. |
| .github/scripts/generate-cpp-ci-config.py | Adds runId to generated matrix entries by joining the matrix entry fields. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Joining the entry fields is only collision-free while no axis value
contains the separator: ("libc++", "debug-x") and ("libc++-debug", "x")
render the same runId. Two jobs sharing one would quietly share a Conan
cache bucket, each restoring the other's packages and rebuilding anyway,
with nothing turning red.
Check the invariant that matters -- distinct entries, distinct runIds --
rather than restricting what an axis value may contain. It also catches
a duplicated table row, and fails in the resolver job instead of nine
build jobs.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/scripts/generate-cpp-ci-config.py:420
- Avoid using
assertfor validatingrunIduniqueness. Python assertions can be stripped with-O, which would silently skip this safety check and potentially cause cache-key collisions. Use an explicit runtime check and raise an exception instead.
# Two jobs sharing a runId would quietly share one Conan cache bucket, each
# restoring the other's packages and rebuilding anyway. Nothing downstream
# would fail, so catch it here.
run_ids = {entry["runId"] for entry in include}
assert len(run_ids) == len(include), "runId values are not unique"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
preset-namestep built a string per matrix job by reassembling the four axes in shell. Its name no longer described it: the CMake preset is the literalci, and the string's only consumers are the Conan cache key prefixes in cmake-project-setup and cmake-project-finalize. Now that the generator emits the matrix, the same value can come from the entry it already describes, so the step is redundant.Add a
runIdfield, joining every field of the entry so it stays unique by construction even if the matrix grows another axis, and read it asconan-${{ matrix.runId }}where the cache prefix is needed. The prefixes must stay distinct per scenario: cmake-project-setup falls back to restoring on the bare prefix, so a shared one would have every job pull a sibling configuration's packages, miss on package_id and rebuild from source anyway.The vestigial
ci-infix is dropped, since it described the preset name this string no longer is. That changes every prefix, so the existing Conan caches are orphaned and the first run rebuilds from source before saving under the new keys.