Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update CMake build to define RUNPREFIX different than CMAKE_INSTALL_PREFIX for package building compatibility #980

Closed
amessina opened this issue Dec 2, 2024 · 5 comments
Assignees

Comments

@amessina
Copy link

amessina commented Dec 2, 2024

Is your feature request related to a problem? Please describe.
The CMake build assumes write access to CMAKE_INSTALL_PREFIX. Packages are typically built for redistribution and prepend DESTDIR to CMAKE_INSTALL_PREFIX. In that way, the package can be "installed" in a temporary buildroot while maintaining the runtime prefix of CMAKE_INSTALL_PREFIX after they are packaged and installed on other systems.

Describe the solution you'd like
The CMake system needs to be updated to install to ${DESTDIR}${CMAKE_INSTALL_PREFIX} during the build so it can be packaged properly.

Describe alternatives you've considered
I've tried setting CMAKE_INSTALL_PREFIX to a place in the builddir (/builddir/build/BUILD/mythtv-34.0-build/install) then moving that install location to /usr which fails at runtime (on another computer) as the runtime prefix is also set to /builddir/build/BUILD/mythtv-34.0-build/install and I get errors like the following as of course it should be looking in /usr/lib64/mythtv/plugins:

mythfrontend[1320]: No plugins directory /builddir/build/BUILD/mythtv-34.0-build/install/lib64/mythtv/plugins
@amessina
Copy link
Author

amessina commented Dec 6, 2024

I was able to "resolve" this challenge by updating the RUNPREFIX to /usr in mythtv/libs/libmythbase/CMakeLists.txt using the following patch. It would be helpful to have a CMake definition like MYTHTV_RUNPREFIX that could default to CMAKE_INSTALL_PREFIX if unset, that way MYTHTV_RUNPREFIX could be overridden (just like with the non-CMake build).

 mythtv/libs/libmythbase/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mythtv/libs/libmythbase/CMakeLists.txt b/mythtv/libs/libmythbase/CMakeLists.txt
