rpms / lld

Clone
Source Code
GIT

#65 explicitly define directories for compat builds
Merged a year ago by nikic. Opened a year ago by sentry.
rpms/ sentry/lld rawhide-llvm-config  into  rawhide

file modified
+1 
@@ -109,6 +109,7 @@ 
 

  	-DCMAKE_SKIP_RPATH:BOOL=ON \
 

  	-DPYTHON_EXECUTABLE=%{__python3} \
 

  %if %{with compat_build}
 

+ 	-DLLVM_CMAKE_DIR=%{install_libdir}/cmake/llvm \
 

  	-DLLVM_INCLUDE_TESTS=OFF \
 

  %else
 

  	-DLLVM_INCLUDE_TESTS=ON \

Previously lld was not using the correct llvm for compat builds causing build failures.
Since the use of llvm-config is deprecated with LLVM 15 and to be removed with 16 this adds explicit definitions for the directories.

Build succeeded.

Could you please explain in more detail what problem this is trying to solve? I don't think lld uses the LLVM_CONFIG variable at all -- to point to a specific LLVM installation, we'd have to specify LLVM_CMAKE_DIR or CMAKE_MODULE_PATH instead.

some part of it seems to be using it, since before this it complains about being unable to include GNUInstallPackageDir.

I'll track this down and find out where its used and when this was added.

The LLVM_CONFIG variable is still used in the 15.0.4 LLD release https://github.com/llvm/llvm-project/blob/llvmorg-15.0.4/lld/CMakeLists.txt

But it appears that llvm-config is on its way out
https://github.com/llvm/llvm-project/commit/91f3f0bf31d6fdcbbd750670840e22f9b2b36625

Right now, this change mirrors what the clang spec does, should I migrate lld and clang over
or is this good enough as is?

Ah yes, I was looking at the code on the main branch, where llvm-config isn't supported anymore. I think it would be best to directly specify the cmake module, as that should work both with lld 15 and newer versions.

rebased onto a85451a
a year ago

Build succeeded.

changed it so that it no longer depends on llvm-config

clang equivalent https://src.fedoraproject.org/rpms/clang/pull-request/186

Sorry for the review delay, I was on PTO. My remaining question here would be whether it is really necessary to explicitly specify LLVM_BINARY_DIR and LLVM_INCLUDE_DIRS. My understanding is that these variables are usually provided by the LLVM cmake module (they are part of LLVMConfig.cmake.in).

rebased onto d29087f
a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto 154be76
a year ago

You are correct, those values are correctly provided by LLVMConfig.cmake, so we don't need to manually set it.
Rebased on the latest commit to try and fix the scratch build failing.

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

The scratch build will currently always fail due to an ongoing side tag build. We can't test the actual compat package build anyway (outside of copr), because it would require building an llvm15 package first. This looks correct to me though, so I'll just go ahead and merge it.

Pull-Request has been merged by nikic
a year ago
Metadata
None
No Tags
Changes Summary 1
+1 -0
file changed
lld.spec
Powered by Pagure 5.13.3
Documentation • File an Issue • About this Instance • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.