llvmPackages_13.libcxx: fix darwin build by midchildan · Pull Request #147289 · NixOS/nixpkgs

midchildan · 2021-11-24T19:18:18Z

Motivation for this change

This succeeds #146517. It fixes the Darwin build of libcxx by applying the following fixes:

Use a newer version of Clang to avoid compilation errors
Break the circular dependency between libcxx and libcxxabi. libcxxabi needs libcxx headers to build, but libcxx has a runtime dependency on libcxxabi. To fix this, this PR creates a header-only variant of the libcxx derivation for building libcxxabi.

The Linux build seems to have avoided the circular dependency problem by dropping the libcxx dependency on libcxxabi. This approach appears not to be possible on Darwin.

llvmPackages_git probably needs the same fix too, but I haven't looked into it closely yet.

ZHF: #144627

Things done

Like in #146517, I've tested on the master branch and rebased against staging to avoid the overwhelming amount of local builds.

midchildan · 2021-11-24T19:59:42Z

I'm marking this as draft because I found out that building llvmPackages_13 with llvmPackages_12 fixes libcxx, but breaks llvm.

building
'/nix/store/ba45qlpblgyabh8kl00d14fxzb8b1307-llvm-13.0.0/lib' -> '/private/tmp/nix-build-llvm-13.0.0.drv-1/source/llvm/build/lib'
build flags: -j4 -l4 SHELL=/nix/store/r42x7q316gznkm5y9b0cl4564g174zyl-bash-5.1-p8/bin/bash
[  0%] Generating VCSRevision.h
[  0%] Creating export file for LLVMHello
[  0%] Building CXX object utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/AsmMatcherEmitter.cpp.o
[  1%] Building CXX object lib/Demangle/CMakeFiles/LLVMDemangle.dir/Demangle.cpp.o
CMake Warning at /tmp/nix-build-llvm-13.0.0.drv-1/source/llvm/cmake/modules/VersionFromVCS.cmake:49 (message):
  Git not found.  Version cannot be determined.
Call Stack (most recent call first):
  /tmp/nix-build-llvm-13.0.0.drv-1/source/llvm/cmake/modules/GenerateVersionFromVCS.cmake:23 (get_source_info)
  /tmp/nix-build-llvm-13.0.0.drv-1/source/llvm/cmake/modules/GenerateVersionFromVCS.cmake:45 (append_info)


[  1%] Built target LLVMHello_exports
[  1%] Building CXX object lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o
[  1%] Built target llvm_vcsrevision_h
[  1%] Building CXX object lib/Demangle/CMakeFiles/LLVMDemangle.dir/MicrosoftDemangle.cpp.o
[  1%] Building CXX object utils/PerfectShuffle/CMakeFiles/llvm-PerfectShuffle.dir/PerfectShuffle.cpp.o
[  2%] Linking CXX executable ../../bin/llvm-PerfectShuffle
[  2%] Building C object utils/count/CMakeFiles/count.dir/count.c.o
Undefined symbols for architecture x86_64:
  "operator new(unsigned long)", referenced from:
      std::__1::ostreambuf_iterator<char, std::__1::char_traits<char> > std::__1::__pad_and_output<char, std::__1::char_traits<char> >(std::__1::ostreambuf_iterator<char, std::__1::char_traits<char> >, char const*, char const*, char const*, std::__1::ios_base&, char) in PerfectShuffle.cpp.o
      Operator::Operator(unsigned short, char const*, unsigned int, unsigned int) in PerfectShuffle.cpp.o
  "operator delete(void*)", referenced from:
      std::__1::vector<Operator*, std::__1::allocator<Operator*> >::~vector() in PerfectShuffle.cpp.o
      std::__1::ostreambuf_iterator<char, std::__1::char_traits<char> > std::__1::__pad_and_output<char, std::__1::char_traits<char> >(std::__1::ostreambuf_iterator<char, std::__1::char_traits<char> >, char const*, char const*, char const*, std::__1::ios_base&, char) in PerfectShuffle.cpp.o
      Operator::Operator(unsigned short, char const*, unsigned int, unsigned int) in PerfectShuffle.cpp.o
ld: symbol(s) not found for architecture x86_64
clang-12: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [utils/PerfectShuffle/CMakeFiles/llvm-PerfectShuffle.dir/build.make:97: bin/llvm-PerfectShuffle] Error 1
make[1]: *** [CMakeFiles/Makefile2:25003: utils/PerfectShuffle/CMakeFiles/llvm-PerfectShuffle.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

midchildan · 2021-11-25T11:04:33Z

This is now ready for review. LLVM, Clang, and libcxx all built successfully after I replaced llvmPackages_12.stdenv with llvmPackages_11.stdenv.

SuperSandro2000 · 2021-11-25T11:11:16Z

pkgs/development/compilers/llvm/13/libcxx/default.nix

+    pushd "$dev"
+    rmdir -p include/c++/v1
+    popd


Suggested change

pushd "$dev"

rmdir -p include/c++/v1

popd

rmdir -p $dev/include/c++/v1

I believe this would result in different behavior. I want the directories v1, c++, and include to be deleted in order while making sure that each of those directories are completely empty before deletion. The suggested change would also attempt to delete each path component in $dev too.

pkgs/development/compilers/llvm/13/libcxx/default.nix

Co-authored-by: Sandro <[email protected]>

domenkozar · 2021-11-25T12:44:55Z

@toonn

toonn

Looks like a good fix overall.

toonn · 2021-11-25T17:11:21Z

pkgs/development/compilers/llvm/13/default.nix

-      inherit llvm_meta;
-      stdenv = if stdenv.hostPlatform.useLLVM or false
+    libcxxabi = let
+      stdenv_ = if stdenv.hostPlatform.useLLVM or false


I think what you want to do here is test stdenv.cc.isClang. useLLVM isn't true for Darwin, for example. (Unless that changed recently?)

I'm not exactly sure what the intention behind that conditional expression is, so I left it unchanged in this PR. Does your suggestion apply to other parts of the code too? I see the same conditional expression used everywhere throughout the LLVM packaging code.

Hmm, I guess leave it if you've tested it on Darwin anyway?

toonn · 2021-11-25T17:12:34Z

pkgs/development/compilers/llvm/13/default.nix

+      cxx-headers = callPackage ./libcxx {
+        inherit llvm_meta;
+        stdenv = stdenv_;
+        isCxxHeaders = true;


I've seen headersOnly as the argument for triggering a headers only build a couple times, might be nice to align with that convention?

I've now changed it to headersOnly.

toonn · 2021-11-25T17:15:14Z

pkgs/development/compilers/llvm/13/libcxx/default.nix

+  sourceRoot = "source/libcxx";

-  outputs = [ "out" "dev" ];
+  outputs = [ "out" ] ++ lib.optional (!isCxxHeaders) "dev";


Wouldn't it be simpler if dev would be the single output when only building the headers?

I tried, but it failed to evaluate when I did this.

❯ nix-build -A llvmPackages_13.libcxx these 3 derivations will be built: /nix/store/9za4wnq6g4kzls8kda2lma2axhppgis4-cxx-headers-13.0.0.drv /nix/store/yhfjvzqrz6d1rs8d7mfifsvm4i6c0rdd-libcxxabi-13.0.0.drv /nix/store/2c18dsqr57z2sl6bw0y85ziz44349jn7-libcxx-13.0.0.drv building '/nix/store/9za4wnq6g4kzls8kda2lma2axhppgis4-cxx-headers-13.0.0.drv'... Error: _assignFirst found no valid variant!

toonn · 2021-11-25T17:18:39Z

pkgs/top-level/all-packages.nix

    targetLlvmLibraries = targetPackages.llvmPackages_13.libraries;
  } // lib.optionalAttrs (stdenv.hostPlatform.isi686 && buildPackages.stdenv.cc.isGNU) {
    stdenv = gcc7Stdenv;
+  } // lib.optionalAttrs stdenv.hostPlatform.isDarwin {


Note that the LLVM bump to 11 #126411 was just merged to staging so I don't think it's necessary to explicitly pick the LLVM version here.

I removed this change in the latest commit.

toonn

LGTM, didn't have time to run test builds though.

github-actions · 2021-11-26T14:15:52Z

Backport failed for release-21.11, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin release-21.11
git worktree add -d .worktree/backport-147289-to-release-21.11 origin/release-21.11
cd .worktree/backport-147289-to-release-21.11
git checkout -b backport-147289-to-release-21.11
ancref=$(git merge-base d71611fb7221eaa8804a355ca22fadecacc4b9f6 7994b1dfc0a5431cbb4c052c740db8992045af8c)
git cherry-pick -x $ancref..7994b1dfc0a5431cbb4c052c740db8992045af8c

pkgs/development/compilers/llvm/13/libcxx/default.nix

midchildan requested a review from matthewbauer as a code owner November 24, 2021 19:18

midchildan mentioned this pull request Nov 24, 2021

Downgrade llvmPackages_latest to 12 on Darwin #146517

Closed

12 tasks

ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Nov 24, 2021

midchildan marked this pull request as draft November 24, 2021 19:59

ofborg bot requested review from 7c6f434c, dtzWill, lovek323 and primeos November 24, 2021 21:58

midchildan force-pushed the fix/libcxx13-darwin branch from 2b1f2e1 to 944de65 Compare November 25, 2021 10:54

midchildan marked this pull request as ready for review November 25, 2021 10:54

llvmPackages_13.libcxx: fix darwin build

e157b5d

midchildan force-pushed the fix/libcxx13-darwin branch from 944de65 to e157b5d Compare November 25, 2021 10:59

SuperSandro2000 reviewed Nov 25, 2021

View reviewed changes

pkgs/development/compilers/llvm/13/libcxx/default.nix Outdated Show resolved Hide resolved

fixup! llvmPackages_13.libcxx: fix darwin build

d8338cb

Co-authored-by: Sandro <[email protected]>

toonn reviewed Nov 25, 2021

View reviewed changes

fixup! llvmPackages_13.libcxx: fix darwin build

7994b1d

toonn approved these changes Nov 25, 2021

View reviewed changes

veprbl added the backport release-21.11 label Nov 26, 2021

domenkozar merged commit 845225e into NixOS:staging Nov 26, 2021

midchildan mentioned this pull request Nov 26, 2021

[Backport release-21.11] llvmPackages_13.libcxx: fix darwin build #147514

Merged

13 tasks

midchildan deleted the fix/libcxx13-darwin branch November 26, 2021 17:51

midchildan mentioned this pull request Nov 28, 2021

llvmPackages_git.libcxx: fix darwin build #147668

Merged

13 tasks

alyssais reviewed Nov 4, 2023

View reviewed changes

pkgs/development/compilers/llvm/13/libcxx/default.nix Show resolved Hide resolved

rrbutani added the 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related label May 27, 2024

Uh oh!

Conversation

midchildan commented Nov 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for this change

Things done

Uh oh!

midchildan commented Nov 24, 2021

Uh oh!

midchildan commented Nov 25, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

midchildan Nov 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

domenkozar commented Nov 25, 2021

Uh oh!

toonn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toonn left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 26, 2021

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

midchildan commented Nov 24, 2021 •

edited

Loading

midchildan Nov 25, 2021 •

edited

Loading