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

darwin.stdenv: Update to LLVM 15 and clang to 15.0.7 #229786

Closed
wants to merge 12 commits into from

Conversation

reckenrode
Copy link
Contributor

@reckenrode reckenrode commented May 4, 2023

Description of changes

Bumps LLVM and clang to 15.0.7 on Darwin. This was prompted by #229210 because the 13.3 SDK does not build with LLVM 11. This PR reworks the Darwin stdenv to decouple it from the bootstrap tools is written not to need updates to bootstrap tools and includes changes to allow LLVM to be bumped without requiring a bump to the bootstrap tools.

See the comments in 309dc9d explaining the rework of the Darwin stden bootstrap process. My hope is this will make future bumps easier and more approachable for non-Darwin developers. It should also make experimentation a little safer. I’d like to see if any space could be saved by enabling LTO during the bootstrap, but that’s out of scope for this PR.

Issues to be resolved

These have all been resolved. The changes needed for the bootstrap are included in this PR. The others are being handled in separate PRs, which will be linked below.

  • Disable LLVM tests when building the stdenv. This causes an infinite recursion currently. I have a workaround, but ideally it should be disabled in the bootstrap.
  • pkg-config requires disabling a warning, or the build fails. It’s vendoring a very old version of GLib. Unfortunately, nixpkgs does not provide a way to use the nixpkgs glib package. I also tried, and it caused an infinite recursion.
  • codesign in bootstrap tools is too old for LLVM, but I’ve included a workaround. A bootstrap tools update would obviate the change.
  • Updating platform definition now that LLVM understands apple-m1. This should likely go in another PR, but it’s included here for testing. See apple-m1 compiler optimisations for aarch64-darwin? #166401.
Testing Done

Running nixpkgs-review on a stdenv change is obviously going to rebuild an infeasible amount of packages. I have attempted to build and run a number of packages to catch common failures. I’ve tried to change things needed to build the stdenv from those that fix common problems. See below for PRs related to those (and sandbox issues I encountered along the way).

Related PRs

These fail to build due to the changes.

These are Darwin sandbox-related fixes. They’re not strictly necessary, but they allow Darwin users to build more things with the sandbox enabled.

