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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake: detect libc location at runtime #181431

Merged
merged 1 commit into from
Aug 12, 2022

Conversation

stephank
Copy link
Contributor

@stephank stephank commented Jul 14, 2022

Description of changes

This PR updates the patch to Modules/Platform/UnixPaths.cmake to detect the libc location at runtime, based on the cc-wrapper specified in $NIX_CC. If $NIX_CC is not (or incorrectly) set, it falls back to the original behavior (ie.: use the libc that was used to build CMake itself).

This allows CMake to automatically pick up an alternate libc in package builds. The alternative would be to do something like -DCMAKE_SYSTEM_INCLUDE_PATH=${stdenv.cc.libc_dev}/include in every package build that wants a different libc.

This may sound niche, but I suspect may become a more common thing on Darwin as the option to use different macOS SDKs becomes available. #176661 was recently merged, making a second SDK available for x86_64-darwin by doing darwin.apple_sdk_11_0.callPackage ./my-package { }, which also swaps in a different stdenv with a different version of libSystem.

I'm specifically working on the Swift build on Darwin, which is picking up the wrong libSystem in a couple of places. A simple one is a call to find_package(Backtrace), which does a find_path(Backtrace_INCLUDE_DIR "execinfo.h"). Both calls can be easily tested in isolation.

Besides that, I also tested rebuilding fish on Linux with this change in place on master. Fish uses CMake, and apparently also pulls in a handful of CMake-using deps. That all worked fine and produced a working build.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@stephank
Copy link
Contributor Author

I've now also successfully tested building and running fish on Darwin with this change on master, which includes the stdenv rebuild.

Copy link
Member

@AndersonTorres AndersonTorres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@trofi
Copy link
Contributor

trofi commented Aug 8, 2022

Caveat: I have very little understanding of how darwin packaging works.

Do I understand it correctly you are planning to use cmake (and a bucnh of other packages) built against one libc to build a few select packages against another libc? Is it typical on darwin to do such a thing? Or do people usually build the whole world with one SDK?

darwin.apple_sdk_11_0.callPackage ./my-package { }

What does it do? Does it only substitute stdenv (and a few darwin-specific packages) just for this package? Or does it apply the same for all it's dependencies? I suspect it's the former and cmake is only the first example of the breakage. AFAUY ideally you somehow need it to be propagated to all the packages who care. glibc/musl or libstdc/libc++ (to pick an extreme) do it via pkgs/pksMusl or pkgs/pkgsLLVM to ensure consistent environment.

Would something like that work for darwin?

@stephank
Copy link
Contributor Author

stephank commented Aug 8, 2022

Do I understand it correctly you are planning to use cmake (and a bucnh of other packages) built against one libc to build a few select packages against another libc? Is it typical on darwin to do such a thing? Or do people usually build the whole world with one SDK?

This is a recent change in nixpkgs. It used to be Intel was built with the 10.12 SDK, and Apple Silicon was built with 11.0 SDK. The recent change is to opt-in per-package into the 11.0 SDK on Intel too.

What that means technically is also something I'm not an expert on. For example, libc / libsystem on darwin is typically injected by the loader (kernel?) at run-time, so I imagine despite linking against different libcs, they use the same at run-time. I imagine frameworks provided by Apple also use the same binary at run-time (sans injection), but maybe there are some exceptions for the ones we have source code of. I'm really speculating here, because I don't know the specifics. 🙂

In practice this PR may not necessarily be darwin specific. It's about about using cmake to build with a different libc (via cc wrapper override), without having to rebuild cmake itself.

darwin.apple_sdk_11_0.callPackage ./my-package { }

What does it do? Does it only substitute stdenv (and a few darwin-specific packages) just for this package? Or does it apply the same for all it's dependencies? I suspect it's the former and cmake is only the first example of the breakage. AFAUY ideally you somehow need it to be propagated to all the packages who care. glibc/musl or libstdc/libc++ (to pick an extreme) do it via pkgs/pksMusl or pkgs/pkgsLLVM to ensure consistent environment.

Would something like that work for darwin?

I do feel that a separate callPackage has limitations (and layer violations), but I'm not sure who is making those decisions. (Edit: This was rushed. I meant to ask, where this was discussed, and where we should further discuss it.)

The relevant part of that special callPackage for this PR is:

let
  clang = stdenv.cc.override {
    bintools = stdenv.cc.bintools.override { libc = packages.Libsystem; };
    libc = packages.Libsystem;
  };
in
  overrideCC stdenv clang;

But yes, it overrides specific packages (beyond stdenv), and all the Apple frameworks. See: pkgs/os-specific/darwin/apple-sdk-11.0/default.nix

I don't believe it recurses into dependencies.

@stephank
Copy link
Contributor Author

stephank commented Aug 8, 2022

Sorry, I rushed the last comment a bit, and made some small edits to it just now.

I think a relevant previous comment is #176661 (comment), quoting:

I tried a PoC pkgsApple.sdk_<version>, but that didn’t work. It requires a lot of rebuilds, which broke in a few places (see my comments on Matrix). I also fear it would result in tons of extra packages built by Hydra that wouldn’t be used.

I can't easily find the Matrix discussion mentioned, unfortunately.

From what I can tell, a pkgsMusl or pkgsLLVM is there for the user to specifically request an alternate build of some package? Whereas what we want here is to simply have a working nix-env -iA go on Darwin (and the user shouldn't have to care what SDK is used.)

Whether we do apple_sdk_11_0.callPackages or pkgsApple.sdk_11_0, we need to have pkgs.go somehow annotated to use the newer SDK. When I said limitations / layer violations, what I meant is that the current callPackages solution does not combine, afaict. (ie.: hypothetically, if another platform needs a similar solution, I don't believe we can stack the two.)

For a pkgsApple-type solution, I guess we'd need some way to do similar annotation in all-packages.nix, but I'm not sure what that'd look like. (For example, a simple go = pkgsApple.sdk_11_0.go is recursive.)

Another difference is that a pkgsApple annotation probably only makes sense when applied to end-user applications, whereas callPackages allows building individual libraries with different SDKs. (I think that answers your question about mixing packages?)

So, there is I think good reason to at least prefer callPackages over pkgsApple. I wonder if we can slowly tackle all the breakages like this PR? (Another darwin-specific example I know of is xcbuild, which is currently also tied to whatever SDK it was built with. That's why it's also currently part of the set overridden by callPackages.)

I wonder if we can maybe even reduce the amount of overrides to just be limited to the darwin.* namespace, so we could potentially drop callPackages and replace it with a simple // optionalAttrs stdenv.isDarwin darwin.pkgs_11_0 (which does combine). But that's probably wishful thinking, as #180931 is currently set to expand the overrides.

P.S.: In an ideal world, I believe that we'd always use the latest SDK and set -mmacosx-version-min (the "deployment target") to the oldest version we want to support. But we don't, and I guess that's because it doesn't work well in practice?

@trofi
Copy link
Contributor

trofi commented Aug 8, 2022

Whether we do apple_sdk_11_0.callPackages or pkgsApple.sdk_11_0, we need to have pkgs.go somehow annotated to use the newer SDK. When I said limitations / layer violations, what I meant is that the current callPackages solution does not combine, afaict. (ie.: hypothetically, if another platform needs a similar solution, I don't believe we can stack the two.)

For a pkgsApple-type solution, I guess we'd need some way to do similar annotation in all-packages.nix, but I'm not sure what that'd look like. (For example, a simple go = pkgsApple.sdk_11_0.go is recursive.)

Yeah, pkgs* are more like cross-targets that assume pkgs already works. I agree pkgsApple would not make default case simpler. At beast it would help building parallel world with non-default SDK.

Another difference is that a pkgsApple annotation probably only makes sense when applied to end-user applications, whereas callPackages allows building individual libraries with different SDKs. (I think that answers your question about mixing packages?)

Yeah. That makes sense.

So, there is I think good reason to at least prefer callPackages over pkgsApple. I wonder if we can slowly tackle all the breakages like this PR? (Another darwin-specific example I know of is xcbuild, which is currently also tied to whatever SDK it was built with. That's why it's also currently part of the set overridden by callPackages.)

Would putting cmake override along with xcbuild also be ~ok? AFAIU it should Just Work without patching if built against stdenv with modified SDK (and everything else required). That would make me a bit less nervous.

P.S.: In an ideal world, I believe that we'd always use the latest SDK and set -mmacosx-version-min (the "deployment target") to the oldest version we want to support. But we don't, and I guess that's because it doesn't work well in practice?

Do @NixOS/darwin-maintainers have a vision how things should be laid out in multiple SDKs world? Reading #101229 AFAIU we normally expect only one default SDK. That is somewhat similar to linux stdenv that defaults to a single current compiler (and attempts to use something else frequently breaks when too old libstdc++ gets pulled in).

@reckenrode
Copy link
Contributor

reckenrode commented Aug 8, 2022

It was my understanding that Go 1.18 is already being built with the 11.0 SDK, and Go 1.18 is the default in unstable. Is there additional context I’m missing? (I’m aware some packages still have problems building, but I believe that’s due to golang/go#51091).

Regarding cmake: I added cmake to callPackages in #180931, which is blocked due to needing updates to the bootstrap-tools on x86_64-darwin. The changes for that were in #181550, which has been merged, but stdenvBootstrapTools is not building on Hydra currently. I believe #182058 is attempting to fix that. After all that is done, and I go through the process of updating the bootstrap tools, then I can move #180931 forward.

Regarding the SDK: As I understand it, the default SDK on x86_64-darwin is still 10.12. Making the 11.0 (and eventually newer) SDKs available was meant to be an escape hatch for packages that don’t build on the default SDK (my motivating example being MoltenVK, but Go and some other packages also wouldn’t build on the 10.12 SDK). Meaning, if packages can build with the 10.13 SDK once the SDK is bumped, those packages should be reverted to the default stdenv.

@uri-canva
Copy link
Contributor

I don't know anything about cmake, so I can't review the code change, but I've been thinking about this exact issue in our build tooling more broadly, have started a discussion here: #185742.

I completely agree with the approach of detecting libc at runtime, though I would prefer if it was done with a mechanism that isn't nix specific.

Copy link
Contributor

@trofi trofi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha. Let's give runtime detection a try.

@bobby285271 bobby285271 added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Aug 12, 2022
@AndersonTorres
Copy link
Member

Okay then!

@AndersonTorres AndersonTorres merged commit a1d622b into NixOS:staging Aug 12, 2022
@vcunat
Copy link
Member

vcunat commented Aug 21, 2022

This PR broke a build: https://hydra.nixos.org/build/187642932

@trofi
Copy link
Contributor

trofi commented Aug 21, 2022

That is probably a pkgsLLVM.llvmPackages.compiler-rt:

CMake Error at /nix/store/xjg2fzw513iig1cghd4mvcq5fh2cyv4y-cmake-3.24.0/share/cmake-3.24/Modules/Platform/UnixPaths.cmake:53 (file):
  file STRINGS file
  "/nix/store/4hiy0nfv99gsfc1hyss3rcl3sn6panmc-x86_64-unknown-linux-gnu-clang-wrapper-11.1.0/nix-support/orig-libc-dev"
  cannot be read.

Where we fail to read orig-libc-dev. /cc @stephank

@trofi
Copy link
Contributor

trofi commented Aug 21, 2022

orig-libc-dev is not yet there. My guess is that it's a bootstrap $CC without libc support and it can't yet produce executable binaries:

$ ls /nix/store/4hiy0nfv99gsfc1hyss3rcl3sn6panmc-x86_64-unknown-linux-gnu-clang-wrapper-11.1.0/nix-support/
add-flags.sh  add-hardening.sh  add-local-cc-cflags-before.sh  cc-cflags  cc-ldflags  orig-cc  propagated-build-inputs  setup-hook  utils.bash

$ nix develop -i /nix/store/73cdvx5gzdmbgva41b6my2hzzs3cnxvw-compiler-rt-x86_64-unknown-linux-gnu-11.1.0.drv
$$ dev>printf "int main(){}" | $CC -x c - -o a
x86_64-unknown-linux-gnu-ld: error: unable to find library -lgcc
x86_64-unknown-linux-gnu-ld: error: unable to find library -lgcc_s
x86_64-unknown-linux-gnu-ld: error: unable to find library -lc
x86_64-unknown-linux-gnu-ld: error: unable to find library -lgcc
x86_64-unknown-linux-gnu-ld: error: unable to find library -lgcc_s
clang-11: error: linker command failed with exit code 1 (use -v to see invocation)

@trofi
Copy link
Contributor

trofi commented Aug 21, 2022

Proposed possible fix by falling back to old mechanism if libc bits are not present in nix-support directory: #187773 #187777

trofi added a commit to trofi/nixpkgs that referenced this pull request Aug 21, 2022
Without this change pkgsLLVM fails to build any packages
as compiler-rt fails early in cmake:

    CMake Error at ...-cmake-3.24.0/share/cmake-3.24/Modules/Platform/UnixPaths.cmake:53 (file):
      file STRINGS file
      "...-x86_64-unknown-linux-gnu-clang-wrapper-11.1.0/nix-support/orig-libc-dev"
      cannot be read.

It's a regression caused by 871cf9f "cmake: detect libc location
at runtime NixOS#181431" where we started using `orig-libc-dev` as a libc pointer.

During pkgsLLVM pootstrap first compiler has no libc support yet.

The change skips runtime detection if there are no libc signs.
@stephank stephank deleted the feat/cmake-libc branch November 2, 2022 15:47
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.

7 participants