Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature request] Add an option to skip installation #959

Closed
ToKiNoBug opened this issue Oct 18, 2023 · 15 comments
Closed

[Feature request] Add an option to skip installation #959

ToKiNoBug opened this issue Oct 18, 2023 · 15 comments

Comments

@ToKiNoBug
Copy link
Contributor

ToKiNoBug commented Oct 18, 2023

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.

@serge-sans-paille
Copy link
Contributor

Thanks for using xsimd :-) PR welcome!

@maxmarsc
Copy link
Contributor

@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)

@ToKiNoBug
Copy link
Contributor Author

@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
Copy link
Contributor Author

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.

@maxmarsc
Copy link
Contributor

@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 ?

@ToKiNoBug
Copy link
Contributor Author

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.

@maxmarsc
Copy link
Contributor

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)

@ToKiNoBug
Copy link
Contributor Author

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.

@maxmarsc
Copy link
Contributor

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 ?

@ToKiNoBug
Copy link
Contributor Author

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.

@maxmarsc
Copy link
Contributor

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

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
Copy link
Contributor

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

@ToKiNoBug
Copy link
Contributor Author

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.

@maxmarsc
Copy link
Contributor

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 git@github.com: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 git@github.com: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

@ToKiNoBug ToKiNoBug changed the title [Feature request] Skip installation when introduced with FetchContent [Feature request] Add an option to skip installation Nov 25, 2023
@serge-sans-paille
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants