Skip to content

v6 Changes Bundle#306

Merged
CodeGat merged 9 commits intov6from
dev-v6
Aug 11, 2025
Merged

v6 Changes Bundle#306
CodeGat merged 9 commits intov6from
dev-v6

Conversation

@CodeGat
Copy link
Copy Markdown
Member

@CodeGat CodeGat commented Jul 29, 2025

References #311

Background

Since we seem to have a tradition of bundling major-level changes into one PR, we have a few updates that will take us from v5 to v6, namely:

As well as a few bug fixes along the way.

Testing

Testing details for each PR is in the linked PR.

Unit Tests

Unit tests can be run with python3 -m pytest, all of them succeeded.

E2E Tests

Tested in ACCESS-NRI/ACCESS-TEST#52 using the dev-v6_TEST, as well as ACCESS-NRI/ACCESS-TEST#53

Test manifests with existing projections

Tested in https://github.com/ACCESS-NRI/ACCESS-TEST/actions/runs/16611404535, using this packages.json and this manifest. It led to this injection: https://github.com/ACCESS-NRI/ACCESS-TEST/actions/runs/16611404535/job/46995084145#step:11:40

Test manifests with no projections

Tested in https://github.com/ACCESS-NRI/ACCESS-TEST/actions/runs/16611534408, using the same packages.json and this manifest. It led to this injection: https://github.com/ACCESS-NRI/ACCESS-TEST/actions/runs/16611534408/job/46995451420?pr=52#step:11:40

CodeGat and others added 2 commits July 24, 2025 15:13
* 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]>
* 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
@CodeGat CodeGat self-assigned this Jul 29, 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 29, 2025
@CodeGat
Copy link
Copy Markdown
Member Author

CodeGat commented Aug 3, 2025

@aidanheerdegen I've updated this branch with the projection fixes required

@CodeGat CodeGat marked this pull request as draft August 4, 2025 01:29
@CodeGat
Copy link
Copy Markdown
Member Author

CodeGat commented Aug 4, 2025

Drafting this so I can incorporate #239 to further the goal of #276

* 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
@aidanheerdegen
Copy link
Copy Markdown
Member

Current pre-release module projections have been modified to use a namespace approach, putting all the dependent modules under a single namespace of the pre-release environment to make them easier to reliably remove when a PR is closed.

The current approach is to use module names like:

access-test/pr53-4
access-test/pr53-4/access-test-component/main-joh6ivi

but this is flawed because access-test/pr53-4 is a file but the same path is being used a directory in the dependent module.

One solution would be to put the dependent modules in a hidden subdirectory, e.g. access-test/.dependencies/pr53-4/

It's slightly messier, but would allow an rm -rf access-test/.dependencies/pr53* operation to remove all module files related to a PR, and the hidden directory doesn't get scanned by the module system, which would result in a confusing bunch of module files being visible with module avail.

@CodeGat
Copy link
Copy Markdown
Member Author

CodeGat commented Aug 7, 2025

We ended up going with ROOT_SPEC/dependencies/prX-Y/COMPONENT/VERSION-{hash:7}

@CodeGat CodeGat marked this pull request as ready for review August 7, 2025 23:20
@CodeGat CodeGat requested a review from aidanheerdegen August 7, 2025 23:20
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.

LGTM. Just a question about cd cd.

@CodeGat CodeGat merged commit 13008b5 into v6 Aug 11, 2025
3 checks passed
@CodeGat CodeGat deleted the dev-v6 branch November 7, 2025 02:53
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