-
-
Notifications
You must be signed in to change notification settings - Fork 14k
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
cctools-llvm: init at 11.1.0-973.0.1 #234859
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good to me though I don't think we should target staging. The cycles aren't usually longer than a couple of days so I don't think that'll unnecessarily delay things.
IIUC this won't actually be used until a later PR, right?
I’m doing a rebuild to make sure it works, but I’ll be pushing an update to this PR later tonight that incorporates your feedback @toonn. I’ve also made a few additional changes:
|
Correct. Once the prerequisite PRs are merged, I’ll open a PR with the stdenv rework that decouples it from the bootstrap tools. It will still target LLVM 11, but that will set things up to do a separate PR that only bumps LLVM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to clarify what I meant regarding the man pages.
bfcf9f2
to
50e19bf
Compare
The latest push fixes the issue with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like case-insensitive HFS strikes again for cctools-apple
:
trying https://opensource.apple.com/tarballs/ld64/ld64-609.tar.gz
% Total % Received % Xferd Average Speed Time Time Time Current
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
Dload Upload Total Spent Left Speed
100 257 100 257 0 0 2776 0 --:--:-- --:--:-- --:--:-- 2793
100 257 100 257 0 0 2394 0 --:--:-- --:--:-- --:--:-- 2401
0 0 0 0 0 0 0 0 --:--:-- 0:00:01 --:--:-- 0
0 0 0 0 0 0 0 0 --:--:-- 0:00:01 --:--:-- 0
100 754k 0 754k 0 0 355k 0 --:--:-- 0:00:02 --:--:-- 1253k
error: hash mismatch in fixed-output derivation '/nix/store/529jabac9d4h16ppwchichndhhg93z07-ld64-609.tar.gz.drv':
specified: sha256-SqQ7SqmK+uOPijzxOTqtkEu3qYmcth6H7rrQ03R1Q+4=
got: sha256-1idktrbJAsDhZcJj3t4aXbYGAGFKr0FKiNtamdUtqiU=
And cctools-llvm
seems to be missing machocheck
, I don't think that was intentional?
/nix/store/nwrx6x29jpn2s4g4bfqymrqxfy6x7kl4-cctools-llvm-11.1.0-973.0.1/bin/machocheck: sh: /nix/store/nwrx6x29jpn2s4g4bfqymrqxfy6x7kl4-cctools-llvm-11.1.0-973.0.1/bin/machocheck: No such file or directory
I’m using case-sensitive APFS, and I get the same error. I think the hashes changed when Apple started redirecting downloads to GitHub.
I only linked binaries corresponding to ones Apple ships (as part of macOS or Xcode), so an omission may be intentional. However, I don’t see |
@reckenrode, not macrocheck, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still getting different hashes. I'll try GC'ing tomorrow after the configd build gets a gcroot but I think Nix wouldn't give different paths nonetheless if cctools
and cctools-port
were the same thing?
toonn@terra ~/s/n/r/pr234859-cctools-llvm> nix-build --no-out-link --pure -A dar
win.cctools
/nix/store/k3aflp9mdq8ks8pzslj9sz1jml9si6rq-cctools-port-973.0.1
toonn@terra ~/s/n/r/pr234859-cctools-llvm> nix-build --no-out-link --pure -A dar
win.cctools-port
/nix/store/b6dlp7jb98wn5fggshbcksipyyxp7gjc-cctools-port-973.0.1
I created #235975 to fix the hashes on cctools-apple. I’ll also be opening a separate PR to fix cctools-port on x86_64-darwin when built with clang 16. |
Replying here to document since this was discussed on Matrix: Apple doesn’t appear to ship |
I think I fixed the cctools ≠ cctools-port issue. I’m not sure using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still getting different paths for them even with the recursive attribute set and I can't explain why. I've GC'ed so there's no FOD shenanigans involved.
> git log -1 --oneline; nix-build --pure --no-out-link -A darwin.cctools -A darwin.cctools-port; sed -n '63p;103,115p' pkgs/top-level/darwin-packages.nix
bc81c565e6c5 (HEAD, reckenrode/cctools-llvm) cctools-llvm: init at 11.1.0-973.0.1
/nix/store/k3aflp9mdq8ks8pzslj9sz1jml9si6rq-cctools-port-973.0.1
/nix/store/b6dlp7jb98wn5fggshbcksipyyxp7gjc-cctools-port-973.0.1
impure-cmds // appleSourcePackages // chooseLibs // rec {
cctools = cctools-port;
cctools-apple = callPackage ../os-specific/darwin/cctools/apple.nix {
stdenv = if stdenv.isDarwin then stdenv else pkgs.libcxxStdenv;
};
cctools-llvm = callPackage ../os-specific/darwin/cctools/llvm.nix {
stdenv = if stdenv.isDarwin then stdenv else pkgs.libcxxStdenv;
};
cctools-port = callPackage ../os-specific/darwin/cctools/port.nix {
stdenv = if stdenv.isDarwin then stdenv else pkgs.libcxxStdenv;
};
@toonn I figured out the problem. The stdenv defines an overlay that sets darwin.cctools to the one built during the stdenv build. That results in darwin.cctools having a different value from darwin.cctools-port. I didn’t catch that during development because my stdenv rework branch was already including the additional attributes. When I cherry-picked the commit onto master, I didn’t update the stdenv to include cctools-port. I pushed an update that does. |
$ nix-build --pure --no-out-link -A darwin.cctools -A darwin.cctools-port
/nix/store/3p2xdd5xmrc0qyi10lf95nbydcyprqy9-cctools-port-973.0.1
/nix/store/3p2xdd5xmrc0qyi10lf95nbydcyprqy9-cctools-port-973.0.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the rationale for keeping the override BTW? We could also drop both altogether, right?
a530ce6
to
94565e6
Compare
The latest push fixed a few typos. Even with man pages enabled, it wasn’t installing any of them because the check for |
7c7b83d
to
b735f72
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, am I wrong in expecting this to give me the man pages?
> nix-build --no-out-link --pure -E 'let ps = (import ./. {}); in ps.lib.getMan ps.darwin.cctools-llvm'
/nix/store/s8vk4mzjzq6q06fja0rd4ainb9ck0wbi-cctools-llvm-11.1.0-973.0.1-man
> ls -a /nix/store/s8vk4mzjzq6q06fja0rd4ainb9ck0wbi-cctools-llvm-11.1.0-973.0.1-man
./ ../
No, you’re not wrong. Are you using the latest commits? I pushed some fixes last night when I found that enabling the man pages in my rework branch wasn’t working as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more quandary. The symlink for the otool
man page isn't working for some reason, even though the target works fine. Can you reproduce this?
> ls -l /nix/store/2jk0qgn8xwiqcm5pd6zjf9aq5sgdqryc-cctools-llvm-11.1.0-973.0.1-man/share/man/man1/otool.1.gz
lrwxr-xr-x 1 root wheel 94 1 jan 1970 /nix/store/2jk0qgn8xwiqcm5pd6zjf9aq5sgdqryc-cctools-llvm-11.1.0-973.0.1-man/share/man/man1/otool.1.gz@ -> /nix/store/vscadj4dfp9vd6ymhw9y3nk0a8ca1nkf-cctools-port-973.0.1-man/share/man/man1/otool.1.gz
> man /nix/store/vscadj4dfp9vd6ymhw9y3nk0a8ca1nkf-cctools-port-973.0.1-man/share/man/man1/otool.1.gz
> man /nix/store/2jk0qgn8xwiqcm5pd6zjf9aq5sgdqryc-cctools-llvm-11.1.0-973.0.1-man/share/man/man1/otool.1.gz
fopen: No such file or directory
Cannot open man page /nix/store/2jk0qgn8xwiqcm5pd6zjf9aq5sgdqryc-cctools-llvm-11.1.0-973.0.1-man/share/man/man1/llvm-otool.1.gz
No manual entry for /nix/store/2jk0qgn8xwiqcm5pd6zjf9aq5sgdqryc-cctools-llvm-11.1.0-973.0.1-man/share/man/man1/otool.1.gz
Another slightly puzzling thing is even though there's a symlink for the ranlib
man page, man
opens the man page for libtool
, the contents of the pages seems like it may be equivalent but the files differ according to diff. Not sure whether there's a way to get the exact page to show up? (This is mostly out of interest, a single redundant man page shouldn't stop this PR.)
I see what’s happening with the man pages. The man page for Edit: Removed bit about compressed man pages. |
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, the opportunity was taken to replace other provided they are functional replacements. Note: 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; * lipo - crashes when running the LLVM test suite; * install_name_tool - fails when trying to build swift-corefoundation; and. * randlib - not completely a drop-in replacement, so leaving it out for now. If other incompatabilities are found, the tools can be reverted or made conditional. For example, cctools `strip` is preferred on older versions of LLVM (which lack the compiler-rt issue) or when cctools itself is a new enough version because `llvm-strip` on LLVM 11 produces files that older verions of `codesign_allocate` cannot process correctly. One final caveat/note: Some tools are not duplicated or linked from cctools-port. The names of the tools and which ones were linked was determined based on what is provided upstream in Xcode and is installed on macOS system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Note: This is targeting staging because it will be needed by Darwin stdenv changes that will need to go through staging.
Description of changes
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. Sincestrip
needs replaced, the opportunity was taken to replace other provided they are functional replacements.Note: 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:
If other incompatabilities are found, the tools can be reverted or made conditional. For example, cctools
strip
is preferred on older versions of LLVM (which lack the compiler-rt issue) or when cctools itself is a new enough version becausellvm-strip
on LLVM 11 produces files that older verions ofcodesign_allocate
cannot process correctly.One final caveat/note: Some tools are not duplicated or linked from cctools-port. The names of the tools and which ones were linked was determined based on what is provided upstream in Xcode and is installed on macOS system.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)