Skip to content

Speed up FindCSP / get all vars in one call and cache for subsequent … - #745

Open
robambalu wants to merge 1 commit into
mainfrom
rba/find_speedup
Open

Speed up FindCSP / get all vars in one call and cache for subsequent …#745
robambalu wants to merge 1 commit into
mainfrom
rba/find_speedup

Conversation

@robambalu

Copy link
Copy Markdown
Collaborator

…finds

…finds

Signed-off-by: Rob Ambalu <robert.ambalu@point72.com>
@timkpaine timkpaine added part: build Issues and PRs related to the build process type: enhancement Issues and PRs related to improvements to existing features labels Aug 14, 2026

@malhotrashivam malhotrashivam left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI-assisted review, please let me know if anything needs more explanation, thanks!

list(GET __csp_query_list 2 __csp_lib_path)
list(GET __csp_query_list 3 __csp_version)

set(__csp_base_path "${__csp_base_path}" CACHE INTERNAL "csp package base path")

@malhotrashivam malhotrashivam Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Note: CACHE INTERNAL persists across separate CMake configure runs, so reusing a build directory after changing CSP may retain old paths and versions. Can you please verify if that's okay? Should we use a CMake global property instead?

RESULT_VARIABLE __csp_query_result)

if(NOT __csp_query_result EQUAL 0)
message(FATAL_ERROR "Failed to query csp package paths via ${Python_EXECUTABLE}")

@malhotrashivam malhotrashivam Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Note: Previously, a failed CSP import let find_package(CSP REQUIRED) fail while find_package(CSP QUIET) returned CSP_FOUND=FALSE. The new FATAL_ERROR aborts both. Can you please once verify if it even matters? Like do we need to support the REQUIRED/QUIET paramaters?

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

Labels

part: build Issues and PRs related to the build process type: enhancement Issues and PRs related to improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants