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
cc-wrapper: Clean up dynamic linking with x86 multilib #29568
Conversation
@Ericson2314, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @orivej and @abbradar to be potential reviewers. |
-m) | ||
case "$p" in | ||
elf_i386) link32=1;; | ||
*) link32=0;; |
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 assume only the last -m
flag matters.
fd4ecec
to
6e56d6e
Compare
4411f99
to
2eea93f
Compare
b3b79c6
to
c8b5f99
Compare
c8b5f99
to
d5e80eb
Compare
46361c0
to
706e281
Compare
Ok, this one is ready for review next! |
@@ -71,8 +71,18 @@ declare -a libDirs | |||
declare -A libs | |||
relocatable= |
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.
It seems that link32
may be left undefined. I'd suggest declaring it here and moving these declarations under the following comment.
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 surprised I didn't hit that with set -u
.
It's better layering to do everything in ld-wrapper. Also, use numeric comparisons for `relocatable`.
706e281
to
fdbda21
Compare
Doing rebuilds overnight. Hope you all like it tomorrow! edit suceeded. |
Motivation for this change
It's better layering to do everything in ld-wrapper.
I pulled this out of #29396 as its good to consider on its own. It was reviewed there by @orivej, however.
Things done
build-use-sandbox
innix.conf
on non-NixOS)statifier
, some multilib-using Linux derivation. Tested that 32- and 64-bit binaries both ran.nix-shell -p nox --run "nox-review wip"
./result/bin/
)CC @orivej