-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix evaluation of release-20.03-aarch64 jobset #82886
Fix evaluation of release-20.03-aarch64 jobset #82886
Conversation
The release-20.03-aarch64 jobset on hydra only evals for aarch64, so the x86_64 jobs do not exists. We need to make sure that the tested job only aggregates jobs that actually exist. This commit solves the issue by generating the tested job constituents names based on the supported systems.
This removes tests from the tested aggregate on aarch64 which are not available for that platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Recording the change discussed on IRC here)
This change should first be made on master
, then backported (though the backport will not be a trivial cherry-pick).
There is a version of this PR for master on https://github.com/bennofs/nixpkgs/tree/fix-nixos-aarch64-eval-master. Note however that the situation for master is a little bit different, as we don't have split jobsets for |
Is it OK to keep this PR for discussion? Or should I create a new one against master? |
I guess that means we'd need two PRs, with this one merged just for 20.03. |
@worldofpeace ok, I created a second PR for master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is fine, though I'm not sure how I feel about the function names (can't suggest better right now I'm afraid).
I did some prior work on a related topic. When the hydra eval changed we had to apply the conversion to strings on release-19.09 as well: I tried to be as close to the "old" style definition as possible to make it more familiar and reduce conflicts if we would be cherry-picking changes. No real opinion just felt like that might be a good piece of information. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/go-no-go-meeting-nixos-20-03-markhor/6495/16 |
nixos/release-combined.nix: fix tested/supportedSystems (master version of #82886)
Motivation for this change
The
nixos-20.03-aarch64
jobset on hydra currently fails to eval. This is because the constituents of the tested job are hardcoded to refer to thex86_64-linux
tests, which aren't available in that jobset.Proposed solution
This PR uses helper functions to only generate
tested
constituents for the available platforms. I also checked which tests are supported on aarch64, and limited everything else tox86_64-linux
.Variant for master: https://github.com/bennofs/nixpkgs/tree/fix-nixos-aarch64-eval-master
Things done
tested.constituents
did stay the same before and after the change: diff of the output fromnix-instantiate --eval --strict --json -A tested.constituents nixos/release-combined.nix | jq
before and after the change:tested
job can be instantiated foraarch64
: