xtensor-blas
xtensor-blas copied to clipboard
Moves target_link_libraries() and target_compile_features() from cmak…
This fixes #169 and correctly determines BLAS and LAPACK location.
Both target_compile_features() and target_link_libraries() are meant to be used in CMakeLists.txt file (not the config). CMake will then export this information to the targets file.
The find_dependency() calls in the config file are supposed to be matched with find_package() calls in the CMakeLists.txt file.
Is there a reason why BLAS is not available during the appveyor test? Should BLAS and LAPACK be optional?
Thanks for the PR.
I think that your suggestion is not what is desired for general shipping of a library. If I’m not mistaken you will now encode the install-time dependencies in the config file. If install time is elsewhere (e.g. conda-build) the library will be left with irrelevant paths.
Instead, what is the current behaviour is that the dependencies are ‘searched’ when calling find_package(xtensor-blas)
and are thus the correct ones for you current environment (and/or system)
A second this is that what would fix #169 is to get rid of more modern convenience functions. As specified in the issue, replacing:
target_link_libraries(xtensor-blas INTERFACE ${BLAS_LIBRARIES} ${LAPACK_LIBRARIES})
by
set_target_properties(xtensor-blas PROPERTIES
INTERFACE_LINK_LIBRARIES "${BLAS_LIBRARIES};${LAPACK_LIBRARIES}"
)
Which retains the functionality fully but also works for CMake versions that do not yet have the convenience function target_link_libraries
I see. This is not an issue with conda as they set placeholder prefix paths when building and then do a string replace when users run conda install
(see https://stackoverflow.com/a/65136313/1034772). This is how it works for the conda-forge shrinkwrap package, which is a header-only library that depends on three other libraries.
But I agree that this approach would likely be a problem if you want to support other package managers beyond conda. Both deb and rpm packages would contain the build paths instead of the install paths.
I updated the PR to remove the find_package and target_link_library calls from CMakeLists.txt. INTERFACE_LINK_LIBRARIES is now set in the config file. I left target_compile_features (which has been available since v3.1) in CMakeLists.txt since this will be exported to the targets file.