Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

cc-wrapper: Enter C++ mode when -lc++ is passed #269694

Closed
wants to merge 1 commit into from

Conversation

judofyr
Copy link
Contributor

@judofyr judofyr commented Nov 24, 2023

Description of changes

When linking C++ programs in LLVM we're currently explicitly passing in -lc++abi as this appears to be required by recent versions of Clang. However, it also appears to b common for packages to use the C compiler to link binaries which uses C++, in which case they do pass in -lc++. This changes the wrapper to also take this into consideration and enter "C++ mode" whenever -lc++ is passed as an argument.

Fixes #166205.
Fixes #150655

Things done

I'm very new to nixpkgs and haven't really tested this change directly. I did manually apply the same patch in a local version of the cc-wrapper script and saw that I was able to link C++ binaries using the C compiler (more specifically: I was able to build Postgis).

I believe this change will cause quite a lot of rebuilds? Hence I targeted the staging branch.

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Priorities

Add a 馃憤 reaction to pull requests you find important.

When linking C++ programs in LLVM we're currently explicitly passing
in -lc++abi as this appears to be required by recent versions of
Clang. However, it also appears to b common for packages to use
the C compiler to link binaries which uses C++, in which case they
do pass in -lc++. This changes the wrapper to also take this into
consideration and enter "C++ mode" whenever -lc++ is passed as an
argument.

Fixes NixOS#166205
@ghost
Copy link

ghost commented Nov 30, 2023

Is this ever necessary with gcc?

If not, could we make this behavior isClang-specific?

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-macos-monthly/12330/49

@hzeller
Copy link
Contributor

hzeller commented Dec 21, 2023

To clarify, this change is only intending to fix linking, right @Artturin ?

The matter of the #150655 bug was about finding the c++ headers when clang is invoked with a c++ file (it only works if explicitly invoked with clang++ which is unlike any other Linux distribution).

I verified that the following would not be resolved with this PR:

nix-shell  \
    -E '{ pkgs ? import <nixpkgs> {} }: pkgs.clang13Stdenv.mkDerivation { name="foo";}' \
    --run 'echo "#include <vector>" > /tmp/foo.cc ; clang -c /tmp/foo.cc -o /tmp/foo.o'

@ghost
Copy link

ghost commented Mar 17, 2024

superseded by #292043 which fixes the build.

@ghost ghost closed this Mar 17, 2024
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants