sccn / liblsl

C++ lsl library for multi-modal time-synched data transmission over the local network
Other
120 stars 70 forks source link

CPACK might be over-configured on Linux #182

Open cboulay opened 1 year ago

cboulay commented 1 year ago
  1. According to this, I think we aren't supposed to be using CPACK_INSTALL_PREFIX here and should instead be using CPACK_PACKAGING_INSTALL_PREFIX, if at all.
  2. It seems like we can drop that argument entirely (same as default) and similarly drop CPACK_SET_DESTDIR.

I came upon this because I noticed that LabRecorder{xxx}.deb installs to /usr/LabRecorder though we'd probably prefer it installs to /usr/bin/LabRecorder or even /usr/local/bin/LabRecorder. I don't have a preference for /usr/bin or /usr/local/bin, but it feels bad to pollute /usr directly.

Assigning @tstenner because you're still listed as the CPACK_DEBIAN_PACKAGE_MAINTAINER :P

cboulay commented 1 year ago

image

cboulay commented 1 year ago

After a quick read, I've updated my preference. I am fine with liblsl going in /usr/lib and /usr/include but I'd prefer if apps like LabRecorder were in /usr/local.

cboulay commented 1 year ago
(base) chad@chad-jammy:/usr/LabRecorder$ readelf -d LabRecorder

Dynamic section at offset 0x450e8 contains 35 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libQt6Widgets.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libQt6Network.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [liblsl.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libQt6Gui.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libQt6Core.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x000000000000001d (RUNPATH)            Library runpath: [$ORIGIN:$ORIGIN/../LSL/lib]

That last line seems strange to me. Is that a holdover from the in-tree builds? That should probably be excluded when packaging debs. But maybe it's harmless to have it there.

tstenner commented 1 year ago

Assigning @tstenner because you're still listed as the CPACK_DEBIAN_PACKAGE_MAINTAINER :P

Oh well...

  1. It seems like we can drop that argument entirely (same as default) and similarly drop CPACK_SET_DESTDIR.

Agreed, I don't even use CMake with make any more and as you said it's not needed any more.

I came upon this because I noticed that LabRecorder{xxx}.deb installs to /usr/LabRecorder though we'd probably prefer it installs to /usr/bin/LabRecorder or even /usr/local/bin/LabRecorder. I don't have a preference for /usr/bin or /usr/local/bin, but it feels bad to pollute /usr directly.

/usr/${PROJECT_NAME} is far from ideal, but only chosen by default when LSL_UNIXFOLDERS is disabled. With this option, everything is as it should be:

$ cmake -DLSL_UNIXFOLDERS=ON . && cpack -G DEB && dpkg -c *.deb
drwxr-xr-x root/root         0 2022-12-28 13:06 ./usr/bin/
-rwxr-xr-x root/root    515472 2022-12-28 13:06 ./usr/bin/LabRecorder
-rwxr-xr-x root/root    264520 2022-12-28 13:06 ./usr/bin/LabRecorderCLI

Creating a Debian package without LSL_UNIXFOLDERS set or the install prefix set to /opt should abort, but I don't know where to do this at the moment.

After a quick read, I've updated my preference. I am fine with liblsl going in /usr/lib and /usr/include but I'd prefer if apps like LabRecorder were in /usr/local.

IMHO: /usr/local is for manually installed programs where removal is left to the user. With a proper package, the package manager can remove it so it doesn't matter where it is. One problem with both paths are the auxiliary files that get bundled alongside the binaries in /usr/bin (e.g. the license and default configs). These should be in /usr/share/${PROJECT_NAME}, but each program needs to search there (e.g. for the default config).

tstenner commented 1 year ago

With [QStandardPaths::AppDataLocation|https://doc.qt.io/qt-6/qstandardpaths.html#StandardLocation-enum] Qt will search in ~/.local/share/<APPNAME>, /usr/local/share/<APPNAME> and /usr/share/<APPNAME>. I have added it to the LabRecorder config search path, but the default config is still installed to /usr/bin. I'll push a PR to address this...