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

cctools-llvm: init at 11.1.0-973.0.1 #234859

Merged
merged 1 commit into from
Jun 8, 2023
Merged

Conversation

reckenrode
Copy link
Contributor

@reckenrode reckenrode commented May 29, 2023

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. 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.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Copy link
Contributor

@toonn toonn left a 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?

pkgs/os-specific/darwin/cctools/llvm.nix Outdated Show resolved Hide resolved
pkgs/os-specific/darwin/cctools/llvm.nix Show resolved Hide resolved
pkgs/os-specific/darwin/cctools/llvm.nix Outdated Show resolved Hide resolved
pkgs/os-specific/darwin/cctools/llvm.nix Outdated Show resolved Hide resolved
@reckenrode
Copy link
Contributor Author

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:

  • I dropped useLLD because it’s untested and complicates things; and
  • Reverted back to ranlib from cctools because llvm-ranlib is not a drop-in replacement.

@reckenrode
Copy link
Contributor Author

IIUC this won't actually be used until a later PR, right?

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.

@reckenrode reckenrode changed the title cctools-llvm: add at 973.0.1-11.1.0 cctools-llvm: init at 973.0.1-11.1.0 Jun 3, 2023
@reckenrode reckenrode changed the title cctools-llvm: init at 973.0.1-11.1.0 cctools-llvm: init at 11.1.0-973.0.1 Jun 3, 2023
@reckenrode reckenrode changed the base branch from staging to master June 3, 2023 12:03
Copy link
Contributor

@toonn toonn left a 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.

pkgs/os-specific/darwin/cctools/llvm.nix Outdated Show resolved Hide resolved
pkgs/os-specific/darwin/cctools/llvm.nix Outdated Show resolved Hide resolved
@reckenrode
Copy link
Contributor Author

The latest push fixes the issue with darwin.cctools not being the same thing as darwin.cctools-port.

Copy link
Contributor

@toonn toonn left a 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

@reckenrode
Copy link
Contributor Author

reckenrode commented Jun 3, 2023

Looks like case-insensitive HFS strikes again for cctools-apple

I’m using case-sensitive APFS, and I get the same error. I think the hashes changed when Apple started redirecting downloads to GitHub.

And cctools-llvm seems to be missing machocheck, I don't think that was intentional?

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 macrocheck in cctools-port, and grepping the source turns up nothing. I’m not sure what that is.

@toonn
Copy link
Contributor

toonn commented Jun 4, 2023

@reckenrode, not macrocheck, machocheck.

Copy link
Contributor

@toonn toonn left a 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

@reckenrode
Copy link
Contributor Author

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.

@reckenrode
Copy link
Contributor Author

@reckenrode, not macrocheck, machocheck.

Replying here to document since this was discussed on Matrix: Apple doesn’t appear to ship machocheck, so it’s not included. It looks like it’s used by ld64 for some tests.

@reckenrode
Copy link
Contributor Author

reckenrode commented Jun 5, 2023

I think I fixed the cctools ≠ cctools-port issue. I’m not sure using rec was the right approach, but neither self.cctools-port nor pkgs.targetPackages.darwin.cctools-port is the same as the cctools-port that’s created directly by callPackage.

Copy link
Contributor

@toonn toonn left a 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;
  };

@reckenrode
Copy link
Contributor Author

@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.

@reckenrode
Copy link
Contributor Author

$ 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

Copy link
Contributor

@toonn toonn left a 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?

pkgs/stdenv/darwin/default.nix Outdated Show resolved Hide resolved
@reckenrode reckenrode force-pushed the cctools-llvm branch 2 times, most recently from a530ce6 to 94565e6 Compare June 6, 2023 00:08
@reckenrode
Copy link
Contributor Author

reckenrode commented Jun 6, 2023

The latest push fixed a few typos. Even with man pages enabled, it wasn’t installing any of them because the check for $sourcePath was missing the leading $. It also needed to properly access the ${source} and ${target} in the function to link the man pages.

@reckenrode reckenrode force-pushed the cctools-llvm branch 2 times, most recently from 7c7b83d to b735f72 Compare June 6, 2023 00:23
Copy link
Contributor

@toonn toonn left a 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
./  ../

@reckenrode
Copy link
Contributor Author

Hmm, am I wrong in expecting this to give me the man pages?

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.

Copy link
Contributor

@toonn toonn left a 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.)

@reckenrode
Copy link
Contributor Author

reckenrode commented Jun 6, 2023

I see what’s happening with the man pages. The man page for otool just includes llvm-otool.1, which does not exist in the cctools-llvm man output. I can special case it. The pushed commit contains special-casing for otool and ld64.

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.
Copy link
Contributor

@toonn toonn left a comment

Choose a reason for hiding this comment

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

LGTM!

@toonn toonn merged commit 782dbaf into NixOS:master Jun 8, 2023
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.

2 participants