From 81cecbd6475b9e34b5519d909fffd9ec73a864a1 Mon Sep 17 00:00:00 2001 From: Cary Phillips Date: Thu, 1 Feb 2024 14:40:59 -0800 Subject: [PATCH 1/5] Add uninstall target Satisfy the OpenSSF Best Practices Badge requirement for an insta/uninstall process: https://www.bestpractices.dev/en/criteria/1#1.installation_common CMake does not support a standard "uninstall" target, but the community recommends implementing an "uninstal" target that remove files named in the `install_manifest.txt`: https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake However, our existing process of installing the symlink to the "bare" library, i.e. the symlink from libImath-3_2.so to libImath.so, fails to add the symlink to the manifest, so "make uninstall" misses the symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))". This changes that to use a simpler "file(CREATE_LINK)" and "install(FILES)" to accomplish the same thing while also registering the symlink the the manifest. Also, this fixes an issue where `OpenEXRConfig.h` was passed to `install()` twice, producing two entries in `install_manifest.txt`. Signed-off-by: Cary Phillips --- CMakeLists.txt | 11 +++++++++++ cmake/CMakeLists.txt | 9 --------- cmake/LibraryDefine.cmake | 3 ++- cmake/cmake_uninstall.cmake.in | 23 +++++++++++++++++++++++ 4 files changed, 36 insertions(+), 10 deletions(-) create mode 100644 cmake/cmake_uninstall.cmake.in diff --git a/CMakeLists.txt b/CMakeLists.txt index 5a97330051..2862560bcd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -52,6 +52,17 @@ if(OPENEXR_INSTALL_TOOLS AND NOT OPENEXR_INSTALL) message(SEND_ERROR "OPENEXR_INSTALL_TOOLS requires OPENEXR_INSTALL") endif() +# uninstall target +if(NOT TARGET uninstall) + configure_file( + "${CMAKE_CURRENT_SOURCE_DIR}/cmake/cmake_uninstall.cmake.in" + "${CMAKE_CURRENT_BINARY_DIR}/cmake_uninstall.cmake" + IMMEDIATE @ONLY) + + add_custom_target(uninstall + COMMAND ${CMAKE_COMMAND} -P ${CMAKE_CURRENT_BINARY_DIR}/cmake_uninstall.cmake) +endif() + include(cmake/LibraryDefine.cmake) include(cmake/OpenEXRSetup.cmake) add_subdirectory(cmake) diff --git a/cmake/CMakeLists.txt b/cmake/CMakeLists.txt index 3a4dbf991e..9348702611 100644 --- a/cmake/CMakeLists.txt +++ b/cmake/CMakeLists.txt @@ -74,15 +74,6 @@ configure_file(OpenEXRConfigInternal.h.in ${CMAKE_CURRENT_BINARY_DIR}/OpenEXRCon # make a temp copy in the binary dir for OpenEXRConfig.h to include configure_file(../src/lib/OpenEXRCore/openexr_version.h ${CMAKE_CURRENT_BINARY_DIR}/OpenEXRCore/openexr_version.h COPYONLY) -if(OPENEXR_INSTALL) -install( - FILES - ${CMAKE_CURRENT_BINARY_DIR}/OpenEXRConfig.h - DESTINATION - ${CMAKE_INSTALL_INCLUDEDIR}/${OPENEXR_OUTPUT_SUBDIR} -) -endif() - ################################################### ####### IexConfig.h and IexConfigInternal.h diff --git a/cmake/LibraryDefine.cmake b/cmake/LibraryDefine.cmake index a242ec8519..0127480bbe 100644 --- a/cmake/LibraryDefine.cmake +++ b/cmake/LibraryDefine.cmake @@ -93,7 +93,8 @@ function(OPENEXR_DEFINE_LIBRARY libname) string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE) set(verlibname ${CMAKE_SHARED_LIBRARY_PREFIX}${libname}${OPENEXR_LIB_SUFFIX}${CMAKE_${uppercase_CMAKE_BUILD_TYPE}_POSTFIX}${CMAKE_SHARED_LIBRARY_SUFFIX}) set(baselibname ${CMAKE_SHARED_LIBRARY_PREFIX}${libname}${CMAKE_${uppercase_CMAKE_BUILD_TYPE}_POSTFIX}${CMAKE_SHARED_LIBRARY_SUFFIX}) - install(CODE "execute_process(COMMAND ${CMAKE_COMMAND} -E chdir \"\$ENV\{DESTDIR\}${CMAKE_INSTALL_FULL_LIBDIR}\" ${CMAKE_COMMAND} -E create_symlink ${verlibname} ${baselibname})") + file(CREATE_LINK ${verlibname} ${baselibname} SYMBOLIC) + install(FILES ${CMAKE_BINARY_DIR}/${baselibname} DESTINATION ${CMAKE_INSTALL_FULL_LIBDIR}) install(CODE "message(STATUS \"Creating symlink ${CMAKE_INSTALL_FULL_LIBDIR}/${baselibname} -> ${verlibname}\")") set(verlibname) set(baselibname) diff --git a/cmake/cmake_uninstall.cmake.in b/cmake/cmake_uninstall.cmake.in new file mode 100644 index 0000000000..1e5d2bb876 --- /dev/null +++ b/cmake/cmake_uninstall.cmake.in @@ -0,0 +1,23 @@ +# Source: https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake + +if(NOT EXISTS "@CMAKE_BINARY_DIR@/install_manifest.txt") + message(FATAL_ERROR "Cannot find install manifest: @CMAKE_BINARY_DIR@/install_manifest.txt") +endif() + +file(READ "@CMAKE_BINARY_DIR@/install_manifest.txt" files) +string(REGEX REPLACE "\n" ";" files "${files}") +foreach(file ${files}) + message(STATUS "Uninstalling $ENV{DESTDIR}${file}") + if(IS_SYMLINK "$ENV{DESTDIR}${file}" OR EXISTS "$ENV{DESTDIR}${file}") + exec_program( + "@CMAKE_COMMAND@" ARGS + "-E remove \"$ENV{DESTDIR}${file}\"" + OUTPUT_VARIABLE rm_out + RETURN_VALUE rm_retval) + if(NOT "${rm_retval}" STREQUAL 0) + message(FATAL_ERROR "Problem when removing $ENV{DESTDIR}${file}") + endif() + else(IS_SYMLINK "$ENV{DESTDIR}${file}" OR EXISTS "$ENV{DESTDIR}${file}") + message(STATUS "File $ENV{DESTDIR}${file} does not exist.") + endif() +endforeach() From 3e57c10e931ffb4330eb5c56b28aeef90965098d Mon Sep 17 00:00:00 2001 From: Cary Phillips Date: Thu, 1 Feb 2024 15:04:15 -0800 Subject: [PATCH 2/5] mention uninstall in install instructions Signed-off-by: Cary Phillips --- website/install.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/website/install.rst b/website/install.rst index 7f1d8aee27..6d50030769 100644 --- a/website/install.rst +++ b/website/install.rst @@ -277,6 +277,22 @@ You can customize these options three ways: 2. Use the UI ``cmake-gui`` or ``ccmake``. 3. Specify them as command-line arguments when you invoke cmake. +Uninstall +~~~~~~~~~ + +If you have installed from source, you can uninstall via: + +.. code-block:: + + % cmake --build $builddir --target uninstall + +or if using ``make``: + +.. code-block:: + + % make uninstall + + Library Naming Options ~~~~~~~~~~~~~~~~~~~~~~ From 59f6f4075bc1ae96a2915f144e42f9601569fc80 Mon Sep 17 00:00:00 2001 From: Cary Phillips Date: Thu, 1 Feb 2024 15:07:03 -0800 Subject: [PATCH 3/5] poke Signed-off-by: Cary Phillips --- CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2862560bcd..01cdb05b37 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -58,7 +58,6 @@ if(NOT TARGET uninstall) "${CMAKE_CURRENT_SOURCE_DIR}/cmake/cmake_uninstall.cmake.in" "${CMAKE_CURRENT_BINARY_DIR}/cmake_uninstall.cmake" IMMEDIATE @ONLY) - add_custom_target(uninstall COMMAND ${CMAKE_COMMAND} -P ${CMAKE_CURRENT_BINARY_DIR}/cmake_uninstall.cmake) endif() From 73d64d706f072cec4ff99bec635bb946f7bf93fc Mon Sep 17 00:00:00 2001 From: Cary Phillips Date: Thu, 8 Feb 2024 16:44:00 -0800 Subject: [PATCH 4/5] COPY_ON_ERROR Signed-off-by: Cary Phillips --- cmake/LibraryDefine.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/LibraryDefine.cmake b/cmake/LibraryDefine.cmake index 0127480bbe..3b273b07f0 100644 --- a/cmake/LibraryDefine.cmake +++ b/cmake/LibraryDefine.cmake @@ -93,7 +93,7 @@ function(OPENEXR_DEFINE_LIBRARY libname) string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE) set(verlibname ${CMAKE_SHARED_LIBRARY_PREFIX}${libname}${OPENEXR_LIB_SUFFIX}${CMAKE_${uppercase_CMAKE_BUILD_TYPE}_POSTFIX}${CMAKE_SHARED_LIBRARY_SUFFIX}) set(baselibname ${CMAKE_SHARED_LIBRARY_PREFIX}${libname}${CMAKE_${uppercase_CMAKE_BUILD_TYPE}_POSTFIX}${CMAKE_SHARED_LIBRARY_SUFFIX}) - file(CREATE_LINK ${verlibname} ${baselibname} SYMBOLIC) + file(CREATE_LINK ${verlibname} ${baselibname} COPY_ON_ERROR SYMBOLIC) install(FILES ${CMAKE_BINARY_DIR}/${baselibname} DESTINATION ${CMAKE_INSTALL_FULL_LIBDIR}) install(CODE "message(STATUS \"Creating symlink ${CMAKE_INSTALL_FULL_LIBDIR}/${baselibname} -> ${verlibname}\")") set(verlibname) From 50fcd31ea949e3dedff6a954066d54099c5747b4 Mon Sep 17 00:00:00 2001 From: Cary Phillips Date: Thu, 8 Feb 2024 16:57:06 -0800 Subject: [PATCH 5/5] clarify the uninstall instructions Signed-off-by: Cary Phillips --- website/install.rst | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/website/install.rst b/website/install.rst index 6d50030769..153b706138 100644 --- a/website/install.rst +++ b/website/install.rst @@ -280,7 +280,12 @@ You can customize these options three ways: Uninstall ~~~~~~~~~ -If you have installed from source, you can uninstall via: +If you did a binary instal of OpenEXR via a package manager +(`apt-get`, `yum`, `port`, `brew`, etc), use the package manager to +uninstall. + +If you have installed from source, *and you still have the build +tree from which you installed*, you can uninstall via: .. code-block:: @@ -292,6 +297,8 @@ or if using ``make``: % make uninstall +The `uninstall` relies on CMake's `install_manifest.txt` for the record +of what was installed. Library Naming Options ~~~~~~~~~~~~~~~~~~~~~~