chore: allow build without Boost Stacktrace #466
Conversation
WalkthroughThe changes in this pull request primarily involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CMake
participant Boost
participant SGP4
User->>CMake: Configure project
CMake->>Boost: Check BUILD_WITH_BOOST_STACKTRACE
CMake->>Boost: Check BUILD_WITH_BOOST_STATIC
CMake->>CMake: Set Boost_USE_STATIC_LIBS based on options
CMake->>CMake: Update CMAKE_MODULE_PATH
CMake->>CMake: Configure files in tools/cmake
CMake->>SGP4: Find SGP4 library
SGP4-->>CMake: Return SGP4_LIBRARIES and SGP4_INCLUDE_DIRS
CMake->>User: Build project
Warning Rate limit exceeded@tapiab has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 28 minutes and 9 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #466 +/- ##
=======================================
Coverage 91.12% 91.12%
=======================================
Files 85 85
Lines 8639 8639
=======================================
Hits 7872 7872
Misses 767 767 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
CMakeLists.txt (1)
28-29: LGTM! Consider adding option documentation.The new build options are well-named and align with CMake conventions. Their default values maintain backward compatibility.
Consider adding comments to document these options:
+# Enable/disable building with Boost Stacktrace (uses basic stacktrace implementation when disabled) OPTION (BUILD_WITH_BOOST_STACKTRACE "Build with Boost Stacktrace instead of stacktrace basic" ON) +# Enable/disable building with Boost static libraries OPTION (BUILD_WITH_BOOST_STATIC "Build with Boost Static lib" ON)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
CMakeLists.txt(6 hunks)tools/cmake/FindSGP4.cmake(1 hunks)
🔇 Additional comments (5)
tools/cmake/FindSGP4.cmake (1)
10-10: LGTM! Verify system paths don't cause conflicts.
Allowing search in system paths increases flexibility. However, ensure this doesn't lead to version conflicts if SGP4 is installed in multiple locations.
✅ Verification successful
Since we're in a sandbox environment without access to system paths, let's verify if there are any other SGP4-related files in the repository that might indicate potential conflicts:
Let's check how SGP4 is being used in the CMakeLists.txt to understand the dependency management better:
Removing NO_DEFAULT_PATH is safe in this context
The removal of NO_DEFAULT_PATH from FindSGP4.cmake is safe because:
- The project uses SGP4 as a required dependency with proper variable checks
- The CMake configuration properly validates the found SGP4 through SGP4_FOUND
- The include and library directories are explicitly set using the variables from the find module
- The project maintains a consistent linking strategy for both static and shared libraries
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for multiple SGP4 installations that could cause conflicts
# Search for SGP4.h in common system paths
echo "Checking for SGP4.h installations:"
fd SGP4.h /usr/include /usr/local/include
# Search for SGP4 libraries
echo -e "\nChecking for SGP4 library installations:"
fd "libsgp4" /usr/lib /usr/local/lib
Length of output: 421
Script:
#!/bin/bash
# Check for any SGP4-related files in the repository
echo "SGP4-related files:"
fd -t f SGP4
# Check for any SGP4-related CMake files
echo -e "\nSGP4-related CMake files:"
fd -e cmake SGP4
# Check if SGP4 is mentioned in any CMake cache or configuration files
echo -e "\nSGP4 mentions in CMake files:"
rg -l "SGP4" -g "*.cmake" -g "CMakeLists.txt"
Length of output: 659
Script:
#!/bin/bash
# Check how SGP4 is configured and used in CMakeLists.txt
echo "SGP4 configuration in CMakeLists.txt:"
rg -A 5 -B 5 "SGP4" CMakeLists.txt
# Check if there are any version requirements or specific configurations
echo -e "\nSGP4 find_package configurations:"
rg "find_package.*SGP4" CMakeLists.txt
# Check for any SGP4 related variables
echo -e "\nSGP4 related variables:"
rg "SGP4_" CMakeLists.txt
Length of output: 2534
CMakeLists.txt (4)
39-39: LGTM! Good improvement to module path handling.
Using LIST(APPEND) instead of SET is a better practice as it preserves any existing module paths that might have been set by parent projects or user configurations.
243-247: LGTM! Clean conditional Boost static library configuration.
The conditional setting of Boost_USE_STATIC_LIBS based on the new option is well implemented.
629-639: LGTM! Good improvement to CMake config installation paths.
Moving CMake config files to a cmake subdirectory follows modern CMake conventions and improves the organization of installed files.
644-644: LGTM! Consistent path update for uninstall target.
The uninstall target configuration file path has been updated consistently with other changes.
This MR allows building OSTk Core without Boost Stacktrace (use basic Stacktrace instead) due to some target incompatibility with it.
Summary by CodeRabbit