stdenv/darwin: enable tapi support in cctools #93912
Conversation
ofborg
bot
added
6.topic: darwin
ofborg
bot
added
10.rebuild-darwin: 501+
|
|
||
| # ncurses is required here to avoid a reference to bootstrap-tools, which is | ||
| # not allowed for the stdenv. | ||
| buildInputs = [ ncurses clang_6.cc ]; |
There was a problem hiding this comment.
We should really get this to build with the current compiler if it's enabled by default, bootstrapping multiple versions is a huge pain and llvm 6 is already quite old at this point.
There was a problem hiding this comment.
Looking closer, I don't think clang is intended to be an external dependency. I've pushed an update but I'm still waiting for it to test building stdenv locally.
|
I see there's a link to |
installTarget looks like a typo for installTargets. This causes a lot of llvm and clang to be built and installed. Clang is not intended to be an external dependency. The source bundle includes llvm and clang. Adding include paths and building clangBasic first is sufficient to use the internal clang components.
I sent an email to the listed address but no reply yet. |
thefloweringash
force-pushed
the
cctools-tapi
branch
from
ab3ab02 to
7a09a40
Compare
|
Tests for the updated libtapi finished building, seems ok. Updated main comment. |
| nativeBuildInputs = [ cmake python3 ]; | ||
| buildInputs = [ clang_6.cc ]; |
There was a problem hiding this comment.
i can't remember why clang_6 was needed here - but if everything builds correctly it sounds good to remove
There was a problem hiding this comment.
This project seems to be intended to be built from its source tree without an external clang. The source tree includes the parts of clang that are required. I didn't look at exactly how much is included or built, but the build is a lot shorter than a full llvm/clang build.
Taking clang out of the build inputs required adding clangBasic to the built targets to generate the required headers (compare to upstream) and fixing the include directories to include the generated headers (compare to upstream). The external clang was probably filling in these holes in the build system.
| ''; | ||
|
|
||
| cmakeFlags = [ "-DLLVM_INCLUDE_TESTS=OFF" ]; | ||
| buildFlags = [ "clangBasic" "libtapi" ]; |
There was a problem hiding this comment.
do you need clangBasic here? we're already building it elsewhere, so i was hoping we could leave it off
There was a problem hiding this comment.
This is required after removing clang_6.cc from buildInputs. I'm guessing it's an undeclared dependency in the build system. If this isn't specified then the build fails with:
In file included from /tmp/nix-build-libtapi-1000.10.8.drv-0/source/src/llvm/projects/libtapi/lib/Core/Architecture.cpp:18:
In file included from /private/tmp/nix-build-libtapi-1000.10.8.drv-0/source/src/llvm/projects/clang/include/clang/Basic/Diagnostic.h:18:
/private/tmp/nix-build-libtapi-1000.10.8.drv-0/source/src/llvm/projects/clang/include/clang/Basic/DiagnosticIDs.h:71:10: fatal error: 'clang/Basic/DiagnosticCommonKinds.inc' file not found
#include "clang/Basic/DiagnosticCommonKinds.inc"
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
we're already building it elsewhere, so i was hoping we could leave it off
I'm making a bit of an assumption that "clangBasic" is a small internal clang target and not something that should be shared between builds, but a quick web search doesn't show up documentation for what it contains. The library definition is fairly small.
| @@ -100,6 +100,8 @@ in rec { | |||
| cp ${cctools_}/bin/$i $out/bin | |||
| done | |||
|
|
|||
| cp -d ${darwin.libtapi}/lib/libtapi* $out/lib | |||
There was a problem hiding this comment.
Is this actually needed? If so, we will have to regenerate bootstrap tools
There was a problem hiding this comment.
libtapi is enabled everywhere, so the ld that's included in the bootstrap tools now depends on libtapi. The intent is to make sure that the bootstrap tools can be updated, but this change doesn't require a bootstrap tools update.
Removing this causes
$ nix-build pkgs/stdenv/darwin/make-bootstrap-tools.nix -A test
[...]
dyld: Library not loaded: @rpath/libtapi.dylib
Referenced from: /nix/store/ym4nscqv1h7sxn6b6dby0i4106lcw9sr-unpack/bin/ld
Reason: image not found
I do want to regenerate the bootstrap tools as part of my overall goals. Next step is a .tbd based libSystem which I have on a branch. It can link hello on Big Sur, but there's still a decent amount of work before I can open that as a PR.
|
Looks good! I think it may cause conflicts with #93596 but this one is small enough to go first. |
|
I'm interested in advancing this as a step towards a Big Sur compatible stdenv. Is there anything I can do to help get this merged? |
|
It looks good to me. Maybe @LnL7 could rereview though. |
thefloweringash
force-pushed
the
cctools-tapi
branch
from
7a09a40 to
630f5d3
Compare
|
I noticed this was causing a large amount of extra building in stage 4 by building a full cmake. I've pushed an updated version that avoids this by reusing cmake in stage 4. This change now only introduces a build of libtapi in stage 1 and stage 4. I've attempted to show the changes in the history of a gist. |
Motivation for this change
Working towards #19906 with the overall goal of Big Sur compatibility via .tbd files.
Comes with the unfortunate downside of requiring clang_6 to build the stdenv, which increases the time required.nix-build -A hellonix-build pkgs/stdenv/darwin/make-bootstrap-tools.nix -A testcc @matthewbauer
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)