cc-wrapper: fix -Bprefix to not confuse lib/libc++.so and bin/c++ #317224
Conversation
ofborg
bot
added
10.rebuild-darwin-stdenv
|
@ju1m This fixed all our haskell builds on Arm64 darwin, which were trying to link in clang++... |
|
ping. What is this PR waiting on and can it move forward? I would like this to be merged so that we can drop the patch in haskell.nix. I recently duplicated this work (https://gitlab.haskell.org/ghc/ghc/-/merge_requests/13005) because I was unaware that this had been upstreamed. Similarly, landing this PR would close https://gitlab.haskell.org/ghc/ghc/-/issues/23138. cc @Ericson2314 |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/1812 |
|
@Ericson2314 are you able to help out with a review here, or do you know if any of the other |
|
I found https://gitlab.haskell.org/ghc/ghc/-/issues/23138 while investigating the failed built of fmt on staging-next #328673 due to ld64 upgrade in #307880, which lead me to this PR.
I’m probably the one you want. This seems fine. I did a hacky test modifying I don’t know if any other cc-wrapper maintainers want to weigh in, but once I confirm it builds, I’ll approve and merge. |
Before this commit, `pkgs/build-support/cc-wrapper/add-flags.sh` was using `-B@out@/bin` instead of `-B@bintools@/bin` to force `cc` to use `ld-wrapper.sh` when calling `ld`. That was confusing `cc` when asked to print the location of a library precisely named `c++` because `-B` prefixes are also used by `cc` to find libraries, see https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html#index-B Indeed, instead of having `cc --print-file-name c++` failing to found a `c++` library and just returning the given `c++` string to let a linker resolve it thereafter, it was finding that `@out@/bin/c++` executable, mistaking it for a library and returning its absolute path, forcing the linker to load an executable as a library. Before this commit: ```console $ nix run -f . stdenv.cc -- --print-file-name c++ /nix/store/9bv7dcvmfcjnmg5mnqwqlq2wxfn8d7yi-gcc-wrapper-13.2.0/bin/c++ ``` After this commit: ```console $ nix run -f . stdenv.cc -- --print-file-name c++ c++ ``` Fixes https://gitlab.haskell.org/ghc/ghc/-/issues/23138#note_567034 where this behavior was breaking GHC on Darwin. [Confirmed by @414owen](NixOS#317224 (comment)): > This fixed all our haskell builds on Arm64 darwin, which were trying > to link in clang++...
|
|
Once checks are green, this can be merged. |
Before this commit,
pkgs/build-support/cc-wrapper/add-flags.shwas using-B@out@/bininstead of-B@bintools@/binto forceccto useld-wrapper.shwhen callingld. That was confusingccwhen asked to print the location of a library precisely namedc++because-Bprefixes are also used byccto find libraries, see https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html#index-BIndeed, instead of having
cc --print-file-name c++failing to find ac++library and just returning the givenc++string to let a linker resolve it thereafter, it was finding that@out@/bin/c++executable, mistaking it for a library and returning its absolute path, forcing the linker to load an executable as a library.Users should likely ask for
libc++.soinstead ofc++, but that's not what's done,For instance this confusion was breaking with GHC on Darwin:
https://gitlab.haskell.org/ghc/ghc/-/issues/23138#note_567034
It's not clear to me why only the Darwin port of GHC was affected since this
-Bwas also injected in the Linux toolchain.And I don't have a Darwin host to test whether this fix is enough to solve the GHC bug mentioned.
But the result of
cc --print-file-name c++is clearly wrong to motivate this fix:Before this commit:
After this commit:
Description of changes
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.