-
-
Notifications
You must be signed in to change notification settings - Fork 14k
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
llvm: add in a missing check dep #218865
llvm: add in a missing check dep #218865
Conversation
0785a31
to
cd2654b
Compare
Port of 6d0c876 ("llvmPackages_15.llvm: add in a missing check dep"). I did not include the sysctl nativeCheckInputs change from that commit, as it looks like it was included by mistake and was supposed to be in c7231c0 ("llvmPackages_15.llvm: run the tests on macOS"). I've also included the doCheck default from that commit (without the change to run on non-Linux), as 6d0c876 just set it to true.
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.
Apologies for the delay.
I did not include the sysctl nativeCheckInputs change from that commit, as it looks like it was included by mistake and was supposed to be in c7231c0 ("llvmPackages_15.llvm: run the tests on macOS").
I've also included the doCheck default from that commit (without the change to run on non-Linux), as 6d0c876 just set it to true.
Ah, thanks for untangling that; I definitely bungled the commits in that PR a bit.
This looks good!
Just to confirm: are these changes intentionally not backported to llvmPackages_5
? I haven't tried running the test suite to verify but a quick peek suggests that the dep was present in lit
even then.
, doCheck ? stdenv.isLinux && (!stdenv.isx86_32) && (!stdenv.hostPlatform.isMusl) && (!stdenv.hostPlatform.isRiscV) | ||
&& (stdenv.hostPlatform == stdenv.buildPlatform) |
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.
Just in case anyone else was also wondering: #211728 copies this condition (which LLVM 9 and 11 have) and disables the tests on RISC-V hosts for LLVM 12 - 14 (though not LLVM 10...); it'll have to be adjusted once this PR is merged.
It was intentional, as in my testing it didn't emit the warning message. I could try testing if the actual number of tests run changes. |
LLVM 6, with psutil:
LLVM 6, without psutil:
LLVM 5, with psutil:
LLVM 5, without psutil:
So it looks to me like LLVM 5, unlike LLVM 6, doesn't skip any tests as "unsupported" if psutil is missing. |
Description of changes
Port of 6d0c876 ("llvmPackages_15.llvm: add in a missing check dep").
I did not include the sysctl nativeCheckInputs change from that commit, as it looks like it was included by mistake and was supposed to be in c7231c0 ("llvmPackages_15.llvm: run the tests on macOS"). I've also included the doCheck default from that commit (without the change to run on non-Linux), as 6d0c876 just set it to true.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)