index b5976ff5f1..3b652d18f4 100644
--- a/mythtv/libs/libmythbase/CMakeLists.txt
+++ b/mythtv/libs/libmythbase/CMakeLists.txt
@@ -266,7 +266,7 @@ set_source_files_properties(
   mythdirs.cpp
   PROPERTIES
     COMPILE_DEFINITIONS
-    "RUNPREFIX=\"${CMAKE_INSTALL_PREFIX}\";LIBDIRNAME=\"${CMAKE_INSTALL_LIBDIR}\""
+    "RUNPREFIX=\"/usr\";LIBDIRNAME=\"${CMAKE_INSTALL_LIBDIR}\""
 )
 target_include_directories(
   mythbase

@amessina amessina changed the title Update CMake build to install to DESTDIR for package building compatibility Update CMake build to define RUNPREFIX different than CMAKE_INSTALL_PREFIX for package building compatibility Dec 6, 2024
@linuxdude42
Copy link
Contributor

How about this? Does this give you the control you need?

index 1dfac3b601..ebf22af5ee 100644
--- a/cmake/MythOptions.cmake
+++ b/cmake/MythOptions.cmake
@@ -21,6 +21,11 @@ set(MYTH_DEFAULT_LIBS_PREFIX
     CACHE PATH
           "Default library install prefix if not specified on the command line."
 )
+set(MYTH_RUN_PREFIX
+    ""
+    CACHE PATH
+          "The prefix where MythTV is expected to be at runtime.  This may differ from MYTH_DEFAULT_PREFIX or CMAKE_INSTALL_PREFIX for packagers."
+)
 
 # Location for downloaded tarballs.
 #
diff --git a/mythtv/libs/libmythbase/CMakeLists.txt b/mythtv/libs/libmythbase/CMakeLists.txt
index c51e056175..5bde73d5b9 100644
--- a/mythtv/libs/libmythbase/CMakeLists.txt
+++ b/mythtv/libs/libmythbase/CMakeLists.txt
@@ -261,11 +261,15 @@ target_compile_definitions(mythbase PRIVATE MBASE_API)
 set_source_files_properties(mythcdrom.cpp PROPERTIES COMPILE_DEFINITIONS
                                                      HAVE_LIBUDFREAD)
 
+if(NOT MYTH_RUN_PREFIX)
+  message(VERBOSE "Setting MYTH_RUN_PREFIX to ${CMAKE_INSTALL_PREFIX}")
+  set(MYTH_RUN_PREFIX ${CMAKE_INSTALL_PREFIX})
+endif()
 set_source_files_properties(
   mythdirs.cpp
   PROPERTIES
     COMPILE_DEFINITIONS
-    "RUNPREFIX=\"${CMAKE_INSTALL_PREFIX}\";LIBDIRNAME=\"${CMAKE_INSTALL_LIBDIR}\""
+    "RUNPREFIX=\"${MYTH_RUN_PREFIX}\";LIBDIRNAME=\"${CMAKE_INSTALL_LIBDIR}\""
 )
 target_include_directories(
   mythbase

@linuxdude42 linuxdude42 self-assigned this Dec 23, 2024
@amessina
Copy link
Author

How about this? Does this give you the control you need?

index 1dfac3b601..ebf22af5ee 100644
--- a/cmake/MythOptions.cmake
+++ b/cmake/MythOptions.cmake
@@ -21,6 +21,11 @@ set(MYTH_DEFAULT_LIBS_PREFIX
     CACHE PATH
           "Default library install prefix if not specified on the command line."
 )
+set(MYTH_RUN_PREFIX
+    ""
+    CACHE PATH
+          "The prefix where MythTV is expected to be at runtime.  This may differ from MYTH_DEFAULT_PREFIX or CMAKE_INSTALL_PREFIX for packagers."
+)
 
 # Location for downloaded tarballs.
 #
diff --git a/mythtv/libs/libmythbase/CMakeLists.txt b/mythtv/libs/libmythbase/CMakeLists.txt
index c51e056175..5bde73d5b9 100644
--- a/mythtv/libs/libmythbase/CMakeLists.txt
+++ b/mythtv/libs/libmythbase/CMakeLists.txt
@@ -261,11 +261,15 @@ target_compile_definitions(mythbase PRIVATE MBASE_API)
 set_source_files_properties(mythcdrom.cpp PROPERTIES COMPILE_DEFINITIONS
                                                      HAVE_LIBUDFREAD)
 
+if(NOT MYTH_RUN_PREFIX)
+  message(VERBOSE "Setting MYTH_RUN_PREFIX to ${CMAKE_INSTALL_PREFIX}")
+  set(MYTH_RUN_PREFIX ${CMAKE_INSTALL_PREFIX})
+endif()
 set_source_files_properties(
   mythdirs.cpp
   PROPERTIES
     COMPILE_DEFINITIONS
-    "RUNPREFIX=\"${CMAKE_INSTALL_PREFIX}\";LIBDIRNAME=\"${CMAKE_INSTALL_LIBDIR}\""
+    "RUNPREFIX=\"${MYTH_RUN_PREFIX}\";LIBDIRNAME=\"${CMAKE_INSTALL_LIBDIR}\""
 )
 target_include_directories(
   mythbase

I think that would work, though I'll need to wait until this weekend to try it. The following also references the resulting run prefix so that may be another place to use MYTH_RUN_PREFIX when set.

message("runtime prefix ${CMAKE_INSTALL_PREFIX}")

linuxdude42 added a commit that referenced this issue Dec 23, 2024
The code previously always assigned CMAKE_INSTALL_PREFIX to RUNPREFIX,
the location that the mythfrontend believes is its location.  Add a
new option, MYTH_RUN_PREFIX, allowing this variable to be specified
when the build tree is created.  This is analogous to the configure
--runprefix option.

Refs: #980
@linuxdude42
Copy link
Contributor

Let me know how the testing goes. I'll leave the issue open for now in case you need further tweaks.

@amessina
Copy link
Author

Let me know how the testing goes. I'll leave the issue open for now in case you need further tweaks.

I was able to rebuild with 31ea30c atop fixes/34 and the patch works beautifully. If this qualifies for backport, I'd appreciate it.

Thank you @linuxdude42.

linuxdude42 added a commit that referenced this issue Dec 25, 2024
The code previously always assigned CMAKE_INSTALL_PREFIX to RUNPREFIX,
the location that the mythfrontend believes is its location.  Add a
new option, MYTH_RUN_PREFIX, allowing this variable to be specified
when the build tree is created.  This is analogous to the configure
--runprefix option.

Refs: #980
(cherry picked from commit 31ea30c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants