gcc: tighten platform flags special-case for aarch64-darwin #225328
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eliasnaur
force-pushed
the
platform-flags
branch
from
8810720 to
8d16f74
Compare
ofborg
bot
added
10.rebuild-darwin: 0
eliasnaur
changed the title
eliasnaur
force-pushed
the
platform-flags
branch
from
8d16f74 to
87c7651
Compare
eliasnaur
force-pushed
the
platform-flags
branch
from
87c7651 to
b3cbd02
Compare
|
Gentle ping. |
|
@trofi may I ask for a review? I know this is darwin, but you've had good insights into gcc guts on my other PRs. FWIW, this PR is the only missing piece for getting rid of |
If you can resolve the conflict we can push it to |
eliasnaur
force-pushed
the
platform-flags
branch
from
b3cbd02 to
efe1cce
Compare
|
Done, thank you. |
trofi
reviewed
Apr 27, 2023
| @@ -3,10 +3,13 @@ | |||
| let | |||
| gcc = targetPlatform.gcc or {}; | |||
| p = gcc | |||
| isAarch64Darwin = targetPlatform.isDarwin && targetPlatform.isAarch64; | |||
There was a problem hiding this comment.
You probably need to move it out of the middle of p expression (1 line above or below) as eval fails as:
error: syntax error, unexpected '=', expecting ';'
at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/ofborg-evaluator-4/pkgs/development/compilers/gcc/common/platform-flags.nix:6:19:
5| p = gcc
6| isAarch64Darwin = targetPlatform.isDarwin && targetPlatform.isAarch64;
| ^
There was a problem hiding this comment.
Of course, thank you.
eliasnaur
force-pushed
the
platform-flags
branch
from
efe1cce to
4fc79e2
Compare
The 4aa95e3 commit added support for aarch64-darwin but also ignored platform flags if the build platform is aarch64-darwin. This leads to confusing errors such as `pkgsCross.raspberryPi` packages compiled with soft-float even though the platform supports hard-float (and is built as such on other platforms). The correct way to ignore platform flags is to check `targetPlatform`, not the build platform. This change fixes that. While we're here, tigthen the special-case to cover only the problematic flags: `-with-cpu` and `-with-arch`.
eliasnaur
force-pushed
the
platform-flags
branch
from
4fc79e2 to
c10a195
Compare
|
Done. This time I tested with not 🤦 |
|
Pulled as is. Thank you! |
12 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The 4aa95e3 commit added support for
aarch64-darwin but also ignored platform flags if the build platform
is aarch64-darwin. This leads to confusing errors such as
pkgsCross.raspberryPipackages compiled with soft-float even thoughthe platform supports hard-float (and is built as such on other
platforms).
The correct way to ignore platform flags is to check
targetPlatform,not the build platform. This change fixes that.
While we're here, tigthen the special-case to cover only the problematic
flags:
-with-cpuand-with-arch.Things done
sandbox = trueset 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/)