-
Notifications
You must be signed in to change notification settings - Fork 9
feat: Add CMAKE_SOURCE_DIR parameter to install-remote-tar to support installing projects whose CMakeLists.txt isn't in the root directory.
#72
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.
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
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TaskRunner
participant CMake
User->>TaskRunner: Run install-remote-tar (with optional CMAKE_SOURCE_DIR)
TaskRunner->>TaskRunner: Extract remote tar to EXTRACTION_DIR
TaskRunner->>TaskRunner: Set SOURCE_DIR = EXTRACTION_DIR/CMAKE_SOURCE_DIR (default ".")
TaskRunner->>CMake: Run generate task with SOURCE_DIR
CMake-->>TaskRunner: Generation complete
Possibly related PRs
Suggested reviewers
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (5)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
exports/taskfiles/utils/cmake.yaml(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: sitaowang1998
PR: y-scope/yscope-dev-utils#64
File: exports/taskfiles/utils/boost.yaml:31-37
Timestamp: 2025-06-08T17:55:11.362Z
Learning: For Boost builds in yscope-dev-utils, symlinking only the top-level directories from SOURCE_DIR to BUILD_DIR is sufficient for the build to succeed. Recursive symlinking of all nested files and directories is not necessary.
Learnt from: AVMatthews
PR: y-scope/yscope-dev-utils#13
File: taskfiles/utils.yml:149-149
Timestamp: 2024-10-28T19:02:02.131Z
Learning: In `taskfiles/utils.yml`, avoid using GNU-specific options like `-r` and `--no-run-if-empty` with `xargs` for better portability; instead, use `find`'s `-exec` option.
exports/taskfiles/utils/cmake.yaml (1)
Learnt from: sitaowang1998
PR: y-scope/yscope-dev-utils#64
File: exports/taskfiles/utils/boost.yaml:31-37
Timestamp: 2025-06-08T17:55:11.362Z
Learning: For Boost builds in yscope-dev-utils, symlinking only the top-level directories from SOURCE_DIR to BUILD_DIR is sufficient for the build to succeed. Recursive symlinking of all nested files and directories is not necessary.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit-tests (ubuntu-22.04)
- GitHub Check: unit-tests (ubuntu-24.04)
🔇 Additional comments (1)
exports/taskfiles/utils/cmake.yaml (1)
160-162: Good addition – new parameter clearly documented
The explanatory block forCMAKE_SOURCE_DIRis concise and puts the parameter in the correct context.
kirkrodrigues
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the PR title, how about:
feat: Add `CMAKE_SOURCE_DIR` parameter to `install-remote-tar` to support installing projects whose `CMakeLists.txt` isn't in the root directory.
CMAKE_SOURCE_DIR parameter to install-remote-tar to support installing projects whose CMakeLists.txt isn't in the root directory.
Description
This PR introduces foundational low-level functionality to support a broader initiative: migrating the libarchive dependency to be downloaded and compiled via task, pinned to a secure version across all platforms. For the transitive dependencies
zstdandlz4, the CMake source directory is located at<project root>/build/cmake. These changes are essential for enabling robust, platform-consistent behavior in the CLP repository’s task filesChecklist
breaking change.
Validation performed
Validation was performed by compiling libarchive along with all its transitive dependencies—lz4, xz, zlib, and zstd—using the functionality introduced in this PR.
Summary by CodeRabbit