-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
rustc: Enable profiler on more platforms #314832
Conversation
Wouldn't it be better to assume this works on all platforms except ones where we have a reason to think it doesn't, like Redox? On a platform it doesn't work on, it would be IMO less confusing to get an error about profiler_builtins not building than one about it being missing from the standard library. |
34dafe9
to
5b7f605
Compare
@@ -139,7 +139,7 @@ in stdenv.mkDerivation (finalAttrs: { | |||
"${setBuild}.llvm-config=${llvmSharedForBuild.dev}/bin/llvm-config" | |||
"${setHost}.llvm-config=${llvmSharedForHost.dev}/bin/llvm-config" | |||
"${setTarget}.llvm-config=${llvmSharedForTarget.dev}/bin/llvm-config" | |||
] ++ optionals (stdenv.isLinux && !stdenv.targetPlatform.isRedox) [ | |||
] ++ optionals (stdenv.targetPlatform.isLinux || stdenv.targetPlatform.isDarwin || stdenv.targetPlatform.isAndroid || stdenv.targetPlatform.isFreeBSD || (stdenv.targetPlatform.isWindows && !stdenv.targetPlatform.isMinGW)) [ |
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.
These links in your commit message are great — but maybe they should be in a comment so they can easily be checked and updated?
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.
LGTM as long as OfBorg is happy.
Ofborg failed with a timeout in llvm. Tried building on the build box but
Error happens without this pr too ( |
|
feb77a1
to
ba7c386
Compare
Been a few months so retrying. |
New errors
|
Split out the |
Sources for the conditionals https://www.github.com/rust-lang/rust/blob/master/src/ci/github-actions/jobs.yml https://www.github.com/rust-lang/rust/blob/5fe554350208b72cb8b66ce39bf0b92b1bba6edd/src/ci/docker/host-x86_64/dist-x86_64-freebsd/Dockerfile#L41 https://www.github.com/search?q=repo%3Arust-lang%2Frust%20%2Fenable-profiler%2F&type=code MinGW excluded because there's an open pr to rust to remove it's `enable-profiler` in `jobs.yml` https://www.github.com/rust-lang/rust/pull/122613/files
ba7c386
to
07695c0
Compare
remove the
isRedox
condition which was added becausestdenv.isLinux
useshostPlatform
, after specifyingtargetPlatform
it's no longer necessaryFor error[E0463]: can't find crate for 'profiler_builtins'
#266570
https://logs.ofborg.org/?key=nixos/nixpkgs.266570&attempt_id=450d0b18-fd6a-4c44-893d-30de5941b73f
Description of changes
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.