Conversation
aidanheerdegen
left a comment
There was a problem hiding this comment.
Looks good, but would love some tests, or just some test output in the PR.
|
Can do once the schema is merged :) |
|
Tested out the new workflow @aidanheerdegen - see the Testing section in the PR description |
|
The projections in the artefact look like this: projections:
access-test: '{name}/pr50-3'
access-test-component: '{name}/pr50-3/main-{hash:7}'
openmpi: '{name}/pr50-3/4.1.5-{hash:7}'It doesn't have the desired behaviour of isolating the environment files of a pre-release to the specific PR. I was thinking of something more like this: projections:
access-test: '{name}/pr50-3'
access-test-component: 'access-test/pr50-3/{name}main-{hash:7}'
openmpi: 'access-test/pr50-3/{name}/4.1.5-{hash:7}'So the top level package gets it's version replaced by the PR version, and all the dependent packages have the top level package and PR version pre-pended. This has the advantage of completely isolating the module files for each PR build, and means it should be possible to do when PR 50 is closed or merged, and clean up all the associated module files. |
|
Ahh, yep. Gotcha. Will update this PR with that fix, just for simplicity's sake... |
|
Thanks @aidanheerdegen, updated with your feedback |
Thanks, bit of scope creep there. Sorry. |
* Injection of Modules over Checks (#295) * Add projection injection module * Add requirements for injection modules * Add tests for injection projection module, and inputs/requirements * Add prerelease injection module * Add tests for prerelease injection module * Run injection.projection and injection.prerelease modules * deploy-1-setup: Remove projection checks * Remove unneeded mergedeep requirement * Add module inits to folders * Add step for installing dependencies, update names for checkout steps * Used cd module prefix rather than custom working-directory so we can find the manifest * Added required input for prerelease modification, always print out updated manifest at end * Add back EOT * Update non-root-spec projections to namespaced `{name}/prX-Y/VERSION` format * Updated tests for namespaced non-root-spec projections * Update prerelease module argument from --root-spec-version to --version * Pass in sys.argv to parse_args, add tests * Refactor workflow to split generation of root spec and packages * Rename projections script to modules * Add inject_includes function * For test inputs, comment what is expected * Reordered functions * Updated tests after code updates * Apply suggestions from code review Co-authored-by: Aidan Heerdegen <[email protected]> Signed-off-by: Tommy Gatti <[email protected]> * Add root_spec to includes set, but remove it later when alphabetically sorting the includelist since it always comes first * Updated output of manifest * Ran black * Added getter module for accessing various parts of a spack manifest * Update `[test_]modules.py` to use new getter module * Update `[test_]prerelease.py` to use getter module, update deploy-2-start to remove --root-spec arg * Ran black again * Raise NotImplementedErrors, add comments for successful matches * Remove comments of old code * Apply suggestions from code review Co-authored-by: Aidan Heerdegen <[email protected]> Signed-off-by: Tommy Gatti <[email protected]> * Add example to multi-target spec to test function * Parameterized tests * Fixes following more test cases * Converted manifest with no packages to fixture * Improved from_file classmethod tests * Parameterized valid and mistakenly invalid tests * Remove `root-spec` arg and instead get from manifest directly, update tests * Add new prerelease script flag `--keep-root-spec-intact`, that doesn't remove version information --------- Signed-off-by: Tommy Gatti <[email protected]> Co-authored-by: Aidan Heerdegen <[email protected]> * `vars` to File-Based Special Packages (#305) * Validate MDR config/packages.json * Pass info from MDR config/packages.json to module injection script * Update variable name to `vars.CONFIG_PACKAGES_SCHEMA_VERSION` * Change cwd relative to build-cd rather than model for scripts * Add output of packages to be injected/added to provenance DB * Update prerelease projection to `ROOT_SPEC/prX-Y/{name}/VERSION` * Update tests for new prerelease injection logic * Replace all internal refs with v6 * Dump quoted strings for prerelease projections, updated expected output * Schema Versions as Entrypoint Inputs Over `vars` (#310) * Change schema vars to required inputs, add optional spack manifest schema path for potentially different schemas * Add required schema version inputs, and optional spack manifest schema paths to entrypoint workflows * Get packages for provenance from an earlier step * Update prerelease injection to the form `ROOT_SPEC/.dependencies/PRX-Y/VERSION-{hash:7}` * Fixed up an unused variable * Remove `.` from `dependencies directory --------- Signed-off-by: Tommy Gatti <[email protected]> Co-authored-by: Aidan Heerdegen <[email protected]>
Closes #304
Background
Since the info in
vars.BUILD_DB_PACKAGESis used for more things than just the release database (for example, module injection), we need to put it somewhere more explicit.This PR moves it from a GitHub Actions Variable to
config/packages.jsonThe PR
Testing
Tested on the
304-var-to-file-based-special-packages_TESTbranch, using the ACCESS-NRI/ACCESS-TEST#50 PR.Specifically, in the run https://github.com/ACCESS-NRI/ACCESS-TEST/actions/runs/16583228910, note the successful parsing of the new
config/packages.jsonfile in https://github.com/ACCESS-NRI/ACCESS-TEST/actions/runs/16583228910/job/46903521275, and the Injection of modules specified via that file in https://github.com/ACCESS-NRI/ACCESS-TEST/actions/runs/16583228910/job/46903541833