Replace the workflow_run chain with a single same-run build graph - #18
Merged
Merged
Conversation
The core and PureData workflows were chained with `workflow_run`. That is not
merely awkward, it is wrong: `workflow_run` checks out the *default branch*
rather than the ref that triggered it, while downloading the core artifact from
the triggering run. Dispatching the core build on core_modernization therefore
made the PureData workflow compile old main sources against a new
libzerr_core.a, failing with
undefined reference to zerr::EnvelopeGenerator::EnvelopeGenerator(
zerr::SystemConfigs, std::string, std::string)
because the constructor had become GenMode-typed. That looked like a real defect
and was not.
ci.yml puts every job in one run. Wrapper jobs take the core artifact from their
own run -- no run-id, no github-token, no "Ensure core artifact id present"
guard -- so a cross-commit mix is not representable rather than merely unlikely.
Artifacts are keyed by TOOLCHAIN rather than by platform. Windows needs two
distinct core builds: MinGW for PureData, and MSVC with static CRT for Max/MSP,
whose max-pretarget.cmake forces /MT. A single zerr-core-windows artifact could
never express that, so the old structure could not reach the target matrix this
project is heading for (Mac/Linux/Windows x PD/Max/JACK/SuperCollider). Adding a
platform or a compiler is now one matrix row plus one row in the consumer.
Max/MSP gains its first CI job on any platform. macOS only for now -- Max does
not exist on Linux, and Windows needs the MSVC/static-CRT path above.
Also folded in:
- The Windows core build moves off its inline conan settings onto
profiles/mingw. Those inline settings omitted compiler.cppstd,
compiler.threads and compiler.exception, all of which are package_id inputs,
so the core and PureData jobs could resolve *different binary packages* of
fftw and yaml-cpp for one logical build -- the single-resolve invariant broken
at the CI layer.
- A composite setup-deps action replaces the conan block that was duplicated 6
times, and caches ~/.conan2. fftw and yaml-cpp currently build from source on
macOS and Windows every run; the Windows job spent most of its 3m41s there.
- Path filters dropped. A core change must rebuild every wrapper and a
wrapper-only change must still build; the old filters are why a puredata- or
maxmsp-only change triggered nothing at all.
- concurrency group so superseded runs are cancelled, and checkout v3 -> v4
(v3 targets the deprecated Node 20).
deploy-doxygen.yml is untouched: it fails by design pending the organization
owner's decision on Pages, and is documented in
docs/design/github-pages-deployment.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seven fixes from review of the previous commit. The `arch` one had been wrong since long before this branch. Workflow: - setup-mingw takes `platform`, not `arch`. Run 32314194864 logged "Unexpected input(s) 'arch', valid inputs are ['platform', 'cc', 'version']" in both windows-mingw jobs -- the x64 pin was silently discarded and the builds were 64-bit only because that is the action's default. Carried over verbatim from the deleted workflows, so it had never worked. - Wrapper jobs gain `if: !cancelled()`. `needs` cannot name a single matrix leg, so a macOS-only core failure used to *skip* every wrapper -- including puredata (windows-mingw), which the file itself identifies as the only job that can catch core/wrapper API drift at build time. Now the leg whose artifact is genuinely missing fails at download and the rest still report. The wait on the slowest core leg is inherent to matrix-level `needs` and is left as is, noted in a comment. - if-no-files-found: error on all three uploads. The two leaf jobs had nothing downstream to notice an empty publish, so a wrapper that stopped emitting into puredata/build/zerr or maxmsp/externals would have stayed green. - cancel-in-progress no longer applies to main. Two merges in quick succession would have cancelled the first one's run, leaving that commit without the artifact set the file's own header promises. - Explicit `permissions: contents: read`. No cross-run artifact download remains, so no job needs a token beyond checkout. setup-deps: - ImageVersion added to the cache key. profiles/mingw pins compiler.version=13 while the runners compile with gcc 16.1.0, so neither the package_id nor a conanfile/profile hash changes when the image bumps its toolchain: conan would have restored fftw and yaml-cpp built by the previous compiler and linked them into a core built by the new one -- exactly what the key's own comment claimed to prevent. - restore-keys dropped. A prefix restore is by definition the stale hit that key exists to avoid, and it re-saved the restored tree under the new key, so ~/.conan2 would accrete every historical revision until it hit the 10GB repository ceiling and began evicting live entries. Docs: - dependency-fallbacks.md section 3.7 asserted that the core CI job deliberately bypasses profiles/mingw, and cited a workflow file this branch deletes. Both halves are now wrong: the core job resolves through the profile, and the pinned compiler_executables names verifiably work. Rewritten, with the surviving compiler.version drift and how the cache key compensates. - repo-audit section 2.6 gets a status block, matching how 2.3 was handled, so the dated audit is not read as current state. The arm64-only macOS artifacts are deliberately not addressed here. That is a real design decision -- CI runners are arm64, so a universal binary needs either -G Xcode or an explicit CMAKE_OSX_ARCHITECTURES -- and it is recorded against section 2.5, which already flagged the same gap for build.sh. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit put ${{ env.ImageVersion }} in the cache key to make a runner
toolchain bump a cache miss. It did not work: the `env` context is not populated
with the runner image's variables inside a composite action's `with:` block, so
the expression expanded to an empty string. Run 32787801135 shows the result --
`conan-Windows--profiles/mingw-b46d38c5...`, with a blank field where the image
id should be. The key changed once, which forced a one-time miss, and then went
back to being blind to the compiler.
Read it in a shell step instead, where the variable is actually set, and pass it
through a step output. Falls back to "unknown" rather than empty so a self-hosted
runner without the variable still produces a well-formed key.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Replaces the two chained build workflows with one
ci.yml. Net −94 lines.The actual bug in the old structure
The core and PureData workflows were chained with
workflow_run. That trigger checks out the default branch, not the ref that triggered it, while downloading the core artifact from the triggering run. So dispatching the core build oncore_modernizationmade the PureData workflow compile oldmainsources against a newlibzerr_core.a:— because the constructor had become
GenMode-typed. It looked like a real defect. It wasn't. Nothing about that failure was avoidable by configuration; it is what the trigger does.In
ci.ymlevery job is in one run and wrapper jobs take the core artifact from their own run — norun-id, nogithub-token, no "Ensure core artifact id present" guard. A cross-commit mix is no longer representable.Artifacts keyed by toolchain, not platform
This is the part that matters for where the project is going. Windows needs two core builds — MinGW for PureData, and MSVC with static CRT for Max/MSP, since
max-pretarget.cmakeforces/MT. A singlezerr-core-windowsartifact cannot express that, so the old structure could never have reached Mac/Linux/Windows × PD/Max/JACK/SuperCollider.macosprofiles/macoslinuxprofiles/linuxwindows-mingwprofiles/mingwMinGW Makefileswindows-msvc(follow-up)profiles/windows-msvcAdding a platform or compiler is one row here plus one row in the consumer job.
Max/MSP gets CI for the first time
On any platform. macOS only for now — Max doesn't exist on Linux (README already marks it ➖), and Windows needs the MSVC/static-CRT path above. Tracked as a follow-up.
Also fixed
-s compiler=gcc -s compiler.version=13 -s compiler.libcxx=libstdc++11; PureData used-pr:h=profiles/mingw, which also pinscppstd=17,threads=posix,exception=seh. All arepackage_idinputs, so the two jobs could resolve different binary packages of fftw/yaml-cpp for one logical build — PR Resolve conan dependencies once at the repo root #13's single-resolve invariant, broken at the CI layer. Both now take the profile from the same matrix row.setup-depsaction replaces the conan block duplicated 6×, and caches~/.conan2. fftw and yaml-cpp currently build from source on macOS and Windows every run — most of the Windows job's 3m41s on Run the core build on pull requests #17.puredata/- ormaxmsp/-only change triggered nothing at all.concurrencygroup cancels superseded runs;checkout@v3→@v4(v3 targets deprecated Node 20).deploy-doxygen.ymlis untouched — it fails by design pending the org owner, perdocs/design/github-pages-deployment.md.Review notes
This PR validates itself. Thanks to #17 the checks run here, so what you see below is the verification. Worth watching:
core×3 →puredata×3 +maxmsp×1, wrappers starting only after core.puredata (windows-mingw)is the job that matters most — the only one that can catch core/wrapper API drift at build time. macOS links externals with-undefined suppressand Linux permits undefined symbols in a.so, so both would happily produce an external that fails at load.profiles/mingw, whosecompiler_executablespins thex86_64-w64-mingw32-*names, and whosecompiler.version=13no longer matches the runners' gcc 16.1.0. The old inline settings sidestepped both. If this fails, the fix is to drop thecompiler_executablespin or add a native profile — and we find out here rather than onmain, which is exactly why Run the core build on pull requests #17 went first.🤖 Generated with Claude Code