diff --git a/.github/actions/get-spack-manifest/README.md b/.github/actions/get-spack-manifest/README.md new file mode 100644 index 00000000..671318a4 --- /dev/null +++ b/.github/actions/get-spack-manifest/README.md @@ -0,0 +1,38 @@ +# Get Spack Manifest Information + +Action that returns information about a Spack manifest file. + +## Inputs + +> [!NOTE] +> Action assumes that an appropriate repository is checked out prior to invocation + +| Name | Type | Description | Required | Default | Example | +| ---- | ---- | ----------- | -------- | ------- | ------- | +| `spack-manifest-path` | `string` | The path to the spack manifest file | `false` | `"./spack.yaml"` | `"./some/other.spack.yaml"` | + +## Outputs + +| Name | Type | Description | Example | +| ---- | ---- | ----------- | ------- | +| `deployment-name` | `string` | The name of the deployment as specified in the reserved definition `_name` | `access-om2` | +| `deployment-version` | `string` | The version of the deployment as specified in the reserved definition `_version` | `2025.11.000` | + +## Example + +```yaml +# ... +jobs: + manifest: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - id: spec + uses: access-nri/build-cd/.github/actions/get-spack-manifest@vX # for some version `vX` + with: + spack-manifest-path: ./spack.yaml + + - run: | + echo "Deploying ${{ steps.spec.outputs.deployment-name }} at ${{ steps.spec.outputs.deployment-version }}" +``` diff --git a/.github/actions/get-spack-manifest/action.yml b/.github/actions/get-spack-manifest/action.yml new file mode 100644 index 00000000..9e0c39f5 --- /dev/null +++ b/.github/actions/get-spack-manifest/action.yml @@ -0,0 +1,52 @@ +name: Get Spack Manifest Information +description: Action that returns information about a Spack manifest file +author: Tommy Gatti +inputs: + spack-manifest-path: + required: false + default: ./spack.yaml + description: The path to the spack manifest file +outputs: + deployment-name: + description: | + The name of the deployment as specified in the reserved definition _name, in the spack manifest file. + value: ${{ steps.defs.outputs.name }} + deployment-version: + description: | + The version of the deployment as specified in the reserved definition _version, in the spack manifest file. + value: ${{ steps.defs.outputs.version }} +runs: + using: composite + steps: + - name: Clone build-cd script location + uses: actions/checkout@v4 + with: + repository: access-nri/build-cd + path: ${{ github.workspace }}/getter-script + + - name: Get Current Directory + id: script + # We need to store the current working directory so we can re-construct the inputs.spack-manifest-path, as we run + # the script from the build-cd directory + shell: bash + run: | + pwd=$(pwd) + echo "Action PWD: $pwd" + echo "pwd=$pwd" >> $GITHUB_OUTPUT + + - name: Get reserved definitions + id: defs + env: + PYTHONPATH: ${{ github.workspace }}/getter-script + shell: python + run: | + from scripts.spack_manifest.getter import ReservedDefinitions + import os + + defs = ReservedDefinitions.from_file("${{ steps.script.outputs.pwd }}/${{ inputs.spack-manifest-path }}") + name = defs.get("name") + version = defs.get("version") + + with open(os.environ['GITHUB_OUTPUT'], 'a') as o: + o.write(f"name={name}\n") + o.write(f"version={version}\n") diff --git a/.github/actions/get-spack-root-spec/README.md b/.github/actions/get-spack-root-spec/README.md deleted file mode 100644 index c4c29f38..00000000 --- a/.github/actions/get-spack-root-spec/README.md +++ /dev/null @@ -1,45 +0,0 @@ -# Get Spack Manifest Information - -Action that returns information about a Spack manifest file. - -## Inputs - -> [!NOTE] -> Action assumes that an appropriate repository is checked out prior to invocation - -| Name | Type | Description | Required | Default | Example | -| ---- | ---- | ----------- | -------- | ------- | ------- | -| `spack-manifest-path` | `string` | The path to the spack manifest file | `false` | `"./spack.yaml"` | `"./some/other.spack.yaml"` | - -## Outputs - -| Name | Type | Description | Example | -| ---- | ---- | ----------- | ------- | -| `root-spec` | `string` | The entirety of the root spec in the spack manifest file | `"access-om2@git.2025.01.01=release ~deterministic"` | -| `root-spec-name` | `string` | The name of the root spec in the spack manifest file | `"access-om2"` | -| `root-spec-ref` | `string` | The git ref from the root spec in the spack manifest file | `"2025.01.01"` | -| `root-spec-version` | `string` | The spack version from the root spec in the spack manifest file | `"release"` | -| `yq-root-spec` | `string` (`yq` filter) | The yq filter for obtaining the root spec for the spack manifest file - may change based on whether the root spec is in multi-target or single-target format | `.spack.specs[0]` | - -## Example - -```yaml -# ... -jobs: - manifest: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - - id: spec - uses: access-nri/build-cd/.github/actions/get-spack-root-spec@vX # for some version `vX` - with: - spack-manifest-path: ./spack.yaml - - - run: | - echo "Deploying ${{ steps.spec.outputs.root-spec-name }} at ${{ steps.spec.outputs.root-spec-version }}" - - # You can tack on more filters on top of the `root-spec` one - variants=$(yq '${{ steps.spec.outputs.yq-root-spec }} | capture("[^~+- ]+(.+)") | .[0]' ./spack.yaml) - echo "Variants are $variants" -``` diff --git a/.github/actions/get-spack-root-spec/action.yml b/.github/actions/get-spack-root-spec/action.yml deleted file mode 100644 index 6f8c438c..00000000 --- a/.github/actions/get-spack-root-spec/action.yml +++ /dev/null @@ -1,92 +0,0 @@ -name: Get Spack Manifest Information -description: Action that returns information about a Spack manifest file -author: Tommy Gatti -inputs: - spack-manifest-path: - required: false - default: ./spack.yaml - description: The path to the spack manifest file -outputs: - root-spec: - description: | - The entirety of the root spec in the spack manifest file. - For example: 'access-om2@git.2025.01.01=access-om2 ~deterministic'. - value: ${{ steps.spec.outputs.full }} - root-spec-name: - description: | - The name of the root spec in the spack manifest file. - For example: 'access-om2'. - value: ${{ steps.spec.outputs.name }} - root-spec-ref: - description: | - The git ref from the root spec in the spack manifest file. - For example: '2025.01.01'. - value: ${{ steps.spec.outputs.ref }} - root-spec-version: - description: | - The spack version from the root spec in the spack manifest file. - For example, 'release'. - value: ${{ steps.spec.outputs.version }} - root-spec-variants: - description: | - The variants from the root spec in the spack manifest file. - For example: '~deterministic'. - value: ${{ steps.spec.outputs.variants }} - yq-root-spec: - description: | - The yq filter for the root spec of the spack manifest file. - value: ${{ steps.yq.outputs.filter }} -runs: - using: composite - steps: - - name: Set yq filter for root spec - id: yq - shell: bash - # This attempts to get the spack.yaml model first via the multi-target - # .spack.definitions.root_package[0] method, then the traditional '.spack.specs[0]'. - env: - MULTI_TARGET_ROOT_SPEC_YQ_FILTER: (.spack.definitions[] | select(."ROOT_PACKAGE") | .[][0]) - SINGLE_TARGET_ROOT_SPEC_YQ_FILTER: .spack.specs[0] - run: | - if [[ -n "$(yq '${{ env.MULTI_TARGET_ROOT_SPEC_YQ_FILTER }}' ${{ inputs.spack-manifest-path }})" ]]; then - filter="${{ env.MULTI_TARGET_ROOT_SPEC_YQ_FILTER }}" - elif [[ -n "$(yq '${{ env.SINGLE_TARGET_ROOT_SPEC_YQ_FILTER }}' ${{ inputs.spack-manifest-path }})" ]]; then - filter="${{ env.SINGLE_TARGET_ROOT_SPEC_YQ_FILTER }}" - else - echo "We can't find the root spec!" - exit 1 - fi - - echo "Based on the root spec, we are using '$filter'" - echo "filter=$filter" >> $GITHUB_OUTPUT - - - name: Get spec info from spack manifest - id: spec - shell: bash - run: | - # Example: access-om2@git.2025.01.0=release ~variant +debug some=value - full=$(yq '${{ steps.yq.outputs.filter }}' ${{ inputs.spack-manifest-path }}) - - # Example of captured groups from the above: - # name: anything before `@`. Ex: access-om2 - # ref: anything after `@` or `@git.`; but before `=` (for =VERSION syntax) or ` `/`+`/`~` (for variants). Ex: 2025.01.0 - # version: anything after a `=`, but before ` `/`+`/`~` (for variants). Ex: release - # variants: anything at the end that contains ` `/`+`/`~` Ex. ~variant +debug some=value - - groups_regex='(?.+)@(?:git\\.)?(?[^=+~ ]+)(?:=(?[^~+ ]+))?(?[~+ ].+)?' - - groups=$(yq '${{ steps.yq.outputs.filter }} | capture("'"$groups_regex"'")' ${{ inputs.spack-manifest-path }}) - - # Pull values from groups above - name=$(yq '.name' <<< "$groups") - ref=$(yq '.ref' <<< "$groups") - version=$(yq '.version' <<< "$groups") - variants=$(yq '.variants' <<< "$groups") - - echo "Split '$full' into name: '$name', ref: '$ref', version: '$version', variants: '$variants'" - - echo "full=$full" >> $GITHUB_OUTPUT - echo "name=$name" >> $GITHUB_OUTPUT - echo "ref=$ref" >> $GITHUB_OUTPUT - echo "version=$version" >> $GITHUB_OUTPUT - echo "variants=$variants" >> $GITHUB_OUTPUT diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index be3e39d6..7980bd35 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -114,19 +114,19 @@ jobs: target: ${{ matrix.target }} error-level: error - get-root-spec-ref: - name: Get root spec ref + get-deployment-version: + name: Get Deployment Version runs-on: ubuntu-latest needs: - verify-settings outputs: - root-spec-ref: ${{ steps.tag.outputs.root-spec-ref }} + version: ${{ steps.manifest.outputs.deployment-version }} steps: - uses: actions/checkout@v4 - - name: Get root spec ref - id: tag - uses: access-nri/build-cd/.github/actions/get-spack-root-spec@v8 + - name: Get Deployment Version + id: manifest + uses: access-nri/build-cd/.github/actions/get-spack-manifest@v8 with: spack-manifest-path: ${{ inputs.spack-manifest-path }} @@ -135,7 +135,7 @@ jobs: if: inputs.tag-deployment runs-on: ubuntu-latest needs: - - get-root-spec-ref + - get-deployment-version permissions: contents: write steps: @@ -155,13 +155,8 @@ jobs: - name: Push Tag env: - TAG: ${{ needs.get-root-spec-ref.outputs.root-spec-ref }} + TAG: ${{ needs.get-deployment-version.outputs.version }} run: | - if [[ "${{ env.TAG }}" == "latest" ]]; then - echo "::error::The version 'latest' is reserved for a spack package that moves often and cannot be used as a release tag. Reset the 'main' or 'backport' branch, reopen the merged PR, and update the version." - exit 1 - fi - git tag ${{ env.TAG }} -m "Deployment of ${{ inputs.model }} ${{ env.TAG }} via build-cd 'cd.yml' workflow" git push --tags @@ -169,13 +164,13 @@ jobs: name: Deploy Release needs: - defaults - - get-root-spec-ref + - get-deployment-version - push-tag # Pushing tags is optional if we are doing a non-model deployment if: >- always() && needs.defaults.result == 'success' && - needs.get-root-spec-ref.result == 'success' && + needs.get-deployment-version.result == 'success' && (needs.push-tag.result == 'success' || needs.push-tag.result == 'skipped') strategy: matrix: @@ -185,7 +180,7 @@ jobs: deployment-target: ${{ matrix.target }} deployment-ref: ${{ github.ref_name }} deployment-type: Release - deployment-version: ${{ needs.get-root-spec-ref.outputs.root-spec-ref }} + deployment-version: ${{ needs.get-deployment-version.outputs.version }} spack-manifest-path: ${{ inputs.spack-manifest-path }} expected-root-spec-name: ${{ needs.defaults.outputs.root-sbd }} spack-manifest-schema-path: ${{ inputs.spack-manifest-schema-path }} @@ -204,7 +199,7 @@ jobs: if: inputs.tag-deployment needs: - defaults - - get-root-spec-ref + - get-deployment-version - push-tag - deploy-release runs-on: ubuntu-latest @@ -245,7 +240,7 @@ jobs: id: release-body env: J2_MODEL: ${{ inputs.model }} - J2_VERSION: ${{ needs.get-root-spec-ref.outputs.root-spec-ref }} + J2_VERSION: ${{ needs.get-deployment-version.outputs.version }} J2_ROOT_SBD: ${{ needs.defaults.outputs.root-sbd }} run: | python -m scripts.jinja_template.render_deployment_info \ @@ -256,8 +251,8 @@ jobs: - name: Create Release uses: softprops/action-gh-release@69320dbe05506a9a39fc8ae11030b214ec2d1f87 # v2.0.5 with: - tag_name: ${{ needs.get-root-spec-ref.outputs.root-spec-ref }} - name: ${{ inputs.model}} ${{ needs.get-root-spec-ref.outputs.root-spec-ref }} + tag_name: ${{ needs.get-deployment-version.outputs.version }} + name: ${{ inputs.model}} ${{ needs.get-deployment-version.outputs.version }} body_path: ${{ env.TEMPLATED_RELEASE_BODY_PATH }} generate_release_notes: true fail_on_unmatched_files: true diff --git a/.github/workflows/ci-comment.yml b/.github/workflows/ci-comment.yml index 3895c7e5..ba92a5ad 100644 --- a/.github/workflows/ci-comment.yml +++ b/.github/workflows/ci-comment.yml @@ -73,7 +73,7 @@ jobs: - name: Original version id: original - uses: access-nri/build-cd/.github/actions/get-spack-root-spec@v8 + uses: access-nri/build-cd/.github/actions/get-spack-manifest@v8 - name: Setup id: setup @@ -99,11 +99,11 @@ jobs: echo "version=${tag_date}" >> $GITHUB_OUTPUT echo "bump=major" >> $GITHUB_OUTPUT else - echo "version=${{ steps.original.outputs.root-spec-ref }}" >> $GITHUB_OUTPUT + echo "version=${{ steps.original.outputs.deployment-version }}" >> $GITHUB_OUTPUT echo "bump=current" >> $GITHUB_OUTPUT fi elif [[ "${{ contains(github.event.comment.body, 'minor')}}" == "true" ]]; then - echo "version=${{ steps.original.outputs.root-spec-ref }}" >> $GITHUB_OUTPUT + echo "version=${{ steps.original.outputs.deployment-version }}" >> $GITHUB_OUTPUT echo "bump=minor" >> $GITHUB_OUTPUT else echo "::warning::Usage: `!bump [major|minor]`, got `${{ github.event.comment.body }}`" @@ -131,22 +131,17 @@ jobs: git_tag_gpgsign: true - name: Update, Commit and Push the Bump - env: - # If the root spec contained an '=VERSION' segment, we want to add that back to the updated root spec, too - OPTIONAL_VERSION: ${{ steps.original.outputs.root-spec-version != '' && format('={0}', steps.original.outputs.root-spec-version) || '' }} run: | - updated_spec="${{ needs.defaults.outputs.root-sbd }}@git.${{ steps.bump.outputs.after }}${{ env.OPTIONAL_VERSION }}" - yq -i "${{ steps.original.outputs.yq-root-spec }} = \"$updated_spec\"" spack.yaml - yq -i '${{ env.SPACK_YAML_MODEL_PROJECTION_YQ }} = "{name}/${{ steps.bump.outputs.after }}"' spack.yaml + yq -i '.spack.definitions[]._version[0] = "${{ steps.bump.outputs.after }}"' spack.yaml git add spack.yaml - git commit -m "spack.yaml: Updated ${{ needs.defaults.outputs.root-sbd }} package version from ${{ steps.original.outputs.root-spec-ref }} to ${{ steps.bump.outputs.after }}" + git commit -m "spack.yaml: Updated deployment version from ${{ steps.original.outputs.deployment-version }} to ${{ steps.bump.outputs.after }}" git push - name: Success Notifier uses: access-nri/actions/.github/actions/pr-comment@main with: comment: | - :white_check_mark: Version bumped from `${{ steps.original.outputs.root-spec-ref }}` to `${{ steps.bump.outputs.after }}` :white_check_mark: + :white_check_mark: Version bumped from `${{ steps.original.outputs.deployment-version }}` to `${{ steps.bump.outputs.after }}` :white_check_mark: - name: Failure Notifier if: failure() diff --git a/.github/workflows/deploy-1-setup.yml b/.github/workflows/deploy-1-setup.yml index d7d4f72e..08a069dc 100644 --- a/.github/workflows/deploy-1-setup.yml +++ b/.github/workflows/deploy-1-setup.yml @@ -279,7 +279,7 @@ jobs: pull-requests: write outputs: # Release version of the deployment. Inferred if not given in inputs.deployment-version - release: ${{ steps.current.outputs.root-spec-ref }} + release: ${{ steps.current.outputs.deployment-version }} # Spack env name in form - spack-env-name: ${{ steps.get-env-name.outputs.env-name }} steps: @@ -298,21 +298,19 @@ jobs: - name: Get current (${{ inputs.deployment-ref }}) root spec version id: current - uses: access-nri/build-cd/.github/actions/get-spack-root-spec@v8 + uses: access-nri/build-cd/.github/actions/get-spack-manifest@v8 with: spack-manifest-path: ${{ inputs.spack-manifest-path }} - - name: Check root spec usage - if: env.IS_NOT_DRAFT == 'true' || inputs.deployment-type == 'Release' + - name: Disallow multi-spec Releases + if: inputs.deployment-type == 'Release' + # FIXME: Once the necessary infrastructure/guidance is in place, remove this step. See ACCESS-NRI/build-cd#344 run: | - if [[ "${{ steps.current.outputs.root-spec-ref }}" == "latest" ]]; then - echo "::error::The '@latest' version string is not allowed in non-draft PRs. It is reserved for a spack package that moves often and cannot be used as a future release tag" - exit 1 - fi + root_specs=$(yq '.spack.specs | select(.[] == "${{ steps.current.outputs.deployment-name }}*") | length' ${{ inputs.spack-manifest-path }}) - if [[ "${{ inputs.expected-root-spec-name }}" != "${{ steps.current.outputs.root-spec-name }}" ]]; then - echo "::error::The root spec in the spack manifest (${{ steps.current.outputs.root-spec-name }}) cannot be different from the expected root spec (${{ inputs.expected-root-spec-name }}) in non-Draft PRs." - exit 1 + if [ "$root_specs" -gt 1 ]; then + echo "::error::We don't yet support multiple root specs with the same package name. Contact the Model Release Team for information." + exit 1 fi - name: Checkout base (${{ inputs.prerelease-compare-ref }}) spack manifest @@ -337,7 +335,7 @@ jobs: - name: Get base root spec version id: base if: inputs.deployment-type != 'Release' && steps.checkout-base-spack.outcome != 'failure' - uses: access-nri/build-cd/.github/actions/get-spack-root-spec@v8 + uses: access-nri/build-cd/.github/actions/get-spack-manifest@v8 with: spack-manifest-path: ${{ inputs.spack-manifest-path }} @@ -348,7 +346,7 @@ jobs: if: inputs.deployment-type != 'Release' && steps.checkout-base-spack.outcome != 'failure' && env.IS_NOT_DRAFT == 'true' id: version-modified run: | - if [[ "${{ steps.base.outputs.root-spec-ref }}" == "${{ steps.current.outputs.root-spec-ref }}" ]]; then + if [[ "${{ steps.base.outputs.deployment-version }}" == "${{ steps.current.outputs.deployment-version }}" ]]; then echo "::warning::The version string in ${{ inputs.spack-manifest-path }} hasn't been modified in this PR, but needs to be before merging." exit 1 fi diff --git a/.github/workflows/deploy-2-start.yml b/.github/workflows/deploy-2-start.yml index eab9b23a..eeac5c7f 100644 --- a/.github/workflows/deploy-2-start.yml +++ b/.github/workflows/deploy-2-start.yml @@ -121,7 +121,7 @@ jobs: - name: Spack Manifest - Get Provenance and Injection Packages id: config-packages run: | - packages_for_injection=$(jq -cr '.provenance + .injection | join(" ")' config/packages.json) + packages_for_injection=$(jq -cr '.provenance + .injection | join(",")' config/packages.json) packages_for_provenance=$(jq -cr '.provenance | join(" ")' config/packages.json) echo "Packages to be injected: $packages_for_injection" @@ -132,17 +132,11 @@ jobs: - name: Spack Manifest - Modules Injection # Injects relevant module projections and includes into the spack manifest so users don't have to - id: modules-injection working-directory: build-cd run: | - original_root_spec_projection=$(yq '.spack.modules.default.tcl.projections."${{ steps.manifest.outputs.root-spec-name }}"' ../${{ inputs.spack-manifest-path }}) - if [[ "$original_root_spec_projection" != "null" ]]; then - echo "custom-projection=${original_root_spec_projection}" >> $GITHUB_OUTPUT - fi - python -m scripts.spack_manifest.injection.modules \ --manifest ../${{ inputs.spack-manifest-path }} \ - --packages "${{ steps.config-packages.outputs.packages-for-injection }}" \ + --packages ${{ steps.config-packages.outputs.packages-for-injection }} \ --output ../${{ inputs.spack-manifest-path }} - name: Get SHA for access-spack-packages repository @@ -155,8 +149,7 @@ jobs: - name: Spack Manifest - Prerelease Injection if: inputs.deployment-type == 'Prerelease' # Modifies the name of the prerelease modulefile to the pr-` style. - # Also removes the `@git.VERSION` specifier for Prereleases as it is a tag that doesn't yet exist. - # Also adds a repos section that points to the prereleases specific spack-packages repo. + # Also adds a repos section that points to the prereleases specific access-spack-packages repo. working-directory: build-cd run: | python -m scripts.spack_manifest.injection.prerelease \ @@ -164,7 +157,6 @@ jobs: --version ${{ inputs.version }} \ --spack-packages-path ${{ steps.path.outputs.spack-environment }}/access-spack-packages \ --spack-packages-version-sha ${{ steps.packages-ref.outputs.sha }} \ - ${{ steps.modules-injection.outputs.custom-projection != '' && format('--custom-root-projection {0}', steps.modules-injection.outputs.custom-projection ) || ''}} \ --output ../${{ inputs.spack-manifest-path }} - name: Copy ${{ inputs.spack-manifest-path }} @@ -262,7 +254,7 @@ jobs: hash=$(spack find --format "{^${pkg}.hash}" /$root_spec_hash) version=$(spack find --format '{version}' /$hash) location=$(spack find --format '{prefix}' /$hash) - pkg_repo_url=$(spack python -c "import spack.spec; print(spack.spec.Spec('$pkg').package_class.git)") + pkg_repo_url=$(spack python -c "print(spack.repo.PATH.get_pkg_class('$pkg').git)") # We need to create a tmp output file as jq doesn't support in-place read/write jq \ diff --git a/scripts/spack_manifest/getter.py b/scripts/spack_manifest/getter.py index 9a8ae583..71365897 100644 --- a/scripts/spack_manifest/getter.py +++ b/scripts/spack_manifest/getter.py @@ -15,6 +15,107 @@ class NoSectionComponentError(Exception): pass +class Specs: + def __init__(self, manifest: dict[str, Any]): + self.manifest = manifest + + self.specs: list[str] = self._get_specs_from_manifest_or_raise() + + def _get_specs_from_manifest_or_raise(self) -> list[str]: + defs: list[dict[str, Any]] = self.manifest.get("spack", {}).get( + "definitions", [] + ) + specs: list[str] = self.manifest.get("spack", {}).get("specs", []) + + # It's either in the multi-target format or the single target format, we just need to find which + # The multi-target format is of the form: + # spack: + # definitions: + # - ROOT_PACKAGE: [access-om2] + # # ... + # FIXME: Multi-target-formatted specs only have the first one picked up. See ACCESS-NRI/build-cd#343 + root_package_def = next( + (d["ROOT_PACKAGE"] for d in defs if "ROOT_PACKAGE" in d), [] + ) + if root_package_def != []: + return [root_package_def[0]] + elif len(specs) != 0: + return specs + else: + raise NoSectionError( + "No specs defined in the manifest spack.specs section for a single-target manifest." + ) + + @classmethod + def from_file(cls, manifest_path: str) -> "RootSpec": + from yaml import safe_load + + with open(manifest_path, "r") as file: + manifest = safe_load(file) + + return cls(manifest) + + def get_specs(self) -> list[str]: + return self.specs + + def get_specs_with_name(self, name: str) -> list[str]: + return [s for s in self.specs if s.startswith(name)] + +class ReservedDefinitions: + def __init__(self, manifest: dict[str, Any]): + self.manifest: dict[str, Any] = manifest + + self.reserved_definitions: list[dict[str, Any]] = ( + self._get_reserved_definitions_from_manifest_or_raise() + ) + + def _get_reserved_definitions_from_manifest_or_raise(self) -> dict[str, Any]: + definitions: list[dict[str, Any]] = self.manifest.get("spack", {}).get( + "definitions", [] + ) + + if definitions == []: + raise NoSectionError( + f"spack.definitions section not found in the manifest." + ) + + # Turn something with the spack-specific structure: + # {'definitions': [ + # {'_name': ['access-om2']}, + # {'_version': ['2025.02.100']}, + # {'something': ['else']} + # ]} + # Into a much easier to parse: + # {'name': 'access-om2', 'version': '2025.02.100'} + # Stripping out non-reserved definitions and unneeded single-element lists + reserved_definitions: dict[str, Any] = {} + for definition in definitions: + if len(definition) > 0: + reserved_name, reserved_value_list = list(definition.items())[0] + if reserved_name.startswith("_") and len(reserved_value_list) > 0: + reserved_name_no_underscore = reserved_name.lstrip("_") + # In future if we want to handle other reserved defs as lists, we can add a case statement here + reserved_definitions[reserved_name_no_underscore] = reserved_value_list[0] + + return reserved_definitions + + @classmethod + def from_file(cls, manifest_path: str) -> "ReservedDefinitions": + from yaml import safe_load + + with open(manifest_path, "r") as file: + manifest = safe_load(file) + + return cls(manifest) + + def get(self, definition: str) -> str: + if definition not in self.reserved_definitions: + raise NoSectionComponentError( + f"Reserved definition '{definition}' not found in the manifest spack.definitions section." + ) + + return self.reserved_definitions[definition] + class RootSpec: def __init__(self, manifest: dict[str, Any]): @@ -23,7 +124,9 @@ def __init__(self, manifest: dict[str, Any]): self.root_spec: str = self._get_root_spec_from_manifest_or_raise() def _get_root_spec_from_manifest_or_raise(self) -> str: - defs: list[dict[str, Any]] = self.manifest.get("spack", {}).get("definitions", []) + defs: list[dict[str, Any]] = self.manifest.get("spack", {}).get( + "definitions", [] + ) specs: list[str] = self.manifest.get("spack", {}).get("specs", []) # It's either in the multi-target format or the single target format, we just need to find which @@ -190,7 +293,6 @@ def get_package_full_version_requirement(self, name: str) -> str: return requirements[0] - def get_package_requirements(self, name: str) -> list[str]: if name not in self.packages: raise NoSectionComponentError( @@ -269,3 +371,13 @@ def get(self) -> dict[str, str]: .get("tcl", {}) .get("projections", {}) ) + + def get_projection_with_name(self, name: str) -> str | None: + return ( + self.manifest.get("spack", {}) + .get("modules", {}) + .get("default", {}) + .get("tcl", {}) + .get("projections", {}) + .get(name, None) + ) diff --git a/scripts/spack_manifest/injection/modules.py b/scripts/spack_manifest/injection/modules.py index a7f95c08..8eaebb3d 100644 --- a/scripts/spack_manifest/injection/modules.py +++ b/scripts/spack_manifest/injection/modules.py @@ -4,11 +4,20 @@ from typing import Any from scripts.spack_manifest.getter import ( - RootSpec, + ReservedDefinitions, Packages, Includes, Projections, + Specs ) +from scripts.spack_manifest.injection.yaml_representer import ( + YamlExplicitFlowStyleSequence, + yaml_explicit_flow_style_sequence_representer, + enforce_explicit_flow_style_definitions +) + +# We represent reserved definitions as in flow-style sequences (eg. `[a]` rather than `- a`), so it is more compact. +yaml.add_representer(YamlExplicitFlowStyleSequence, yaml_explicit_flow_style_sequence_representer) ################## # Main functions # @@ -19,25 +28,30 @@ def main(): # Get inputs args = parse_args(sys.argv[1:]) - packages: set[str] = set(args.packages.split()) + packages: set[str] = set(args.packages.split(",")) with open(args.manifest, "r") as file: manifest: dict[str, Any] = yaml.safe_load(file) # Inject manifest with projections and includes - root_spec_name: str = RootSpec(manifest).get_name() + + deployment_name: str = ReservedDefinitions(manifest).get("name") manifest_with_projections: dict[str, Any] = inject_projections( - manifest=manifest, root_spec=root_spec_name, packages=packages + manifest=manifest, root_spec=deployment_name, packages=packages ) manifest_with_projections_and_includes: dict[str, Any] = inject_includes( - manifest=manifest_with_projections, root_spec=root_spec_name, packages=packages + manifest=manifest_with_projections, root_spec=deployment_name, packages=packages + ) + + finalized_manifest: dict[str, Any] = enforce_explicit_flow_style_definitions( + manifest_with_projections_and_includes ) # Output the modified manifest dumped_manifest: str = yaml.dump( - manifest_with_projections_and_includes, + finalized_manifest, default_flow_style=False, sort_keys=False, ) @@ -48,7 +62,6 @@ def main(): with open(args.output, "w") as output_file: output_file.write(dumped_manifest) - def inject_projections( manifest: str, root_spec: str, packages: set[str] ) -> dict[str, Any]: @@ -72,10 +85,9 @@ def inject_projections( # To start with, add the projections that are already defined in the manifest new_projections: dict[str, str] = dict(defined_projections_dict) - if root_spec not in defined_projections: - new_projections.update( - generate_projection_for_root_spec_or_raise(manifest, root_spec) - ) + new_projections.update( + generate_projection_for_root_spec_or_raise(manifest, root_spec, defined_projections_dict.get(root_spec)) + ) for projection in projections_to_generate: new_projections.update( @@ -122,24 +134,41 @@ def inject_includes( def generate_projection_for_root_spec_or_raise( - manifest: dict[str, Any], root_spec_name: str + manifest: dict[str, Any], + root_spec_name: str, + root_spec_projection: str | None = None ) -> dict[str, str]: - root_spec_getter = RootSpec(manifest) + reserved_definitions_getter = ReservedDefinitions(manifest) + specs_getter = Specs(manifest) - root_spec_name_from_definition: str = root_spec_getter.get_name() - version = root_spec_getter.get_ref() - - if root_spec_name_from_definition != root_spec_name: - raise ValueError( - f"Expected root spec name '{root_spec_name}' does not match the name in the root spec definition '{root_spec_name_from_definition}'. The --root-spec needs to be defined the same as the actual root spec." - ) + version = reserved_definitions_getter.get("version") print( - f"Extracted version '{version}' from root spec definition '{root_spec_getter.get()}'" + f"Extracted version '{version}' from _version definition'" ) - # We don't add a hash to the root spec projection, as it is a unique deployment - return {root_spec_name: f"{{name}}/{version}"} + if root_spec_projection: + # We have a custom projection defined for the root spec, so we need to respect that + projection_components = root_spec_projection.split("/", 1) + + if len(projection_components) == 1: + updated_version: str = f"{{name}}/{version}/{projection_components[0]}" + else: + updated_version: str = f"{{name}}/{version}/{projection_components[1]}" + + print(f"Root spec already had a projection ({root_spec_projection}) so we infix the the version ({version}) to give: {updated_version}") + + return {root_spec_name: updated_version} + + root_specs_in_speclist = len(specs_getter.get_specs_with_name(root_spec_name)) + + if root_specs_in_speclist == 0: + raise ValueError(f"No specs with name {root_spec_name} in speclist") + elif root_specs_in_speclist == 1: + return {root_spec_name: f"{{name}}/{version}"} + else: + # If there are multiple of the same root spec (for example, different variants housed under the same environment), we need to demarcate the modulefile with a spack package hash + return {root_spec_name: f"{{name}}/{version}/{{hash:7}}"} def generate_projection_for_package_or_raise( @@ -184,7 +213,7 @@ def parse_args(args: list[str]) -> argparse.Namespace: "--packages", type=str, required=True, - help="List of space-separated packages (excluding the root spec) to be considered for projection injection", + help="List of comma-separated packages (excluding the root spec) to be considered for projection injection", ) parser.add_argument( @@ -196,12 +225,6 @@ def parse_args(args: list[str]) -> argparse.Namespace: parsed_args = parser.parse_args(args) - # Verifying that --packages are space-separated, which is a bit different from the usual comma-separated lists - if "," in parsed_args.packages: - raise ValueError( - "The --packages argument must be a space-separated list of package names." - ) - return parsed_args diff --git a/scripts/spack_manifest/injection/prerelease.py b/scripts/spack_manifest/injection/prerelease.py index 04e4dacb..bf611045 100644 --- a/scripts/spack_manifest/injection/prerelease.py +++ b/scripts/spack_manifest/injection/prerelease.py @@ -7,37 +7,31 @@ from copy import deepcopy from scripts.spack_manifest.getter import ( - RootSpec, + ReservedDefinitions, Projections, + Specs +) +from scripts.spack_manifest.injection.yaml_representer import ( + YamlExplicitFlowStyleSequence, + YamlExplicitQuotedString, + yaml_explicit_flow_style_sequence_representer, + yaml_explicit_quoted_string_representer, + enforce_explicit_flow_style_definitions ) -# PyYaml by default dumps unquoted strings if they look unambiguous, and quoted strings otherwise. -# PyYaml dumps '{name}/prX-Y' as a quoted str as it has '{' at the front and causes ambiguity -# But 'ROOT_SPEC/.dependencies/prX-Y/VERSION-{hash:7}' is dumped as an unquoted str as it is unambiguous -# So we need to wrap projections in a custom class that forces PyYaml to dump them as quoted strings. -class YamlExplicitQuotedString(str): - pass - - -def yaml_explicit_quoted_string_representer(dumper, data): - """ - Custom representer for YAML to ensure that some strings are quoted explicitly. - This is necessary for strings that are used as projections in spack manifests. - """ - return dumper.represent_scalar("tag:yaml.org,2002:str", data, style="'") - - +# The yaml representer sometimes dumps ambiguous strings in the case of projections like `{name}/...` as unquoted strings, +# which is not handled by spack very well. yaml.add_representer(YamlExplicitQuotedString, yaml_explicit_quoted_string_representer) -### Actual methods begin here ### +# We represent reserved definitions as in flow-style sequences (eg. `[a]` rather than `- a`), so it is more compact. +yaml.add_representer(YamlExplicitFlowStyleSequence, yaml_explicit_flow_style_sequence_representer) +### Actual methods begin here ### def inject_prerelease_information( manifest_path: str, version: str, - custom_root_projection: str | None = None, - keep_root_spec_intact: bool = False, spack_packages_path: str | None = None, spack_packages_version_sha: str | None = None, ) -> str: @@ -46,20 +40,16 @@ def inject_prerelease_information( with open(manifest_path, "r") as manifest_file: manifest: dict[str, Any] = yaml.safe_load(manifest_file) - root_spec_from_manifest = RootSpec(manifest) - root_spec_name = root_spec_from_manifest.get_name() + reserved_definitions_from_manifest = ReservedDefinitions(manifest) + root_spec_name = reserved_definitions_from_manifest.get("name") updated_manifest: dict[str, Any] = deepcopy(manifest) - # Remove @git.VERSION information from the root spec, since it will be a tag that does not yet exist for prereleases - # This does not include versions of the form @VERSION, which are the hallmark of software deployment repositories, - # or builds that explicitly ask to keep_root_spec_intact. - if not keep_root_spec_intact: - updated_manifest = remove_potential_root_spec_git_version(manifest) - - # We want the root spec projection to be of the form {name}/prX-Y + # We want the root spec projection to be of the form {name}/prX-Y for single specs, and + # {name}/prX-Y/DEMARCATOR for multiple specs, so we don't have modulefile clashes. + # The DEMARCATOR can be a custom projection, or {hash:7} if not supplied. updated_manifest = update_root_spec_projection_version( - updated_manifest, root_spec_name, version, custom_root_projection + updated_manifest, root_spec_name, version ) # We want all other projections to be of the form {name}/prX-Y/VERSION @@ -73,6 +63,8 @@ def inject_prerelease_information( updated_manifest, spack_packages_path, spack_packages_version_sha ) + updated_manifest = enforce_explicit_flow_style_definitions(updated_manifest) + # Dump the current dict, and add the non-standard 'repo::' section manifest_str: str = yaml.dump( updated_manifest, default_flow_style=False, sort_keys=False @@ -110,43 +102,32 @@ def add_namespace_to_other_projection_versions( return manifest -def remove_potential_root_spec_git_version(manifest: dict[str, Any]) -> dict[str, Any]: - """ - Remove the version information from the root spec in the manifest. - This is necessary for prerelease deployments where the version may not yet exist. - """ - root_spec_from_manifest = RootSpec(manifest) - name = root_spec_from_manifest.get_name() - constraints = root_spec_from_manifest.get_non_version_constraints() - - if root_spec_from_manifest.has_git_ref(): - # Remove the @git version and then add later contraints back - manifest["spack"]["specs"][0] = f"{name} {constraints}".strip() - else: - print( - f"The root spec '{name}' does not have a git ref, so no changes are made." - ) - - return manifest - - def update_root_spec_projection_version( - manifest: dict[str, Any], root_spec_name: str, root_spec_version: str, custom_root_projection: str | None = None + manifest: dict[str, Any], root_spec_name: str, deployment_version: str ) -> dict[str, Any]: + manifest.setdefault("spack", {}).setdefault("modules", {}).setdefault("default", {}).setdefault("tcl", {}).setdefault("projections", {}) - if custom_root_projection is not None and custom_root_projection != "": - projection_components = custom_root_projection.split("/", 1) + current_root_projection = Projections(manifest).get_projection_with_name(root_spec_name) + number_of_root_specs_in_speclist = len(Specs(manifest).get_specs_with_name(root_spec_name)) - if len(projection_components) == 1: - updated_version: str = f"{{name}}/{root_spec_version}/{projection_components[0]}" - else: - updated_version: str = f"{{name}}/{root_spec_version}/{projection_components[1]}" - else: - updated_version: str = f"{{name}}/{root_spec_version}" + if current_root_projection: + # Essentially - replace the original version infix with the prX-Y style, and add back the custom suffix if there was one. + # For example: + # {name}/2025.12.000 -> {name}/prX-Y + # {name}/2025.12.000/{variant.x} -> {name}/prX-Y/{variant.x} + new_root_projection = re.sub(r"(.+?/)[^/]+(/.+)?", fr"\1{deployment_version}\2", current_root_projection) - manifest.setdefault("spack", {}).setdefault("modules", {}).setdefault("default", {}).setdefault("tcl", {}).setdefault("projections", {}) + if number_of_root_specs_in_speclist > 1 and re.match(fr"^.+?/{deployment_version}$", new_root_projection): + # If there are multiple of the same root spec, we need to demarcate them somehow if there was no custom suffix given. + # We use a short package hash as the demarcator if no custom suffix was given. + new_root_projection += "/{hash:7}" + else: + if number_of_root_specs_in_speclist == 1: + new_root_projection = f'{{name}}/{deployment_version}' + else: + new_root_projection = f'{{name}}/{deployment_version}/{{hash:7}}' - manifest["spack"]["modules"]["default"]["tcl"]["projections"][root_spec_name] = updated_version + manifest["spack"]["modules"]["default"]["tcl"]["projections"][root_spec_name] = new_root_projection return manifest @@ -189,22 +170,6 @@ def parse_args(args: list[str]) -> argparse.Namespace: help="Version to be used for projections in the manifest", ) - parser.add_argument( - "--custom-root-projection", - type=str, - required=False, - help="Custom projection string to be used for the root spec in the manifest", - ) - - # This option is for the special case where the root spec defined at the repository level (a bundle with - # a version that doesn't yet exist) is not the same as the root spec defined in the manifest (which could - # be a regular package with a meaningful version). This is not recommended, but can be useful for special builds. - parser.add_argument( - "--keep-root-spec-intact", - action="store_true", - help="If set, the root spec will not be modified to remove git version information.", - ) - parser.add_argument( "--spack-packages-path", type=str, @@ -236,8 +201,6 @@ def main(): injected_manifest: str = inject_prerelease_information( args.manifest, args.version, - args.custom_root_projection, - args.keep_root_spec_intact, args.spack_packages_path, args.spack_packages_version_sha, ) diff --git a/scripts/spack_manifest/injection/yaml_representer.py b/scripts/spack_manifest/injection/yaml_representer.py new file mode 100644 index 00000000..f62be703 --- /dev/null +++ b/scripts/spack_manifest/injection/yaml_representer.py @@ -0,0 +1,46 @@ +from typing import Any + +# For sequences of reserved definitions, we keep it in the flow-style format (i.e., [a, b, c]) rather than block style +# as it is more compact for reserved definitions. +class YamlExplicitFlowStyleSequence(list[str]): + pass + +def yaml_explicit_flow_style_sequence_representer(dumper, data): + """ + Custom representer for YAML to ensure that some sequences are represented in flow style. + This is necessary for sequences that are used as definitions in spack manifests. + """ + return dumper.represent_sequence("tag:yaml.org,2002:seq", data, flow_style=True) + + +# PyYaml by default dumps unquoted strings if they look unambiguous, and quoted strings otherwise. +# PyYaml dumps '{name}/prX-Y' as a quoted str as it has '{' at the front and causes ambiguity +# But 'ROOT_SPEC/.dependencies/prX-Y/VERSION-{hash:7}' is dumped as an unquoted str as it is unambiguous +# So we need to wrap projections in a custom class that forces PyYaml to dump them as quoted strings. +class YamlExplicitQuotedString(str): + pass + +def yaml_explicit_quoted_string_representer(dumper, data): + """ + Custom representer for YAML to ensure that some strings are quoted explicitly. + This is necessary for strings that are used as projections in spack manifests. + """ + return dumper.represent_scalar("tag:yaml.org,2002:str", data, style="'") + +# Custom logic to enforce explicit flow-style sequences for `spack.definitions` that are reserved definitions +def enforce_explicit_flow_style_definitions(manifest: dict[str, Any]) -> dict[str, Any]: + """ + Ensure that the 'definitions' section of the manifest is represented in flow style. + This is necessary for spack manifests to ensure that definitions are correctly interpreted. + """ + if "spack" in manifest and "definitions" in manifest["spack"]: + definitions: list[dict[str, Any]] = manifest["spack"]["definitions"] + for i in range(len(definitions)): + definition = definitions[i] + if len(definition) > 0: + reserved_definition, reserved_value_list = list(definition.items())[0] + + if reserved_definition.startswith("_"): + manifest["spack"]["definitions"][i][reserved_definition] = YamlExplicitFlowStyleSequence(reserved_value_list) + + return manifest diff --git a/tests/scripts/spack_manifest/injection/inputs/prerelease.spack.yaml b/tests/scripts/spack_manifest/injection/inputs/prerelease.spack.yaml index dc44818d..71638bfa 100644 --- a/tests/scripts/spack_manifest/injection/inputs/prerelease.spack.yaml +++ b/tests/scripts/spack_manifest/injection/inputs/prerelease.spack.yaml @@ -4,37 +4,40 @@ # configuration settings. spack: # add package specs to the `specs` list + definitions: + - _name: [access-om2] + - _version: [2024.03.0] specs: - - access-om2@git.2024.03.0=latest + - access-om2 packages: cice5: require: - - '@git.2023.10.19=access-om2' + - '@git.2023.10.19=access-om2' mom5: require: - - '@git.2023.11.09=access-om2' + - '@git.2023.11.09=access-om2' libaccessom2: require: - - '@git.2023.10.26=access-om2' + - '@git.2023.10.26=access-om2' oasis3-mct: require: - - '@git.2023.11.09=access-om2' + - '@git.2023.11.09=access-om2' netcdf-c: require: - - '@4.7.4' + - '@4.7.4' netcdf-fortran: require: - - '@4.5.2' + - '@4.5.2' parallelio: require: - - '@2.5.2' + - '@2.5.2' openmpi: require: - - '@4.0.2' + - '@4.0.2' all: require: - - '%intel@19.0.5.281' - - 'target=x86_64' + - '%intel@19.0.5.281' + - 'target=x86_64' view: true concretizer: unify: true @@ -42,11 +45,11 @@ spack: default: tcl: include: - - access-om2 - - mom5 - - cice5 - - libaccessom2 - - oasis3-mct + - access-om2 + - mom5 + - cice5 + - libaccessom2 + - oasis3-mct projections: access-om2: '{name}/2024.03.0' mom5: '{name}/2023.11.09-{hash:7}' diff --git a/tests/scripts/spack_manifest/injection/inputs/spack.yaml b/tests/scripts/spack_manifest/injection/inputs/spack.yaml index c9626f08..d4f1825d 100644 --- a/tests/scripts/spack_manifest/injection/inputs/spack.yaml +++ b/tests/scripts/spack_manifest/injection/inputs/spack.yaml @@ -4,8 +4,11 @@ # configuration settings. spack: # add package specs to the `specs` list + definitions: + - _name: ["access-om2"] + - _version: ["2024.03.0"] specs: - - access-om2@git.2024.03.0=latest + - access-om2 packages: cice5: require: diff --git a/tests/scripts/spack_manifest/injection/outputs/expected.prerelease.spack.yaml b/tests/scripts/spack_manifest/injection/outputs/expected.prerelease.spack.yaml index 6f7a32f6..e135f0d2 100644 --- a/tests/scripts/spack_manifest/injection/outputs/expected.prerelease.spack.yaml +++ b/tests/scripts/spack_manifest/injection/outputs/expected.prerelease.spack.yaml @@ -1,4 +1,7 @@ spack: + definitions: + - _name: [access-om2] + - _version: [2024.03.0] specs: - access-om2 packages: @@ -43,7 +46,7 @@ spack: - libaccessom2 - oasis3-mct projections: - access-om2: '{name}/pr12-12/2024.03.0' + access-om2: '{name}/pr12-12' mom5: 'access-om2/dependencies/pr12-12/{name}/2023.11.09-{hash:7}' repos: access_spack_packages: diff --git a/tests/scripts/spack_manifest/injection/test_modules.py b/tests/scripts/spack_manifest/injection/test_modules.py index b7cd8714..fda1c90e 100644 --- a/tests/scripts/spack_manifest/injection/test_modules.py +++ b/tests/scripts/spack_manifest/injection/test_modules.py @@ -37,13 +37,13 @@ def test_parse_args__valid_no_optionals_multiple_packages(self): "--manifest", "tests/scripts/spack_manifest/injection/inputs/spack.yaml", "--packages", - "mom5 cice5 libaccessom2" + "mom5,cice5,libaccessom2" ] parsed_args = parse_args(args) assert parsed_args.manifest == "tests/scripts/spack_manifest/injection/inputs/spack.yaml" - assert parsed_args.packages == "mom5 cice5 libaccessom2" + assert parsed_args.packages == "mom5,cice5,libaccessom2" assert parsed_args.output is None, "Output should be None when not specified." @@ -54,31 +54,20 @@ def test_parse_args__valid_with_optionals_multiple_packages(self): "--output", "output.yaml", "--packages", - "mom5 cice5 libaccessom2" + "mom5,cice5,libaccessom2" ] parsed_args = parse_args(args) assert parsed_args.manifest == "tests/scripts/spack_manifest/injection/inputs/spack.yaml" assert parsed_args.output == "output.yaml" - assert parsed_args.packages == "mom5 cice5 libaccessom2" - - def test_parse_args__invalid_comma_separated_packages(self): - args = [ - "--manifest", - "tests/scripts/spack_manifest/injection/inputs/spack.yaml", - "--packages", - "mom5,cice5,libaccessom2" - ] - - with pytest.raises(ValueError): - parse_args(args) + assert parsed_args.packages == "mom5,cice5,libaccessom2" class TestGenerateProjectionForRootSpecOrRaise: def test_generate_projection_for_root_spec_or_raise__valid_single_target(self): - manifest = {"spack": {"specs": ["access-om2@git.2025.05.000 +debug ~mpi"]}} + manifest = {"spack": {"definitions": [{"_name": ["access-om2"]}, {"_version": ["2025.05.000"]}],"specs": ["access-om2 +debug ~mpi"]}} projection = "access-om2" expected_version = {"access-om2": "{name}/2025.05.000"} @@ -92,6 +81,8 @@ def test_generate_projection_for_root_spec_or_raise__valid_multi_target(self): manifest = { "spack": { "definitions": [ + {"_name": ["access-om2"]}, + {"_version": ["2025.05.000"]}, {"ROOT_PACKAGE": ["access-om2@git.2025.05.000 +debug ~mpi"]} ] } @@ -108,7 +99,7 @@ def test_generate_projection_for_root_spec_or_raise__valid_multi_target(self): def test_generate_projection_for_root_spec_or_raise__single_target_no_version_defined( self, ): - manifest = {"spack": {"specs": ["access-om2"]}} + manifest = {"spack": {"definitions": [{"_name": ["access-om2"]}],"specs": ["access-om2"]}} projection = "access-om2" @@ -128,7 +119,7 @@ def test_generate_projection_for_root_spec_or_raise__multi_target_no_version_def def test_generate_projection_for_root_spec_or_raise__single_target_wrong_projection( self, ): - manifest = {"spack": {"specs": ["access-om2@git.2025.05.000 +debug ~mpi"]}} + manifest = {"spack": {"definitions": [{"_name": ["access-om2"]}, {"_version": ["2025.05.000"]}],"specs": ["access-om2 +debug ~mpi"]}} projection = "wrong-projection" @@ -141,7 +132,9 @@ def test_generate_projection_for_root_spec_or_raise__multi_target_wrong_projecti manifest = { "spack": { "definitions": [ - {"ROOT_PACKAGE": ["access-om2@git.2025.05.000 +debug ~mpi"]} + {"_name": ["access-om2"]}, + {"_version": ["2025.05.000"]}, + {"ROOT_PACKAGE": ["access-om2@git.2025.05.000 +debug ~mpi"]}, ] } } @@ -206,7 +199,11 @@ def test_inject_projections__valid(self): expected_output = { "spack": { - "specs": ["access-om2@git.2024.03.0=latest"], + "definitions": [ + {"_name": ["access-om2"]}, + {"_version": ["2024.03.0"]}, + ], + "specs": ["access-om2"], "packages": { "cice5": {"require": ["@git.2023.10.19=access-om2"]}, "mom5": {"require": ["@git.2023.11.09=access-om2"]}, diff --git a/tests/scripts/spack_manifest/injection/test_prerelease.py b/tests/scripts/spack_manifest/injection/test_prerelease.py index 7eee3cb5..1be0a2e0 100644 --- a/tests/scripts/spack_manifest/injection/test_prerelease.py +++ b/tests/scripts/spack_manifest/injection/test_prerelease.py @@ -3,22 +3,24 @@ from scripts.spack_manifest.injection.prerelease import ( inject_prerelease_information, add_prerelease_repos_section, - remove_potential_root_spec_git_version, update_root_spec_projection_version, add_namespace_to_other_projection_versions, parse_args ) class TestUpdateRootSpecProjectionVersion: - def test_update_root_spec_projection_version__valid_existing(self): + def test_update_root_spec_projection_version__valid_existing_single_spec(self): manifest = { "spack": { + "specs": [ + "access-om2", + ], "modules": { "default": { "tcl": { "projections": { - "access-om2": "{name}/1.0.0", - "dependency": "{name}/2.0.0" + "access-om2": "{name}/2025.12.000/1.0.0", + "dependency": "{name}/2025.12.000/2.0.0" } } } @@ -27,19 +29,25 @@ def test_update_root_spec_projection_version__valid_existing(self): } root_spec_name = "access-om2" root_spec_version = "pr12-2" - custom_root_spec_projection = "1.0.0" - updated_manifest = update_root_spec_projection_version(manifest, root_spec_name, root_spec_version, custom_root_spec_projection) + updated_manifest = update_root_spec_projection_version(manifest, root_spec_name, root_spec_version) - assert updated_manifest["spack"]["modules"]["default"]["tcl"]["projections"][root_spec_name] == f"{{name}}/{root_spec_version}/{custom_root_spec_projection}" + assert updated_manifest["spack"]["modules"]["default"]["tcl"]["projections"][root_spec_name] == f"{{name}}/{root_spec_version}/1.0.0" - def test_update_root_spec_projection_version__valid_new(self): + def test_update_root_spec_projection_version__valid_existing_multi_spec(self): manifest = { "spack": { + "specs": [ + "access-om2 +var", + "access-om2 ~var" + ], "modules": { "default": { "tcl": { - "projections": {} + "projections": { + "access-om2": "{name}/2025.12.000/1.0.0", + "dependency": "{name}/2025.12.000/2.0.0" + } } } } @@ -50,14 +58,20 @@ def test_update_root_spec_projection_version__valid_new(self): updated_manifest = update_root_spec_projection_version(manifest, root_spec_name, root_spec_version) - assert updated_manifest["spack"]["modules"]["default"]["tcl"]["projections"][root_spec_name] == f"{{name}}/{root_spec_version}" + assert updated_manifest["spack"]["modules"]["default"]["tcl"]["projections"][root_spec_name] == f"{{name}}/{root_spec_version}/1.0.0" - def test_update_root_spec_projection_version__no_projections(self): + + def test_update_root_spec_projection_version__valid_new_single_spec(self): manifest = { "spack": { + "specs": [ + "access-om2" + ], "modules": { "default": { - "tcl": {} + "tcl": { + "projections": {} + } } } } @@ -69,58 +83,69 @@ def test_update_root_spec_projection_version__no_projections(self): assert updated_manifest["spack"]["modules"]["default"]["tcl"]["projections"][root_spec_name] == f"{{name}}/{root_spec_version}" -class TestRemovePotentialRootSpecGitVersion: - def test_remove_potential_root_spec_git_version__valid_no_constraints(self): + def test_update_root_spec_projection_version__valid_new_multi_spec(self): manifest = { "spack": { "specs": [ - "access-om2@git.1.0.0" - ] - } - } - - updated_manifest = remove_potential_root_spec_git_version(manifest) - - assert updated_manifest["spack"]["specs"][0] == "access-om2" - - def test_remove_potential_root_spec_git_version__valid_with_constraints_no_space(self): - manifest = { - "spack": { - "specs": [ - r"access-om2@git.1.0.0%compiler@2.0.0" - ] + "access-om2 +var", + "access-om2 ~var" + ], + "modules": { + "default": { + "tcl": { + "projections": {} + } + } + } } } + root_spec_name = "access-om2" + root_spec_version = "pr12-2" - updated_manifest = remove_potential_root_spec_git_version(manifest) + updated_manifest = update_root_spec_projection_version(manifest, root_spec_name, root_spec_version) - assert updated_manifest["spack"]["specs"][0] == f"access-om2 %compiler@2.0.0" + assert updated_manifest["spack"]["modules"]["default"]["tcl"]["projections"][root_spec_name] == f"{{name}}/{root_spec_version}/{{hash:7}}" - def test_remove_potential_root_spec_git_version__valid_with_constraints_with_space(self): + def test_update_root_spec_projection_version__no_projections_single_spec(self): manifest = { "spack": { "specs": [ - r"access-om2@git.1.0.0 %compiler@2.0.0 +debug" - ] + "access-om2" + ], + "modules": { + "default": { + "tcl": {} + } + } } } + root_spec_name = "access-om2" + root_spec_version = "pr12-2" - updated_manifest = remove_potential_root_spec_git_version(manifest) + updated_manifest = update_root_spec_projection_version(manifest, root_spec_name, root_spec_version) - assert updated_manifest["spack"]["specs"][0] == f"access-om2 %compiler@2.0.0 +debug" + assert updated_manifest["spack"]["modules"]["default"]["tcl"]["projections"][root_spec_name] == f"{{name}}/{root_spec_version}" - def test_remove_potential_root_spec_git_version__invalid_no_git_version(self): + def test_update_root_spec_projection_version__no_projections_multi_spec(self): manifest = { "spack": { "specs": [ - "access-om2@1.0.0" - ] + "access-om2 +var", + "access-om2 ~var" + ], + "modules": { + "default": { + "tcl": {} + } + } } } + root_spec_name = "access-om2" + root_spec_version = "pr12-2" - updated_manifest = dict(remove_potential_root_spec_git_version(manifest)) + updated_manifest = update_root_spec_projection_version(manifest, root_spec_name, root_spec_version) - assert manifest == updated_manifest, "Manifest should remain unchanged if no git version is present" + assert updated_manifest["spack"]["modules"]["default"]["tcl"]["projections"][root_spec_name] == f"{{name}}/{root_spec_version}/{{hash:7}}" class TestAddPrereleaseReposSection: def test_add_prerelease_repos_section__valid(self): @@ -147,12 +172,11 @@ class TestInjectPrereleaseInformation: def test_inject_prerelease_information__valid_custom_projection(self): manifest_path = "tests/scripts/spack_manifest/injection/inputs/prerelease.spack.yaml" root_spec_version = "pr12-12" - custom_root_projection = "2024.03.0" spack_packages_path = "/some/spack-packages" spack_packages_version_sha = "e8713551c6eee57caf9603543e6dd6daf3c93922" updated_manifest_str: str = inject_prerelease_information( - manifest_path, root_spec_version, custom_root_projection, spack_packages_path, spack_packages_version_sha + manifest_path, root_spec_version, spack_packages_path, spack_packages_version_sha ) expected_manifest_path = "tests/scripts/spack_manifest/injection/outputs/expected.prerelease.spack.yaml" diff --git a/tests/scripts/spack_manifest/test_getter.py b/tests/scripts/spack_manifest/test_getter.py index 537c8ae4..80828e92 100644 --- a/tests/scripts/spack_manifest/test_getter.py +++ b/tests/scripts/spack_manifest/test_getter.py @@ -2,10 +2,12 @@ import yaml from scripts.spack_manifest.getter import ( + ReservedDefinitions, RootSpec, Packages, Includes, Projections, + Specs, NoSectionError, NoSectionComponentError, ) @@ -36,6 +38,55 @@ def manifest_from_file(): } } +class TestReservedDefinitionsGetter: + @pytest.fixture + def manifest_with_reserved_definitions(self): + return { + "spack": { + "definitions": [ + {"_name": ["access-om2"]}, + {"_version": ["2025.11.000"]}, + {"OTHER_DEFINITION": ["some-value"]}, + ] + } + } + def test___init___valid(self, manifest_with_reserved_definitions): + + reserved_definitions_getter = ReservedDefinitions(manifest_with_reserved_definitions) + + expected = { + "name": "access-om2", + "version": "2025.11.000", + } + + assert ( + reserved_definitions_getter.reserved_definitions == expected + ), "Reserved definitions should be correctly initialized from manifest." + + def test___init___invalid_no_definitions_section(self): + manifest = {"spack": {}} + + with pytest.raises(NoSectionError): + ReservedDefinitions(manifest) + + def test_get__valid(self, manifest_with_reserved_definitions): + + reserved_definitions_getter = ReservedDefinitions(manifest_with_reserved_definitions) + definition_value = reserved_definitions_getter.get("name") + + expected = "access-om2" + + assert ( + definition_value == expected + ), "Reserved definitions should be correctly retrieved." + + def test_get__invalid_no_definition(self, manifest_with_reserved_definitions): + + reserved_definitions_getter = ReservedDefinitions(manifest_with_reserved_definitions) + + with pytest.raises(NoSectionComponentError): + reserved_definitions_getter.get("nonexistent_definition") + class TestRootSpecGetter: def test___init___valid_multi_target_spec(self): @@ -448,3 +499,41 @@ def test__get_defined_includes__no_modules_section(self): assert ( result == expected ), "Manifest without a modules section should return an empty set." + + +class TestSpecsGetter: + @pytest.mark.parametrize( + "specs", + [ + ["access-om2"], # Single root spec + ["access-om2 +var", "access-om2 ~var"] # Multiple root specs + ] + ) + def test_get_specs__valid(self, specs): + manifest = {"spack": {"specs": specs}} + + assert Specs(manifest).get_specs() == specs, "Should return all specs" + + @pytest.mark.parametrize( + "specs,expected", + [ + (["access-om2", "access-om3"], ["access-om2"]), # Single root spec + (["access-om2 +var", "access-om2 ~var", "access-om3"], ["access-om2 +var", "access-om2 ~var"]) # Multiple root specs + ] + ) + def test_get_specs_with_name__exist(self, specs, expected): + manifest = {"spack": {"specs": specs}} + + assert Specs(manifest).get_specs_with_name("access-om2") == expected, "Should return specs with the given name" + + @pytest.mark.parametrize( + "specs", + [ + ["access-om3"], + ["access-om3 +var", "access-om3 ~var"] + ] + ) + def test_get_specs_with_name__no_exist(self, specs): + manifest = {"spack": {"specs": specs}} + + assert Specs(manifest).get_specs_with_name("access-om2") == [], "Should return no specs" \ No newline at end of file