AcademySoftwareFoundation / OpenColorIO

A color management framework for visual effects and animation.
https://opencolorio.org
BSD 3-Clause "New" or "Revised" License
1.74k stars 430 forks source link

Add cmake variable OCIO_INSTALL_OCIO #1957

Closed lgritz closed 3 months ago

lgritz commented 3 months ago

Defaults to ON.

This guards all of the install() commands, so if this variable is OFF, no components of OCIO or its dependencies will be installed.

This is helpful when OCIO is not the "top level project", i.e. some other project is building it as an unexposed static dependency and doesn't want the OCIO components to be placed as artifacts in the outer project's install.

lgritz commented 3 months ago

If you approve this and merge it, I would also appreciate a backport into any future 2.{1,2,3} releases that you may do. (Or at least future 2.3.x's.)

remia commented 3 months ago

I'm not sure how you use OpenColorIO exactly in this context but it seems like the EXCLUDE_FROM_ALL parameter from add_subdirectory() or FetchContent_Declare() could do the trick here as well? In any case it seems fine to add the guard variable if the suggested methods do not work as expected or require a newer CMake version.

lgritz commented 3 months ago

I will try the "EXCLUDE_FROM_ALL" and report back.

lgritz commented 3 months ago

I think EXCLUDE_FROM_ALL does what it says -- it excludes from the "all" target, but not from the "install" target. I think I still want a way to install the outer project but have the inner project (in this case, OCIO) NOT install, because I'm already trying to build OCIO statically and want nothing to show up in the install area.

Am I doing this wrong?

lgritz commented 3 months ago

@doug-walker I think it's running OCIO's install process because it's running the install process overall, and OCIO adds install() commands to be run. Do I misunderstand?

remia commented 3 months ago

@lgritz I tried EXCLUDE_FROM_ALL in a toy project (from cmake-consumer in OCIO tests) like so:

...
set(BUILD_SHARED_LIBS OFF)
set(OCIO_BUILD_TESTS OFF)
add_subdirectory(OpenColorIO EXCLUDE_FROM_ALL)
...
install(TARGETS consumer)

Then built with the command: cmake --build . --target install

It only installed the consumer executable in my custom installation directory.

That being said, if you tried it with your build and it didn't work, we must be doing something different and this doesn't work in all cases, so I'm going to approve the PR.

lgritz commented 3 months ago

OK, wait, you are right. I was using EXCLUDE_FROM_ALL wrong! It takes an argument, I was assuming it was a single keyword. When I follow it with YES, it all works the right way.

I will close/withdraw this PR. I now believe it is unnecessary for my use case.

lgritz commented 3 months ago

By the way, I'm experimenting with CPM.cmake, which is a wrapper around FetchContent for cmake based projects that pretty much reduces the whole dependency inclusion process (functionality akin to OCIO's share/cmake/modules/install/foo.cmake files) with a single command per dependency. It's so simple I think there must be a catch, but I haven't found it yet.

remia commented 3 months ago

Ah, in add_subdirectory() it's only the keyword and no need for a boolean, that's unfortunate API design.

Thanks for the mention of CPM, I think we should definitively have a look for OCIO as the dependency management code is quite complex at the moment.