CI: Fix so installation path on Debian

umireon commented 1 year ago

Description

The plugin binary will be installed into /usr/local/lib/obs-plugins but it must be /usr/lib/x86_64-linux-gnu/obs-plugins on Debian-based systems. The install prefix will include local when CPACK_SET_DESTDIR is ON and CPACK_SET_DESTDIR will be handled automatically by DEB generator according to the document so it should leave unset. https://cmake.org/cmake/help/latest/variable/CPACK_SET_DESTDIR.html CMAKE_INSTALL_LIBDIR won't include x86_64-linux-gnu when CMAKE_INSTALL_PREFIX is not /usr and CMAKE_INSTALL_PREFIX is /usr/local by default so we need to provide CMAKE_INSTALL_PREFIX=/usr during CMake configuration time. https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html

Motivation and Context

We have generated the deb from the new build system and distributed it but some Ubuntu users complained that this won't be recognized by OBS. We need to fix this problem.

How Has This Been Tested?

I tested this change on my local machine.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

[x] My code has been run through clang-format.
[x] I have read the contributing document.
[x] My code is not on the master branch.
[x] The code has been tested.
[x] All commit messages are properly formatted and commits squashed where appropriate.
[x] I have included updates to all appropriate documentation.

PatTheMav commented 1 year ago

The install prefix will include local when CPACK_SET_DESTDIR is ON and CPACK_SET_DESTDIR will be handled automatically by DEB generator according to the document so it should leave unset.

That is not correct - the documentation states that only if CPACK_SET_DESTDIR is enabled will the value of CMAKE_INSTALL_PREFIX be used when DESTDIR is provided.

This functionality has been a hard requirement for some Linux distributions, so removing it is not an option.

Explicitly specifying a default prefix I'd be fine with if that prefix is stable across all Linux distributions.

umireon commented 1 year ago

It revealed that CPACK_SET_DESTDIR is not related to the problem that the installation prefix is wrong in Ubuntu. Therefore, I removed the change of CPACK_SET_DESTDIR from this PR.

umireon commented 11 months ago

ping @PatTheMav

PatTheMav commented 11 months ago

I'm still waiting for a authoritative response to this question as I have no ideas if Linux distributions agree on this:

Explicitly specifying a default prefix I'd be fine with if that prefix is stable across all Linux distributions

umireon commented 11 months ago

Are you waiting for the response about the default prefix other than me?

PatTheMav commented 10 months ago

Are you waiting for the response about the default prefix other than me?

I'm waiting for an answer from Linux contributors as I'm not too familiar with the common practises on the platform. CMake defaults to /usr/local as the default prefix, making it seem as if any other prefix needs to be manually provided.

As far as I can tell it makes sense to use /usr/local by default, because you're building the project for local use and that's the correct prefix to install anything then.

This only changes when you build for packaging and in that case the prefix should be changed in the packaging script but probably also need to be adjustable (and correctly passed to cpack).

Just changing it across the board for local builds seems like it's just doctoring around the symptom.

PatTheMav commented 10 months ago

Discussed off-thread, it breaks common Unix convention ("only distribution-provided packages go in /usr/, local installs go into /usr/local") but it's the convention expected by OBS Studio.

While this might break systems like FreeBSD, the build script is intended for CI use and Ubuntu only, so it's a right fit. Any deviations from that need to use custom build scripts or manually configure the project accordingly.

obsproject / obs-plugintemplate