-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
treewide: mass rebuild complicated cleanups #48721
treewide: mass rebuild complicated cleanups #48721
Conversation
Looks scary but it is a noop.
Success on x86_64-linux (full log) Attempted: llvm_5, stdenv Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: llvm_5, stdenv Partial log (click to expand)
|
Thanks!! The last commit should just be dropped; everything else is good. The issue is while technically it is correct to set that env var based on the target platform, the vast majority of package are not sensitiveness to the target platform, and so we'd like to keep it that way way. With that uncommented, all build-time deps of cross compiled packages on darwin must be rebuilt, which is a big annoyance. The solutions I want is @lheckemann's stuff to use absolute dt_needed / sonames, and avoid all this rpath stuff everywhere (on linux and darwin). |
84b48cb
to
91ea34a
Compare
I see. Done.
|
Success on x86_64-linux (full log) Attempted: llvm_5 Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: llvm_5 Partial log (click to expand)
|
@@ -424,7 +424,8 @@ stdenv.mkDerivation ({ | |||
platforms = | |||
stdenv.lib.platforms.linux ++ | |||
stdenv.lib.platforms.freebsd ++ | |||
stdenv.lib.platforms.illumos; | |||
stdenv.lib.platforms.illumos ++ | |||
stdenv.lib.platforms.darwin; |
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.
IIRC gcc 4.8 is broken on darwin. Perhaps, the same is true for 4.9. Should probably use the meta.broken
attribute to indicate that.
@@ -240,7 +240,6 @@ stdenv.mkDerivation ({ | |||
# The builder relies on GNU sed (for instance, Darwin's `sed' fails with | |||
# "-i may not be used with stdin"), and `stdenvNative' doesn't provide it. | |||
++ (optional hostPlatform.isDarwin gnused) | |||
++ (optional hostPlatform.isDarwin targetPackages.stdenv.cc.bintools) |
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 think you missed one on line 238 here.
Waiting for more feedback before merging into staging. |
91ea34a
to
110f16e
Compare
I think you missed one on line 238 here.
Oops, right, fixed.
IIRC gcc 4.8 is broken on darwin. Perhaps, the same is true for 4.9. Should probably use the ```meta.broken``` attribute to indicate that.
But the expression for 4.8 has `darwin` in platforms and I didn't touch that part.
|
Success on x86_64-linux (full log) Attempted: llvm_5 Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: llvm_5 Partial log (click to expand)
|
ping?
All of this builds fine on `x86_64-linux` and `meta` nitpicks can be fixed separately, IMHO.
|
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 don't feel competent to review this since I'm not familiar with the details of our gcc and llvm expressions. |
I am not keeping a close track of the minor details in the expressions, but I can understand what these specific changes do, and I agree these are a mix of useful cleanups and neutral layout changes. |
buildInputs = [ libxml2 libffi ] | ||
# TODO(@Ericson2314): Remove next mass rebuild | ||
++ stdenv.lib.optionals (stdenv.isDarwin && stdenv.hostPlatform == stdenv.buildPlatform) [ libcxxabi ]; | ||
buildInputs = [ libxml2 libffi ]; |
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.
Oh for the record this is because compiler-rt
is now built separately and libcxxabi
was just needed for that. Overall on the LLVM side, tools and runtime libraries are never built in the same derivation, which is good.
Motivation for this change
Continuation of #47233.
The first two patches homogenize gcc expressions (i.e. make
diff
between them as small as possible) in the hope that they could be merged into a single expression eventually.I have no idea whether the third patch does the right thing.
Things done
/cc @Ericson2314