-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
nixos/cc-wrapper: Fix bug if dynamicLinker not found #31209
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
nixos/cc-wrapper: Fix bug if dynamicLinker not found #31209
Conversation
If a dynamic linker for target is not found the generated script fails due to unbound variable error (due to "set -u"). Correct by specifying default value with dynamicLinker:- and not generating ldflagsBefore if no linker is found. This problem was found when cross compiling to mingw32 targets
@Ericson2314 knows cross-compiling best, I guess. |
This does not look right, the variable is always initialized a few lines above. And why would |
Because
dynamicLinker=()
in bash unbinds the variable.
(Try it! E.g.
set -u
vv="dkj"
vv=()
echo $vv
fails with unbound variable)
|
@@ -286,7 +286,7 @@ stdenv.mkDerivation { | |||
*) echo "Multiple dynamic linkers found for platform '${targetPlatform.config}'." >&2;; | |||
esac | |||
if [ -n "$dynamicLinker" ]; then | |||
if [ -n "''${dynamicLinker:-}" ]; then |
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.
This is correct, good catch.
@@ -296,7 +296,9 @@ stdenv.mkDerivation { | |||
echo ${libc_lib}/lib/32/ld-linux.so.2 > $out/nix-support/dynamic-linker-m32 | |||
fi | |||
local ldflagsBefore=(-dynamic-linker "$dynamicLinker") | |||
if [ -n "''${dynamicLinker:-}" ]; then |
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.
This second conditional is unneeded, no?
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's already part of the first, unless I messed up the string interpolation.
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.
You're right, I missed the indentation. Pushing a revised patch now
@nixborg build |
Jobset created at https://hydra.mayflower.de/jobset/nixos/pr-31209 |
@globin: it's included in staging now, so you may want to save resources for other stuff. |
Thanks @bnikolic! Btw I'm happy to pitch on various MinGW-related things, especially as they bitrotted further a bit under my watch. @domenkozar and @DavidEGrayson may also be interested. |
Thanks. Things are now failing a bit further along, looks like a problem
between sys-includes and includes when building mingw32 (sys-includes
need to come first by they do not by default?). I'll try to have a look
at it later in the week but if you one of you already has some ideas why
this could be happening do let me know...
|
@bnikolic I'm not sure if this is helpful for your sys-includes problem, but I use two patches for mingw-w64 GCC to make the search paths correct: https://github.com/DavidEGrayson/nixcrpkgs/blob/6501664/mingw-w64/gcc/cppdefault.patch The default search path for my toolchain is:
You might also want to look at the Arch Linux recipe for mingw-w64-gcc: https://www.archlinux.org/packages/community/x86_64/mingw-w64-gcc/ The default header search path is defined in |
@DavidEGrayson I've resolved the mingw-w64 problems in #33167 -- cross-compilation to win64 target seems to work with this. |
If a dynamic linker for target is not found the generated script fails
due to unbound variable error (in turn due to "set -u"). Correct by specifying
default value with dynamicLinker:- and not generating ldflagsBefore if
no linker is found.
This problem was found when cross compiling to mingw32 targets
Motivation for this change
Cross-compiling for mingw32 targets was currently broken with "variable unbound" errors. This change allows the compilation to proceed further.
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)