refactor(taskfile): Split main dependency Taskfile into category-specific modules; Migrate antlr-runtime and utfcpp to use utils:install-remote-cmake-lib.
#1377
Conversation
WalkthroughAdds many dependency task definitions and reorganizes dependency orchestration: new CMake-based library tasks, misc library tasks, artifact download tasks, and refactored init/utility wiring to centralize checksum and CMake-settings generation and installation flows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Main as deps/main.yaml
participant Core as deps/core.yaml:core
participant Utils as :yscope-dev-utils
participant CMakeLibs as deps/cmake-libs.yaml
participant Misc as deps/misc-libs.yaml
participant Checks as checksum-aggregator
Dev->>Main: run deps (default)
Main->>Core: invoke core
Core->>Utils: cmake:install-deps-and-generate-settings (CMAKE_SETTINGS_DIR, FILE)
Utils->>CMakeLibs: trigger per-lib download/build/install
Utils->>Misc: trigger misc lib installs/artifacts
CMakeLibs->>Checks: emit per-lib checksum files
Misc->>Checks: emit per-lib checksum files
Core->>Checks: combine-cpp-checksum-files
Core-->>Dev: complete
sequenceDiagram
autonumber
participant Dev as Developer
participant Spider as deps/spider.yaml:spider
participant SpiderDep as deps/spider.yaml:spider-dep
participant Utils as :yscope-dev-utils
participant Installer as cmake-installer
Dev->>Spider: run spider (linux/Ubuntu)
Spider->>SpiderDep: ensure deps & settings
SpiderDep->>Utils: clean-outdated-cpp-checksum-files
SpiderDep->>Utils: cmake:install-deps-and-generate-settings
SpiderDep->>Utils: combine-cpp-checksum-files
Spider->>Installer: install-remote-tar (Spider artifact, URL, SHA256, CMAKE args)
Installer-->>Dev: Spider installed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (3 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. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
taskfiles/deps/artifacts.yaml(1 hunks)taskfiles/deps/main.yaml(1 hunks)taskfiles/deps/misc-libs.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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
⏰ 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: package-image
- GitHub Check: ubuntu-jammy-static-linked-bins
- GitHub Check: ubuntu-jammy-dynamic-linked-bins
- GitHub Check: musllinux_1_2-x86_64-static-linked-bins
- GitHub Check: musllinux_1_2-x86_64-dynamic-linked-bins
- GitHub Check: manylinux_2_28-x86_64-static-linked-bins
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: manylinux_2_28-x86_64-dynamic-linked-bins
- GitHub Check: centos-stream-9-dynamic-linked-bins
- GitHub Check: build-macos (macos-14, true)
- GitHub Check: build-macos (macos-15, false)
- GitHub Check: build-macos (macos-14, false)
- GitHub Check: rust-checks (macos-15)
- GitHub Check: rust-checks (ubuntu-22.04)
- GitHub Check: rust-checks (ubuntu-24.04)
- GitHub Check: lint-check (macos-15)
- GitHub Check: lint-check (ubuntu-24.04)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
taskfiles/deps/artifacts.yaml(1 hunks)taskfiles/deps/cmake-libs.yaml(1 hunks)taskfiles/deps/main.yaml(1 hunks)taskfiles/deps/misc-libs.yaml(1 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). (4)
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: rust-checks (ubuntu-22.04)
- GitHub Check: rust-checks (ubuntu-24.04)
- GitHub Check: build-macos (macos-14, false)
| preconditions: | ||
| # NOTE: The MariaDB connector is only required for building Spider, which is only supported on | ||
| # Ubuntu. `INSTALL_LAYOUT` is currently hardcoded to "DEB". To support other Linux distros: | ||
| # - Update the precondition accordingly. | ||
| # - Make `INSTALL_LAYOUT` configurable. | ||
| - >- | ||
| source /etc/os-release && [[ "$ID" == "ubuntu" ]] | ||
| run: "once" |
There was a problem hiding this comment.
Fix bashisms in precondition.
Task preconditions run under /bin/sh, so source and [[ … ]] are undefined and the check exits 127 even on Ubuntu, blocking the install. Use POSIX-compatible syntax instead.
- - >-
- source /etc/os-release && [[ "$ID" == "ubuntu" ]]
+ - >-
+ . /etc/os-release && [ "$ID" = "ubuntu" ]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| preconditions: | |
| # NOTE: The MariaDB connector is only required for building Spider, which is only supported on | |
| # Ubuntu. `INSTALL_LAYOUT` is currently hardcoded to "DEB". To support other Linux distros: | |
| # - Update the precondition accordingly. | |
| # - Make `INSTALL_LAYOUT` configurable. | |
| - >- | |
| source /etc/os-release && [[ "$ID" == "ubuntu" ]] | |
| run: "once" | |
| preconditions: | |
| # NOTE: The MariaDB connector is only required for building Spider, which is only supported on | |
| # Ubuntu. `INSTALL_LAYOUT` is currently hardcoded to "DEB". To support other Linux distros: | |
| # - Update the precondition accordingly. | |
| # - Make `INSTALL_LAYOUT` configurable. | |
| - >- | |
| . /etc/os-release && [ "$ID" = "ubuntu" ] | |
| run: "once" |
🤖 Prompt for AI Agents
In taskfiles/deps/cmake-libs.yaml around lines 160 to 167, the precondition uses
bash-only constructs (`source` and `[[ … ]]`) which fail under /bin/sh; replace
them with POSIX equivalents: use `. /etc/os-release` instead of `source
/etc/os-release`, and use `[ "$ID" = "ubuntu" ]` (single `=`) instead of `[[
"$ID" == "ubuntu" ]]`; ensure the whole command exits with the appropriate
success/failure status (e.g., chain the dot and test so the test runs only if
the file exists).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
taskfiles/deps/cmake-libs.yaml (1)
165-167: Replace bashisms in the MariaDB precondition.
sourceand[[ … ]]aren’t available in/bin/sh, so this guard fails even on Ubuntu. Use POSIX syntax so the task can run.- - >- - source /etc/os-release && [[ "$ID" == "ubuntu" ]] + - >- + [ -r /etc/os-release ] && . /etc/os-release && [ "$ID" = "ubuntu" ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
taskfiles/deps/cmake-libs.yaml(1 hunks)taskfiles/deps/core.yaml(1 hunks)taskfiles/deps/misc-libs.yaml(1 hunks)taskfiles/deps/spider.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
📚 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-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
⏰ 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: ubuntu-jammy-dynamic-linked-bins
- GitHub Check: ubuntu-jammy-static-linked-bins
- GitHub Check: ubuntu-jammy-lint
- GitHub Check: package-image
- GitHub Check: manylinux_2_28-x86_64-static-linked-bins
- GitHub Check: manylinux_2_28-x86_64-dynamic-linked-bins
- GitHub Check: centos-stream-9-dynamic-linked-bins
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: musllinux_1_2-x86_64-dynamic-linked-bins
- GitHub Check: musllinux_1_2-x86_64-static-linked-bins
- GitHub Check: rust-checks (ubuntu-24.04)
- GitHub Check: rust-checks (ubuntu-22.04)
- GitHub Check: lint-check (ubuntu-24.04)
- GitHub Check: build-macos (macos-15, false)
- GitHub Check: build-macos (macos-14, true)
- GitHub Check: build-macos (macos-14, false)
- GitHub Check: build-macos (macos-15, true)
| source /etc/os-release && [[ "$ID" == "ubuntu" ]] | ||
| cmds: |
There was a problem hiding this comment.
Use POSIX-compatible precondition syntax.
source and [[ … ]] are bash-only, so this precondition exits under /bin/sh, blocking the task on Ubuntu. Switch to POSIX forms so the check succeeds where it should.
- - >-
- source /etc/os-release && [[ "$ID" == "ubuntu" ]]
+ - >-
+ [ -r /etc/os-release ] && . /etc/os-release && [ "$ID" = "ubuntu" ]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| source /etc/os-release && [[ "$ID" == "ubuntu" ]] | |
| cmds: | |
| - >- | |
| [ -r /etc/os-release ] && . /etc/os-release && [ "$ID" = "ubuntu" ] | |
| cmds: |
🤖 Prompt for AI Agents
In taskfiles/deps/spider.yaml around lines 15–16, the precondition uses
bash-only syntax ("source" and "[[ ... ]]") which breaks under /bin/sh; replace
with POSIX-compatible forms: use ". /etc/os-release" (or "if [ -r
/etc/os-release ]; then . /etc/os-release; fi") and the single-bracket string
test "[ "$ID" = "ubuntu" ]" (or use grep/awk to check ID from /etc/os-release)
so the conditional runs correctly under /bin/sh.
There was a problem hiding this comment.
@sitaowang1998 Does this comment make sense to you?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
It is ok to use bash only commands. No one is really using the plain old sh, not even the GitHub workflows.
| source /etc/os-release && [[ "$ID" == "ubuntu" ]] | ||
| cmds: |
There was a problem hiding this comment.
@sitaowang1998 Does this comment make sense to you?
Bill-hbrhbr
changed the title
antlr-runtime and utfcpp to use utils:install-remote-cmake-lib.
| - "-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.
| liblzma: | ||
| internal: true | ||
| vars: | ||
| COMMON_CMAKE_GEN_ARGS: |
There was a problem hiding this comment.
COMMON_CMAKE_GEN_ARGS sounds like general arguments for all cmake libraries.
| COMMON_CMAKE_GEN_ARGS: | |
| LIBLZMA_CMAKE_GEN_ARGS: |
| vars: | ||
| BUILD_SHARED_LIBS: true | ||
| COMMON_CMAKE_GEN_ARGS: | ||
| ref: ".COMMON_CMAKE_GEN_ARGS" |
There was a problem hiding this comment.
| ref: ".COMMON_CMAKE_GEN_ARGS" | |
| ref: ".LIBLZMA_CMAKE_GEN_ARGS" |
| - "-DSPDLOG_BUILD_EXAMPLE=OFF" | ||
| - "-DSPDLOG_BUILD_EXAMPLE_HO=OFF" | ||
| - "-DSPDLOG_FMT_EXTERNAL=ON" |
There was a problem hiding this comment.
| - "-DSPDLOG_BUILD_EXAMPLE=OFF" | |
| - "-DSPDLOG_BUILD_EXAMPLE_HO=OFF" | |
| - "-DSPDLOG_FMT_EXTERNAL=ON" | |
| - "-DSPDLOG_BUILD_EXAMPLE=OFF" | |
| - "-DSPDLOG_BUILD_EXAMPLE_HO=OFF" | |
| - "-DSPDLOG_BUILD_TESTS=OFF" | |
| - "-DSPDLOG_BUILD_TESTS_HO=OFF" | |
| - "-DSPDLOG_FMT_EXTERNAL=ON" |
There was a problem hiding this comment.
SPDLOG_BUILD_EXAMPLE_HO is already OFF so we don't need it anymore
| vars: | ||
| CMAKE_GEN_ARGS: | ||
| - "-DCMAKE_BUILD_TYPE=Release" | ||
| - "-DCMAKE_INSTALL_MESSAGE=LAZY" |
There was a problem hiding this comment.
| - "-DCMAKE_INSTALL_MESSAGE=LAZY" | |
| - "-DCMAKE_INSTALL_MESSAGE=LAZY" | |
| - "-DUTF8CPP_ENABLE_TESTS=OFF" |
There was a problem hiding this comment.
UTF8CPP_ENABLE_TESTS is not in utfcpp v4.0.6 but in v4.0.8
There was a problem hiding this comment.
Look like no way to stop test from building in v4.0.6. Maybe we can leave this here with a comment in case we bump the version later.
| - "-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.
| - "spider" | ||
| - "log-viewer" |
There was a problem hiding this comment.
Sort tasks in lexical order. @LinZhihao-723 Could you confirm if we should keep spider as a default task?
| - "spider" | |
| - "log-viewer" | |
| - "log-viewer" | |
| - "spider" |
There was a problem hiding this comment.
Yeah we should. If log-viewer is then Spider should also be.
| source /etc/os-release && [[ "$ID" == "ubuntu" ]] | ||
| cmds: |
There was a problem hiding this comment.
It is ok to use bash only commands. No one is really using the plain old sh, not even the GitHub workflows.
| vars: | ||
| BUILD_SHARED_LIBS: false | ||
| COMMON_CMAKE_GEN_ARGS: | ||
| ref: ".COMMON_CMAKE_GEN_ARGS" |
There was a problem hiding this comment.
| ref: ".COMMON_CMAKE_GEN_ARGS" | |
| ref: ".LIBLZMA_CMAKE_GEN_ARGS" |
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 |
4 participants
Description
Background
Library install tasks in main.yaml are designed to be reusable, but they were only used to build the project
clp-coreviatask deps:core.With the introduction of a new project
task deps:spiderin #1318 , we quickly realized that the library installation utilities in both this repo and in yscope-dev-utils were designed in a way that prevents easily adding a second CPP project.Specifically, there were several problems:
task deps:coreis up-to-date.up-to-datestatus of individual lib installation.cmake-settingsdirectory, instead of one for each project:cmake-settingsfile that lists the union of dependencies required by bothcoreandspider.deps:coreanddeps:spider-depwrite into different top-level CMake configuration files (fixes #1330). #1331 fixes the race conditions that could occur when both projects are trying to combine cmake settings to the same file.Solution
Changes in yscope-dev-utils are required first, and then we use the new utilities to fix the race conditions above:
reset-dirandreset-filetasks and apply across exported Taskfiles; RequireOUTPUT_FILEforremote:curl. yscope-dev-utils#91To make the future bug fix logic clearer for review, this PR introduces a purely cosmetic refactorization of main.yaml first.
Later, when we finally bump the
yscope-dev-utilsversion, we will open subsequent PRs for actual fixes:Main structural changes
Previously, all dependency tasks were bundled inside a single
main.yaml. Now, we split the tasks into modular, categorized files:utils:install-remote-cmake-libdue to well-written CMake project structure.Miscellaneous Changes
Other than the structural copy-paste refactoring, this PR also includes several one big and several small adjustments.
Big adjustment:
utils:install-remote-cmake-libantlr-runtime: instead of extracting the downloaded tar and removing the external layers outside the Cpp directory, we directly specify the directory whereCMakelists.txtlives.utfcpp: being an interface-only library, is still installable. We now install it as a regular library.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:section ofdeps:<target>-all-parallelnow iterates over a list variable.utils:inittask has been moved intomain.yamland is now only invoked by the top-level deps tasks:deps:coredeps:spider:spider-deputils:andyscope-dev-utils:inside some taskfiles are now prefixed with a leading colon (:) to align with updated namespaces.Summary
Restructuring in this PR improves clarity, modularity, and maintainability, and lays the groundwork for properly supporting multiple independent CPP project targets in the future.
For the prototype of the final version, see:
Checklist
breaking change.
Validation performed
Summary by CodeRabbit
New Features
Chores