separateDebugInfo: Use --strip-unneeded #160259
Conversation
ofborg
bot
added
10.rebuild-linux-stdenv
andersk
added
11.by: nixpkgs-member
6.topic: closure size
|
This seems to break the Valgrind tests in |
|
That failure can be reproduced with |
|
I guess we’ll need to patch Valgrind to respect |
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
andersk
force-pushed
the
strip-unneeded
branch
from
96977af to
db54778
Compare
According to https://stackoverflow.com/q/46197810/115030, --only-keep-debug preserves all the information stripped by --strip-unneeded. This reduces the size of the webkitgtk output by 22% (123 MB → 96 MB). Inspired by NixOS#159612. Signed-off-by: Anders Kaseorg <andersk@mit.edu>
andersk
force-pushed
the
strip-unneeded
branch
from
db54778 to
d652556
Compare
| @@ -38,6 +39,7 @@ in stdenv.mkDerivation rec { | |||
| python3 | |||
| libxslt | |||
| ] ++ lib.optionals enableValgrindTests [ | |||
| glibc.debug | |||
There was a problem hiding this comment.
(Should this be in valgrind’s propagatedBuildInputs, or be left up to its users?)
There was a problem hiding this comment.
Good question...!
I'm not sure every use-case of valgrind (in and out of nixpkgs) would expect glibc debug infos to be available. Maybe. 🤷
ofborg
bot
added
10.rebuild-darwin: 11-100
and removed
10.rebuild-darwin: 0
There was a problem hiding this comment.
Ok with the change on libpsl
Result of nixpkgs-review pr 160259 run on x86_64-linux 1
1 package built:
- libpsl
As for --strip-unneeded it seems safe indeed.
Do we have a rough estimate of size difference (and thus the space that will be saved) compared to --strip-debug?
| @@ -34,7 +34,7 @@ _separateDebugInfo() { | |||
| # firmware blobs in QEMU.) | |||
| ( | |||
| $OBJCOPY --only-keep-debug "$i" "$dst/${id:0:2}/${id:2}.debug" | |||
| $STRIP --strip-debug "$i" | |||
| $STRIP --strip-unneeded "$i" | |||
There was a problem hiding this comment.
Will this ever be used on macOS? FWIW it supports neither flags.
strip is an area I've noticed where we tend to use non portable flags, maybe it's time to introduce a more bespoke strip wrapper…
There was a problem hiding this comment.
OK, all this sounds more like future improvements.
|
A more interesting thing would be to switch the default strip flag, i.e. even packages not using |
|
Uh, this PR is most likely what broke elfutils test(s). No idea why, but I can reproduce the issue :-/ |
|
Thread iterators rely on superfluous symbols to be retained in I suspect we over-strip |
|
"unneeded" :-( So I suppose we'll be reverting this PR, at least for now? (fixing glibc will be a stdenv rebuild anyway) |
|
Also, how should the tests look like in case of separate debug into? Looks like elfutils just tries to get a symbolised backtrace out of crashes binary and expects libc symbols from there. From what I understand What is the correct way to pull it in in libc-agnostic way? AFAIU |
|
I expect you rarely need to pull it into a build. For real-life usage it might be most practical to use |
Motivation for this change
According to https://stackoverflow.com/q/46197810/115030,
--only-keep-debugpreserves all the information stripped by--strip-unneeded. This reduces the size of the webkitgtk output by 22% (123 MB → 96 MB).Inspired by #159612.
Not sure whether to target
stagingormaster. I expect this to toucha lot of packagesokay, just about every package (via python, which hasseparateDebugInfo = true), so I went withstagingfor now. But on the other handmasteris currently blocked from feedingnixos-unstabledue to the GNOME ISO size issue.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/)nixos/doc/manual/md-to-db.shto update generated release notes