This fix is related to running the store on case-sensitive APFS. It’s the first thing that has broken for me. Most case sensitivity issues during builds are due to /tmp being insensitive.

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/)
  • 23.05 Release Notes (or backporting 22.11 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.

@RaitoBezarius
Copy link
Member

Hello, thank you for your contribution, please read the CONTRIBUTING guide and target staging adequately (without mass pinging everyone by reading the guide).

@reckenrode reckenrode changed the base branch from master to staging May 4, 2023 04:13
@reckenrode reckenrode changed the title darwin.stdenv: Update to LLVM 15 and clang 15.0.7 darwin.stdenv: Update to LLVM 15 and clang to 15.0.7 May 4, 2023
Comment on lines 388 to 389
arch = "armv8.3-a+crypto+sha2+aes+crc+fp16+lse+simd+ras+rdm+rcpc";
cpu = "apple-a13";
arch = "armv8.5a+crc+crypto+dotprod+fp16fml+ras+lse+rdm+rcpc+sm4+sha3+sha2+aes";
cpu = "apple-m1";
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious: does this change by any chance affect the hack I recently had to add:

# --with-cpu on aarch64-darwin fails with "Unknown cpu used in --with-cpu=apple-a13".
(lib.optional (!isAarch64Darwin && p ? cpu) "--with-cpu=${p.cpu}")

Probably not, but one can hope.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not as far as I can tell. There’s an open issue at iains/gcc-darwin-arm64#86 for support.

Comment on lines 875 to 876
# Having this set when trying to cross-compile to Linux causes problems, so only set it
# when building for Darwin.
Copy link
Contributor

@eliasnaur eliasnaur May 10, 2023

Choose a reason for hiding this comment

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

Nit: consider dropping this comment. It seems clear that darwin specific stuff should be guarded by platform checks, by principle.

@reckenrode
Copy link
Contributor Author

Moving the environment variables to CF sounds like what #230870 does. I mention it in case you need similar hacks to disable the hook during stdenv building.

The original issue indicated it was done to avoid unwanted references between stages. I dropped allowedRequisites in favor of assertions in the reworked bootstrap. allowedRequisites and disallowedRequisites are now used only in the final stdenv, so that shouldn’t be an issue. This approach follows how the Linux stdenv is built.

@reckenrode
Copy link
Contributor Author

I pushed the current branch. I expect I’ll be opening separate PRs to fix things that don’t build with the new clang that aren’t really needed for the bootstrap (or fail immediately). I still need to do some cleanup on commit messages before marking this ready and review any other issues that could be resolved/affected. I dropped the platforms.nix changes for now.

xnu does not require a Python that supports configd, so use a minimal
one without support. Doing this prevents an infinite recuresion when
building configd with xpc, which is needed to build with a newer LLVM.
Clang 15 does not like the fake xpc headers. Use the real ones instead.
Doing this no longer causes an infinite recursion because xnu now
depends on python3Minimal, which does not include configd support.
cctools-llvm is a replacement for cctools that replaces as much of
cctools with equivalents from LLVM that it can reasonably do. This was
motivated by wanting to reduce dependencies on cctools, which are
updated infrequently by upstream.

To provide a motivating example, the version of `strip` included in
cctools cannot properly strip the archives in compiler-rt in LLVM 15.
Paths are left to bootstrap tools, resulting in failed requisites checks
in the final stdenv build. Since `strip` needs replaced, other tools
were replaced as well.

Note that this has to be done in cctools (or some equivalent) because
some derivations (noteably LLVM) use the bintools of the stdenv directly
instead of going through the wrapper.

The following tools from LLVM are not used in this derivation:

* LLD - not fully compatible with ld64 yet and potentially too big of a change;
* libtool - not a drop-in replacement yet because it does not support
  linker passthrough, which is needed by xcbuild; and
* install_name_tool - fails when trying to build swift-corefoundation.

If other incompatibilities are found, other tools can be reverted.
However, testing for the stdenv bump has shown the others seem to work
pretty well as replacements.

One final caveat: some tools are not duplicated or linked from cctools.
As a rule of thumb, the actual tools on a Darwin system were used to
determine which ones to link, so there are some differences (e.g., in
names or availability).
Closes NixOS#230870. Thanks to @eliasnaur for the test case and for rasining
awareness and to @veprbl for the work done on NixOS#111385.

This takes a slightly different approach from those two PRs. The hook is
set unconditionally. The stdenv bootstrap doesn’t really need CF at all,
so setting the hook is harmless. This simplifies things.
@reckenrode
Copy link
Contributor Author

The last push should reflect the final PR. I am keeping this draft until I can get the PRs submitted for the failures due to the stdenv change, which should be tomorrow.

@reckenrode
Copy link
Contributor Author

The ofborg eval is failing on llvmPackages.clang-polly-unwrapped, but that attribute is not part of llvmPackages_15. It appears it’s not included been part of any llvmPackages since 11.

In preparation for bumping the LLVM used by Darwin, this change
refactors and reworks the stdenv build process. When it made sense,
existing behaviors were kept to avoid causing any unwanted breakage.
However, there are some differences. The reasoning and differences are
discussed below.

- Improved cycle times - Working on the Darwin stdenv was a tedious
  process because `allowedRequisites` determined what was allowed
  between stages. If you made a mistake, you might have to wait a
  considerable amount of time for the build to fail. Using assertions
  makes many errors fail at evaluation time and makes moving things
  around safer and easier to do.
- Decoupling from bootstrap tools - The stdenv build process builds as
  much as it can in the early stages to remove the requirement that the
  bootstrap tools need bumped in order to bump the stdenv itself. This
  should lower the barrier to updates and make it easier to bump in the
  future. It also allows changes to be made without requiring additional
  tools be added to the bootstrap tools.
- Patterned after the Linux stdenv - I tried to follow the patterns
  established in the Linux stdenv with adaptations made to Darwin’s
  needs. My hope is this makes the Darwin stdenv more approable for
  non-Darwin developers who made need to interact with it. It also
  allowed some of the hacks to be removed.
- Documentation - Comments were added explaining what was happening and
  why things were being done. This is particular important for some
  stages that might not be obvious (such as the sysctl stage).
- Cleanup - Converting the intermediate `allowedRequisites` to
  assertions revealed that many packages were being referenced that no
  longer exist or have been renamed. Removing them reduces clutter and
  should help make the stdenv bootstrap process be more understandable.
@cole-h
Copy link
Member

cole-h commented May 26, 2023

Running the outpaths.nix check locally, this is what I see right before the eval fails:

[...snip...]
instantiated 'llvm-manpages-15.0.7' -> '/nix/store/2msilg6yh3p7493r1xv7mdb0j432s4mz-llvm-manpages-15.0.7.drv'
evaluating attribute 'recurseForDerivations'
evaluating attribute 'llvmPackages'
evaluating attribute '__unfix__'
evaluating attribute 'recurseForDerivations'
evaluating attribute 'bintools'
evaluating file '/home/vin/workspace/vcs/nixpkgs/master/pkgs/development/compilers/llvm/11/bintools/default.nix'
evaluating attribute 'recurseForDerivations'
evaluating attribute 'bintools-unwrapped'
evaluating attribute 'recurseForDerivations'
evaluating attribute 'bintoolsNoLibc'
evaluating attribute 'recurseForDerivations'
evaluating attribute 'clang'
evaluating attribute 'aarch64-darwin'
evaluating attribute 'recurseForDerivations'
evaluating attribute 'clang-manpages'
evaluating attribute 'aarch64-darwin'
evaluating attribute 'recurseForDerivations'
evaluating attribute 'clang-polly-unwrapped'
evaluating attribute 'aarch64-darwin'
error:
[...snip...]

Note that this is specifically a problem on the aarch64-darwin eval -- all other platforms are fine. You can reproduce this by looking at https://github.com/NixOS/ofborg#running-meta-checks-locally and commenting out all the architectures except for aarch64-darwin. (I ran with -vvv to get the above output.)

Dunno what's going on there, but maybe this will help you diagnose.

@reckenrode
Copy link
Contributor Author

Closing this in favor of tracking issue #234710, so I can submit changes incrementally while working through the ofborg issue and the gnugrep issue on x86_64-darwin.

@reckenrode reckenrode closed this May 29, 2023
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

5 participants