SpartanJ / efsw

efsw is a C++ cross-platform file system watcher and notifier.
Other
662 stars 101 forks source link

Enable install only if main project #149

Closed LucaCiucci closed 1 year ago

SpartanJ commented 1 year ago

Hi, sorry but I'm not versed in CMake at all. What does exactly this change? How it behaved and how it behaves with the change?

LucaCiucci commented 1 year ago

Sorry for the missing description.

Consider a dependent project that downloads and include this library, for example:

FetchContent_Declare(
    efsw
    GIT_REPOSITORY https://github.com/SpartanJ/efsw
)
FetchContent_MakeAvailable(efsw)

which might provide some install targets. When the dependant project is installed, it will also install esfw by default, but this might be an unwanted behaviour.

The lines:

set(ESFW_MAIN_PROJECT FALSE)
if ((CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR) AND (CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME))
  set(ESFW_MAIN_PROJECT TRUE)
endif()

check if this is the main project by comparing the top level project name (CMAKE_PROJECT_NAME) and the current project name (PROJECT_NAME that in this case is "efsw"), the same is done for the source dir.

At this point if this is the main project, we will enable all the installing stuff.

I'm now wondering if this is the best thing to do, maybe a better solution would be to just set the default value of (EFSW_INSTALL) to false when not the main project?

SpartanJ commented 1 year ago

I'm now wondering if this is the best thing to do, maybe a better solution would be to just set the default value of (EFSW_INSTALL) to false when not the main project?

I think that's the correct solution. The flag is there for that reason. I don't know how CMake dependencies work, you can't just set the flag from the main project avoiding any change to the efsw CMakeLists.txt file?

LucaCiucci commented 1 year ago

Yes, you can set the flag from the main project or set the option by hand, but disabling install by defualt when not the main project would make it easier to depend on efsw. I think a good example is presented in yaml-cpp:

option(YAML_CPP_INSTALL "Enable generation of yaml-cpp install targets" ${YAML_CPP_MAIN_PROJECT})
SpartanJ commented 1 year ago

Sounds reasonable, thanks.