llvmPackages_{9,12,13,14,15,16,17,18,git}.clang: fix libLTO.dylib path #302481
Conversation
ofborg
bot
requested review from
rrbutani,
Ericson2314,
sternenseemann,
dtzWill,
alyssais,
RossComputerGuy and
lovek323
ofborg
bot
added
10.rebuild-darwin: 501+
10.rebuild-darwin: 5001+
10.rebuild-linux: 0
|
This is a good set of changes from the looks of it, I'll give this a look over when I have the time. Right now, my priority is on making all LLVM subpackages common and I am hoping to get that done asap so this PR will probably take a bit for me to look over. I also don't want to have to rebase that PR heh. |
| '' + lib.optionalString stdenv.hostPlatform.isDarwin '' | ||
| # Make sure clang passes the correct location of libLTO to ld64 | ||
| substituteInPlace clang/lib/Driver/ToolChains/Darwin.cpp \ | ||
| --replace-fail 'StringRef P = llvm::sys::path::parent_path(D.Dir);' 'StringRef P = "${lib.getLib libllvm}";' |
There was a problem hiding this comment.
Shouldn't this be unconditional, so that Linux users catch it if the replacement starts failing?
There was a problem hiding this comment.
Sure, I can make it unconditional.
There was a problem hiding this comment.
It’s unconditional now. I also moved it to postPatch. Clang 16 already correctly had it there. Now they all should.
reckenrode
force-pushed
the
clang-liblto-fix
branch
from
09910a3
to
d699a8f
Compare
ofborg
bot
added
10.rebuild-linux: 501+
10.rebuild-linux: 5001+
and removed
10.rebuild-linux: 0
alyssais
added
the
2.status: merge conflict
|
Conflict :( |
reckenrode
force-pushed
the
clang-liblto-fix
branch
from
d699a8f
to
f3fd585
Compare
Rebased and dropped clang 9 since it was removed in #302750. |
ofborg
bot
removed
the
2.status: merge conflict
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
fixes NixOS#302629 After updating from `3.15.0 -> 3.19.2` build for `goose` was failing. This was due to the tests failing esp on integration tests which required the docker darmon to be running. Tests with similar nature were already skipped, but on the upgraded version newer integration tests were added. This change adds these changes to the test skip list aswell. Skip following tests which require a docker daemon to be running - TestLockModeAdvisorySession - TestDialectStore - TestGoMigrationStats - TestPostgresSessionLocker Links: - Commit adding the new integration tests: pressly/goose@b2c483a
qt: convert warnings to throw
Firefox: 124.0.2 -> 125.0; 115.9.0esr -> 115.10.0esr
goose: update test skip list with newer tests
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
force-pushed
the
clang-liblto-fix
branch
from
f3fd585
to
6011c50
Compare
|
Closing because of the mass ping. I’ll reopen targeting staging with the consolidated clang. |
rrbutani
added
the
6.topic: llvm/clang
Description of changes
Clang assumes that
libLTO.dylibis located at../libin the same prefix asclang, but that’s not true in nixpkgs.libLTO.dylibis actually located atlibllvm^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
-fltoeven 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=1when running clang, then confirming that the path passed to-lto_libraryactually exists in the store.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-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.