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 bump LLVM to 11 #126411

Merged
merged 54 commits into from
Nov 24, 2021
Merged

Darwin stdenv bump LLVM to 11 #126411

merged 54 commits into from
Nov 24, 2021

Conversation

toonn
Copy link
Contributor

@toonn toonn commented Jun 9, 2021

Motivation for this change

This bumps the LLVM used in the Darwin stdenv to 11, building on #121055, which made the required updates to the bootstrap-tools first.

Some packages the stdenv depends on needed patching because of the LLVM update so it's likely there are many packages that require compiler flags, mostly to bring the behavior back in line with clang 7, so I'm opening this as a draft to get a hydra jobset.

I'm building hello as a test, will report back tomorrow.

@happysalada
Copy link
Contributor

Not specifically related but the two lines begining with the following in darwin stdenv

# Workaround for https://openradar.appspot.com/22671534 on 10.11.

are not required anymore. It was like the comment says a workaround for 10.11. I've tried on local and they can be safely removed.
If you would rather leave that to another PR, no worries.

@happysalada
Copy link
Contributor

One other thing. Would you not make the PR on staging rather than master? The number of packages to be rebuilt would be quite big in this case.

@toonn
Copy link
Contributor Author

toonn commented Jun 10, 2021

@happysalada, I'm targeting master rn on @LnL7's advice. It's so we have a good hydra evaluation to compare to for the custom jobset for this.

IIUC once we get most of the packages building with the LLVM bump we'd change the target to staging.

@toonn
Copy link
Contributor Author

toonn commented Jun 10, 2021

@LnL7, hello built successfully so this is ready for an evaluation.

@LnL7
Copy link
Member

LnL7 commented Jun 10, 2021

Hydra eval is running. Note that this will take a while as this jobset has very low build shares, so other builds from master and staging generally get priority.

https://hydra.nixos.org/eval/1677441?compare=1677022#tabs-unfinished

@prusnak
Copy link
Member

prusnak commented Jun 25, 2021

Shouldn't this be targeted at staging, not master?

@alyssais
Copy link
Member

alyssais commented Jun 25, 2021

@prusnak #126411 (comment)

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 4, 2021
The original problem with the normalization of the filename only
occurred because it was in NFC. However, when trying to fix it by
`mv`ing the file to a normalization-indifferent name, I used the NFD
normalized name from my file system. This means it only works on
normalizing file systems. The filename must be in the original encoding
and will be normalized by normalizing file systems like HFS+.
The original problem with the normalization of the filename only
occurred because it was in NFC. However, when trying to fix it by
`mv`ing the file to a normalization-indifferent name, I used the NFD
normalized name from my file system. This means it only works on
normalizing file systems. The filename must be in the original encoding
and will be normalized by normalizing file systems like HFS+.
Both patches implement the same fix so they'll probably have to be
removed at the same time. This avoids one of them being left behind.
Use `autoreconfHook` rather than manually running `aclocal`.
Clarify removal of `version` file.
LLVM 11 libcxxabi has some flags to support usage in the Darwin stdenv,
in particular, `standalone` and `withLibunwind`.

Darwin stdenv needs the `standalone` flag because its `hostPlatform` set
doesn't have `useLLVM` set to true. And it needs `withLibunwind` to
explicitly disable including `libunwind` as a build input.

We also prefix `install_name_tool` in case we're cross-compiling.
LLVM 11 libcxxabi has some flags to support usage in the Darwin stdenv,
in particular, `standalone` and `withLibunwind`.

Darwin stdenv needs the `standalone` flag because its `hostPlatform` set
doesn't have `useLLVM` set to true. And it needs `withLibunwind` to
explicitly disable including `libunwind` as a build input.

We also prefix `install_name_tool` in case we're cross-compiling.
Copy link
Contributor

@thiagokokada thiagokokada left a comment

Choose a reason for hiding this comment

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

This looks beautiful, congrats 👏 .

@jonringer
Copy link
Contributor

Future me, is going to hate current me. But lets get this going.

@jonringer
Copy link
Contributor

sorry, trying to build ripgrep on my server, to ensure something isn't fundamentally wrong all the way through to the rust toolchain. give me a few hours to do the stdenv rebuild process :(

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

LGTM

Result of nixpkgs-review pr 126411 run on x86_64-linux 1

1 package failed to build:
  • ripgrep

@jonringer jonringer merged commit 5d23e61 into NixOS:staging Nov 24, 2021
@trofi
Copy link
Contributor

trofi commented Dec 1, 2021

Looks like mariadb-client needs a fix:

$ nix build -f. --no-link mariadb-client -L
mariadb-client> unpacking sources
mariadb-client> unpacking source archive /nix/store/zk6b3f70aqyxd8pasyvzl9knmwja24cj-mariadb-10.6.5.tar.gz
mariadb-client> source root is mariadb-10.6.5
mariadb-client> setting SOURCE_DATE_EPOCH to timestamp 1636142613 of file mariadb-10.6.5/wsrep-lib/wsrep-API/v26/wsrep_uuid.c
mariadb-client> patching sources
mariadb-client> mv: cannot stat 'storage/mroonga/version': No such file or directory

@toonn
Copy link
Contributor Author

toonn commented Dec 6, 2021

@trofi, addressed the mariadb issue in #149096.

@vcunat
Copy link
Member

vcunat commented Dec 11, 2021

Nix won't build, probably due to this? There are also other regressions; please have a look at staging-next #148396 (comment)

@vcunat vcunat mentioned this pull request Dec 11, 2021
13 tasks
@happysalada
Copy link
Contributor

Oh that was the upgrade. I wasn't aware that this was merged actually. I'm curious if other darwin people have an opinion on the nix failure on staging-next. I haven't been able to figure it out.

@toonn
Copy link
Contributor Author

toonn commented Dec 11, 2021

Sounds to me like it's trying to use a method introduced in SDK 10.13 and Nixpkgs is currently on SDK 10.12? I assume Clang 11 is just louder about the error and it doesn't actually cause any problems for anyone who's not on 10.12, hence not having been noticed before?

@r-burns
Copy link
Contributor

r-burns commented Dec 11, 2021

I think the availability macros in libcxx are only applicable for the system /usr/lib/libc++.dylib. I think anything built purely in nixpkgs should be able to define _LIBCPP_DISABLE_AVAILABILITY without any issues.

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.