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

llvmPackages_{12,13,14,15,16,17,18,git}.clang: fix libLTO.dylib path #304350

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

reckenrode
Copy link
Contributor

@reckenrode reckenrode commented Apr 15, 2024

Description of changes

Clang assumes that libLTO.dylib is located at ../lib in the same prefix as clang, but that’s not true in nixpkgs. libLTO.dylib is actually located at libllvm^lib/lib.libLTO.dylib.

This is the first piece to fixing LTO on Darwin. The other is updating ld64, which I will be doing in a separate PR later this week. This PR does not depend on that PR because there is no harm passing the correct path to ld64. It does not perform LTO by default, and trying to use -flto even with the correct path remains broken.

Testing was done using my WIP PR for ld64 with clang 16. See #19098 (comment) for output. It can also be validated by setting NIX_DEBUG=1 when running clang, then confirming that the path passed to -lto_library actually exists in the store.

Note: Testing was done on #302481. I’m building against the common clang branch now.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.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.

Add a 👍 reaction to pull requests you find important.

Clang assumes that `libLTO.dylib` is located at `../lib` in the same
prefix as `clang`, but that’s not true in nixpkgs. `libLTO.dylib` is
actually located at `libllvm^lib/lib.libLTO.dylib`.
@reckenrode
Copy link
Contributor Author

@RossComputerGuy I rebased and consolidated the patches. Thanks for your work on #303948.

@wegank
Copy link
Member

wegank commented Apr 18, 2024

@reckenrode Do you want this in 24.05?

@reckenrode
Copy link
Contributor Author

reckenrode commented Apr 18, 2024

@reckenrode Do you want this in 24.05?

It would be nice, but it’s only half the fix to LTO on Darwin. The other half is the ld64 upgrade, which is probably too big for 24.05. I’m still working through getting all the Darwin channel blockers to build.

@wegank wegank merged commit cb96a4d into NixOS:staging Apr 18, 2024
31 checks passed
@reckenrode reckenrode mentioned this pull request Apr 30, 2024
13 tasks
@rrbutani rrbutani added the 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related label May 27, 2024
@nixos-discourse
Copy link

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

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.

4 participants