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
binutils: use shared libs #46056
binutils: use shared libs #46056
Conversation
206 -> 32 MiB, i.e. not like the previous 26, but much better now.
We might additionally consider having the libraries in a separate output, but we should look at the overall design – the separately built libs, etc. |
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.
Note that GDB has the same bad --enable-static
stuff, and probably should have a similar treatment
, fetchurl, zlib, autoreconfHook264 | ||
# Enabling all targets increases output size to a multiple. | ||
, withAllTargets ? false, libbfd, libopcodes | ||
, enableShared ? 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.
I'm not sure we even need this, but I suppose it doesn't hurt.
, noSysDirs, gold ? true, bison ? null | ||
}: | ||
|
||
let | ||
reuseLibs = enableShared && withAllTargets; |
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.
Woo! I am a fan of that.
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'd advocate making it unconditional if only there was a way to prevent it from being built at all, sigh.
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.
Apart from complicating stdenv bootstrap, they're significantly larger due to supporting all targets.
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.
So far the design is that you have "simple" versions of the libs inside the binutils derivation, and then those more "featureful" variants built separately for use by other packages.
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 right, good point.
Success on x86_64-darwin (full log) Attempted: binutils Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: binutils Partial log (click to expand)
|
Timed out, unknown build status on x86_64-linux (full log) Attempted: binutils Partial log (click to expand)
|
+ reuseLibs = enableShared && withAllTargets;
May I ask for a comment explaining what and why?
|
Why: when this condition holds, the libraries will be basically the same as the separately built ones, so it seems suitable to de-duplicate. I originally tried that by default, but there were down-sides so I only kept it conditional #46056 (comment) |
@vcunat I see, thanks. LGTM. |
Can someone confirm this actually fixes the binutils bloat issue? Looks good to merge if so... |
It's down from ~266M to ~70M. So let's go for the merge, as everyone appears to agree :) |
Motivation for this change
Fixing #44936
I had tested this on a different version of staging, including building various packages with the stdenv, the bootstrap-testing job, etc.