Skip to content

vars to File-Based Special Packages#305

Merged
CodeGat merged 7 commits intodev-v6from
304-var-to-file-based-special-packages
Jul 29, 2025
Merged

vars to File-Based Special Packages#305
CodeGat merged 7 commits intodev-v6from
304-var-to-file-based-special-packages

Conversation

@CodeGat
Copy link
Copy Markdown
Member

@CodeGat CodeGat commented Jul 24, 2025

Closes #304

Background

Since the info in vars.BUILD_DB_PACKAGES is 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.json

The PR

  • Validate MDR config/packages.json
  • Pass info from MDR config/packages.json to module injection script, and release database info generation steps

Testing

Tested on the 304-var-to-file-based-special-packages_TEST branch, 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.json file 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

@CodeGat CodeGat added type:enhancement Improvements to existing features priority:high version:MAJOR Requires an update to Model Deployment Repositories CI for:v6 Applies to v6 labels Jul 24, 2025
@CodeGat CodeGat self-assigned this Jul 24, 2025
@CodeGat CodeGat added type:enhancement Improvements to existing features priority:high version:MAJOR Requires an update to Model Deployment Repositories CI for:v6 Applies to v6 labels Jul 24, 2025
Copy link
Copy Markdown
Member

@aidanheerdegen aidanheerdegen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but would love some tests, or just some test output in the PR.

@CodeGat
Copy link
Copy Markdown
Member Author

CodeGat commented Jul 28, 2025

Can do once the schema is merged :)

@CodeGat CodeGat marked this pull request as ready for review July 29, 2025 00:20
@CodeGat CodeGat requested a review from aidanheerdegen July 29, 2025 00:20
@CodeGat
Copy link
Copy Markdown
Member Author

CodeGat commented Jul 29, 2025

Tested out the new workflow @aidanheerdegen - see the Testing section in the PR description

@aidanheerdegen
Copy link
Copy Markdown
Member

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

rm -rf access-test/pr50-*/

when PR 50 is closed or merged, and clean up all the associated module files.

@CodeGat
Copy link
Copy Markdown
Member Author

CodeGat commented Jul 29, 2025

Ahh, yep. Gotcha. Will update this PR with that fix, just for simplicity's sake...

@CodeGat
Copy link
Copy Markdown
Member Author

CodeGat commented Jul 29, 2025

Thanks @aidanheerdegen, updated with your feedback

@aidanheerdegen
Copy link
Copy Markdown
Member

Thanks @aidanheerdegen, updated with your feedback

Thanks, bit of scope creep there. Sorry.

@CodeGat CodeGat merged commit 8a072b2 into dev-v6 Jul 29, 2025
@CodeGat CodeGat deleted the 304-var-to-file-based-special-packages branch July 29, 2025 22:39
@CodeGat CodeGat mentioned this pull request Jul 29, 2025
CodeGat added a commit that referenced this pull request Aug 11, 2025
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

for:v6 Applies to v6 priority:high type:enhancement Improvements to existing features version:MAJOR Requires an update to Model Deployment Repositories CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants