llvmPackages_17.llvm: remove origin variable #316071
Conversation
|
CC @paparodeo since I can't add them as a reviewer |
There was a problem hiding this comment.
in this patch there seem to be places that are patching $ORIGIN that shouldn't.
eg:
+ set(_build_rpath "\$ORIGIN/../${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}" ${extra_libdir})
+ set(_install_rpath "\$ORIGIN/../${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}" ${extra_libdir})
the _build_rpath seems wrong, as i think that is to be able to run tests out of the build directory and be able to find the libs in $ORIGIN/../lib so the fact we are modifying it seems wrong. however, _install_rpath seems like it needs to be modified and can just be set(_install_rpath ${extra_libdir}) or ${LLVM_LIBRARY_DIR} perhaps as i think that is what ${extra_libdir} expands too.
i think this happened was fixed in llvm18 init.
I only synced the patch with the one in LLVM 18. I'll remove those and then we can update the LLVM 18 one to kill off |
6314ee8
to
4c27651
Compare
|
Hmm, looking at the LLVM 18 patch that one looks fine. I'll update this one to fully sync it with LLVM 18. |
4c27651
to
e0df69d
Compare
e0df69d
to
e0b48c1
Compare
|
Move to staging done. |
Description of changes
Related: #316045 (comment)
This PR removes
$ORIGINfrom LLVM 17'sgnu-install-dirs.patchso that it is inline with how LLVM 18 operates.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.