-
-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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: 973.0.1 -> 1010.6 #307880
cctools: 973.0.1 -> 1010.6 #307880
Conversation
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
d9770a1
to
60c33ac
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/darwin-ld-symbol-s-not-found-for-architecture-arm64/46177/1 |
Latest push is up until right before the Darwin cross work. |
I’m paring down this PR to reduce the chance of conflicts and to make it easier to review in aggregate. Cherry-picking commits is going to take too much time, and sometimes they depend on other commits. |
The following will be done in separate PRs:
|
The only thing I have left to do before marking this ready is fix the AvailabilityVersions package to patch out newer macros on SDKs 10.12, 10.13, 10.14, and 10.15. Some packages (like Git and SDL2) check for the presence of certain macros regardless of their value to check SDK version. That feels like a misuse of the macros, but I don’t want to break anything with the switch. |
When the linker signs a Mach-O binary, it sets a flag in the signature’s code directory indicating that the signature was generated by a linker. Tools such as `strip` and `install_name_tool` read this flag and will update ad hoc signatures after they perform their modifications. The updated l64 supports signing binaries automatically. Both the updated cctools and LLVM will check for the linker-signed flag and resign binaries they modify automatically when it’s present. Given that, use of postLinkSignHook is unnecessary and potentially harmful. In particular, if the hook is used and an unwrapped `strip` or `install_name_tool` is on the user’s path, they will not automatically update an ad hoc signature. Instead, they will issue a warning and create a binary with a broken signature. It is more robust to let the tools handled this since the only time a signature would not be linker-signed is when the user is manually invoking `codesign` (or another tool such as `sigtool` or `rcodesign`), which by nature of the invocation updates the signature to a valid one. Since `strip` no longer needs to be wrapped for code-signing, binutils-wrapper now uses the GNU strip wrapper on Darwin. Fixes NixOS#208951.
# GHC needs install_name_tool on all darwin platforms. On aarch64-darwin it is | ||
# part of the bintools wrapper (due to codesigning requirements), but not on | ||
# x86_64-darwin. We decide based on target platform to have consistent tools | ||
# across all GHC stages. |
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.
This comment needs to be rephrased and replace the one for strip below. (In all touched files.) Having the information in a comment is better than piecing it together from a deleted comment and the commit message :)
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 context of the comment for strip
? Darwin now uses the default wrapper, which just passes --enable-deterministic-archives
. Should it not do that with GHC, or should it use the wrapper unconditionally?
# TODO(@sternenseemann): also use wrapper if linker == "bfd" or "gold"
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 updated the comments.
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.
We want to use the wrapper if possible, but unfortunately it is kind of difficult to know when we can use it. I think the TODO
is about the fact that you can sort of bodge detect GNU binutils using linker == "gold" || linker == "bfd"
which means there's a wrapper (like on aarch64-darwin and now x86_64-darwin). IIRC there's no wrapper if bintools come from LLVM, so that's a case you'd need to guard against.
I think I never really attempted to improve it since the wrapper is inconsequential for GHC itself and I was dissatisfied with the detectability. I think this mess ought to be cleaned up in a different way, i.e. just symlinking from the wrapper to the unwrapped derivation if an unwrapped bintool is available.
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 take it using bintools.isGNU
is unreliable? If not, I can update the GHC files to check cc.bintools.isGNU || stdenv.targetPlatform.isDarwin
. Otherwise, I’d like to keep the changes just to Darwin to avoid increasing the scope of this PR more.
(Also, please let me know if any more revisions are needed to the comments I added to GHC regarding install_name_tool
and strip
.)
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 don't know whether it is reliable or not. I think it wasn't a thing back then (would need to check). But agreed w.r.t. scope.
- Unconditionally get `install_name_tool` from cc.bintools.bintools since it is no longer wrapped; and - Use the `strip` wrapper on both Darwin architectures. It’s the default one, and it’s the same between both.
I've kicked off a build on 10.13 hardware to see if the changes to keep <10.14 working are alright, not sure if we want to wait for that to complete though. |
It should work, so I’d be surprised if it didn’t. Anyway, I’m currently waiting on approvals from you and @sternenseemann (or at least confirmation/feedback re: the revised comments). |
Looks good. |
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, like I said no need to block on the build.
Had a couple failures related to the LLVM manpages but I haven't been able to check whether it's not just running out of disk space or such. So let's just go ahead with the merge already.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
cctools was updated and migrated to the `by-name` hierarchy in nixpkgs, which moves it to the top-level. It is also being added to `darwin-aliases.nix`, which will make the old name unavailable for use in nixpkgs. This change preferentially uses the new name while falling back to the old one for out-of-tree users. Relevant nixpkgs PRs: - NixOS/nixpkgs#307880 - NixOS/nixpkgs#328077
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/breaking-changes-announcement-for-unstable/17574/57 |
cctools: 973.0.1 -> 1010.6
After NixOS#307880 was merged, ld64 is 951.9, which is the version shipped with Xcode 15.4. It is new enough to support all of the linker flags used by the upstream bindist. The only remaining blockers to bootstrapping with the bindist on Darwin is the GHC issue that tries to use `ar -L`, which creates archives that cannot be linked by ld64. Overriding the setting in the bindist suppresses that behavior.
Description of changes
This PR updates cctools and ld64. It also fixes related breakage. It is currently set to draft until 24.05 is released. I will be rebasing weekly against staging, resolving any merge conflicts then. Before updating the ld64 branch, I build my configs and the Darwin channel blockers to confirm no regressions.
The following notes are copied from https://discourse.nixos.org/t/darwin-updates-news/42249/10.
Packaging Changes
Note: ld-prime is will not be packaged because it is not currently included in the source releases. While I think it’s unlikely, ld-prime will be added separately as a new package should the source be released.
New Features
Breaking Changes
-rpath
when merging Mach-O object files fails.-q
and-Q
flags are supported to control whether it calls GNU as or the clang assembler (matching the upstream behavior ofas
).strip
andinstall_name_tool
are no longer wrapped withsigtool
. If you are modifying linker-signed binaries, which should be the typical case, they will update the signatures automatically. If you are manually invokingcodesign
, you will need to update the signatures manually after runningstrip
orinstall_name_tool
.stdenv updates
darwin.bintools changes
darwin.bintools changes
llvm_cmds
andcctools_cmds
respectively). All binaries are symlinked to their traditional names (e.g., llvm-strip is symlinked to strip).isCCTools
and notisLLVM
because it does not use lld by default. Maybe that could change with lld 18, but I’m doubtful.Resolved Issues
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.