Skip to content

Configuration - Parallel installing and improve catching cmake version #914

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.

Sign up for GitHub

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

Draft
dpasukhi wants to merge 1 commit into
base: IR
Choose a base branch
from
+34 −15
Draft

Configuration - Parallel installing and improve catching cmake version #914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 13 additions & 15 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,21 +206,23 @@ if (BUILD_FORCE_RelWithDebInfo)
set (CMAKE_CONFIGURATION_TYPES Release Debug CACHE INTERNAL "" FORCE)
endif()

# option to enable or disable use of precompiled headers
if (NOT DEFINED BUILD_USE_PCH)
set (BUILD_USE_PCH OFF CACHE BOOL "${BUILD_USE_PCH_DESCR}")
endif()
# Version-gated build options (enabled by default when CMake version is sufficient)
OCCT_VERSION_GATED_OPTION (BUILD_USE_PCH "3.16" ON "${BUILD_USE_PCH_DESCR}")
OCCT_VERSION_GATED_OPTION (BUILD_INCLUDE_SYMLINK "3.14" ON "${BUILD_INCLUDE_SYMLINK_DESCR}")
OCCT_VERSION_GATED_OPTION (BUILD_INSTALL_PARALLEL "3.30" ON "${BUILD_INSTALL_PARALLEL_DESCR}")

if (CMAKE_VERSION VERSION_LESS "3.16")
OCCT_CHECK_AND_UNSET (BUILD_USE_PCH)
elseif (NOT BUILD_USE_PCH)
set (CMAKE_DISABLE_PRECOMPILE_HEADERS ON)
else ()
# Apply precompiled headers setting
if (BUILD_USE_PCH)
set (CMAKE_DISABLE_PRECOMPILE_HEADERS OFF)
elseif (DEFINED BUILD_USE_PCH)
set (CMAKE_DISABLE_PRECOMPILE_HEADERS ON)
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic has an edge case issue: when CMake version is < 3.16, BUILD_USE_PCH is unset, so DEFINED BUILD_USE_PCH is false, and neither branch executes. This means CMAKE_DISABLE_PRECOMPILE_HEADERS retains whatever value it had before. For clarity and correctness, consider adding an else clause to explicitly set CMAKE_DISABLE_PRECOMPILE_HEADERS ON when BUILD_USE_PCH is not defined, ensuring consistent behavior across all CMake versions.

Suggested change
set (CMAKE_DISABLE_PRECOMPILE_HEADERS ON)
set (CMAKE_DISABLE_PRECOMPILE_HEADERS ON)
else()
set (CMAKE_DISABLE_PRECOMPILE_HEADERS ON)

Copilot uses AI. Check for mistakes.
endif()

if (NOT DEFINED BUILD_INCLUDE_SYMLINK)
set (BUILD_INCLUDE_SYMLINK OFF CACHE BOOL "${BUILD_INCLUDE_SYMLINK_DESCR}")
# Apply parallel installation setting
if (BUILD_INSTALL_PARALLEL)
set_property (GLOBAL PROPERTY INSTALL_PARALLEL ON)
elseif (DEFINED BUILD_INSTALL_PARALLEL)
set_property (GLOBAL PROPERTY INSTALL_PARALLEL OFF)
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the BUILD_USE_PCH logic, when CMake version is < 3.30, BUILD_INSTALL_PARALLEL is unset, so neither branch executes and the INSTALL_PARALLEL property may retain an unexpected value. Consider adding an else clause to explicitly set INSTALL_PARALLEL OFF when BUILD_INSTALL_PARALLEL is not defined, ensuring the property is always explicitly set regardless of CMake version.

Suggested change
set_property (GLOBAL PROPERTY INSTALL_PARALLEL OFF)
set_property (GLOBAL PROPERTY INSTALL_PARALLEL OFF)
else()
set_property (GLOBAL PROPERTY INSTALL_PARALLEL OFF)

Copilot uses AI. Check for mistakes.
endif()

# Overview
Expand All @@ -237,10 +239,6 @@ if (NOT DEFINED INSTALL_DOC_RefMan)
set (INSTALL_DOC_RefMan OFF CACHE BOOL "${INSTALL_DOC_RefMan_DESCR}")
endif()

if (CMAKE_VERSION VERSION_LESS "3.14")
OCCT_CHECK_AND_UNSET (BUILD_INCLUDE_SYMLINK)
endif()

# copy samples to install directory
set (INSTALL_SAMPLES OFF CACHE BOOL "${INSTALL_SAMPLES_DESCR}")

Expand Down
17 changes: 17 additions & 0 deletions adm/cmake/occt_macros.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,23 @@ macro (OCCT_CHECK_AND_UNSET VARNAME)
endif()
endmacro()

# Macro to define a version-gated boolean option.
# If CMake version is sufficient, option defaults to DEFAULT_ON value.
# If CMake version is too old, option is unset and unavailable.
# VARNAME - variable name
# MIN_VERSION - minimum CMake version required
# DEFAULT_ON - default value when version is sufficient (ON/OFF)
# DESCRIPTION - variable description
macro (OCCT_VERSION_GATED_OPTION VARNAME MIN_VERSION DEFAULT_ON DESCRIPTION)
if (CMAKE_VERSION VERSION_LESS "${MIN_VERSION}")
OCCT_CHECK_AND_UNSET (${VARNAME})
else()
if (NOT DEFINED ${VARNAME})
set (${VARNAME} ${DEFAULT_ON} CACHE BOOL "${DESCRIPTION}")
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The macro doesn't respect user-specified values when the variable is already defined in the cache. If a user has explicitly set the variable to a value different from DEFAULT_ON, this logic won't update the cache description, potentially leaving outdated help text. Consider using set(${VARNAME} ${DEFAULT_ON} CACHE BOOL \"${DESCRIPTION}\" FORCE) when the variable exists but needs the description updated, or document that the description won't be updated for pre-existing cache entries.

Suggested change
set (${VARNAME} ${DEFAULT_ON} CACHE BOOL "${DESCRIPTION}")
set (${VARNAME} ${DEFAULT_ON} CACHE BOOL "${DESCRIPTION}")
else()
# Update the cache entry's description and type, but keep the user's value
set (${VARNAME} "${${VARNAME}}" CACHE BOOL "${DESCRIPTION}" FORCE)

Copilot uses AI. Check for mistakes.
endif()
endif()
endmacro()

macro (OCCT_CHECK_AND_UNSET_GROUP GROUPNAME)
get_cmake_property(VARS VARIABLES)
string (REGEX MATCHALL "(^|;)${GROUPNAME}[A-Za-z0-9_]*" GROUPNAME_VARS "${VARS}")
Expand Down
4 changes: 4 additions & 0 deletions adm/cmake/vardescr.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ set (BUILD_INCLUDE_SYMLINK_DESCR
OFF - using a reference file with direct include to the origin,
ON - symbolic link to the origin file are created")

set (BUILD_INSTALL_PARALLEL_DESCR
"Enable parallel installation (requires CMake 3.30+).
When enabled, cmake --install can use -j option to install in parallel.")

# install variables
set (INSTALL_DIR_DESCR
"The place where built OCCT libraries, headers, test cases (INSTALL_TEST_CASES variable),
Expand Down
Loading