Skip to content

ENH: Refactor CMake project infrastructure#27

Open
RafaelPalomar wants to merge 1 commit into
commontk:masterfrom
SystoleOS:refactor/cmake_infrastructure
Open

ENH: Refactor CMake project infrastructure#27
RafaelPalomar wants to merge 1 commit into
commontk:masterfrom
SystoleOS:refactor/cmake_infrastructure

Conversation

@RafaelPalomar

Copy link
Copy Markdown

This PR updates the cmake project definition for qRestAPI:

  • Removed old configuration and use CMake files in favor ofqRestAPIConfig.cmake.in
  • Enable install configuration through implementation of qRestAPIInstallConfig.cmake.in
  • Enabled installation of development files (optional).

This PR has been greatly inspired on the project definition for vtkAddon project. The motivation for this PR is to enable this library as a standalone library that can be installed and used in other projects and will be useful to build 3D Sclier with SUPERBUILD=OFF and support the SystoleOS.

Since these changes affect the cmake configuration files (most notably removing the need for UseqRestAPI.cmake file), projects using the library will need to update their cmake project definitions.

Installation examples

Library installation

cmake -S . -B build
❯ make install
[ 65%] Built target qRestAPI
[100%] Built target qRestAPITests
Install the project...
-- Install configuration: ""
-- Installing: /tmp/test-prefix/lib/qRestAPI/libqRestAPI.so
-- Installing: /tmp/test-prefix/cmake/qRestAPI/qRestAPIConfig.cmake

Library and development files installation

cmake -S . -B build -DqRestAPI_INSTALL_NO_DEVELOPMENT=OFF
❯ make install
[ 65%] Built target qRestAPI
[100%] Built target qRestAPITests
Install the project...
-- Install configuration: ""
-- Installing: /tmp/test-prefix/include/qRestAPI/qGirderAPI.h
-- Installing: /tmp/test-prefix/include/qRestAPI/qMidasAPI.h
-- Installing: /tmp/test-prefix/include/qRestAPI/qRestAPI.h
-- Installing: /tmp/test-prefix/include/qRestAPI/qRestAPI_p.h
-- Installing: /tmp/test-prefix/include/qRestAPI/qRestResult.h
-- Installing: /tmp/test-prefix/include/qRestAPI/qRestAPI_Export.h
-- Up-to-date: /tmp/test-prefix/lib/qRestAPI/libqRestAPI.so
-- Installing: /tmp/test-prefix/cmake/qRestAPI/qRestAPIConfig.cmake

content of install configuration:

❯ cat /tmp/test-prefix/cmake/qRestAPI/qRestAPIConfig.cmake
set(qRestAPI_CMAKE_DIR "/tmp/test-prefix/cmake/qRestAPI")
set(qRestAPI_INCLUDE_DIRS "/tmp/test-prefix/include/qRestAPI")
set(qRestAPI_LIB_DIR "/tmp/test-prefix/lib/qRestAPI")

- Removed old configuration and use CMake files in favor of`qRestAPIConfig.cmake.in`
- Enable install configuration through implementation of qRestAPIInstallConfig.cmake.in
- Enabled installation of development files.
@ferdymercury

Copy link
Copy Markdown

@lassoan could this PR be reviewed? Installation is crucial for packaging purposes. Thanks!

@RafaelPalomar

Copy link
Copy Markdown
Author

@lassoan could this PR be reviewed? Installation is crucial for packaging purposes. Thanks!

@ferdymercury I'm curious about your use case. What type of package are you thinking of?

@ferdymercury

Copy link
Copy Markdown

Mostly building slicer with superbuild=off but potentially also creating homebrew formula tap

@lassoan

lassoan commented Jul 7, 2026

Copy link
Copy Markdown
Member

We discussed this today with @RafaelPalomar and he'll have a fresh look at it and if it seems all good then we'll get this merged.

@ferdymercury have you tested this PR - if the changes solve your packaging needs?

@ferdymercury

Copy link
Copy Markdown

Yes, thanks for the reply. I tried with this branch and it seems to work:

cmake -DCMAKE_POLICY_VERSION_MINIMUM=3.5 -DCMAKE_INSTALL_PREFIX=../install -DqRestAPI_QT_VERSION=5 -DqRestAPI_INSTALL_NO_DEVELOPMENT=OFF ..
make -j8
make install

gives:

Install the project...
-- Install configuration: ""
-- Installing: /tmp/qRestAPI/install/include/qRestAPI/qGirderAPI.h
-- Installing: /tmp/qRestAPI/install/include/qRestAPI/qMidasAPI.h
-- Installing: /tmp/qRestAPI/install/include/qRestAPI/qRestAPI.h
-- Installing: /tmp/qRestAPI/install/include/qRestAPI/qRestAPI_p.h
-- Installing: /tmp/qRestAPI/install/include/qRestAPI/qRestResult.h
-- Installing: /tmp/qRestAPI/install/include/qRestAPI/qRestAPI_Export.h
-- Installing: /tmp/qRestAPI/install/lib/qRestAPI/libqRestAPI.so
-- Installing: /tmp/qRestAPI/install/cmake/qRestAPI/qRestAPIConfig.cmake

so it looks fine.

However, as a potential improvement one could do the following changes so outside users know better what to do:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 29fe52b..526ba20 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -41,9 +41,7 @@ option(BUILD_TESTING "Test the project" ON)
 #
 # Qt
 #
-set(qRestAPI_QT_VERSION "4" CACHE STRING "Expected Qt version")
-mark_as_advanced(qRestAPI_QT_VERSION)
-set_property(CACHE qRestAPI_QT_VERSION PROPERTY STRINGS 4 5 6)
+set(qRestAPI_QT_VERSION "4" CACHE STRING "Expected Qt version (4, 5 or 6)")
 if(NOT (qRestAPI_QT_VERSION VERSION_EQUAL "4" OR qRestAPI_QT_VERSION VERSION_EQUAL "5" OR qRestAPI_QT_VERSION VERSION_EQUAL "6"))
   message(FATAL_ERROR "Expected value for qRestAPI_QT_VERSION is either '4', '5' or '6'")
 endif()
@@ -91,9 +89,7 @@ set(KIT_SRCS
 # --------------------------------------------------------------------------
 # Install headers
 # --------------------------------------------------------------------------
-if(NOT DEFINED ${PROJECT_NAME}_INSTALL_NO_DEVELOPMENT)
-  set(${PROJECT_NAME}_INSTALL_NO_DEVELOPMENT ON)
-endif()
+set(${PROJECT_NAME}_INSTALL_NO_DEVELOPMENT "ON" CACHE STRING "Set to OFF when wanting to create a package / install dir")
 
 if(NOT ${PROJECT_NAME}_INSTALL_NO_DEVELOPMENT)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants