intel / libvpl

Intel® Video Processing Library (Intel® VPL) API, dispatcher, and examples
https://intel.github.io/libvpl/
MIT License
262 stars 80 forks source link

Configuration installing to (prefix)/etc/ when prefix is used #94

Closed AdamWill closed 11 months ago

AdamWill commented 1 year ago

When an install prefix is used (which is typical for installation on Linux), the configuration files wind up in (prefix)/etc . So in the typical case of CMAKE_INSTALL_PREFIX being /usr , we wind up with:

/usr/etc/modulefiles
/usr/etc/modulefiles/vpl
/usr/etc/vpl
/usr/etc/vpl/vars.sh

I don't think this is correct or intended, but I don't know what the 'right' way to fix it would be.

CMake's documented special handling here only covers "(dir) equal to SYSCONFDIR, LOCALSTATEDIR or RUNSTATEDIR". What we do is this:

if(NOT ONEAPI_INSTALL_ENVDIR)
  set(ONEAPI_INSTALL_ENVDIR ${CMAKE_INSTALL_SYSCONFDIR}/${PROJECT_NAME})
endif()

if(NOT ONEAPI_INSTALL_MODFILEDIR)
  set(ONEAPI_INSTALL_MODFILEDIR ${CMAKE_INSTALL_SYSCONFDIR}/modulefiles)
endif()

and then we install things to those directories, e.g.:

  install(
    PROGRAMS vpl
    DESTINATION ${ONEAPI_INSTALL_MODFILEDIR}
    COMPONENT dev)

which, I think, results in cmake expanding ONEAPI_INSTALL_MODFILEDIR , since it's a relative path, and including the prefix, so we wind up with /usr/etc/modulefiles.

I did come up with a bit of an ugly hack, which is to explicitly set ONEAPI_INSTALL_FULL_ENVDIR and ONEAPI_INSTALL_FULL_MODFILEDIR in oneAPIInstallDirs.cmake and then install the files to the _FULL_ paths not the relative ones. But that also requires adding extra stuff in the if(USE_ONEAPI_INSTALL_LAYOUT) branch and just generally feels kinda ugly. This seems like the kind of thing where you'd expect it to be something that's commonly encountered and a solved problem already, but I couldn't find anything for it, weirdly.

AdamWill commented 1 year ago

Well, that workaround was silly, so here's another. This "fixes" it, but still feels icky, so I'm not sending it as a PR...

From 89068d21b5a24bd160b3487b8d8c8f05482ee1f9 Mon Sep 17 00:00:00 2001
From: Adam Williamson <awilliam@redhat.com>
Date: Mon, 13 Mar 2023 16:32:31 -0700
Subject: [PATCH] Fix config install destinations when prefix is /usr (#94)

When CMAKE_INSTALL_PREFIX is /usr , we wind up installing config
files to /usr/etc , which is wrong (and breaks ostree composes).
This hacks around that.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
---
 cmake/oneAPIInstallDirs.cmake | 7 +++++++
 env/CMakeLists.txt            | 2 +-
 modulefiles/CMakeLists.txt    | 2 +-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/cmake/oneAPIInstallDirs.cmake b/cmake/oneAPIInstallDirs.cmake
index c6ab7d7..1a04675 100644
--- a/cmake/oneAPIInstallDirs.cmake
+++ b/cmake/oneAPIInstallDirs.cmake
@@ -95,6 +95,13 @@ foreach(
                                           ONEAPI_INSTALL_${dir} ${dir})
 endforeach()

+# GNUInstallDirs_get_absolute_install_dir gets this case wrong
+# https://github.com/oneapi-src/oneVPL/issues/94
+if(CMAKE_INSTALL_PREFIX STREQUAL "/usr")
+  set(ONEAPI_INSTALL_FULL_ENVDIR /${ONEAPI_INSTALL_ENVDIR})
+  set(ONEAPI_INSTALL_FULL_MODFILEDIR /${ONEAPI_INSTALL_MODFILEDIR})
+endif()
+
 if(WIN32)
   set(CMAKE_INSTALL_SYSTEM_RUNTIME_DESTINATION ${CMAKE_INSTALL_BINDIR})
 else()
diff --git a/env/CMakeLists.txt b/env/CMakeLists.txt
index 4ea8ab2..cc75873 100644
--- a/env/CMakeLists.txt
+++ b/env/CMakeLists.txt
@@ -44,6 +44,6 @@ else()
   configure_file("sh/vars.sh.in" "${ENV_SCRIPT_NAME}" @ONLY)
   install(
     PROGRAMS "${CMAKE_CURRENT_BINARY_DIR}/${ENV_SCRIPT_NAME}"
-    DESTINATION "${ONEAPI_INSTALL_ENVDIR}"
+    DESTINATION "${ONEAPI_INSTALL_FULL_ENVDIR}"
     COMPONENT dev)
 endif()
diff --git a/modulefiles/CMakeLists.txt b/modulefiles/CMakeLists.txt
index 4094662..e9b5076 100644
--- a/modulefiles/CMakeLists.txt
+++ b/modulefiles/CMakeLists.txt
@@ -8,6 +8,6 @@ cmake_minimum_required(VERSION 3.10.2)
 if(UNIX)
   install(
     PROGRAMS vpl
-    DESTINATION ${ONEAPI_INSTALL_MODFILEDIR}
+    DESTINATION ${ONEAPI_INSTALL_FULL_MODFILEDIR}
     COMPONENT dev)
 endif()
-- 
2.39.2
Conan-Kudo commented 1 year ago

This patch looks reasonably correct, since this project has its own derivative module that doesn't correctly handle the /etc exception.

mav-intel commented 11 months ago

Fixed in 69d6b59bbdfc092d02f45b1ee550d703e85e4b6e

scaronni commented 11 months ago

I think that commit is not enough, it's missing an adjustment on the paths in env/modulfiles:

diff -Naur oneVPL-2023.3.0.old/env/CMakeLists.txt oneVPL-2023.3.0/env/CMakeLists.txt
--- oneVPL-2023.3.0.old/env/CMakeLists.txt  2023-07-14 16:43:05.607855171 +0200
+++ oneVPL-2023.3.0/env/CMakeLists.txt  2023-07-14 16:44:32.808448799 +0200
@@ -44,6 +44,6 @@
   configure_file("sh/vars.sh.in" "${ENV_SCRIPT_NAME}" @ONLY)
   install(
     PROGRAMS "${CMAKE_CURRENT_BINARY_DIR}/${ENV_SCRIPT_NAME}"
-    DESTINATION "${VPL_INSTALL_ENVDIR}"
+    DESTINATION "${VPL_INSTALL_FULL_ENVDIR}"
     COMPONENT dev)
 endif()
diff -Naur oneVPL-2023.3.0.old/modulefiles/CMakeLists.txt oneVPL-2023.3.0/modulefiles/CMakeLists.txt
--- oneVPL-2023.3.0.old/modulefiles/CMakeLists.txt  2023-07-14 16:43:05.617855354 +0200
+++ oneVPL-2023.3.0/modulefiles/CMakeLists.txt  2023-07-14 16:43:47.542621545 +0200
@@ -8,6 +8,6 @@
 if(UNIX)
   install(
     PROGRAMS vpl
-    DESTINATION ${VPL_INSTALL_MODFILEDIR}
+    DESTINATION ${VPL_INSTALL_FULL_MODFILEDIR}
     COMPONENT dev)
 endif()