cmake: allow to disable install and additional targets

[jcelerier]

In case of building liblsl as part of another software project
(in my case https://github.com/ossia/score) the library will install lots of unneeded things)

[cboulay]

A couple things...

First, the edit to cmake/LSLCMake.cmake must be reverted. This file gets packaged with LSL for installation and can be used by other applications' build systems to assist with common config and install steps. I don't want these other applications' build systems to now require setting a variable just so they can call a function that is optional anyway.
I'm curious though, why did you identify this as something to change? Your other edits should have prevented this function from ever being called, no?

Second, is there no other way to tell cmake not to run the install targets on dependencies? If you are including the source tree then I get the problem, but if you're using find_package semantics then I wouldn't expect it to run any of the liblsl install targets.

[cboulay]

I had some wrong assumptions about how you were using liblsl. For most of the first party LSL applications, we actually treat liblsl as something that's expected to be built and installed already -- maybe on demand in a shell script -- then we link it and optionally bundle it in the package (depending on platform).

However, I see now that you have the repo as a submodule and include the entire liblsl project in your project, and build the static lib.
https://github.com/ossia/score-addon-lsl/blob/main/CMakeLists.txt

I also see that you have many other 3rd party dependencies using this pattern so you're unlikely to want to do things differently for this one library.

I'm not against having an option to disable installation. The name will probably be different, however, because we already have LSL_INSTALL_ROOT which is intended to be a hint to find_package about where to find the lsl install location if not in one of the standard places. I might prefer LSL_DISABLE_INSTALL (default to OFF) or similar.

ETA: We can probably make it so that the default behaviour is if the LSL project is not the root project then installation is disabled, or maybe 'not root project and build static'. Let me know if any of your other dependencies are a good exemplar with an elegant solution.

[jcelerier]

Heya!
Yes, my use case is with static linking directly to the library (especially as the app I develop has to be shipped as binary that can just be run from macos / windows / any linux distro, from people who won't have experience installing packages or dev dependencies).

I'm curious though, why did you identify this as something to change?

For the LSLCMake change, you're right, I added more for consistency for other users.

I'll change it to

if((DEFINED LSL_INSTALL) AND (NOT LSL_INSTALL))
	return()
endif()

so that there is no breaking change for users that get this from an installed version of liblsl.

Completely unrelated: I think that if you target more recent Qt (from 2-3 years ago) you can replace this with the Qt6 cmake deployment scripts: the following code will automatically run windeployqt / macdeployqt / linuxdeployqt

cmake_minimum_required(VERSION 3.16...3.22)
project(MyThings)

find_package(Qt6 REQUIRED COMPONENTS Core)
qt_standard_project_setup()

qt_add_executable(MyApp main.cpp)

install(TARGETS MyApp
    BUNDLE  DESTINATION .
    RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
)

qt_generate_deploy_app_script(
    TARGET MyApp
    OUTPUT_SCRIPT deploy_script
    NO_UNSUPPORTED_PLATFORM_ERROR
)
install(SCRIPT ${deploy_script})

Second, is there no other way to tell cmake not to run the install targets on dependencies? If you are including the source tree then I get the problem,

yes, that's my use case. There are a couple hacks to do this: https://stackoverflow.com/questions/35344132/cpack-exclude-install-commands-from-subdirectory-googletest-directory but I think it's cleaner to have a variable for explicit and specific control.

ETA: We can probably make it so that the default behaviour is if the LSL project is not the root project then installation is disabled, or maybe 'not root project and build static'. Let me know if any of your other dependencies are a good exemplar with an elegant solution.

that was my first idea but then I can imagine a few case where this would break existing users, which I'd like to avoid.
Do you have any suggestion for the name ? maybe LSL_ENABLE_INSTALL_TARGET to be more specific?

[jcelerier]

also, are the broken unit tests on windows the fault of this PR?

[cboulay]

also, are the broken unit tests on windows the fault of this PR?

No. We have some integration tests that push the limits of the system and fail about 80% of the time on the GHA windows-latest runners but never seem to fail on a normal PC. It's very hard to reproduce these segfaults and they don't happen in practice so we haven't been motivated to fix them. I'll just re-run the tests a few times until they pass.

[cboulay]

For the LSLCMake change, I still don't understand why someone would call that function and have LSL_ENABLE_INSTALL_TARGET defined and set to OFF. The installLSLApp function is entirely optional; you don't have to call it. Yes, we call it inside the liblsl project, but your other proposed methods of disabling installing the additional targets would prevent those calls. I think any 3rd party apps would probably similarly decide whether to call this function or not using its own internal options -- and not rely on a sub-project's option determining the behaviour of a top-level project's function call.

And yes, I've started to migrate to modern Qt install scripts. My plan is to get this working in AudioCapture and LabRecorder, then decide whether I need a helper function or if examples are sufficient. Unfortunately, it seems like there are too many 'gotchas' so I'm leaning toward keeping the to-be-updated helper function.

For example, for the AudioCapture app, here is the minimum that I think is required if not using a helper function:
https://github.com/labstreaminglayer/App-AudioCapture/blob/cboulay/apple_framework/CMakeLists.txt

• The Qt installer script skips non-Qt libraries on Windows (at least it does for liblsl for me).
• The Qt installer script skips the lsl.framework, and annoyingly also removes /Library/System from the app's RPATH so I can't even use system lsl.framework by default.
• Unsurprising, but the Qt install script also invalidates any signatures that Xcode might do so I disable signing to begin with and have a post-install signing step.

If the Qt installer could actually bundle lsl.dll / liblsl.so / lsl.framework properly then I'd be happy to skip over LSLCMake.cmake entirely.