xsimd icon indicating copy to clipboard operation
xsimd copied to clipboard

[Feature request] Add an option to skip installation

Open ToKiNoBug opened this issue 2 years ago • 14 comments

I tried to introduce xsimd into my project with FetchContent, it works well, but all xsimd headers are installed together with my project files, which is not expected. And I failed to find any way to control xsimd's installation.

So I hope to skip the installation of xsimd when it's introduced with FetchContent. This can be done with comparing CMAKE_SOURCE_DIR and PROJECT_SOURCE_DIR. If they are euqal, then xsimd is built as an individual project; otherwise it's built as a subproject of others.

If this feature is not implemented yet, I'm willing to do it myself and start a pr.

ToKiNoBug avatar Oct 18 '23 03:10 ToKiNoBug

Thanks for using xsimd :-) PR welcome!

serge-sans-paille avatar Nov 12 '23 11:11 serge-sans-paille

@ToKiNoBug I actually need to install xsimd alongside my project, because it's a library with xsimd needed in the headers. So I actually use the behaviour you wanna get rid of.

Do you think it could be possible to add an option to keep it ? Or should I rather use a cmake feature I'm not aware of ? (sorry for the question but I'm recently struggling a lot with modern cmake setup)

maxmarsc avatar Nov 12 '23 12:11 maxmarsc

@ToKiNoBug I actually need to install xsimd alongside my project, because it's a library with xsimd needed in the headers. So I actually use the behaviour you wanna get rid of.

Do you think it could be possible to add an option to keep it ? Or should I rather use a cmake feature I'm not aware of ? (sorry for the question but I'm recently struggling a lot with modern cmake setup)

Ofcourse, it' really easy to implement. But I'm not sure what the default behavior should be like. When used as subproject, should we skip installation by default?

ToKiNoBug avatar Nov 12 '23 14:11 ToKiNoBug

Do you think it could be possible to add an option to keep it ? Or should I rather use a cmake feature I'm not aware of ? (sorry for the question but I'm recently struggling a lot with modern cmake setup)

I have dig into this problem but I found indeed there is no such cmake feature. The installation behavior of subprojects can not be controlled by parent project, unless the subproject provide an option to do it.

ToKiNoBug avatar Nov 12 '23 14:11 ToKiNoBug

@ToKiNoBug I actually need to install xsimd alongside my project, because it's a library with xsimd needed in the headers. So I actually use the behaviour you wanna get rid of.

Do you think it could be possible to add an option to keep it ? Or should I rather use a cmake feature I'm not aware of ? (sorry for the question but I'm recently struggling a lot with modern cmake setup)

Ofcourse, it' really easy to implement. But I'm not sure what the default behavior should be like. When used as subproject, should we skip installation by default?

I would say this should depends on the PUBLIC / PRIVATE / INTERFACE dependency type you declared. Imo both public and interface should make sure your dependency is installed alongside your project, whereas private should not.

But I'm not sure this is the actual behaviour of cmake. In which cas are you ?

maxmarsc avatar Nov 12 '23 14:11 maxmarsc

I would say this should depends on the PUBLIC / PRIVATE / INTERFACE dependency type you declared. Imo both public and interface should make sure your dependency is installed alongside your project, whereas private should not.

But I'm not sure this is the actual behaviour of cmake. In which cas are you ?

Usually I use external projects privately, but this time I decide to leave the default behavior unchanged. I will add an option named XSIMD_SKIP_INSTALL, the default value is OFF. If anyone like me don't want to install xsimd files, just set this option to ON before introducing xsimd.

ToKiNoBug avatar Nov 12 '23 15:11 ToKiNoBug

I would say this should depends on the PUBLIC / PRIVATE / INTERFACE dependency type you declared. Imo both public and interface should make sure your dependency is installed alongside your project, whereas private should not. But I'm not sure this is the actual behaviour of cmake. In which cas are you ?

Usually I use external projects privately, but this time I decide to leave the default behavior unchanged. I will add an option named XSIMD_SKIP_INSTALL, the default value is OFF. If anyone like me don't want to install xsimd files, just set this option to ON before introducing xsimd.

