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

[Backport release-22.11] llvmPackages_15: init at 15.0.7 #213092

Merged
merged 30 commits into from
Jan 28, 2023

Conversation

github-actions[bot]
Copy link
Contributor

Bot-based backport to release-22.11, triggered by a label in #194634.

  • Before merging, ensure that this backport complies with the Criteria for Backporting.
    • Even as a non-commiter, if you find that it does not comply, leave a comment.

None of the patches required any touch-up; the only change of note is:
  - due to changes in the libc++/libc++abi build
    (https://reviews.llvm.org/D120719 and https://reviews.llvm.org/D131037)
    we have to add an extra build option to the libc++ header only
    build that sidesteps bits of the libc++ build config that assume
    libc++-abi is present in the build:
    https://github.com/llvm/llvm-project/blob/4f827318e3e8ccab4ff131e06234caa827e91e4e/libcxx/src/CMakeLists.txt#L255-L256

Rather than maintaining a precise set of build options that let us dodge
referencing libc++-abi variables in the libc++ header only build, we set
`LIBCXX_CXX_ABI` to `none`, as suggested by @lovesegfault.

More discussion about this here: #194634 (comment)

Co-authored-by: Bernardo Meurer <[email protected]>
(cherry picked from commit bc4dbee)
`llvmPackages_15` originates from `llvmPackages_git` which does
not include this change

(cherry picked from commit 3b6d98d)
See the comments here for context: #194634 (comment)

Co-authored-by: Weijia Wang <[email protected]>
(cherry picked from commit 4fabcf4)
libc++ has switched to using `__attribute__((using_if_exists))` to handle
incomplete libc implementations; see: llvm/llvm-project@a9c9183

These essentially require a modern C++ compiler (clang gained support in
LLVM 13: llvm/llvm-project@369c648,
gcc appears to not have support yet: https://gcc.gnu.org/bugzilla//show_bug.cgi?id=105584).

Previously this was not an issue for us (despite the transition happening
around LLVM 13) but something about the changes to the libc++/libc++-abi
build has made it so that on platforms with incomplete libc impls (i.e.
Darwin is missing `quick_exit`/`at_quick_exit`) we error during the `libcxx-abi`
build when the stdenv's (older, not supporting `using_if_exists`) compiler
tries to import libc symbols that aren't present.

The libc++ docs suggest we use a modern compiler to build libc++ anyways
(https://releases.llvm.org/15.0.0/projects/libcxx/docs/index.html#platform-and-compiler-support)
so this commit uses stdenv's containing the package set's clang to build
libcxx/libcxx-abi.

This is similar to how libc++ bootstrapping builds (https://releases.llvm.org/15.0.0/projects/libcxx/docs/BuildingLibcxx.html#bootstrapping-build)
work.

(cherry picked from commit ca59a20)
this introduces a codesigning related patch that we can drop once #195107
goes through

see: #194634 (comment)
(cherry picked from commit 00839fe)
see the discussion here for context:
#194634 (comment)

(cherry picked from commit f91fad4)
…= "glibc"`

This restores this check to what it originally was in #196909 (see:
#196909 (comment)) and
lets `compiler-rt` eval successfully when trying to compile the
`llvmPackages_15` set for mingw targets (i.e. a platform that *is* GNU
but does *not* use glibc).

---

It's not clear to me what the `haveLibc` check is doing here (platforms
that seem to use glibc like `x86_64-linux` and have
`plat.libc == "glibc"` have `haveLibc = false` because `stdenv.cc.libc`
is `null`).

(cherry picked from commit d729907)
as detailed within, adding `asm/ptrace.h` leads to `asm/ptrace-abi.h`
being included which defines preprocessor symbols that clash with
identifiers used in the LLVM headers (`FS` and `CS` only defined on
i686)

(cherry picked from commit b4ee532)
…nfigure time

this previously worked because, when using Make, this variable was
expanded at build time

(cherry picked from commit 4d3857d)
this comment has a more complete history: #194634 (comment)

in short: polly was disabled in the llvm 11 -> llvm 12 copy, mostly by
accident

most of the Polly install dirs patch has been upstreamed; one change
remains

(cherry picked from commit 2a58596)
there are a few parts to this:
  - adding darwin specific check deps
  - working around referencing LLVM dylibs during the checkPhase in a
    way that supports darwin
    + previously we just set `$LD_LIBRARY_PATH` and/or made some
      strategic symlinks
    + now we have LLVM's `lit` config set the appropriate env vars as
      needed (as is done for other LLVM subprojects)
    + in retrospect switching to `installCheckPhase` might have been the
      better move..
  - patching `lit` to deal with `$DYLD_LIBRARY_PATH` being purged for
    new "protected" processes

more details within.

(cherry picked from commit c7231c0)
Details within but ultimately there isn't a satisfying resolution for
any of the three test failures we were seeing and all three deserve
further exploration.

For the `sw_vers` macOS version issue in particular, it's possible to
observe the nixpkgs provided `CoreFoundation` vs system `CoreFoundation`
for `x86_64` and `aarch64` like so (on a host running macOS `13.0.1`):

```console
$ nix-shell -p darwin.DarwinTools --system aarch64-darwin --command "sw_vers"
ProductName:    macOS
ProductVersion: 13.0.1
BuildVersion:   22A400

$ nix-shell -p darwin.DarwinTools --system x86_64-darwin --command "sw_vers"
ProductName:    Mac OS X
ProductVersion: 10.16
BuildVersion:   22A400
```

Where `/System/Library/CoreServices/SystemVersion.plist` has:
```console
$ cat /System/Library/CoreServices/SystemVersion.plist | grep ProductVersion
-A 1
	<key>ProductVersion</key>
	<string>13.0.1</string>
```

Further:
```console
$ nix-shell -p darwin.DarwinTools --system aarch64-darwin --command 'otool -L $(which sw_vers)'
/nix/store/nb2q33ak2zif49ndcpc6m823z0vhmy8y-DarwinTools-1/bin/sw_vers:
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1770.255.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.60.1)

$ nix-shell -p darwin.DarwinTools --system x86_64-darwin --command 'otool -L $(which sw_vers)'
/nix/store/88v4kjvgwl71byfpvd0baviiq7l5appc-DarwinTools-1/bin/sw_vers:
	@rpath/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1454.90.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1238.60.2)
```

For the `x86_64` `sw_vers` binary we can see rpath:
```console
$ nix-shell -p darwin.DarwinTools --system x86_64-darwin --command 'otool -l $(which sw_vers)' | grep LC_RPATH -A 2 -B 1
Load command 13
          cmd LC_RPATH
      cmdsize 120
         path /nix/store/zvr4wypbgskhhw9cawfn7mmxfa75nh8f-swift-corefoundation-unstable-2018-09-14/Library/Frameworks (offset 12)
```

And we can confirm that the nixpkgs provided `CoreFoundation` is what
ultimately gets loaded:
```console
$ nix-shell -p darwin.DarwinTools --system x86_64-darwin --command 'DYLD_PRINT_LIBRARIES=1 sw_vers'
dyld[16215]: <no uuid> /nix/store/88v4kjvgwl71byfpvd0baviiq7l5appc-DarwinTools-1/bin/sw_vers
dyld[16215]: <no uuid> /nix/store/zvr4wypbgskhhw9cawfn7mmxfa75nh8f-swift-corefoundation-unstable-2018-09-14/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
dyld[16215]: <no uuid> /nix/store/xd2a4xh8kdwq0j67hzgw720npdw5hzkk-ICU-66108/lib/libicucore.A.dylib
<snipped>
```

```bash
nix-diff \
    $(nix path-info nixpkgs#legacyPackages.aarch64-darwin.darwin.DarwinTools --derivation) \
    $(nix path-info nixpkgs#legacyPackages.x86_64-darwin.darwin.DarwinTools --derivation)
```
doesn't show any _obvious_ discrepancies

(cherry picked from commit eafb8fb)
see this thread for context: #194634 (comment)

Co-authored-by: misuzu <[email protected]>
(cherry picked from commit 5e5ed7d)
…lation

The two scenarios described within where splicing doesn't handle
selecting the right package for us are observable in the following
(nix repl session):
```
> np = import <nixpkgs> { system = "x86_64-linux"; crossSystem = { config = "aarch64-linux"; }; }

> np.__splicedPackages.hello ? __spliced
true

> np.__splicedPackages.python3Packages.psutil ? __spliced
true

> np.__splicedPackages.python3.pkgs.psutil ? __spliced
false

> (np.__splicedPackages.python3.withPackages (ps: with ps; [psutil])) ? __spliced
false
```

See: #211340
(cherry picked from commit 3436075)
the tests still don't all pass so `doCheck` is still disabled

(cherry picked from commit 8f16b4b)
(cherry picked from commit f8cbbdd)
this is in preparation for the next commit which exposes the release
information and monorepo source as overridable args (which makes it
easier for users to use their own LLVM in nixpkgs)

this commit only adds a check in the `llvm` package's postConfigure that
makes sure the LLVM source provided matches the version we were given;
the actual machinery (functionally just a cosmetic change; causes no
rebuilds) is in the next commit

(cherry picked from commit 8afa321)
…s overridable args

this makes it easier for users to use their own LLVM in nixpkgs

(cherry picked from commit d231d18)
@github-actions github-actions bot mentioned this pull request Jan 28, 2023
92 tasks
@wegank
Copy link
Member

wegank commented Jan 28, 2023

#211401 also needs to be backported.

@wegank
Copy link
Member

wegank commented Jan 28, 2023

@ofborg eval

@wegank wegank force-pushed the backport-194634-to-release-22.11 branch from 3efec43 to 7270892 Compare January 28, 2023 08:04
@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Jan 28, 2023
@wegank wegank merged commit 1035e58 into release-22.11 Jan 28, 2023
@wegank wegank deleted the backport-194634-to-release-22.11 branch January 28, 2023 18:58
@primeos
Copy link
Member

primeos commented May 6, 2023

Oh, strange that CI didn't catch this... :o I guess our CI still needs a bit of work.

I coincidentally noticed this when backporting llvmPackages_16 and was already surprised that it affects llvmPackages_15 as well (I simply ran nix-instantiate -A llvmPackages_15 for a quick eval test). It's fixed now as I've included libcxxrt in #230327.

Thanks @max-privatevoid for catching this! :)

@rrbutani rrbutani added the 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related label May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants