[Libxc] update to version 7.0.0 #9910
Conversation
L/Libxc/Libxc_GPU/build_tarballs.jl
Outdated
|
|
||
| mkdir libxc_build | ||
| cd libxc_build | ||
| cmake -DCMAKE_INSTALL_PREFIX=$prefix -DCMAKE_TOOLCHAIN_FILE=${CMAKE_TARGET_TOOLCHAIN} \ | ||
|
|
||
| mv ${WORKSPACE}/destdir/cuda/lib ${WORKSPACE}/destdir/cuda/lib64 |
There was a problem hiding this comment.
Doesn't this leave leftovers in the final tarballs? Why do you need to move it?
There was a problem hiding this comment.
CMake looks for the cuda installation in cuda/lib64 rather than cuda/lib. Other CUDA packages either do this, or they create a symlink. I'm happy to change for the latter, if it is indeed cleaner.
There was a problem hiding this comment.
The point is that the tarball has a cuda directory full of empty subdirectories. That shouldn't exist at all.
There was a problem hiding this comment.
Cleaned-up. It looks like the offending line was skip_audit=true, dont_dlopen=true in the arguments of the build_tarball() function. I am not sure what it did, but it does not seem necessary. I successfully tested the new build as well.
L/Libxc/Libxc_GPU/build_tarballs.jl
Outdated
| -DCMAKE_BUILD_TYPE=Release -DENABLE_XHOST=OFF -DBUILD_SHARED_LIBS=ON \ | ||
| -DENABLE_CUDA=ON -DCMAKE_CUDA_COMPILER=$prefix/cuda/bin/nvcc -DBUILD_TESTING=OFF \ | ||
| -DENABLE_FORTRAN=OFF -DDISABLE_KXC=ON .. | ||
| -DENABLE_CUDA=ON -DCMAKE_CUDA_COMPILER=$prefix/cuda/bin/nvcc \ |
There was a problem hiding this comment.
libxc is using enable_language(CUDA), i.e. the FindCUDAToolkit CMake module, so it should be sufficient (and a bit more proper) to specify the CUDA toolkit root, i.e. CUDAToolkit_ROOT=$prefix/cuda - instead of specifying the path to nvcc.
... but for unknown reasons, it seems CUDA_TOOLKIT_ROOT_DIR=$prefix/cuda is usually what actually works - though that is the variable specified for the deprecated FindCUDA
There was a problem hiding this comment.
None of the CUDA_TOOLKIT_ROOT_DIR or CUDAToolkit_ROOT option works out of the box in this build, without exporting the location of the nvcc compiler. I left the DCMAKE_CUDA_COMPILER as it is.
There was a problem hiding this comment.
Did you try setting CUDA_PATH?
L/Libxc/Libxc_GPU/build_tarballs.jl
Outdated
| -DENABLE_CUDA=ON -DCMAKE_CUDA_COMPILER=$prefix/cuda/bin/nvcc \ | ||
| -DBUILD_TESTING=OFF -DENABLE_FORTRAN=OFF \ | ||
| -DDISABLE_KXC=ON .. |
There was a problem hiding this comment.
Breaking lists into separate parts and sorting them is good practice (helps shorten future diffs etc.):
| -DENABLE_CUDA=ON -DCMAKE_CUDA_COMPILER=$prefix/cuda/bin/nvcc \ | |
| -DBUILD_TESTING=OFF -DENABLE_FORTRAN=OFF \ | |
| -DDISABLE_KXC=ON .. | |
| -DBUILD_TESTING=OFF \ | |
| -DCMAKE_CUDA_COMPILER=$prefix/cuda/bin/nvcc \ | |
| -DENABLE_CUDA=ON \ | |
| -DENABLE_FORTRAN=OFF \ | |
| -DDISABLE_KXC=ON .. |
This should of course be extended for the entire cmake "configure" call, but GitHub prevents making a suggestion for the entire cmake configure call (due to some deleted lines, apparently).
L/Libxc/Libxc_GPU/build_tarballs.jl
Outdated
|
|
||
| build_tarballs(ARGS, name, version, sources, script, [platform], | ||
| products, [dependencies; cuda_deps]; lazy_artifacts=true, | ||
| julia_compat="1.7", augment_platform_block, | ||
| skip_audit=true, dont_dlopen=true) | ||
| julia_compat="1.8", augment_platform_block=CUDA.augment, preferred_gcc_version=v"8") |
There was a problem hiding this comment.
Consider sorting keyword args (also lazy_artifacts) and putting each on a separate line.
|
Applied cosmetic changes suggested by @stemann |
L/Libxc/Libxc_GPU/build_tarballs.jl
Outdated
| # The products that we will ensure are always built | ||
| products = [ | ||
| LibraryProduct("libxc", :libxc) | ||
| ] | ||
|
|
||
| # Dependencies that must be installed before this package can be built | ||
| dependencies = [ | ||
| Dependency(PackageSpec(name="CompilerSupportLibraries_jll", uuid="e66e0078-7015-5450-92f7-15fbd957f2ae")), | ||
| Dependency(PackageSpec(name="CompilerSupportLibraries_jll", uuid="e66e0078-7015-5450-92f7-15fbd957f2ae")) |
There was a problem hiding this comment.
Please don't make unrelated unneeded and undesired cosmetic changes such as removing trailing commas that are there for a very good reason (i.e. not to break git blame when adding new lines)
| Dependency(PackageSpec(name="CompilerSupportLibraries_jll", uuid="e66e0078-7015-5450-92f7-15fbd957f2ae")) | |
| Dependency(PackageSpec(name="CompilerSupportLibraries_jll", uuid="e66e0078-7015-5450-92f7-15fbd957f2ae")), |
Straight forward version update. No need for patches anymore, because of upstream fixes. Note that I gave up on a previous approach to add
aarch64CPU + CUDA binaries (#9676).