I'm not sure to understand, in your target_link_libraries did you declared xsimd as a public, private or interface dependency ? (I'm curious)

maxmarsc avatar Nov 12 '23 15:11 maxmarsc

I'm not sure to understand, in your target_link_libraries did you declared xsimd as a public, private or interface dependency ? (I'm curious)

Yes, for examlpe, when distributing shared libs or executables.

ToKiNoBug avatar Nov 12 '23 16:11 ToKiNoBug

I'm not sure to understand, in your target_link_libraries did you declared xsimd as a public, private or interface dependency ? (I'm curious)

Yes, for examlpe, when distributing shared libs or executables.

Sorry, I was not clear. Which one of these (PUBLIC / PRIVATE / INTERFACE) are you using when you're having "unwanted" installation of xsimd ?

maxmarsc avatar Nov 12 '23 17:11 maxmarsc

I'm not sure to understand, in your target_link_libraries did you declared xsimd as a public, private or interface dependency ? (I'm curious)

Yes, for examlpe, when distributing shared libs or executables.

Sorry, I was not clear. Which one of these (PUBLIC / PRIVATE / INTERFACE) are you using when you're having "unwanted" installation of xsimd ?

PRIVATE. For me, xsimd headers are not required by my public headers. I am developing an application instead of library, so I only need to distribute executables and shared libs, there is no need to distribute internal headers to users.

ToKiNoBug avatar Nov 12 '23 18:11 ToKiNoBug

Ok, so I would also assume Cmake should not install xsimd alongside your binaries :thinking:

Might be interesting to ask more experienced folks on the CMake discourse forum about how they think we should handle this. I'll try to do so later in the week.

maxmarsc avatar Nov 12 '23 18:11 maxmarsc

I asked the question on CMake discourse, I'll let you know what they think

https://discourse.cmake.org/t/should-cmake-install-an-interface-library-when-linked-as-private-dependency/9449

maxmarsc avatar Nov 17 '23 15:11 maxmarsc

I asked the question on CMake discourse, I'll let you know what they think

https://discourse.cmake.org/t/should-cmake-install-an-interface-library-when-linked-as-private-dependency/9449

I look forward for official opinons, but I think maybe you have paid too much attention on linkage type or library type, they are not that important.

In my opinon, it is the type of software(app or lib) that decides whether a subproject should be installed. xsimd can never guess whether it should be installed, so we need an option to control.

However, the default behavior is still worth to be disgussed.

ToKiNoBug avatar Nov 17 '23 19:11 ToKiNoBug

Ok so I think I found an acceptable solution, which doesn't require modifying the xsimd library @ToKiNoBug

For short, with cmake <3.28 you’d use :

include(FetchContent)

FetchContent_Declare(
  xsimd
  GIT_REPOSITORY [email protected]:xtensor-stack/xsimd.git
  GIT_TAG 11.1.0
  GIT_SHALLOW ON
)

FetchContent_GetProperties(xsimd)
if(NOT xsimd_POPULATED)
  FetchContent_Populate(xsimd)
  add_subdirectory(${xsimd_SOURCE_DIR} ${xsimd_BINARY_DIR} EXCLUDE_FROM_ALL)
endif()

And with cmake >= 3.28 we’ll be able to use this syntax :

include(FetchContent)

FetchContent_Declare(
  xsimd
  GIT_REPOSITORY [email protected]:xtensor-stack/xsimd.git
  GIT_TAG 11.1.0
  GIT_SHALLOW ON
  # Other content options...
  EXCLUDE_FROM_ALL
)

FetchContent_MakeAvailable(xsimd)

If you want more details it all relies on the EXCLUDE_FROM_ALL flag of the FetchContent functions. You can check this StackOverflow post and the CMake Discourse post mentionned earlier

maxmarsc avatar Nov 23 '23 11:11 maxmarsc

Since #972 got merged, I think we can close this bug, @ToKiNoBug

serge-sans-paille avatar Apr 15 '24 06:04 serge-sans-paille