-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
libcxxabi: remove link with build libcxxabi #185766
Conversation
Removing the loop was apparently a mistake, because sometimes it's more than one dylib? Looks like just another copy of the same. Any way, I restored the loop. I did a successful stdenv rebuild on aarch64-darwin and confirmed my problem went away in the build I was trying. |
I'm trying to test this but using
|
1946e62
to
96e4a0f
Compare
Rebased to fix conflicts. I've only seen this cause |
Ping! What can we do to move this forward? |
I was intending to review this as part of the work on Swift, which IIUC is ready for review? I don't think it's blocking anything else? Will try to review both PRs this week. Others should not be held back from reviewing by this, of course. |
Yes, the Swift PR is ready. I don't have anything to add to it. 👍 And yes, afaik this PR doesn't block anything else. Should be fine to merge it together with the Swift PR or by itself, whichever we prefer. |
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.
Does what it says on the tin.
Before:
> otool -L /nix/store/srj4s0xsrz6fdhc4zip4ah5b8lhqvdhr-libcxxabi-13.0.1/lib/libc++abi.1.dylib
/nix/store/srj4s0xsrz6fdhc4zip4ah5b8lhqvdhr-libcxxabi-13.0.1/lib/libc++abi.1.dylib:
/nix/store/srj4s0xsrz6fdhc4zip4ah5b8lhqvdhr-libcxxabi-13.0.1/lib/libc++abi.1.dylib (compatibility version 1.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1238.60.2)
/nix/store/36grdzp9lha3isiwiagnj1vyr4y10qvd-libcxxabi-11.1.0/lib/libc++abi.1.dylib (compatibility version 1.0.0, current version 1.0.0)
After:
> otool -L /nix/store/l34pp467vl2xnccqrllnxvigvsmdgjqk-libcxxabi-13.0.1/lib/libc++abi.1.dylib
/nix/store/l34pp467vl2xnccqrllnxvigvsmdgjqk-libcxxabi-13.0.1/lib/libc++abi.1.dylib:
/nix/store/l34pp467vl2xnccqrllnxvigvsmdgjqk-libcxxabi-13.0.1/lib/libc++abi.1.dylib (compatibility version 1.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1238.60.2)
/nix/store/l34pp467vl2xnccqrllnxvigvsmdgjqk-libcxxabi-13.0.1/lib/libc++abi.1.dylib (compatibility version 1.0.0, current version 1.0.0)
I assume the repetition of the dylib is harmless?
It appears to be harmless in all my testing. I looked for a way to remove the entry completely, but there doesn't appear to be a tool for that? |
Looks like something similar needs to be done for libcxx as well.
libc++ should link against its own version of libc++abi, but current links the version from the previous stage stdenv which built libc++. |
Same adjustment as made for libc++abi in NixOS#185766, for the same reason: the unamended dylib links to the libc++abi in the build stdenv, which is the wrong version. Tested on Darwin with LLVM 14 stdenv, but the phase is added to all versions, including 11 - so this will cause a mass rebuild. See: NixOS#185766
Same adjustment as made for libc++abi in NixOS#185766, for the same reason: the unamended dylib links to the libc++abi in the build stdenv, which is the wrong version. Tested on Darwin with LLVM 14 stdenv, but the phase is added to all versions, including 11 - so this will cause a mass rebuild. See: NixOS#185766
Same adjustment as made for libc++abi in #185766, for the same reason: the unamended dylib links to the libc++abi in the build stdenv, which is the wrong version. Tested on Darwin with LLVM 14 stdenv, but the phase is added to all versions, including 11 - so this will cause a mass rebuild. See: #185766 (cherry picked from commit 67f11a2)
Description of changes
This is a fix for the issue seen in #181485 (comment).
I updated the snippet across all LLVM versions, but that probably implies a Darwin stdenv rebuild (via v11). I've not yet tested this, just the library build itself on v13 (and verified
otool -L
output).The previous snippet did a for-loop over
lib/*.dylib
, but all but one of those are symlinks. This also meant the final install name was whatever it processed last. It now preserves the exact basename.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/
)nixos/doc/manual/md-to-db.sh
to update generated release notes