LSLConfig.cmake forces default install folder to be ${CMAKE_BINARY_DIR}/install

[acompagn]

Hi,

I just faced an issue with the LSLConfig which always includes the LCLCmake.cmake which in turn forces the CMAKE_INSTALL_PREFIX to point to ${CMAKE_BINARY_DIR}/install when no values are passed to it. This behavior reflects on my project too, which is importing LSL using the find_package utility of cmake and thus the default installation folder becomes ${CMAKE_BINARY_DIR}/install for my project too.

Would it be possible to leave the CMAKE_INSTALL_PREFIX pointing to its default value when no other values are assigned to it?

[tstenner]

The default install dirs (e.g. C:\\Program Files\\… on Windows, /usr/local on Linux) are typically not writable by regular users and even (or rather especially) if they run CMake as admin, ${CMAKE_BINARY_DIR}/install is the better choice for 99% of users. Also, the remaining 1% of users (e.g. you) are more likely to have an alternate install dir and/or correctly diagnose the issue so I'd rather have occasionally explain why it's in there than to find out the default install dir isn't writable after remote diagnosing the issue each time someone doesn't set an install prefix.

The easiest way to skip this would be a tiny change in:

if(CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT)
    set(CMAKE_INSTALL_PREFIX "${CMAKE_BINARY_DIR}/install" CACHE PATH
        "Where to put redistributable binaries" FORCE)
    message(WARNING "CMAKE_INSTALL_PREFIX default initialized to ${CMAKE_INSTALL_PREFIX}")
endif()

to if(CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT AND NOT LSL_KEEP_INSTALL_PREFIX) so you could just set LSL_KEEP_INSTALL_PREFIX and be done with it.

[acompagn]

Thank you for your explanation. My 2 cents on this would be to avoid to change the default behavior unless really necessary as it would have side effects, as in my case. IMHO having the install folder in the compilation folder is not an ideal choice for at least two reasons: you already have the output of your compilation in there, so probably with a different design of the CMakeList you could achieve the same behavior you currently have (i.e., everything packed in the same folder), the compilation folder is most probably not in any PATH of your system, with the direct consequence that if you need to link against liblsl, you need to include the CMAKE_BINARY_DIR in your INCLUDE_PATH (similar if you need to run an executable that linked liblsl, to mention just a couple of examples). Besides that I doubt that your 99% of users find this a straightforward action to do, I don't think it reflects the meaning of "installation".

[tstenner]

My 2 cents on this would be to avoid to change the default behavior unless really necessary

The build folder isn't optimal, but in almost all cases it's better than a system location the user can't write to and the build artifacts are either supposed to be put into a package with CPack (so the install location doesn't matter) or the user wants just the liblsl.so, e.g. for the Python bindings. An other default location would be possible, but "install"ing into the build directory makes it a lot easier to work with separate build directories at once (i.e. what Visual Studio does by default and what I do with separate Clang+Debug and GCC+Release build folders).

the compilation folder is most probably not in any PATH of your system, with the direct consequence that if you need to link against liblsl, you need to include the CMAKE_BINARY_DIR in your INCLUDE_PATH (similar if you need to run an executable that linked liblsl, to mention just a couple of examples). Besides that I doubt that your 99% of users find this a straightforward action to do, I don't think it reflects the meaning of "installation".

Most end users get liblsl with an app (e.g. LabRecorder or bundled in the vendor acquisition software), but we want to encourage even those who build software with liblsl to download a precompiled package (and for those who want to build it themselves, I prefer the explicit approach with -DLSL_INSTALL_ROOT).

Also, in almost all labs the PCs used for development are not those where liblsl is used productively so CMake isn't (and shouldn't) be used to "install" a library in the traditional sense; even on Debian/Ubuntu where we're most likely to have a dev environment present we prefer the .deb packages (which get found by find_package automatically) and for Windows we could build an .msi installer without too much effort.

In theory, users could copy the liblsl library from the build folder to the folder they need, but then install() won't fix up the RPATH.

I've pushed a small adjustment (2a0f6ba23581dde6415cd4eeea866aefaa30e638) to leave the CMAKE_INSTALL_PREFIX untouched if LSL_PRESERVE_INSTALL_PREFIX is set, so starting with the next release it's possible to install to the default path if needed.

[cboulay]

This came up today in conversation with a user who was trying to make an Arch Linux distribution.

With more and more Linux users, I'm thinking that we're now at or past the point where there is sufficient benefit to having two different distribution workflows that it outweighs the cost in maintaining disparate workflows. (1) On Linux and Mac, liblsl should be installed system wide then apps and wrappers (e.g., pylsl) are expected to find the system-level liblsl; (2) On Windows it stays mostly as it is now, where liblsl is shipped with each application and we discourage adding any liblsl64.dll / lsl.dll to the PATH.

That means that many of our cmake hacks to simplify user builds will need to be wrapped in if(WIN32) guards.

[cboulay]

Regarding this issue specifically, I'm of the opinion that we should move the set(CMAKE_INSTALL_PREFIX ... command up to liblsl/CMakeLists.txt, and guard it in if(CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT AND WIN32).

Then applications that find_package(LSL...) will no longer have the install prefix overridden, so we'll have to add a similar section (without the WIN32 criterion?) to the template app and other apps that we don't want to install at the system level. This is a bit tedious, but I think it's worth it because we shouldn't be accidentally setting install paths for integrators that want to use liblsl without manual CMAKE_INSTALL_PREFIX overrides.

[acompagn]

For what it's worth I support @cboulay proposal.

[tstenner]

Regarding this issue specifically, I'm of the opinion that we should move the set(CMAKE_INSTALL_PREFIX ... command up to liblsl/CMakeLists.txt, and guard it in if(CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT AND WIN32).

This wouldn't update the default install path for all LSL apps, only for liblsl. There are some quality of life adjustments in the LSLCMake script that are helpful to most app developers, but they can hinder distributors. OTOH, I'm surprised we've got interesting to distributors and I guess they are better at distributing binaries than we are so I could live with an opt-in solution (e.g. an additional option LSL_COMFY_DEFAULTS) that enables them.

[tstenner]

Changed in 31c187c0579ba430eac22fc54ccc057f09d0e6a7.