Skip to content

Conversation

@CodeGat
Copy link
Member

@CodeGat CodeGat commented Aug 10, 2025

Background

In order to get the {{ scheduled_ref }} for the manifests, we need to git rev-parse the HEAD of the branch. We had updated the checkout logic to checkout access-esm1.5 instead of master so we could get that HEAD in the access-esm1.5 case, but the manifests themselves weren't in that branch! So we should move the manifests that deal with scheduled checks out of the master branch, into the branch that they'll be used.

So now, instead of the old directory structure, which namespaced scheduled tests by branch name on master, we don't have to separate them, as they are on their own branch. Instead, we have:

On master:
.github/
├── build-ci/
│   ├── data/
│   │   └── standard.json
│   └── manifests/
│       ├── pr/
│       │   └── ...
│       └── scheduled/
│           ├──  gcc_mom_access-esm1.6.spack.yaml.j2
│           └── ...
└── workflows/
    ├── ci.yml
    └── scheduled.yml

On access-esm1.5:
.github/
├── build-ci/
│   ├── data/
│   │   └── standard.json
│   └── manifests/
│       ├── pr/
│       │   └── ...
│       └── scheduled/
│           ├── gcc_mom_access-esm1.5.spack.yaml.j2
│           ├── intel_mom_access-esm1.5.spack.yaml.j2
│           └── oneapi_mom_access-esm1.5.spack.yaml.j2
└── workflows/
    └── ci.yml

So this PR removes the namespacing of scheduled tests, and deletes the access-esm1.5-specific ones (to be moved to the access-esm1.5 branch).

The PR

  • Move scheduled/master manifests under scheduled
  • Delete scheduled/access-esm1.5 (move to access-esm1.5 branch)
  • Remove branch namespacing, rev-parse HEAD instead of origin/BRANCH

@CodeGat CodeGat added the bug Something isn't working label Aug 10, 2025
@CodeGat CodeGat self-assigned this Aug 10, 2025
@CodeGat CodeGat added the bug Something isn't working label Aug 10, 2025
@CodeGat CodeGat requested a review from dougiesquire August 10, 2025 22:21
@CodeGat CodeGat marked this pull request as ready for review August 10, 2025 22:26
Copy link
Collaborator

@dougiesquire dougiesquire left a comment

Choose a reason for hiding this comment

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

Thanks @CodeGat. oasis3_mct_version_esm1p5 can now be removed from ./build-ci/data/standard.json, but otherwise LGTM

@CodeGat CodeGat requested a review from dougiesquire August 10, 2025 22:52
@CodeGat CodeGat force-pushed the master-scheduled-fix branch from c98a9d7 to ae9dfd2 Compare August 10, 2025 23:26
Copy link
Collaborator

@dougiesquire dougiesquire left a comment

Choose a reason for hiding this comment

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

It looks like the current exclusion is actually in the wrong place. We want:

  • master: exclude .github/build-ci/manifests/scheduled/gcc_mom_access-esm1.6.spack.yaml.j2
  • access-esm1.5: exclude .github/build-ci/manifests/scheduled/gcc_mom_access-esm1.5.spack.yaml.j2

Copy link
Collaborator

@dougiesquire dougiesquire left a comment

Choose a reason for hiding this comment

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

It also looks like the version of oasis3-mct that we use in access-esm1.5 doesn't build with oneapi. Let's exclude:

  • access-esm1.5: exclude .github/build-ci/manifests/scheduled/oneapi_mom_access-esm1.5.spack.yaml.j2

Copy link
Collaborator

@dougiesquire dougiesquire left a comment

Choose a reason for hiding this comment

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

Approving now because I'm so confident we're there.

Test of the scheduled workflow is running here: https://github.com/ACCESS-NRI/MOM5/actions/runs/16870417116

@CodeGat CodeGat merged commit 627a321 into master Aug 11, 2025
20 of 22 checks passed
@CodeGat CodeGat deleted the master-scheduled-fix branch August 11, 2025 04:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants