Navigation Menu

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

stdenv/darwin: assemble full clang toolchain #94427

Closed

Conversation

thefloweringash
Copy link
Member

@thefloweringash thefloweringash commented Aug 1, 2020

Adapted from main expression for clang 7.

Motivation for this change

Second part of #94426, and must only be merged when updating bootstrap tools to include #94426. See that ticket for the main context. This integrates compiler-rt into the toolchain, both from the re-added bootstrap level and for the higher level stdenvs.

I'm not confident about the composition of the stages or if compiler-rt is rebuilt at the appropriate times. The idea is to preserve the bootstrap version until llvm is available to build a new one.

cc @matthewbauer and @LnL7

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@thefloweringash
Copy link
Member Author

The current state of this now is able to build perlPackages.LocaleGettext from an updated bootstrap tools. I put together a simple analysis for the structure of the stdenv and verified that this doesn't not change the structure in any interesting way. Since llvmPackages_7.compiler-rt depends on llvm, it is delayed until the same stage as the first llvm rebuild by copying it up from bootstrap tools until then.

The only remaining mystery is if libcxxabi is required. Including it in the same way as the main clang expression causes libcxx to reference two different libcxxabi packages.

@LnL7 LnL7 added this to Big Sur in Darwin Sep 17, 2020
Adapted from main expression for clang 7.
Build the llvm support libraries (libcxx, libcxxabi) from scratch
without using the existing llvm libraries. This is the same spirit and
similar implementation as the "useLLVM" bootstrap in llvm package
sets. Critically it avoids having libcxxabi provided by the cc-wrapper
when building libcxx, which otherwise results in two libcxxabi
instances.

$ otool -L /nix/store/vd4vvgs9xngqbjzpg3qc41wl6jh42s9i-libc++-7.1.0/lib/libc++.dylib
/nix/store/vd4vvgs9xngqbjzpg3qc41wl6jh42s9i-libc++-7.1.0/lib/libc++.dylib:
        /nix/store/vd4vvgs9xngqbjzpg3qc41wl6jh42s9i-libc++-7.1.0/lib/libc++.1.0.dylib (compatibility version 1.0.0, current version 1.0.0)
        /nix/store/gmpwk5fyp3iasppqrrdpswxvid6kcp8r-libc++abi-7.1.0/lib/libc++abi.dylib (compatibility version 1.0.0, current version 1.0.0)
        /nix/store/3hn7azynqgp2pm5gpdg45gpq0ia72skg-libc++abi-7.1.0/lib/libc++abi.dylib (compatibility version 1.0.0, current version 1.0.0)
        /nix/store/1nq94scbxs6bk7pimqhvz76q6cfmbv97-Libsystem-osx-10.12.6/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)

Additionally move some utilities (clang, binutils, coreutils, gnugrep)
to the stage layers so they can be replaced before the final
stdenv. This should cause most of stage4 to be built from the
toolchain assembled as of stage3 instead of the bootstrap toolchain.
Copy link
Member Author

@thefloweringash thefloweringash left a comment

Choose a reason for hiding this comment

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

There are two commits here:

  • the first is the minimal set of changes to produce a functional stdenv after updating the bootstrap tools
  • the second is to try to improve the correctness of the bootstrapping change, and is not complete (looking for feedback)

The main change in the second commit is to match the toolchain wrapper in llvm/7/default.nix and include libcxxabi in extraPackages. Testing with only the first commit shows that it works without this, so maybe it's not important. In order to include libcxxabi, I had to be careful when building it to avoid a reference to the bootstrap version. For this I took the bootstrapping logic from llvm/7/default.nix [1], [2]. This existing bootstrapping logic is driven by stdenv.hostPlatform.useLLVM, which I initially expected to be true for Darwin as it is an llvm/clang based stdenv, but it appears to be unrelated (used for wasi cross compilation).

I've spent a bit of time inspecting the darwin stdenv. I noticed that the bootstrap tools don't appear to be swapped out incrementally through the stages as they are in linux. The second commit also includes an attempt to make this more well behaved.

@@ -1,4 +1,5 @@
{ stdenv, cmake, fetch, libcxx, llvm, version
, standalone ? false
Copy link
Member Author

Choose a reason for hiding this comment

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

This has very similar effects to stdenv.hostPlatform.useLLVM, in supporting bootstraping an llvm/clang based toolchain. Though useLLVM is only defined in versions 8, 9, 10 and 11. We should find a consistent approach to this. If possible I'd like to remove useLLVM from llvm/*/libc++abi.nix for something reusable that describes its effects.

@thefloweringash
Copy link
Member Author

This has been subsumed by #98541.

@LnL7 LnL7 moved this from Big Sur to Done in Darwin Nov 22, 2020
@thefloweringash thefloweringash deleted the bootstrap-unpack branch July 10, 2021 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Darwin
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

1 participant