Open CodeGat opened 3 weeks ago
We're at war with our smarter selves. I wonder who will win?
Ironically, this is probably one of the arguments for using patch files instead of a branch that tracks upstream with local changes as commits.
Hi @CodeGat , Could we do something like:
diff --git a/config/settings.json b/config/settings.json
index 94fe836..7c3fbf2 100644
--- a/config/settings.json
+++ b/config/settings.json
@@ -5,16 +5,19 @@checks
"Release": {
"0.20": {
"spack": "6812713cf470b473a607f0de0e8e1cf53f804fb7",
+ "access-spack": "x",
"spack-config": "2024.03.22"
},
"0.22": {
"spack": "21da7d7e2b5e2680cd9d2e0a2fb4a7d13d8baa9d",
+ "access-spack": "y",
"spack-config": "2024.07.05"
}
},
"Prerelease": {
"0.22": {
"spack": "21da7d7e2b5e2680cd9d2e0a2fb4a7d13d8baa9d",
+ "access-spack": "z",
"spack-config": "2024.07.05"
}
}
where:
spack
denotes the latest upstream commit
access-spack
denotes the latest ACCESS-NRI commit
Then only do your checks on the spack
variable.
But we test against the commit hash of the deployed access-nri/spack
hash - what does that have to do with the upstream one?
The latest upstream commit hash will be an ancestor of the latest ACCESS-NRI commit. I think you should be able to create a better test when you take into account how the ACCESS-NRI fork of Spack operates.
@harshula and myself came to two possibilities to fix this:
.deployment.TARGET.TYPE.MAJOR.spack-upstream
which is used to test that it, and therefore it's child commit (the spack
commit hash), is on the access-nri/spack
MAJOR
branch. One downside to this is that editors of config/settings.json
must do their own homework on what the value of upstream-spack
is, all for a single check. spack
field that is on the upstream spack
, and check that that is on the access-nri/spack
MAJOR
branch. Issues with this is that it might be difficult to determine what the last true commit on spack/spack
is, as opposed to one that is introduced during an interative rebase in which we swap/drop/create new commits. Potential solution is to link the upstream as a remote and check against upstream/releases/vMAJOR
. Both of those solutions will take time, so currently I'll opt for Option 3 for now, which is:
See warning in
Validate [config/settings.json] Deployment Settings
here: https://github.com/ACCESS-NRI/build-cd/actions/runs/11007422156/job/30563301689#step:5:245This is due to ACCESS-NRI/spack@21da7d7e2b5e2680cd9d2e0a2fb4a7d13d8baa9d no longer existing on the
releases/v0.22
branch ofaccess-nri/spack
. This is because after syncing our fork with the upstream for its0.22.2
updates, @harshula rebased our ACCESS-NRI-specific fixes on top of that.This currently prevents Model Deployment Repo PRs from being merged as any warnings in validation are converted to errors upon merging.
We need to come to a decision where either:
release/v$VERSION
branch. This would remove a guardrail that prevents people from modifying the0.XX
install to a commit hash not within that major version. Relevant code here: https://github.com/ACCESS-NRI/build-cd/blob/ab2b346275632e203996392442df7b628b94e3ec/.github/actions/validate-deployment-settings/action.yml#L83-L120spack
fixes commits stable