fix(cmake): Correctly report system BLAS in summary#7230
Merged
ssheorey merged 2 commits intoisl-org:mainfrom May 26, 2025
Merged
fix(cmake): Correctly report system BLAS in summary#7230ssheorey merged 2 commits intoisl-org:mainfrom
ssheorey merged 2 commits intoisl-org:mainfrom
Conversation
|
Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes. |
ad57d25 to
4513697
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the CMake summary table so that enabling USE_SYSTEM_BLAS shows “yes (system)” instead of “no” and documents the change in the changelog.
- Correct CMake logic to report system BLAS in the configuration summary.
- Add an entry to
CHANGELOG.md.
Files not reviewed (1)
- cmake/Open3DPrintConfigurationSummary.cmake: Language not supported
CHANGELOG.md
Outdated
| - Fix infinite loop in segment_plane if num_points < ransac_n (PR #7032) | ||
| - Add select_by_index method to Feature class (PR #7039) | ||
|
|
||
| - Fix CMake configuration summary incorrectly reporting `no` for system BLAS. |
There was a problem hiding this comment.
[nitpick] Remove the empty bullet entry at line 58 to avoid an extraneous blank item in the changelog list.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(cmake): Correctly report system BLAS in configuration summary
Type
Motivation and Context
When configuring Open3D with the
USE_SYSTEM_BLAS=ONoption, the final configuration summary table incorrectly reportedBLAS ..... no. This occurred even though CMake successfully found the system BLAS/LAPACK libraries and configured the build to use them (verified viaCMakeCache.txtand the addition of theUSE_BLASpreprocessor definition).This misleading summary output could cause confusion for users building with system BLAS. This change corrects the summary output to accurately reflect the configuration.
Checklist:
python util/check_style.py --applyto apply Open3D code style to my code.Description
This PR modifies the
cmake/Open3DPrintConfigurationSummary.cmakescript.The original logic for printing the status of third-party dependencies checked for the existence of an internal CMake target (
Open3D::3rdparty_${dep_lower}). However, whenUSE_SYSTEM_BLAS=ON, this target (Open3D::3rdparty_blas) is apparently not created, causing the script to fall into theelseblock and print"no".The fix adds a specific condition within this
elseblock: if the dependency being checked is"BLAS"and theUSE_SYSTEM_BLASvariable is ON, it now correctly prints"yes (system)"instead of"no". This ensures the summary accurately reflects the use of a system-provided BLAS library.Before:
-- Third-Party Dependencies: -- BLAS .................................... no
After:
-- Third-Party Dependencies: -- BLAS .................................... yes (system)