Refactor CI build matrix to use explicit per-job attributes - #338
Draft
cdvonbargen wants to merge 6 commits into
Draft
Refactor CI build matrix to use explicit per-job attributes#338cdvonbargen wants to merge 6 commits into
cdvonbargen wants to merge 6 commits into
Conversation
Replace per-step build-name string comparisons in sketcher-builder.yml with explicit matrix attributes (build-type, needs-venv, needs-msvc, needs-emscripten, linux-qt-deps, runs-pytest, runs-wasm-tests, runs-memtest, etc.), so adding or renaming a build no longer requires editing every conditional. Rename matrix entries to reflect what they actually build (macos-arm64, windows-x64, linux-x64, debug). Make build-external explicit about its inputs by adding required cache-tag and use-emsdk parameters, removing its implicit dependency on the caller's matrix variables. Update release packaging to skip debug artifacts and ship linux-x64 as a tar.gz alongside the zips.
Revert build-external to its prior shape: the cache key is keyed off matrix.runner / matrix.build-name as before, and emsdk sourcing is gated on matrix.build-name == 'wasm' inside the action. The earlier change traded ~10 lines of caller boilerplate and a required input for theoretical reusability that the single caller does not need.
…enum
cmake-wrapper was 1:1 with needs-emscripten (emscripten ships emcmake),
so derive it inline at the two call sites instead of carrying both
attributes.
The three runs-pytest / runs-wasm-tests / runs-memtest booleans are
mutually exclusive by construction. Replace them with a single
'tests: {cpp, wasm, memtest}' enum, which encodes the exclusivity in
the schema and shortens each matrix entry by a line.
The explicit per-job matrix attributes added churn without enough payoff. Restore sketcher-builder.yml to its prior shape and apply only the build-name renames (macos -> macos-arm64, windows -> windows-x64, ubuntu -> linux-x64, memtest -> debug), updating the matching string conditionals and the runner-cleanup comment. The release.yml change (skip *-debug, tar.gz for linux-x64) and the pre-commit JSON drop stay.
The .pre-commit-config.yaml change was unrelated to the build-name renames; it's now PR schrodinger#339. Restore the file to match main here.
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.
Description
This refactors the
sketcher-builder.ymlworkflow so that each matrix entry declares explicit attributes (build-type,needs-venv,needs-msvc,needs-emscripten,linux-qt-deps,runs-pytest,runs-wasm-tests,runs-memtest,cmake-wrapper,cmake-extra-flags,test-prefix,free-space) rather than having every step branch on thebuild-namestring. Adding or renaming a build no longer requires editing a dozen scattered conditionals, and the matrix entries themselves now read as a clear declaration of what each job needs. The matrix entries are also renamed to describe what they actually build (macos-arm64,windows-x64,linux-x64,debug) instead of the prior OS-only names plus the misleadingmemtestlabel.The
build-externalcomposite action previously reached up into the caller's matrix context forrunner,build-name, and the implicitwasmuse-emsdk behavior, which made it impossible to reuse from a job with a different matrix shape. It now takescache-tag(required) anduse-emsdkas explicit inputs, with the caller passing\${{ env.CACHE_TAG }}and\${{ matrix.needs-emscripten }}from the new attributes. Finally,release.ymlis updated to match the renamed artifacts: it skips*-debugoutputs (no longer published), and shipslinux-x64as atar.gzto preserve POSIX permissions and symlinks alongside the existing zips for the other platforms.Testing Done
CI on this branch will exercise all five matrix entries (macos-arm64, windows-x64, linux-x64, wasm, debug) end-to-end, which is the meaningful test of the refactor since the changes are entirely workflow-driven. The release workflow change will be exercised the next time a release tag is cut.