CatxFish / obs-v4l2sink

obs studio output plugin for Video4Linux2 device
GNU General Public License v2.0
930 stars 99 forks source link

cmake: Use proper variables for install paths #26

Closed rossmeier closed 4 years ago

rossmeier commented 4 years ago

Hello,

I am trying to create an rpm package for this. On some system, using ${CMAKE_INSTALL_PREFIX} is not enough (eg Fedora has libs for x86_64 in /usr/lib64). Therefore use ${SHARE_INSTALL_PREFIX} and ${LIB_INSTALL_DIR} specifically.

displague commented 4 years ago

Could this be the root cause of #14?

rossmeier commented 4 years ago

pretty sure. However, apparently some versions of Debian expect obs libraries in hardcoded /usr/lib instead of the correct lib prefix (see also https://github.com/fzwoch/obs-gnome-screencast/pull/13#issuecomment-606622989). Fixing this would be the packages job of the affected OSs (and should be easily possible with this PR)

marcusmueller commented 4 years ago

I agree with the intention (the installation directories aren't right many distros), but this is not the way to go: it breaks if you specify CMAKE_INSTALL_PREFIX; I don't even know where your SHARE_INSTALL_PREFIX would come from!

So, while this seems to fix it for you (probably because you set these externally somehow? They are not standard CMake variables), this is a change for the worse for the rest of us, and it'd at least be so surprising that it would break any default CMake build and install routine in the Fedora and Debian packaging macros.

Fixing this would be the packages job of the affected OSs

Um, and that would be an unnecessary regression. The whole point of having CMake, autoconf, Scons et al. is that the user (in this case: packager) sets as little paths manually as possible; that would be CMAKE_INSTALL_PREFIX, if anything, and the rest should be detected. Rest of the world does that, we can do that, too. :) Maybe I'll fix that later today.

rossmeier commented 4 years ago

Those are the variables set by the rpm cmake macro. I just assumed, they were standard or at least de-facto standard for packaging

jktjkt commented 4 years ago

@veecue , can you take a look at #35? I think that using a standard CMake's GNUInstallDirs is a cleaner fix.

rossmeier commented 4 years ago

Closing in favor of @jktjkt more general fix.