refactor(taskfile): Split main dependency Taskfile into category-specific modules; Migrate antlr-runtime and utfcpp to use utils:install-remote-cmake-lib.
#1377
Conversation
WalkthroughIntroduces modular YAML taskfiles for C++ dependency installs and orchestration: new per-library install taskfiles, a registry mapping, refactored main/core/spider workflows that centralize init/dirs/checksum handling, and parallel per-dependency task expansion via internal "*-all-parallel" tasks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer/Invoker
participant Main as deps:core / deps:spider
participant Init as :init
participant Clean as :utils:clean-outdated-cpp-checksum-files
participant InstallGen as :utils:install-deps-and-generate-settings
participant Combine as :utils:combine-cpp-checksums
participant Parallel as core-all-parallel / spider-all-parallel
participant Item as :<per-item>
Dev->>Main: run core (or spider)
Main->>Init: depend -> run once
Main->>Clean: run once
Main->>InstallGen: install/generate settings (CMAKE_SETTINGS_DIR/FILE)
InstallGen->>Combine: write/emit per-lib checksum files
Combine->>Main: combined checksum
Main->>Parallel: trigger internal parallel task (run: once)
Parallel->>Item: for each CPP_*_TASKS -> invoke :<item>
Item-->>Parallel: done
Parallel-->>Main: all per-item tasks invoked
Main-->>Dev: complete
Notes: diagram focuses on primary control flow for core/spider orchestration: init → clean → install/generate → combine → parallel per-item expansion. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Rationale: Many new, mostly-homogeneous install files reduce per-file complexity, but orchestration changes (main/utils/core/spider), checksum handling, colon-prefixed refactors, platform preconditions, and dependency iteration require careful verification of control flow, URLs/SHA256, and interaction order. Possibly related issues
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
Bill-hbrhbr
changed the title
Bill-hbrhbr
changed the title
Bill-hbrhbr
requested review from
LinZhihao-723,
davidlion,
kirkrodrigues and
sitaowang1998
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
taskfile.yaml (1)
130-162: Restore the dependency on the Spider build task.
deps:spider:spider-depstops after installing Spider’s prerequisites; it never runs thespiderbuild that produces{{.G_SPIDER_BUILD_DIR}}/spider-build/src/spider/spider_schedulerandspider_worker. Thersyncin the package recipe will therefore fail with missing inputs. Please depend on the full Spider build (e.g.,deps:spider) so these binaries exist before packaging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
taskfile.yaml(1 hunks)taskfiles/deps/cmake-libs.yaml(1 hunks)taskfiles/deps/core.yaml(1 hunks)taskfiles/deps/log-viewer.yaml(1 hunks)taskfiles/deps/main.yaml(1 hunks)taskfiles/deps/misc-libs.yaml(1 hunks)taskfiles/deps/spider.yaml(1 hunks)taskfiles/deps/utils.yaml(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-28T18:33:30.155Z
Learnt from: anlowee
PR: y-scope/clp#925
File: taskfiles/deps/main.yaml:97-106
Timestamp: 2025-05-28T18:33:30.155Z
Learning: In the taskfiles dependency system (taskfiles/deps/main.yaml), echo commands are used to generate .cmake settings files that are consumed by the main CMake build process. These files set variables like ANTLR_RUNTIME_HEADER to point to dependency locations for use during compilation.
Applied to files:
taskfiles/deps/misc-libs.yaml
📚 Learning: 2025-07-23T09:54:45.185Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
Applied to files:
taskfiles/deps/cmake-libs.yaml
⏰ 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). (6)
- GitHub Check: package-image
- GitHub Check: musllinux_1_2-x86_64-static-linked-bins
- GitHub Check: ubuntu-jammy-lint
- GitHub Check: musllinux_1_2-x86_64-dynamic-linked-bins
- GitHub Check: build-macos (macos-15, false)
- GitHub Check: build-macos (macos-15, true)
|
If I understand correctly the scenario is that:
Is it correct to say the fix in this PR is splitting ==== (Assuming my understanding above is correct.) ==== On a side note, in the future only request a single reviewer at a time to reduce confusion and prevent duplicate comments. The yscope-maintainers group automatically notifies a bunch of people, but you probably want to talk to people to figure out the order of reviews (if you want multiple) or who has time. |
|
@CodeRabbit Open a PR in |
|
Tip For best results, initiate chat on the files or code changes. @sitaowang1998: I want to clarify your request. Based on the comment chain, davidlion mentioned opening an issue to track this problem. However, you mentioned opening a PR. I can help you:
Would you like me to open an issue in the
Please confirm if you'd like me to proceed with creating an issue. |
|
@CodeRabbit. Sorry I meant opening an issue |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 53 @sitaowang1998: No problem! I've created the issue in the 🔗 Issue #89: y-scope/yscope-dev-utils#89 The issue includes:
The issue is now ready for tracking and future implementation. |
This is already done in #1331. This PR is just a refactor of the taskfile structure with some minor fixes. The main problem is that The problem of including unnecessary settings in combined cmake setting files is not a big issue, as it does not affect the correctness of the build system. |
taskfiles/deps/cmake-libs.yaml
Outdated
| - "-DANTLR4_INSTALL=ON" | ||
| - "-DCMAKE_BUILD_TYPE=Release" |
There was a problem hiding this comment.
| - "-DANTLR4_INSTALL=ON" | |
| - "-DCMAKE_BUILD_TYPE=Release" | |
| - "-DANTLR4_INSTALL=ON" | |
| - "-DBUILD_TESTING=OFF" | |
| - "-DCMAKE_BUILD_TYPE=Release" |
There was a problem hiding this comment.
BUILD_TESTING does not exist in antlr4-runtime.
There was a problem hiding this comment.
BUILD_TESTING is a variable in CTest. If a project use enable_testing, we can set BUILD_TESTING to turn off the test build. See https://cmake.org/cmake/help/latest/variable/BUILD_TESTING.html.
There was a problem hiding this comment.
I've found a better flag: -DANTLR_BUILD_CPP_TESTS=OFF
taskfiles/deps/cmake-libs.yaml
Outdated
| - "-DCMAKE_BUILD_TYPE=Release" | ||
| - "-DCMAKE_INSTALL_MESSAGE=LAZY" | ||
| - "-DCMAKE_INSTALL_PREFIX={{.G_DEPS_CPP_DIR}}/{{.LIB_NAME}}-install" | ||
| - "-DZLIB_BUILD_EXAMPLES=OFF" |
There was a problem hiding this comment.
| - "-DZLIB_BUILD_EXAMPLES=OFF" | |
| - "-DZLIB_BUILD_EXAMPLES=OFF" | |
| - "-DZLIB_BUILD_TESTING=OFF" |
There was a problem hiding this comment.
ZLIB_BUILD_TESTING is not present in zlib-1.3.1
There was a problem hiding this comment.
In 1.3.1, zlib uses enable_testing. However, it does not build test if example build if off. Same with utf8, let's leave it here with a comment.
Co-authored-by: sitaowang1998 <[email protected]>
|
To provide some context:
Many libraries expose test-related flags, but most are already Policy options we can adopt going forward:
Thoughts or preferences? |
|
As a side note, I'm trying to avoid scope creep in this PR. We should do as much copy-paste and moving code around as possible. But I guess I did modify the install methods of two libraries ( |
For 1. I think we can only apply |
I prefer to stick with 3 since it's the clearest to reason about. |
This reverts commit 5a2150c.
Co-authored-by: sitaowang1998 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
taskfiles/deps/cmake-libs.yaml (3)
220-238: Add test build flag.Based on past review comments, explicitly setting
-DENABLE_TESTS=OFFwas suggested to disable test building for mongocxx.Apply this diff to disable test building:
- "-DBUILD_SHARED_AND_STATIC_LIBS=ON" - "-DBUILD_SHARED_LIBS_WITH_STATIC_MONGOC=ON" - "-DCMAKE_BUILD_TYPE=Release" - "-DCMAKE_INSTALL_MESSAGE=LAZY" - "-DENABLE_EXAMPLES=OFF" + - "-DENABLE_TESTS=OFF" - "-DENABLE_UNINSTALL=OFF"
68-79: Add test build flag.Based on past review comments, explicitly setting
-DENABLE_DATE_TESTING=OFFwas suggested to disable test building for the date library.Apply this diff to disable test building:
CMAKE_GEN_ARGS: - "-DCMAKE_BUILD_TYPE=Release" - "-DCMAKE_INSTALL_MESSAGE=LAZY" + - "-DENABLE_DATE_TESTING=OFF"
295-316: Add test build flag.Based on past review comments, explicitly setting
-DSPDLOG_BUILD_TESTS=OFFwas suggested. The current configuration includesSPDLOG_BUILD_TESTS_HO=OFFbut is missing the mainSPDLOG_BUILD_TESTSflag.Apply this diff to disable test building:
- "-DCMAKE_POLICY_DEFAULT_CMP0074=NEW" - "-DSPDLOG_BUILD_EXAMPLE=OFF" + - "-DSPDLOG_BUILD_TESTS=OFF" - "-DSPDLOG_BUILD_TESTS_HO=OFF" - "-DSPDLOG_BUILD_SHARED=ON"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
taskfiles/deps/cmake-libs.yaml(1 hunks)taskfiles/deps/main.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1156
File: components/core/CMakeLists.txt:772-772
Timestamp: 2025-08-09T04:07:27.083Z
Learning: In the CLP project's CMakeLists.txt, when reviewing changes related to the ${zstd_TARGET} variable usage in test linking, the team is planning a refactoring PR to improve this mechanism. Guards for undefined target variables should be deferred to that separate PR rather than being added in focused dependency migration PRs.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
📚 Learning: 2025-07-23T09:54:45.185Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
Applied to files:
taskfiles/deps/cmake-libs.yaml
📚 Learning: 2025-09-15T18:53:15.844Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1226
File: taskfiles/deps/main.yaml:522-527
Timestamp: 2025-09-15T18:53:15.844Z
Learning: When ystdlib is built with -Dystdlib_BUILD_TESTING=OFF in taskfiles/deps/main.yaml, it does not require Catch2 settings to be injected via CMAKE_GEN_ARGS since testing is disabled and Catch2 is not needed during the build process.
Applied to files:
taskfiles/deps/cmake-libs.yaml
📚 Learning: 2025-08-19T07:08:15.583Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1229
File: taskfiles/deps/main.yaml:494-504
Timestamp: 2025-08-19T07:08:15.583Z
Learning: In zlib v1.3.1's CMake build system, only ZLIB_BUILD_EXAMPLES is a valid option. Other ZLIB_BUILD_* flags (MINIZIP, SHARED, STATIC, TESTING) and ZLIB_INSTALL are not defined in the CMakeLists.txt and have no effect. Use BUILD_SHARED_LIBS=ON for shared library control and ZLIB_BUILD_EXAMPLES=OFF to disable examples.
Applied to files:
taskfiles/deps/cmake-libs.yaml
📚 Learning: 2025-08-06T07:32:28.462Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1156
File: taskfiles/deps/main.yaml:70-71
Timestamp: 2025-08-06T07:32:28.462Z
Learning: In LZ4's CMake build system (build/cmake/CMakeLists.txt), the option to build static libraries is `BUILD_STATIC_LIBS=ON`, not `LZ4_BUILD_STATIC=ON`. The correct options for building both shared and static LZ4 libraries are `-DBUILD_SHARED_LIBS=ON` and `-DBUILD_STATIC_LIBS=ON`.
Applied to files:
taskfiles/deps/cmake-libs.yaml
📚 Learning: 2025-08-09T04:07:27.083Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1156
File: components/core/CMakeLists.txt:772-772
Timestamp: 2025-08-09T04:07:27.083Z
Learning: In the CLP project's CMakeLists.txt, when reviewing changes related to the ${zstd_TARGET} variable usage in test linking, the team is planning a refactoring PR to improve this mechanism. Guards for undefined target variables should be deferred to that separate PR rather than being added in focused dependency migration PRs.
Applied to files:
taskfiles/deps/cmake-libs.yaml
⏰ 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: rust-checks (ubuntu-22.04)
- GitHub Check: rust-checks (ubuntu-24.04)
🔇 Additional comments (21)
taskfiles/deps/main.yaml (4)
3-22: LGTM! Well-organized includes structure.The categorization of includes (project targets, utils, component libraries) improves clarity and maintainability. The flattening of component library tasks is appropriate for direct access.
29-30: LGTM! Checksums directory properly exposed.Making
G_DEPS_CPP_CHECKSUMS_DIRa public variable supports the modular dependency structure and aligns with the checksum management workflow described in the PR objectives.
42-46: LGTM! Spider correctly added to default dependencies.The explicit inclusion of spider in the default task aligns with the PR objectives and addresses the discussion in past review comments about keeping spider as a default task.
48-54: LGTM! Init task properly consolidated.The init task correctly creates directories in hierarchical order (G_DEPS_DIR → G_DEPS_CPP_DIR → G_DEPS_CPP_CHECKSUMS_DIR → G_DEPS_CPP_CMAKE_SETTINGS_DIR). The
internal: trueandrun: "once"attributes are appropriate for this initialization task.taskfiles/deps/cmake-libs.yaml (17)
3-12: LGTM! Clear variable definitions and policy documentation.The library name constants promote consistency, and the NOTE about CMP0074 policy provides valuable context for when and why the policy is set to NEW.
29-48: LGTM! ANTLR runtime correctly configured.The task properly addresses the test build flag discussion from past comments by using
-DANTLR_BUILD_CPP_TESTS=OFF. TheCMAKE_SOURCE_DIR: "runtime/Cpp"correctly points to the ANTLR runtime subdirectory, and the CMP0135 policy ensures extracted files use current timestamps.
50-66: LGTM! Catch2 properly configured.The test framework is correctly configured with testing disabled (
-DCATCH_BUILD_TESTING=OFF) and unnecessary extras excluded. The C++20 requirement aligns with the project's standards.
81-95: LGTM! fmt library properly configured.The task correctly builds shared libraries and disables both documentation and tests with
-DFMT_DOC=OFFand-DFMT_TEST=OFF.
97-143: LGTM! LibLZMA correctly configured with dual builds.The dual-build structure (shared and static) is necessary for LibLZMA and is implemented correctly. The test flag (
-DBUILD_TESTING=OFF) and tool flags are properly set. The join pattern on lines 139-140 works correctly in this context, as confirmed in past review discussions.
145-162: LGTM! log-surgeon properly configured with dependencies.The task correctly declares dependencies on fmt and microsoft.gsl, injects their CMake settings via
-Cflags, and disables test building with-Dlog_surgeon_BUILD_TESTING=OFF.
181-204: LGTM! MariaDB connector properly configured with platform restrictions.The Linux-only platform restriction and Ubuntu precondition are appropriate for Spider dependencies. The unit tests are correctly disabled with
-DWITH_UNIT_TESTS=OFF.
206-218: LGTM! Microsoft.GSL correctly configured.The task properly disables tests with
-DGSL_TEST=OFFand uses the library name variable consistently.
262-278: LGTM! nlohmann_json properly configured.The task correctly disables tests with
-DJSON_BuildTests=OFF. The NOTE about using the GitHub-generated tarball is helpful context for developers facing firewall restrictions.
280-293: LGTM! simdjson correctly configured.The task properly builds both shared and static libraries. Per past review discussions, simdjson does not support a BUILD_TESTING flag, so the configuration is complete as-is.
318-329: LGTM! utfcpp correctly configured.The task uses v4.0.6 which, per past review discussions, does not support
UTF8CPP_ENABLE_TESTSflag. The configuration is appropriate for this version.
331-343: LGTM! yaml-cpp properly configured.The task correctly disables tests with
-DYAML_CPP_BUILD_TESTS=OFF.
345-359: LGTM! ystdlib properly configured.The task correctly depends on Boost, disables testing with
-Dystdlib_BUILD_TESTING=OFF, and sets position-independent code generation. Per previous learnings, with testing disabled, Catch2 is not required during the build.Based on learnings
361-377: LGTM! zlib correctly configured.The task properly disables examples with
-DZLIB_BUILD_EXAMPLES=OFF. Per previous learnings, zlib v1.3.1 usesenable_testingbut does not build tests when examples are disabled, so the configuration is complete. The custom install prefix is appropriate.Based on learnings
379-395: LGTM! zstd properly configured.The task correctly builds both shared and static libraries, disables programs and tests with appropriate flags, and specifies the correct
CMAKE_SOURCE_DIRfor zstd's build system.
14-27: No explicit ABSL_BUILD_TESTING flag required.Abseil’s CMakeLists defaults
ABSL_BUILD_TESTINGto OFF, so adding-DABSL_BUILD_TESTING=OFFis unnecessary.
164-179: No test-related CMake options found; no change required.
There was a problem hiding this comment.
Actionable comments posted: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (26)
taskfiles/deps/installs/absl.yaml(1 hunks)taskfiles/deps/installs/all.yaml(1 hunks)taskfiles/deps/installs/antlr-jar.yaml(1 hunks)taskfiles/deps/installs/antlr-runtime.yaml(1 hunks)taskfiles/deps/installs/boost.yaml(1 hunks)taskfiles/deps/installs/catch2.yaml(1 hunks)taskfiles/deps/installs/date.yaml(1 hunks)taskfiles/deps/installs/fmt.yaml(1 hunks)taskfiles/deps/installs/liblzma.yaml(1 hunks)taskfiles/deps/installs/log-surgeon.yaml(1 hunks)taskfiles/deps/installs/lz4.yaml(1 hunks)taskfiles/deps/installs/mariadb-connector-cpp.yaml(1 hunks)taskfiles/deps/installs/microsoft.gsl.yaml(1 hunks)taskfiles/deps/installs/mongocxx.yaml(1 hunks)taskfiles/deps/installs/msgpack-cxx.yaml(1 hunks)taskfiles/deps/installs/nlohmann_json.yaml(1 hunks)taskfiles/deps/installs/simdjson.yaml(1 hunks)taskfiles/deps/installs/spdlog.yaml(1 hunks)taskfiles/deps/installs/sqlite3.yaml(1 hunks)taskfiles/deps/installs/utfcpp.yaml(1 hunks)taskfiles/deps/installs/yaml-cpp.yaml(1 hunks)taskfiles/deps/installs/ystdlib.yaml(1 hunks)taskfiles/deps/installs/zlib.yaml(1 hunks)taskfiles/deps/installs/zstd.yaml(1 hunks)taskfiles/deps/main.yaml(1 hunks)taskfiles/deps/utils.yaml(4 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1156
File: components/core/CMakeLists.txt:772-772
Timestamp: 2025-08-09T04:07:27.083Z
Learning: In the CLP project's CMakeLists.txt, when reviewing changes related to the ${zstd_TARGET} variable usage in test linking, the team is planning a refactoring PR to improve this mechanism. Guards for undefined target variables should be deferred to that separate PR rather than being added in focused dependency migration PRs.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
📚 Learning: 2025-09-15T18:53:15.844Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1226
File: taskfiles/deps/main.yaml:522-527
Timestamp: 2025-09-15T18:53:15.844Z
Learning: When ystdlib is built with -Dystdlib_BUILD_TESTING=OFF in taskfiles/deps/main.yaml, it does not require Catch2 settings to be injected via CMAKE_GEN_ARGS since testing is disabled and Catch2 is not needed during the build process.
Applied to files:
taskfiles/deps/installs/catch2.yamltaskfiles/deps/installs/ystdlib.yaml
📚 Learning: 2025-05-28T18:33:30.155Z
Learnt from: anlowee
PR: y-scope/clp#925
File: taskfiles/deps/main.yaml:97-106
Timestamp: 2025-05-28T18:33:30.155Z
Learning: In the taskfiles dependency system (taskfiles/deps/main.yaml), echo commands are used to generate .cmake settings files that are consumed by the main CMake build process. These files set variables like ANTLR_RUNTIME_HEADER to point to dependency locations for use during compilation.
Applied to files:
taskfiles/deps/installs/antlr-runtime.yamltaskfiles/deps/installs/antlr-jar.yaml
📚 Learning: 2025-07-23T09:54:45.185Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
Applied to files:
taskfiles/deps/installs/zlib.yamltaskfiles/deps/installs/lz4.yamltaskfiles/deps/installs/liblzma.yaml
📚 Learning: 2025-08-25T07:27:19.449Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1262
File: docs/src/dev-guide/components-core/index.md:50-50
Timestamp: 2025-08-25T07:27:19.449Z
Learning: In the CLP project, nlohmann_json version 3.11.3 is installed through their dependency management process, but they don't enforce version constraints in CMake find_package calls - they control versions through installation rather than CMake version pinning.
Applied to files:
taskfiles/deps/installs/nlohmann_json.yaml
🪛 GitHub Actions: clp-lint
taskfiles/deps/installs/msgpack-cxx.yaml
[error] 31-31: empty-lines: too many blank lines (1 > 0) in YAML file.
🪛 GitHub Check: lint-check (ubuntu-24.04)
taskfiles/deps/installs/antlr-runtime.yaml
[failure] 25-25:
25:1 [empty-lines] too many blank lines (1 > 0)
taskfiles/deps/installs/mongocxx.yaml
[failure] 24-24:
24:1 [empty-lines] too many blank lines (1 > 0)
taskfiles/deps/installs/msgpack-cxx.yaml
[failure] 31-31:
31:1 [empty-lines] too many blank lines (1 > 0)
🪛 YAMLlint (1.37.1)
taskfiles/deps/installs/antlr-runtime.yaml
[error] 25-25: too many blank lines (1 > 0)
(empty-lines)
taskfiles/deps/installs/mongocxx.yaml
[error] 24-24: too many blank lines (1 > 0)
(empty-lines)
taskfiles/deps/installs/msgpack-cxx.yaml
[error] 31-31: too many blank lines (1 > 0)
(empty-lines)
⏰ 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). (17)
- GitHub Check: musllinux_1_2-x86_64-dynamic-linked-bins
- GitHub Check: musllinux_1_2-x86_64-static-linked-bins
- GitHub Check: centos-stream-9-dynamic-linked-bins
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: manylinux_2_28-x86_64-static-linked-bins
- GitHub Check: ubuntu-jammy-dynamic-linked-bins
- GitHub Check: manylinux_2_28-x86_64-dynamic-linked-bins
- GitHub Check: package-image
- GitHub Check: ubuntu-jammy-lint
- GitHub Check: ubuntu-jammy-static-linked-bins
- GitHub Check: build-macos (macos-14, true)
- GitHub Check: build-macos (macos-15, true)
- GitHub Check: build-macos (macos-15, false)
- GitHub Check: build-macos (macos-14, false)
- GitHub Check: rust-checks (ubuntu-22.04)
- GitHub Check: rust-checks (ubuntu-24.04)
- GitHub Check: rust-checks (macos-15)
🔇 Additional comments (14)
taskfiles/deps/utils.yaml (2)
54-64: LGTM! Well-structured dynamic source glob.The INSTALL_SOURCE_GLOB logic correctly constructs a Task glob pattern that includes the install task's own YAML file plus all dependency YAML files. This ensures proper source tracking and cache invalidation when dependencies change.
74-97: LGTM! Namespace alignment with colon-prefixed task references.The task dependencies and commands now use fully-qualified colon-prefixed references (e.g.,
:yscope-dev-utils:checksum:validate), which improves namespace clarity and aligns with the PR's modularization objectives.taskfiles/deps/main.yaml (1)
47-54: LGTM! Centralized initialization addresses concurrency concerns.Moving the
inittask to main.yaml centralizes directory creation and ensures it's invoked only by top-level dependency tasks. This aligns with the PR's objective to address the race condition where multiple targets (core, spider) were invokinginstall-deps-and-generate-settingsconcurrently with the sameCMAKE_SETTINGS_DIR.taskfiles/deps/installs/spdlog.yaml (1)
30-32: LGTM! Helpful compatibility note.The comment about spdlog-fmt version compatibility is valuable documentation for future maintainers who may need to upgrade either library.
taskfiles/deps/installs/catch2.yaml (1)
15-16: Verify C++20 requirement is necessary.The explicit C++20 requirement may constrain projects that need to build with earlier C++ standards. Ensure this is intentional and aligns with the project's minimum C++ standard requirements.
taskfiles/deps/installs/ystdlib.yaml (1)
19-19: LGTM! Testing disabled as expected.As per learnings, when ystdlib is built with
-Dystdlib_BUILD_TESTING=OFF, it does not require Catch2 settings since testing is disabled. This is correctly implemented here.Based on learnings
taskfiles/deps/installs/zstd.yaml (1)
10-21: zstd install config looks good.Flags match upstream CMake options; source dir and tarball details look correct.
taskfiles/deps/installs/all.yaml (1)
8-31: Mapping file looks consistent.Includes are coherent and align with per-dependency taskfiles.
taskfiles/deps/installs/absl.yaml (1)
10-18: absl install config LGTM.Reasonable defaults (Release, C++20); tarball URL/SHA provided.
taskfiles/deps/installs/microsoft.gsl.yaml (1)
14-16: LIB_NAME is correctly sourced. G_GSL_LIB_NAME is defined in taskfiles/deps/main.yaml as "Microsoft.GSL", matching the expected CMake package name.taskfiles/deps/installs/lz4.yaml (1)
11-16: Retain BUILD_STATIC_LIBS as valid LZ4 option
BUILD_STATIC_LIBS is a supported, LZ4-specific CMake switch that LZ4 honours alongside BUILD_SHARED_LIBS.taskfiles/deps/installs/mongocxx.yaml (1)
12-18: No changes required
BUILD_SHARED_AND_STATIC_LIBS and BUILD_SHARED_LIBS_WITH_STATIC_MONGOC are valid upstream CMake flags in mongo-cxx-driver ≥ 3.5 (including r4.1.1).taskfiles/deps/installs/log-surgeon.yaml (1)
3-13: Structure and deps orchestration look good.The deps loop and once semantics align with the modularised flow. No issues spotted with namespacing or invocation order.
taskfiles/deps/installs/sqlite3.yaml (1)
17-23: Cannot verify task parameter contract; submodule not available in sandbox.The yscope-dev-utils repository is a Git submodule (https://github.com/y-scope/yscope-dev-utils.git) that is not cloned in this environment. Without access to the actual
download-and-extract-ziptask definition, I cannot confirm whether bothCHECKSUM_FILEandFILE_SHA256are valid simultaneously, nor can I determine the task's validation logic.Your original concern remains valid: passing both an MD5-named checksum file and an inline SHA256 hash to the same task is potentially ambiguous and warrants clarification from the external task's documentation or source code.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
taskfiles/deps/installs/log-surgeon.yaml (1)
24-24: Prefer full commit SHA in the TARBALL_URL for reproducibility.The URL uses a short 7-character commit SHA, which can be ambiguous and less stable than a full 40-character commit SHA. After updating the URL to use the full commit SHA, ensure TARBALL_SHA256 is updated to match the new tarball.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (27)
taskfiles/deps/core.yaml(1 hunks)taskfiles/deps/installs/absl.yaml(1 hunks)taskfiles/deps/installs/antlr-jar.yaml(1 hunks)taskfiles/deps/installs/antlr-runtime.yaml(1 hunks)taskfiles/deps/installs/boost.yaml(1 hunks)taskfiles/deps/installs/catch2.yaml(1 hunks)taskfiles/deps/installs/date.yaml(1 hunks)taskfiles/deps/installs/fmt.yaml(1 hunks)taskfiles/deps/installs/liblzma.yaml(1 hunks)taskfiles/deps/installs/log-surgeon.yaml(1 hunks)taskfiles/deps/installs/lz4.yaml(1 hunks)taskfiles/deps/installs/mariadb-connector-cpp.yaml(1 hunks)taskfiles/deps/installs/microsoft.gsl.yaml(1 hunks)taskfiles/deps/installs/mongocxx.yaml(1 hunks)taskfiles/deps/installs/msgpack-cxx.yaml(1 hunks)taskfiles/deps/installs/nlohmann_json.yaml(1 hunks)taskfiles/deps/installs/simdjson.yaml(1 hunks)taskfiles/deps/installs/spdlog.yaml(1 hunks)taskfiles/deps/installs/sqlite3.yaml(1 hunks)taskfiles/deps/installs/utfcpp.yaml(1 hunks)taskfiles/deps/installs/yaml-cpp.yaml(1 hunks)taskfiles/deps/installs/ystdlib.yaml(1 hunks)taskfiles/deps/installs/zlib.yaml(1 hunks)taskfiles/deps/installs/zstd.yaml(1 hunks)taskfiles/deps/main.yaml(2 hunks)taskfiles/deps/spider.yaml(1 hunks)taskfiles/deps/utils.yaml(3 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1156
File: components/core/CMakeLists.txt:772-772
Timestamp: 2025-08-09T04:07:27.083Z
Learning: In the CLP project's CMakeLists.txt, when reviewing changes related to the ${zstd_TARGET} variable usage in test linking, the team is planning a refactoring PR to improve this mechanism. Guards for undefined target variables should be deferred to that separate PR rather than being added in focused dependency migration PRs.
📚 Learning: 2025-07-23T09:54:45.185Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
Applied to files:
taskfiles/deps/installs/liblzma.yamltaskfiles/deps/installs/zlib.yamltaskfiles/deps/installs/lz4.yaml
📚 Learning: 2025-09-15T18:53:15.844Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1226
File: taskfiles/deps/main.yaml:522-527
Timestamp: 2025-09-15T18:53:15.844Z
Learning: When ystdlib is built with -Dystdlib_BUILD_TESTING=OFF in taskfiles/deps/main.yaml, it does not require Catch2 settings to be injected via CMAKE_GEN_ARGS since testing is disabled and Catch2 is not needed during the build process.
Applied to files:
taskfiles/deps/installs/ystdlib.yamltaskfiles/deps/installs/catch2.yaml
📚 Learning: 2025-08-25T07:27:19.449Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1262
File: docs/src/dev-guide/components-core/index.md:50-50
Timestamp: 2025-08-25T07:27:19.449Z
Learning: In the CLP project, nlohmann_json version 3.11.3 is installed through their dependency management process, but they don't enforce version constraints in CMake find_package calls - they control versions through installation rather than CMake version pinning.
Applied to files:
taskfiles/deps/installs/nlohmann_json.yaml
📚 Learning: 2025-05-28T18:33:30.155Z
Learnt from: anlowee
PR: y-scope/clp#925
File: taskfiles/deps/main.yaml:97-106
Timestamp: 2025-05-28T18:33:30.155Z
Learning: In the taskfiles dependency system (taskfiles/deps/main.yaml), echo commands are used to generate .cmake settings files that are consumed by the main CMake build process. These files set variables like ANTLR_RUNTIME_HEADER to point to dependency locations for use during compilation.
Applied to files:
taskfiles/deps/installs/antlr-runtime.yaml
🔇 Additional comments (24)
taskfiles/deps/installs/absl.yaml (1)
1-17: LGTM — Standard library install pattern.The task structure and CMake configuration look correct. Verify that the absence of an explicit
:initdependency aligns with your top-level orchestrator's invocation strategy (i.e., whether:initis called bydeps:core-all-parallelor similar parent tasks before individual library installs).taskfiles/deps/installs/yaml-cpp.yaml (1)
1-16: LGTM — Consistent testing flag application.The
-DYAML_CPP_BUILD_TESTS=OFFflag aligns with the testing-disable policy discussed in the PR. Configuration and structure are correct.taskfiles/deps/installs/ystdlib.yaml (1)
1-22: Verify global variable availability and:initdependency consistency.Two observations:
- Line 17 references
{{.G_BOOST_LIB_NAME}}, which is not defined locally in this file or in boost.yaml. Confirm this variable is initialized globally (e.g., in main.yaml or utils.yaml).- This file explicitly depends on
:init(line 11), but simpler install tasks like absl.yaml and yaml-cpp.yaml do not. Clarify whether individual library installs should depend on:initor if orchestration handles this at the top level (per PR objectives: "utils:init moved into main.yaml and is invoked only by top-level deps tasks").taskfiles/deps/installs/boost.yaml (1)
1-26: VerifyG_BOOST_LIB_NAMEis set after boost task completion.The boost install uses a specialized
:yscope-dev-utils:boost:download-and-installtask rather than the generic:utils:install-remote-cmake-libused by other libraries. Verify that this task setsG_BOOST_LIB_NAME = "boost"as a global variable, since downstream tasks (ystdlib.yaml, msgpack-cxx.yaml) reference this variable in their CMake settings path (line 17 of ystdlib.yaml).Also clarify whether the explicit
:initdependency here is required, given the PR objective to move:initinvocation to top-level tasks only.taskfiles/deps/installs/lz4.yaml (1)
1-19: LGTM — Correct library-specific CMake configuration.The
CMAKE_SOURCE_DIR: "build/cmake"override correctly targets lz4's actual CMakeLists.txt location, and the disable flags are appropriate. Note the consistency question about:initdependency mentioned in other files.taskfiles/deps/installs/mongocxx.yaml (1)
1-22: Verify no trailing blank lines present.Past review comments flagged a trailing blank line in this file. Ensure the file ends immediately after line 22 with no extra blank lines, per YAML linting requirements.
Otherwise, the mongocxx install task structure and CMake configuration look correct.
taskfiles/deps/installs/utfcpp.yaml (1)
1-24: LGTM — Appropriate header-only library handling.The approach of downloading, extracting, and generating a CMake include-directory file is correct for header-only libraries like utfcpp. The shell command (lines 21–24) properly generates the
.cmakefile that downstream tasks can reference.Confirm that the variable
CLP_UTFCPP_INCLUDE_DIRECTORYnaming is consistent with how CMakeLists.txt expects it.taskfiles/deps/installs/msgpack-cxx.yaml (1)
1-27: Verify no trailing blank lines and global variable availability.Two items:
- Past review flagged a trailing blank line in this file. Ensure the file ends immediately after line 27 with no extra blank lines.
- Line 18 references
{{.G_BOOST_LIB_NAME}}, which must be available globally after the boost task completes (verified by dependency expansion on lines 10–13). Confirm this variable is initialized.Otherwise, the msgpack-cxx install task structure, per-item dependency expansion, and CMake configuration are correct.
taskfiles/deps/installs/simdjson.yaml (1)
1-17: LGTM! Follows established installation pattern.The SIMDJSON installation task correctly delegates to the remote CMake installer with appropriate build configuration (shared and static libraries in Release mode).
taskfiles/deps/installs/zlib.yaml (1)
7-18: LIB_NAME is redundantly re-assigned at task scope.Line 7 defines
LIB_NAME: "zlib", which is then re-assigned at line 18 asLIB_NAME: "{{.LIB_NAME}}"(referencing the parent scope). If this is intentional for Taskfile scoping semantics, keep it; otherwise, line 18 can be removed.taskfiles/deps/installs/catch2.yaml (1)
1-20: LGTM! Comprehensive build flags and C++ standard well configured.Catch2 installation properly disables testing, documentation, and extras to minimize build overhead, and correctly specifies C++20 standard for v3.8.0 compatibility.
taskfiles/deps/installs/spdlog.yaml (1)
1-29: LGTM! Dependency handling pattern well implemented.The spdlog task correctly manages fmt dependency by: (1) invoking fmt installation first via the
deps:section, (2) including fmt's CMake settings file via-Cflag, and (3) enabling external fmt linking. The comment clarifying fmt compatibility is valuable for future maintainers.taskfiles/deps/installs/microsoft.gsl.yaml (1)
1-16: LGTM! Simple and focused GSL installation configuration.The templated
LIB_NAMEvariable and focused set of CMake arguments appropriately configure GSL v4.0.0 for installation with tests disabled.taskfiles/deps/installs/date.yaml (1)
1-15: LGTM! Minimal, clean date library installation.The date library installation correctly specifies v3.0.1 with standard Release and LAZY install message flags, following the established pattern.
taskfiles/deps/installs/fmt.yaml (1)
1-18: LGTM! fmt library properly configured with shared build and testing disabled.The fmt v11.2.0 installation appropriately enables shared libraries while disabling documentation and tests, and uses templated
LIB_NAMEconsistent with other global variables.taskfiles/deps/installs/antlr-runtime.yaml (2)
15-19: LGTM! CMAKE_SOURCE_DIR and CMP0135 policy well configured.The
CMAKE_SOURCE_DIR: "runtime/Cpp"correctly targets ANTLR's subdirectory containing CMakeLists.txt, and the CMP0135 policy with explanatory comment ensures proper timestamp handling for extracted files and rebuild triggers.
22-23: Verify line continuation in TARBALL_URL.The TARBALL_URL spans lines 22-23 using line continuation. Ensure YAML parser correctly interprets this continuation and that the resulting URL is valid. Verify the URL renders correctly in test execution.
taskfiles/deps/spider.yaml (4)
25-26: Verify external variable definitions.Line 25 references
{{.G_SPIDER_BUILD_DIR}}and line 26 references{{.G_CPP_MAX_PARALLELISM_PER_BUILD_TASK}}, neither of which are defined in the provided snippet. Ensure these variables are defined in a parent Taskfile or included configuration file that feeds into this file.Also applies to: 28-28
11-31: Correct structure and namespacing for spider build task.The platform restriction, Ubuntu-only precondition, and namespaced
:yscope-dev-utils:cmake:install-remote-tarinvocation are all correctly configured. The CMake generator arguments follow the established policy (Release build type, lazy install message, target-specific BUILD_TESTING flag).
33-43: Correct orchestration of spider dependency installation.The spider-dep task correctly depends on init and checksum cleanup, invokes the settings generator with spider:spider-all-parallel as the dependency task, and finalizes with checksum combination. Namespacing is consistent.
45-62: All CPP_SPIDER_TASKS are correctly defined and available.Verification confirms that all nine tasks (absl, antlr-jar, antlr-runtime, boost, fmt, mariadb-connector-cpp, msgpack-cxx, spdlog, ystdlib) are defined in taskfiles/deps/installs/all.yaml. The for-loop expansion in spider-all-parallel will correctly generate parallel task dependencies via the
:{{.ITEM}}pattern.taskfiles/deps/main.yaml (3)
3-17: Correct modularization of includes for target-specific workflows.The public includes now properly expose core, log-viewer, and spider as separate target workflows, with components flattened into the root namespace. This structure enables clean separation of concerns and supports the new modular dependency installation approach described in the PR objectives.
19-29: Clear variable organization and new checksum directory variable.The vars section correctly adds G_DEPS_CPP_CHECKSUMS_DIR as a derived path and improves documentation through section comments. Naming and scoping are consistent with project conventions.
40-54: Correct default task expansion and centralized init task structure.The default task now properly includes spider alongside core and log-viewer per prior review approval. The centralized init task correctly creates the full directory hierarchy (parent G_DEPS_CPP_DIR before child directories) with idempotent mkdir -p commands, marked to run exactly once. All referenced directories have corresponding variables defined.
| version: "3" | ||
|
|
||
| tasks: | ||
| default: | ||
| internal: true | ||
| run: "once" | ||
| cmds: | ||
| - task: ":utils:install-remote-cmake-lib" | ||
| vars: | ||
| CMAKE_GEN_ARGS: | ||
| - "-DCMAKE_BUILD_TYPE=Release" | ||
| - "-DCMAKE_INSTALL_MESSAGE=LAZY" | ||
| - "-DZSTD_BUILD_PROGRAMS=OFF" | ||
| - "-DZSTD_BUILD_SHARED=ON" | ||
| - "-DZSTD_BUILD_STATIC=ON" | ||
| - "-DZSTD_BUILD_TESTS=OFF" | ||
| CMAKE_SOURCE_DIR: "build/cmake" | ||
| LIB_NAME: "zstd" | ||
| TARBALL_SHA256: "eb33e51f49a15e023950cd7825ca74a4a2b43db8354825ac24fc1b7ee09e6fa3" | ||
| TARBALL_URL: "https://github.com/facebook/zstd/releases/download/v1.5.7/zstd-1.5.7.tar.gz" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify SHA-256 hash integrity and ZSTD_BUILD_TESTS policy compliance.
The file follows the established installation pattern, but requires verification on two fronts:
- SHA-256 hash: Confirm the TARBALL_SHA256 matches the official zstd v1.5.7 release for security.
- ZSTD_BUILD_TESTS flag: Per PR discussions, the team is implementing a policy where testing flags are disabled only if they default to ON (option 3). Verify that ZSTD_BUILD_TESTS defaults to ON in zstd's CMakeLists.txt; if it defaults to OFF, this flag is redundant.
- CMAKE_SOURCE_DIR: Confirm that "build/cmake" is the correct subdirectory path containing the CMakeLists.txt in the zstd v1.5.7 release.
🏁 Script executed:
#!/bin/bash
# Description: Verify zstd v1.5.7 release integrity and CMakeLists.txt build option defaults
set -e
# Create temp directory for extraction
TEMP_DIR=$(mktemp -d)
trap "rm -rf $TEMP_DIR" EXIT
cd "$TEMP_DIR"
# Download zstd v1.5.7 release
echo "Downloading zstd v1.5.7..."
curl -sLo zstd-1.5.7.tar.gz "https://github.com/facebook/zstd/releases/download/v1.5.7/zstd-1.5.7.tar.gz"
# Verify SHA-256
EXPECTED_SHA="eb33e51f49a15e023950cd7825ca74a4a2b43db8354825ac24fc1b7ee09e6fa3"
ACTUAL_SHA=$(sha256sum zstd-1.5.7.tar.gz | awk '{print $1}')
echo "Expected SHA-256: $EXPECTED_SHA"
echo "Actual SHA-256: $ACTUAL_SHA"
if [ "$EXPECTED_SHA" != "$ACTUAL_SHA" ]; then
echo "ERROR: SHA-256 mismatch!"
exit 1
fi
echo "✓ SHA-256 verified"
# Extract and check structure
tar -xzf zstd-1.5.7.tar.gz
ls -la zstd-1.5.7/
# Check for CMakeLists.txt in build/cmake directory
if [ -f "zstd-1.5.7/build/cmake/CMakeLists.txt" ]; then
echo "✓ Found CMakeLists.txt at build/cmake/"
else
echo "✗ CMakeLists.txt not found at build/cmake/"
echo "Checking alternative locations..."
find zstd-1.5.7 -name "CMakeLists.txt" -type f
fi
# Extract ZSTD_BUILD_TESTS default value from CMakeLists.txt
echo ""
echo "Checking ZSTD_BUILD_TESTS default in build/cmake/CMakeLists.txt:"
if grep -n "ZSTD_BUILD_TESTS" zstd-1.5.7/build/cmake/CMakeLists.txt; then
echo "Found ZSTD_BUILD_TESTS option definition"
else
echo "ZSTD_BUILD_TESTS not found in build/cmake/CMakeLists.txt"
fiLength of output: 202
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: Verify zstd v1.5.7 SHA-256 hash using alternative methods and inspect CMakeLists.txt
set -e
# Create temp directory for extraction
TEMP_DIR=$(mktemp -d)
trap "rm -rf $TEMP_DIR" EXIT
cd "$TEMP_DIR"
# Download zstd v1.5.7 release
echo "Downloading zstd v1.5.7..."
curl -sLo zstd-1.5.7.tar.gz "https://github.com/facebook/zstd/releases/download/v1.5.7/zstd-1.5.7.tar.gz"
# Verify SHA-256 using Python
EXPECTED_SHA="eb33e51f49a15e023950cd7825ca74a4a2b43db8354825ac24fc1b7ee09e6fa3"
echo "Verifying SHA-256 hash using Python..."
ACTUAL_SHA=$(python3 -c "import hashlib; print(hashlib.sha256(open('zstd-1.5.7.tar.gz', 'rb').read()).hexdigest())")
echo "Expected SHA-256: $EXPECTED_SHA"
echo "Actual SHA-256: $ACTUAL_SHA"
if [ "$EXPECTED_SHA" = "$ACTUAL_SHA" ]; then
echo "✓ SHA-256 verified successfully"
else
echo "✗ SHA-256 mismatch detected"
exit 1
fi
# Extract tarball
echo ""
echo "Extracting zstd-1.5.7.tar.gz..."
tar -xzf zstd-1.5.7.tar.gz
# Check directory structure
echo ""
echo "Checking zstd-1.5.7 directory structure:"
ls -la zstd-1.5.7/
# Check for CMakeLists.txt in build/cmake directory
echo ""
echo "Searching for CMakeLists.txt files in zstd-1.5.7:"
find zstd-1.5.7 -name "CMakeLists.txt" -type f
# Check if build/cmake/CMakeLists.txt exists
if [ -f "zstd-1.5.7/build/cmake/CMakeLists.txt" ]; then
echo "✓ Found CMakeLists.txt at zstd-1.5.7/build/cmake/CMakeLists.txt"
# Search for ZSTD_BUILD_TESTS option
echo ""
echo "Searching for ZSTD_BUILD_TESTS in build/cmake/CMakeLists.txt:"
grep -n "ZSTD_BUILD_TESTS" zstd-1.5.7/build/cmake/CMakeLists.txt || echo "Not found in build/cmake/CMakeLists.txt"
# Also check parent CMakeLists.txt if it exists
if [ -f "zstd-1.5.7/CMakeLists.txt" ]; then
echo ""
echo "Searching for ZSTD_BUILD_TESTS in zstd-1.5.7/CMakeLists.txt:"
grep -n "ZSTD_BUILD_TESTS" zstd-1.5.7/CMakeLists.txt || echo "Not found in root CMakeLists.txt"
fi
# Display the context around ZSTD_BUILD_TESTS for default value inspection
echo ""
echo "Context around ZSTD_BUILD_TESTS definitions:"
grep -B 2 -A 2 "ZSTD_BUILD_TESTS" zstd-1.5.7/build/cmake/CMakeLists.txt zstd-1.5.7/CMakeLists.txt 2>/dev/null || true
else
echo "✗ CMakeLists.txt not found at zstd-1.5.7/build/cmake/CMakeLists.txt"
fi
Length of output: 3636
Remove redundant ZSTD_BUILD_TESTS=OFF flag per PR policy.
Verification confirms SHA-256 hash integrity and CMAKE_SOURCE_DIR path are correct. However, the ZSTD_BUILD_TESTS CMake option defaults to OFF in zstd v1.5.7 (its default depends on the BUILD_TESTING variable, which itself defaults to OFF in CMake). Per the PR's stated policy of disabling testing flags only when they default to ON, this flag is redundant and should be removed from CMAKE_GEN_ARGS.
🤖 Prompt for AI Agents
In taskfiles/deps/installs/zstd.yaml around lines 1 to 20, remove the redundant
"-DZSTD_BUILD_TESTS=OFF" entry from the CMAKE_GEN_ARGS list because zstd v1.5.7
already defaults tests off; just delete that line and leave the other CMake args
(and keep the existing TARBALL_SHA256, TARBALL_URL and CMAKE_SOURCE_DIR
unchanged).
5 participants
Description
To address #1477, we split each library install task into its own file inside
taskfiles/deps/installs. In addition, we also group tasks for each major target into respective files. So far the major targets arecore,log-viewerandspider.Other than the structural copy-paste refactoring, this PR also includes several one big and several small adjustments.
Big adjustment:
antlr-runtimehas been modified to be installable viautils:install-remote-cmake-lib. Instead of extracting the downloaded tar and removing the external layers outside the Cpp directory, we directly specify the directory whereCMakelists.txtlives.Small adjustments:
deps:coreanddeps:spider-depno longer resets the cmake settings directory, which is already done byyscope-dev-utils:cmake:install-deps-and-generate-settingsdeps:sections ofdeps:<target>-all-parallelnow iterate over list variables.deps:sections of lib install tasks that depend on other libraries.utils:inittask has been moved intomain.yaml.utils:andyscope-dev-utils:inside some taskfiles are now prefixed with a leading colon (:) to align with updated namespaces.Checklist
breaking change.
Validation performed
Summary by CodeRabbit