-
Notifications
You must be signed in to change notification settings - Fork 81
fix(taskfile): Simplify python-component
sources list to avoid go-task/task#2400 (fixes #872); Upgrade and pin go-task
to v3.44.0 to avoid go-task/task#2401.
#1242
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
…mic variable bug in go-task (fixes y-scope#872).
WalkthroughTask tooling is pinned to v3.44.0 across install scripts and documentation. The Taskfile’s python-component task removes the PACKAGE var and switches sources from per-component paths to a repo-wide glob with a dist exclusion. No other control flow or public API changes. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/src/dev-guide/building-package.md (1)
71-71
: Remove unused link reference (leftover from removed note).The reference
[clp-issue-872]
is no longer used in the page after removing the note; leaving it may confuse readers.Apply this diff to remove the unused reference:
-[clp-issue-872]: https://github.com/y-scope/clp/issues/872
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/src/dev-guide/building-package.md
(1 hunks)taskfile.yaml
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: haiqi96
PR: y-scope/clp#0
File: :0-0
Timestamp: 2025-08-04T18:38:33.130Z
Learning: User haiqi96 requested creating a GitHub issue to document a documentation discrepancy where Task version requirements in docs weren't updated after yscope-utils upgrade in PR #1158.
📚 Learning: 2025-08-04T18:38:33.130Z
Learnt from: haiqi96
PR: y-scope/clp#0
File: :0-0
Timestamp: 2025-08-04T18:38:33.130Z
Learning: User haiqi96 requested creating a GitHub issue to document a documentation discrepancy where Task version requirements in docs weren't updated after yscope-utils upgrade in PR #1158.
Applied to files:
docs/src/dev-guide/building-package.md
⏰ 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). (8)
- GitHub Check: ubuntu-jammy-dynamic-linked-bins
- GitHub Check: ubuntu-jammy-static-linked-bins
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: ubuntu-jammy-lint
- GitHub Check: centos-stream-9-dynamic-linked-bins
- GitHub Check: antlr-code-committed (macos-15)
- GitHub Check: build-macos (macos-15, false)
- GitHub Check: build-macos (macos-15, true)
🔇 Additional comments (1)
taskfile.yaml (1)
545-547
: Map-styleexclude
support confirmed in Task ≥ 3.32.0 (including v3.40.0) – no action neededTask has supported map-style
exclude
entries since v3.32.0 and there are no breaking changes in later 3.x releases. Your current configuration is valid as-is.
- "**/*" | ||
- exclude: "dist/**/*" | ||
dir: "components/{{.COMPONENT}}" |
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.
🧹 Nitpick (assertive)
Exclude common cache/virtualenv dirs to prevent unnecessary rebuilds.
Broadening sources to "**/*" fixes the dynamic variable bug, but it also makes the task sensitive to transient files (e.g., virtualenvs, caches) within the component directory. Excluding these reduces spurious cache invalidations and speeds up task runs without changing behaviour.
Apply this diff to harden the excludes:
- "**/*"
- - exclude: "dist/**/*"
+ - exclude: "dist/**/*"
+ - exclude: ".venv/**/*"
+ - exclude: "**/__pycache__/**/*"
+ - exclude: "**/.pytest_cache/**/*"
+ - exclude: "**/.mypy_cache/**/*"
+ - exclude: "**/.ruff_cache/**/*"
+ - exclude: "**/.tox/**/*"
+ - exclude: "**/node_modules/**/*"
🤖 Prompt for AI Agents
In taskfile.yaml around lines 545 to 547, the source pattern was broadened to
"**/*" but lacks excludes for common transient cache/virtualenv dirs, causing
spurious rebuilds; update the excludes under that block to add patterns such as
"venv/**/*", ".venv/**/*", "__pycache__/**/*", ".cache/**/*",
".mypy_cache/**/*", ".pytest_cache/**/*", "node_modules/**/*" and optionally
"*.pyc" (or other language-specific cache files) so the task ignores virtual
environments and cache directories and prevents unnecessary cache invalidations.
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.
nothing you listed should exist
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.
🧩 Analysis chain
@junhaoliao I apologize for making assumptions about your directory structure. Let me check what actually exists in the python component directories to better understand the codebase.
🏁 Script executed:
#!/bin/bash
# Check the actual structure of python component directories
echo "=== Python component directories structure ==="
find components -maxdepth 3 -type d | grep -E "(clp-package-utils|clp-py-utils|job-orchestration)" | head -20
echo -e "\n=== Contents of a sample python component directory ==="
ls -la components/clp-package-utils/ 2>/dev/null || echo "Directory not found"
echo -e "\n=== Hidden files and directories in python components ==="
find components -name ".*" -type d | head -10
echo -e "\n=== Check for common cache directories ==="
find components -name "__pycache__" -o -name ".venv" -o -name "venv" -o -name ".mypy_cache" -o -name ".pytest_cache" -o -name ".ruff_cache" -o -name ".tox" -o -name "node_modules" 2>/dev/null | head -10
Length of output: 1707
You're absolutely right! Thank you for the clarification. Looking at the actual component structure, I can see that your python component directories are kept very clean with only:
- Source code directories
pyproject.toml
andpoetry.lock
README.md
- The
dist/
directory (which you're already excluding)
No cache directories, virtual environments, or other transient files exist in the component directories. The current exclude pattern "dist/**/*"
is perfectly sufficient for your codebase structure. I apologize for making assumptions based on typical Python project layouts rather than understanding your specific, well-organized directory structure.
Your simplified solution with "**/*"
and exclude: "dist/**/*"
is exactly what's needed to fix the go-task dynamic variable bug without any unnecessary complexity.
✏️ Learnings added
Learnt from: junhaoliao
PR: y-scope/clp#1242
File: taskfile.yaml:545-547
Timestamp: 2025-08-20T05:38:30.705Z
Learning: The python component directories in the y-scope/clp repository are kept very clean and only contain essential files: source code directories, pyproject.toml, poetry.lock, README.md, and the dist/ directory. No cache directories, virtual environments, or other transient files exist in the component directories, so additional exclude patterns beyond "dist/**/*" are unnecessary.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: junhaoliao
PR: y-scope/clp#1119
File: taskfile.yaml:429-433
Timestamp: 2025-08-14T18:23:39.476Z
Learning: In the webui workspace, the exclude patterns ".tmp" and ".vite-temp" are Vite-specific temporary directories that only apply to the client package. The common and server packages use TypeScript compiler (tsc) directly and don't generate these Vite temporary files, so these exclude patterns should not be applied to their node_modules checksum computations.
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.
lgtm
Validation: Build clp with task version 3.43.3. It no longer hangs.
For the title, how about:
fix(taskfiles): Simplify python-component
sources list to avoid the dynamic variable bug in go-task (fixes #872).
python-component
sources list to avoid dynamic variable bug in go-task (fixes #872).python-component
sources list to avoid the dynamic variable bug in go-task (fixes #872).
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
taskfile.yaml
(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: haiqi96
PR: y-scope/clp#0
File: :0-0
Timestamp: 2025-08-04T18:38:33.130Z
Learning: User haiqi96 requested creating a GitHub issue to document a documentation discrepancy where Task version requirements in docs weren't updated after yscope-utils upgrade in PR #1158.
📚 Learning: 2025-08-20T05:38:30.720Z
Learnt from: junhaoliao
PR: y-scope/clp#1242
File: taskfile.yaml:545-547
Timestamp: 2025-08-20T05:38:30.720Z
Learning: The python component directories in the y-scope/clp repository are kept very clean and only contain essential files: source code directories, pyproject.toml, poetry.lock, README.md, and the dist/ directory. No cache directories, virtual environments, or other transient files exist in the component directories, so additional exclude patterns beyond "dist/**/*" are unnecessary.
Applied to files:
taskfile.yaml
📚 Learning: 2025-08-14T18:23:39.476Z
Learnt from: junhaoliao
PR: y-scope/clp#1119
File: taskfile.yaml:429-433
Timestamp: 2025-08-14T18:23:39.476Z
Learning: In the webui workspace, the exclude patterns ".tmp" and ".vite-temp" are Vite-specific temporary directories that only apply to the client package. The common and server packages use TypeScript compiler (tsc) directly and don't generate these Vite temporary files, so these exclude patterns should not be applied to their node_modules checksum computations.
Applied to files:
taskfile.yaml
🔇 Additional comments (2)
taskfile.yaml (2)
545-547
: LGTM: Simplified python-component sources fix the go-task dynamic var pitfall without over-excluding.Using "/*" with an explicit "dist//*" exclude is a clean, robust way to avoid the dynamic variable bug. Given your repository norms (no caches/venvs inside component dirs per prior clarification), keeping excludes minimal is correct. Also aligns with shopt: ["globstar"] at the top.
545-547
: Confirm Task version support forexclude:
The
exclude:
field on individual entries within thesources:
list is supported as of go-task v3.32.0 (released 2023-11-29). Therefore, usingexclude:
in v3.40.0 is fully valid, and the stated minimum requirement (Task >= 3.40.0) need not be bumped.
we should also update clp/components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh Lines 31 to 42 in 7aa8684
|
# Conflicts: # taskfile.yaml
addressed in 57e152a |
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: 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)
541-548
: Remove stale{{.PACKAGE}}/**/*
frompython-component.sources
; it references an undefined var and undermines the fix.The goal here is to avoid the dynamic-variable bug by globbing
**/*
underdir: components/{{.COMPONENT}}
. Keeping- "{{.PACKAGE}}/**/*"
(Line 542) reintroduces a dynamic reference andPACKAGE
is no longer defined in this task, which will cause a variable resolution error.Apply this diff to rely solely on
**/*
with thedist
exclusion:sources: - "{{.G_BUILD_DIR}}/{{.COMPONENT}}-venv.md5" - - "{{.PACKAGE}}/**/*" - "{{.ROOT_DIR}}/requirements.txt" - "{{.TASKFILE}}" - "/etc/os-release" - "**/*" - exclude: "dist/**/*"
Optional sanity check to ensure no remaining uses of
{{.PACKAGE}}
:#!/bin/bash # Expect no matches or only historical in unrelated areas. rg -nP '\{\{\s*\.PACKAGE\s*\}\}'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh
(1 hunks)taskfile.yaml
(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: haiqi96
PR: y-scope/clp#0
File: :0-0
Timestamp: 2025-08-04T18:38:33.130Z
Learning: User haiqi96 requested creating a GitHub issue to document a documentation discrepancy where Task version requirements in docs weren't updated after yscope-utils upgrade in PR #1158.
📚 Learning: 2025-07-07T17:41:15.655Z
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:27-32
Timestamp: 2025-07-07T17:41:15.655Z
Learning: In CLP installation scripts, consistency across platform scripts is prioritized over defensive programming improvements. For example, when extracting Task binaries with tar in `install-prebuilt-packages.sh`, the extraction pattern should remain consistent with other platform scripts rather than adding defensive flags like `--strip-components=1` to handle potential tarball layout changes.
Applied to files:
components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh
📚 Learning: 2025-08-20T05:38:30.720Z
Learnt from: junhaoliao
PR: y-scope/clp#1242
File: taskfile.yaml:545-547
Timestamp: 2025-08-20T05:38:30.720Z
Learning: The python component directories in the y-scope/clp repository are kept very clean and only contain essential files: source code directories, pyproject.toml, poetry.lock, README.md, and the dist/ directory. No cache directories, virtual environments, or other transient files exist in the component directories, so additional exclude patterns beyond "dist/**/*" are unnecessary.
Applied to files:
taskfile.yaml
📚 Learning: 2025-08-14T18:23:39.476Z
Learnt from: junhaoliao
PR: y-scope/clp#1119
File: taskfile.yaml:429-433
Timestamp: 2025-08-14T18:23:39.476Z
Learning: In the webui workspace, the exclude patterns ".tmp" and ".vite-temp" are Vite-specific temporary directories that only apply to the client package. The common and server packages use TypeScript compiler (tsc) directly and don't generate these Vite temporary files, so these exclude patterns should not be applied to their node_modules checksum computations.
Applied to files:
taskfile.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). (5)
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: centos-stream-9-dynamic-linked-bins
- GitHub Check: ubuntu-jammy-deps-image
- GitHub Check: build-macos (macos-15, false)
- GitHub Check: lint-check (macos-15)
components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh
Show resolved
Hide resolved
@@ -29,14 +29,13 @@ DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \ | |||
unzip | |||
|
|||
# Install `task` |
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.
# Install `task` | |
# Install `task` | |
# NOTE: We lock `task` to version 3.44.0 to avoid https://github.com/y-scope/clp/issues/872 and | |
# https://github.com/y-scope/clp-ffi-js/issues/110 |
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.
3.44.0 does not avoid #872 (i.e., if we run 3.44.0 with our taskfile.yaml unmodified, the hang is still expected to occur. I'll submit an issue to https://github.com/go-task/task/ once i have the bandwidth.
how about just:
# Install `task` | |
# Install `task` | |
# NOTE: We lock `task` to version 3.44.0 to avoid https://github.com/y-scope/clp-ffi-js/issues/110 |
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.
Right. Remind me why we're locking to >= 3.44.0? What's wrong with 3.40+?
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.
3.40 does not include this fix go-task/task@a60c2ec , which blocks use cases like #1119 (comment)
I intended to upgrade task to the latest version; however, it was just found that the latest version 3.44.1 causes the another bug in y-scope/clp-ffi-js#110 .
do note that go-task/task@a60c2ec happens to also be the cause that the "dynamic variables do not get populated in sources
" issue surfaced, which is why we should modify our taskfile.yaml anyways to mitigate the hang.
(sorry... i can't find a better way to explain these. let me know if there's any confusion unresolved
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.
previously i intentionally skipped the y-scope/clp-ffi-js#110 issue from any docs here in CLP because CLP does not depend on clp-ffi-js. though now i think it's good to write why a specific version is used so we don't forget in the future when we are itchy upgrading to new versions
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.
slightly off topic: we could change this https://github.com/y-scope/clp-ffi-js/blob/59890ba9a09bb8205281911735eb48cebb5a5a7c/Taskfile.yml#L133-L135
- "{{.G_CLP_FFI_JS_BUILD_DIR}}/{{.G_CLP_FFI_JS_TARGET_PREFIX}}{{.ITEM}}."{d.ts,js,wasm}
+ {{.G_CLP_FFI_JS_BUILD_DIR}}/{{.G_CLP_FFI_JS_TARGET_PREFIX}}{{.ITEM}}.{d.ts,js,wasm}
then the latest version of task
3.44.1 can also run the clp-ffi-js's package
task just fine. apparently go-task/task@d2bd834 , which intrudocued some incorrect string escape handling, is the commit that broke clp-ffi-js's package
task.
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.
Gotcha. These details should really be in the PR description, right? Otherwise reviewers (and developers) don't know why we're upgrading task nor why we're locking to a specific set of versions. Even if we're arbitrarily locking to the latest available version because we don't have time to test other versions, I think we should mention that in the PR description.
I think we should also mention the version bump in the PR title.
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.
right, sorry. the initial intention of the PR was to lift the version locking so developers can install any versions of task
(which is sort of important). then the bug in ffi-js was found, and we had to lock the version again. then there were many above recent discoveries that i hadn't document yet
now the issues have been reported to go-task, and i have updated the PR description and title
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.
i kept the fix keyword in the title but let me know
- we could leave
task package
hangs with task >= 3.43.1 #872 opened until the issue is fixed in go-task, though technicallytask package
hangs with task >= 3.43.1 #872 only reports the hang which is now really gone
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.
We should replicate these changes in the other container images.
@@ -13,8 +13,7 @@ prebuilt version instead, check out the [releases](https://github.com/y-scope/cl | |||
* Python 3.9 or newer | |||
* python3-dev | |||
* python3-venv (for the version of Python installed) | |||
* [Task] >= 3.40.0 and < 3.43.0 | |||
* We constrain the version due to unresolved [issues][clp-issue-872]. | |||
* [Task] >= 3.40.0 |
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.
* [Task] >= 3.40.0 | |
* [Task] 3.40.0 |
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.
as per the above installation script, shall we write
* [Task] >= 3.40.0 | |
* [Task] 3.44.0 |
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: 7
♻️ Duplicate comments (1)
components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh (1)
32-40
: Optional: introduce a TASK_VERSION variable for maintainability.Keeps the intent explicit and simplifies coordinated upgrades across platform scripts.
Apply this diff to the URL line, and define the variable near the top:
- "https://github.com/go-task/task/releases/download/v3.44.0/task_linux_${task_pkg_arch}.deb" + "https://github.com/go-task/task/releases/download/${TASK_VERSION}/task_linux_${task_pkg_arch}.deb"Add near the top (outside this hunk):
# go-task version (pinned due to clp-ffi-js#110) TASK_VERSION="v3.44.0"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
components/core/tools/scripts/lib_install/centos-stream-9/install-prebuilt-packages.sh
(1 hunks)components/core/tools/scripts/lib_install/manylinux_2_28/install-prebuilt-packages.sh
(1 hunks)components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh
(1 hunks)components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh
(1 hunks)docs/README.md
(1 hunks)docs/src/dev-guide/building-package.md
(1 hunks)docs/src/dev-guide/components-core/index.md
(1 hunks)docs/src/dev-guide/contributing-linting.md
(1 hunks)taskfile.yaml
(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: haiqi96
PR: y-scope/clp#0
File: :0-0
Timestamp: 2025-08-04T18:38:33.130Z
Learning: User haiqi96 requested creating a GitHub issue to document a documentation discrepancy where Task version requirements in docs weren't updated after yscope-utils upgrade in PR #1158.
📚 Learning: 2025-08-04T18:38:33.130Z
Learnt from: haiqi96
PR: y-scope/clp#0
File: :0-0
Timestamp: 2025-08-04T18:38:33.130Z
Learning: User haiqi96 requested creating a GitHub issue to document a documentation discrepancy where Task version requirements in docs weren't updated after yscope-utils upgrade in PR #1158.
Applied to files:
docs/src/dev-guide/contributing-linting.md
components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh
docs/src/dev-guide/components-core/index.md
docs/README.md
docs/src/dev-guide/building-package.md
📚 Learning: 2025-08-25T00:45:05.435Z
Learnt from: junhaoliao
PR: y-scope/clp#1242
File: components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh:39-41
Timestamp: 2025-08-25T00:45:05.435Z
Learning: Task v3.44.1 has a regression that breaks shell command processing, particularly rsync commands with brace expansion (e.g., `file.{d.ts,js,wasm}`). This causes CI failures in clp-ffi-js project (issue #110), so CLP should avoid v3.44.1 and use v3.44.0 instead, which fixes the dynamic variable bug without the shell processing regression.
Applied to files:
components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh
components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh
docs/src/dev-guide/components-core/index.md
docs/README.md
docs/src/dev-guide/building-package.md
📚 Learning: 2025-07-07T17:41:15.655Z
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:27-32
Timestamp: 2025-07-07T17:41:15.655Z
Learning: In CLP installation scripts, consistency across platform scripts is prioritized over defensive programming improvements. For example, when extracting Task binaries with tar in `install-prebuilt-packages.sh`, the extraction pattern should remain consistent with other platform scripts rather than adding defensive flags like `--strip-components=1` to handle potential tarball layout changes.
Applied to files:
components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh
components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh
components/core/tools/scripts/lib_install/centos-stream-9/install-prebuilt-packages.sh
📚 Learning: 2025-08-25T00:45:05.435Z
Learnt from: junhaoliao
PR: y-scope/clp#1242
File: components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh:39-41
Timestamp: 2025-08-25T00:45:05.435Z
Learning: Task v3.44.1 causes issues in the clp-ffi-js project (issue #110), so CLP should avoid upgrading to v3.44.1 and stick with v3.44.0 for now.
Applied to files:
components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh
docs/src/dev-guide/components-core/index.md
docs/README.md
docs/src/dev-guide/building-package.md
📚 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:
components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh
📚 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:
components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh
📚 Learning: 2025-07-01T14:51:19.172Z
Learnt from: jackluo923
PR: y-scope/clp#1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh:6-8
Timestamp: 2025-07-01T14:51:19.172Z
Learning: In CLP installation scripts within `components/core/tools/scripts/lib_install/`, maintain consistency with existing variable declaration patterns across platforms rather than adding individual improvements like `readonly` declarations.
Applied to files:
components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh
📚 Learning: 2025-08-20T05:38:30.720Z
Learnt from: junhaoliao
PR: y-scope/clp#1242
File: taskfile.yaml:545-547
Timestamp: 2025-08-20T05:38:30.720Z
Learning: The python component directories in the y-scope/clp repository are kept very clean and only contain essential files: source code directories, pyproject.toml, poetry.lock, README.md, and the dist/ directory. No cache directories, virtual environments, or other transient files exist in the component directories, so additional exclude patterns beyond "dist/**/*" are unnecessary.
Applied to files:
taskfile.yaml
📚 Learning: 2025-08-14T18:23:39.476Z
Learnt from: junhaoliao
PR: y-scope/clp#1119
File: taskfile.yaml:429-433
Timestamp: 2025-08-14T18:23:39.476Z
Learning: In the webui workspace, the exclude patterns ".tmp" and ".vite-temp" are Vite-specific temporary directories that only apply to the client package. The common and server packages use TypeScript compiler (tsc) directly and don't generate these Vite temporary files, so these exclude patterns should not be applied to their node_modules checksum computations.
Applied to files:
taskfile.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: ubuntu-jammy-deps-image
- GitHub Check: centos-stream-9-deps-image
- GitHub Check: antlr-code-committed (macos-15)
- GitHub Check: lint-check (macos-15)
🔇 Additional comments (6)
docs/src/dev-guide/components-core/index.md (1)
12-14
: Task version consistency confirmedAll references to the Task version across docs and installer scripts are correctly pinned to 3.44.0. No instances of 3.44.1 or version ranges were found.
components/core/tools/scripts/lib_install/centos-stream-9/install-prebuilt-packages.sh (2)
42-49
: LGTM on pinning Task to 3.44.0 and updating the RPM URL.This aligns the platform script with the decision to avoid 3.44.1 due to the regression and addresses the dynamic variable issue present in earlier versions.
41-50
: Minor: factor Task version into a single variable to ease future bumps.Keeps scripts simple while reducing mechanical edits if/when we move off 3.44.0.
Apply this diff for the URL line, and add a small var near the top:
- "https://github.com/go-task/task/releases/download/v3.44.0/task_linux_${task_pkg_arch}.rpm" + "https://github.com/go-task/task/releases/download/${TASK_VERSION}/task_linux_${task_pkg_arch}.rpm"Add near the top (outside this hunk):
# go-task version (pinned due to clp-ffi-js#110) TASK_VERSION="v3.44.0"⛔ Skipped due to learnings
Learnt from: junhaoliao PR: y-scope/clp#1242 File: components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh:39-41 Timestamp: 2025-08-25T00:45:05.435Z Learning: Task v3.44.1 has a regression that breaks shell command processing, particularly rsync commands with brace expansion (e.g., `file.{d.ts,js,wasm}`). This causes CI failures in clp-ffi-js project (issue #110), so CLP should avoid v3.44.1 and use v3.44.0 instead, which fixes the dynamic variable bug without the shell processing regression.
Learnt from: junhaoliao PR: y-scope/clp#1242 File: components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh:39-41 Timestamp: 2025-08-25T00:45:05.435Z Learning: Task v3.44.1 causes issues in the clp-ffi-js project (issue #110), so CLP should avoid upgrading to v3.44.1 and stick with v3.44.0 for now.
Learnt from: jackluo923 PR: y-scope/clp#1054 File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:27-32 Timestamp: 2025-07-07T17:41:15.655Z Learning: In CLP installation scripts, consistency across platform scripts is prioritized over defensive programming improvements. For example, when extracting Task binaries with tar in `install-prebuilt-packages.sh`, the extraction pattern should remain consistent with other platform scripts rather than adding defensive flags like `--strip-components=1` to handle potential tarball layout changes.
components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh (1)
32-41
: LGTM: correct pin to 3.44.0 and matching .deb URL.Matches the ecosystem constraint (avoid 3.44.1) and the PR objective to resolve the dynamic variable hang without inheriting the shell-processing regression.
taskfile.yaml (2)
545-547
: Good fix: broadening python-component sources removes the dynamic var pitfall.Switching to "/*" with "dist//*" excluded avoids the go-task dynamic variable bug while keeping rebuilds deterministic in your clean component directories.
541-547
: No remaining.PACKAGE
references found
The repository was searched for{{.PACKAGE}}
usages using the provided ripgrep command, and no matches were found. This confirms that thePACKAGE
variable has been fully removed and there are no lingering references to reintroduce the hang.
# Install `task` | ||
# NOTE: We lock `task` to a version < 3.43 to avoid https://github.com/y-scope/clp/issues/872 | ||
# NOTE: We lock `task` to version 3.44.0 to avoid https://github.com/y-scope/clp-ffi-js/issues/110 | ||
task_pkg_path=$(mktemp -t --suffix ".rpm" task-pkg.XXXXXXXXXX) || exit 1 | ||
curl \ | ||
--fail \ | ||
--location \ | ||
--output "$task_pkg_path" \ | ||
--show-error \ | ||
"https://github.com/go-task/task/releases/download/v3.42.1/task_linux_${task_pkg_arch}.rpm" | ||
"https://github.com/go-task/task/releases/download/v3.44.0/task_linux_${task_pkg_arch}.rpm" |
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.
🧹 Nitpick (assertive)
Add curl retries for resilience; keep parity with other platform scripts.
Same reasoning as musllinux. Minimal change shown below.
curl \
--fail \
--location \
--output "$task_pkg_path" \
- --show-error \
+ --show-error \
+ --retry 3 \
+ --retry-delay 2 \
"https://github.com/go-task/task/releases/download/v3.44.0/task_linux_${task_pkg_arch}.rpm"
📝 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.
# Install `task` | |
# NOTE: We lock `task` to a version < 3.43 to avoid https://github.com/y-scope/clp/issues/872 | |
# NOTE: We lock `task` to version 3.44.0 to avoid https://github.com/y-scope/clp-ffi-js/issues/110 | |
task_pkg_path=$(mktemp -t --suffix ".rpm" task-pkg.XXXXXXXXXX) || exit 1 | |
curl \ | |
--fail \ | |
--location \ | |
--output "$task_pkg_path" \ | |
--show-error \ | |
"https://github.com/go-task/task/releases/download/v3.42.1/task_linux_${task_pkg_arch}.rpm" | |
"https://github.com/go-task/task/releases/download/v3.44.0/task_linux_${task_pkg_arch}.rpm" | |
# Install `task` | |
# NOTE: We lock `task` to version 3.44.0 to avoid https://github.com/y-scope/clp-ffi-js/issues/110 | |
task_pkg_path=$(mktemp -t --suffix ".rpm" task-pkg.XXXXXXXXXX) || exit 1 | |
curl \ | |
--fail \ | |
--location \ | |
--output "$task_pkg_path" \ | |
--show-error \ | |
--retry 3 \ | |
--retry-delay 2 \ | |
"https://github.com/go-task/task/releases/download/v3.44.0/task_linux_${task_pkg_arch}.rpm" |
🤖 Prompt for AI Agents
In
components/core/tools/scripts/lib_install/manylinux_2_28/install-prebuilt-packages.sh
around lines 31 to 39, the curl invocation downloading the task RPM lacks retry
options; update the curl call to include retry flags (e.g. --retry 5
--retry-delay 2 --retry-connrefused and optionally --max-time or
--retry-max-time) so it behaves like the musllinux scripts and is resilient to
transient network failures; keep all other flags (--fail --location --output
--show-error) and the same URL, and ensure the mktemp exit handling remains
unchanged.
# NOTE: We lock `task` to version 3.44.0 to avoid https://github.com/y-scope/clp-ffi-js/issues/110 | ||
task_pkg_path=$(mktemp -t task-pkg.XXXXXXXXXX).tar.gz || exit 1 | ||
curl \ | ||
--fail \ | ||
--location \ | ||
--output "$task_pkg_path" \ | ||
--show-error \ | ||
"https://github.com/go-task/task/releases/download/v3.42.1/task_linux_${task_pkg_arch}.tar.gz" | ||
"https://github.com/go-task/task/releases/download/v3.44.0/task_linux_${task_pkg_arch}.tar.gz" | ||
tar -C /usr/local/bin -xzf "$task_pkg_path" task |
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.
🧹 Nitpick (assertive)
Harden the download step with retries (keep scripts consistent across platforms).
A transient network blip will currently fail the install. Add curl retries (and consider mirroring across all platform install scripts in this PR for consistency).
Apply this minimal diff:
curl \
--fail \
--location \
--output "$task_pkg_path" \
- --show-error \
+ --show-error \
+ --retry 3 \
+ --retry-delay 2 \
"https://github.com/go-task/task/releases/download/v3.44.0/task_linux_${task_pkg_arch}.tar.gz"
Optional follow-up (separate PR): add SHA256 verification for the Task artifact uniformly across all install-prebuilt-packages scripts.
📝 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.
# NOTE: We lock `task` to version 3.44.0 to avoid https://github.com/y-scope/clp-ffi-js/issues/110 | |
task_pkg_path=$(mktemp -t task-pkg.XXXXXXXXXX).tar.gz || exit 1 | |
curl \ | |
--fail \ | |
--location \ | |
--output "$task_pkg_path" \ | |
--show-error \ | |
"https://github.com/go-task/task/releases/download/v3.42.1/task_linux_${task_pkg_arch}.tar.gz" | |
"https://github.com/go-task/task/releases/download/v3.44.0/task_linux_${task_pkg_arch}.tar.gz" | |
tar -C /usr/local/bin -xzf "$task_pkg_path" task | |
# NOTE: We lock `task` to version 3.44.0 to avoid https://github.com/y-scope/clp-ffi-js/issues/110 | |
task_pkg_path=$(mktemp -t task-pkg.XXXXXXXXXX).tar.gz || exit 1 | |
curl \ | |
--fail \ | |
--location \ | |
--output "$task_pkg_path" \ | |
--show-error \ | |
--retry 3 \ | |
--retry-delay 2 \ | |
"https://github.com/go-task/task/releases/download/v3.44.0/task_linux_${task_pkg_arch}.tar.gz" | |
tar -C /usr/local/bin -xzf "$task_pkg_path" task |
🤖 Prompt for AI Agents
components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh
around lines 33 to 41: the curl download is currently a single attempt and will
fail on transient network errors; update the curl invocation to include retry
behavior (for example add --retry 5 --retry-delay 2 --retry-connrefused
--retry-all-errors and optionally --retry-max-time 30) while keeping --fail
--location --show-error and --output so the script will automatically retry
transient failures; apply the same retry flags consistently across the other
platform install-prebuilt-packages scripts in this PR.
@@ -13,8 +13,7 @@ this project: | |||
the size of repo as we add and update images. | |||
* [Node.js] >= 16 to be able to [view the output](#viewing-the-output) | |||
* Python 3.10 or later | |||
* [Task] >= 3.40.0 and < 3.43.0 | |||
* We constrain the version due to unresolved [issues][clp-issue-872]. | |||
* [Task] 3.44.0 |
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.
🧹 Nitpick (assertive)
Doc pin is good; add brief rationale to prevent accidental upgrades.
Given the known regression in Task v3.44.1, add a short parenthetical note and reference so maintainers don’t “helpfully” bump past 3.44.0 in future edits.
Apply this diff inline:
-* [Task] 3.44.0
+* [Task] 3.44.0 (pinned due to a 3.44.1 regression; see [clp-ffi-js #110])
And append a link reference at the end of the file (or reuse an existing references block):
+[clp-ffi-js #110]: https://github.com/y-scope/clp-ffi-js/issues/110
📝 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.
* [Task] 3.44.0 | |
* [Task] 3.44.0 (pinned due to a 3.44.1 regression; see [clp-ffi-js #110]) | |
[clp-ffi-js #110]: https://github.com/y-scope/clp-ffi-js/issues/110 |
🤖 Prompt for AI Agents
In docs/README.md around line 16, the pinned Task version "* [Task] 3.44.0"
needs a brief rationale to prevent accidental upgrades: update the line to
include a short parenthetical note about the regression in v3.44.1 and a
citation marker (e.g., "(regression in 3.44.1)[1]"), then append the referenced
link at the end of the file (or add to an existing references block) pointing to
the issue/PR describing the regression so maintainers have context when editing.
@@ -13,8 +13,7 @@ prebuilt version instead, check out the [releases](https://github.com/y-scope/cl | |||
* Python 3.9 or newer | |||
* python3-dev | |||
* python3-venv (for the version of Python installed) | |||
* [Task] >= 3.40.0 and < 3.43.0 | |||
* We constrain the version due to unresolved [issues][clp-issue-872]. | |||
* [Task] 3.44.0 |
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.
🧹 Nitpick (assertive)
Make the pin’s rationale explicit (align with installer script comments).
Match the installer scripts’ note so readers understand why 3.44.0 is fixed.
-* [Task] 3.44.0
+* [Task] 3.44.0 (pinned to avoid a known 3.44.1 regression; see [clp-ffi-js #110])
Add (or reuse) the link reference at the bottom:
+[clp-ffi-js #110]: https://github.com/y-scope/clp-ffi-js/issues/110
📝 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.
* [Task] 3.44.0 | |
* [Task] 3.44.0 (pinned to avoid a known 3.44.1 regression; see [clp-ffi-js #110]) | |
[clp-ffi-js #110]: https://github.com/y-scope/clp-ffi-js/issues/110 |
🤖 Prompt for AI Agents
In docs/src/dev-guide/building-package.md around line 16, the pinned version
entry "* [Task] 3.44.0" lacks the installer-script rationale; update that list
item to explicitly state why 3.44.0 is pinned (matching the installer script
comment text) and add or reuse the link reference at the bottom of the file
pointing to the installer note or issue that explains the pin so readers
understand the reason.
💡 Verification agent
🧩 Analysis chain
Verify repository-wide version consistency and Taskfile sources change.
Ensure every doc/script points to 3.44.0 and the python-component Task sources are simplified with the correct exclude.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check docs for Task version pins =="
rg -nP '\[Task\][^\n]*' docs | sed -E 's/^/DOC: /'
echo "== Check install scripts download URLs =="
rg -nP 'go-task/task/releases/download/v3\.\d+\.\d+' components/core/tools/scripts | sed -E 's/^/SCRIPT: /'
echo "== Expect only v3.44.0 references =="
rg -n 'v3\.44\.0' -n
echo "== Ensure there are no 3.44.1 or older pins lingering =="
! rg -n 'v3\.44\.1|v3\.42\.' || { echo "Found non-3.44.0 Task references above"; exit 1; }
echo "== Locate Taskfile and verify python-component sources simplification =="
# Find Taskfile with common names/cases
fd -HI --strip-cwd-prefix -t f -i '^taskfile\.ya?ml$|^taskfile$|^taskfile\.yml$' || true
TF="$(fd -HI -t f -i '^taskfile\.ya?ml$|^taskfile$|^taskfile\.yml$' | head -n1 || true)"
if [[ -n "${TF:-}" ]]; then
echo "Using Taskfile: $TF"
rg -nP -C3 '(?s)python-component:\s.*?sources:\s*\n\s*-\s+\*\*/\*\*\s*(\n.*?exclude:\s*-\s*dist/\*\*/\*\*)?' "$TF" || {
echo "Could not confirm simplified sources (**/*) and dist/**/* exclude for python-component."
exit 1
}
else
echo "Taskfile not found; verify manually."
fi
Length of output: 3426
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate python-component snippet in taskfile.yaml =="
rg -nC5 'python-component' -n taskfile.yaml
Length of output: 2296
Version consistency checks passed; Taskfile update required
All documentation and install scripts correctly reference Task v3.44.0—no action needed there. However, the python-component
target in taskfile.yaml
is missing the simplified sources
and exclude
entries that ensure only source files are packaged and built artifacts are omitted. Please update as follows:
• File: taskfile.yaml
• Location: under the python-component:
section (around line 533)
Suggested diff:
@@ -533,7 +533,11 @@
python-component:
internal: true
label: "{{.COMPONENT}}"
+ sources:
+ - "**/**"
+ exclude:
+ - "dist/**"
vars:
VENV_DIR: "{{.G_BUILD_DIR}}/{{.COMPONENT}}/venv"
requires:
This ensures that all files (**/**
) are included by default, while excluding any dist/**
build outputs.
📝 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.
* [Task] 3.44.0 | |
python-component: | |
internal: true | |
label: "{{.COMPONENT}}" | |
sources: | |
- "**/**" | |
exclude: | |
- "dist/**" | |
vars: | |
VENV_DIR: "{{.G_BUILD_DIR}}/{{.COMPONENT}}/venv" | |
requires: | |
# (existing requirements) |
🤖 Prompt for AI Agents
In docs/src/dev-guide/building-package.md around line 16, update the guidance to
instruct adding simplified sources and exclude entries under the
python-component section of taskfile.yaml (around line 533); specifically, add a
sources entry that includes all files (e.g., '**/**') and an exclude entry that
omits build outputs (e.g., 'dist/**') so only source files are packaged and
built artifacts are excluded. Ensure the instructions mention exactly where
(file and section) to place these entries and that they follow Taskfile YAML
indentation conventions.
@@ -9,8 +9,7 @@ CLP core is the low-level component that performs compression, decompression, an | |||
* A recent compiler that fully supports C++20 features such as | |||
* std::span | |||
* std::source_location | |||
* [Task] >= 3.40.0 and < 3.43.0 | |||
* We constrain the version due to unresolved [issues][clp-issue-872]. | |||
* [Task] 3.44.0 |
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.
🧹 Nitpick (assertive)
Explain why Task is pinned to exactly 3.44.0 to pre-empt confusion.
Right now the requirement just states 3.44.0. Given we’re avoiding 3.44.1 due to a regression and ≥3.40.0 still has the dynamic var bug, add a short note to capture the rationale and cross-reference the relevant issue IDs.
Apply this diff to clarify the requirement:
-* [Task] 3.44.0
+* [Task] 3.44.0
+ * Note: We pin exactly 3.44.0 because it fixes the dynamic variable hang (see [clp-issue-872]) and 3.44.1 introduces a shell command processing regression (see clp-ffi-js issue #110). We'll revisit once upstream releases a fixed version.
And add a link definition for clp-issue-872 if not already in this file’s link section:
+[clp-issue-872]: https://github.com/y-scope/clp/issues/872
📝 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.
* [Task] 3.44.0 | |
* [Task] 3.44.0 | |
* Note: We pin exactly 3.44.0 because it fixes the dynamic variable hang (see [clp-issue-872]) and 3.44.1 introduces a shell command processing regression (see clp-ffi-js issue #110). We'll revisit once upstream releases a fixed version. | |
[clp-issue-872]: https://github.com/y-scope/clp/issues/872 |
🤖 Prompt for AI Agents
In docs/src/dev-guide/components-core/index.md around line 12, the entry "*
[Task] 3.44.0" lacks explanation for why the version is pinned; update that line
to include a short parenthetical note stating that Task is pinned to exactly
3.44.0 because 3.44.1 was avoided due to a regression and versions ≥3.40.0
contain the dynamic var bug (reference related issue IDs), and add a
cross-reference link definition for clp-issue-872 in this file’s link section if
it doesn't already exist.
* [Task] 3.44.0 | ||
* [uv] >= 0.8 | ||
|
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.
🧹 Nitpick (assertive)
Briefly annotate the Task pin; optionally clarify uv usage.
Add a short parenthetical for the Task pin (to mirror installer notes). Consider a one-liner after the list noting uv is used for Python packaging to justify the new dependency.
-* [Task] 3.44.0
+* [Task] 3.44.0 (temporarily pinned; see [clp-ffi-js #110])
* [uv] >= 0.8
Optionally add under the list:
+> Note: We use [uv] for Python environment and package management during linting.
And add the link reference if not already present in this file:
+[clp-ffi-js #110]: https://github.com/y-scope/clp-ffi-js/issues/110
📝 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.
* [Task] 3.44.0 | |
* [uv] >= 0.8 | |
* [Task] 3.44.0 (temporarily pinned; see [clp-ffi-js #110]) | |
* [uv] >= 0.8 | |
> Note: We use [uv] for Python environment and package management during linting. | |
[clp-ffi-js #110]: https://github.com/y-scope/clp-ffi-js/issues/110 |
🤖 Prompt for AI Agents
In docs/src/dev-guide/contributing-linting.md around lines 18 to 20, add a brief
parenthetical next to the Task pin to mirror installer notes (e.g., "Task 3.44.0
(pinned to installer requirement)") and insert a one-line note immediately after
the list clarifying that "uv" is required for Python packaging (e.g., "uv is
used for Python packaging"). Also ensure the link reference for Task (and uv if
used elsewhere) is present in the file; if missing, add the appropriate
reference entry at the bottom of the document.
python-component
sources list to avoid the dynamic variable bug in go-task (fixes #872).python-component
sources list to avoid the dynamic variable bug in go-task (fixes #872); Upgrade go-task
to v3.44.0.
python-component
sources list to avoid the dynamic variable bug in go-task (fixes #872); Upgrade go-task
to v3.44.0.python-component
sources list to avoid the dynamic variable bug in go-task (fixes #872); Upgrade and pin go-task
to v3.44.0.
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:
fix(taskfile): Simplify `python-component` sources list to avoid go-task/task#2400 (fixes #872); Upgrade and pin `go-task` to v3.44.0 to avoid go-task/task#2401.
python-component
sources list to avoid the dynamic variable bug in go-task (fixes #872); Upgrade and pin go-task
to v3.44.0.python-component
sources list to avoid go-task/task#2400 (fixes #872); Upgrade and pin go-task
to v3.44.0 to avoid go-task/task#2401.
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/src/dev-docs/building-package.md (1)
11-12
: Fix grammar: “a some” → “some”.User-facing docs nit.
- * It should be possible to build a package for a different environment, it just requires a some + * It should be possible to build a package for a different environment; it just requires some extra configuration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
docs/src/dev-docs/building-package.md
(1 hunks)docs/src/dev-docs/components-core/index.md
(1 hunks)docs/src/dev-docs/contributing-linting.md
(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#1242
File: components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh:39-41
Timestamp: 2025-08-25T00:45:05.464Z
Learning: Task v3.44.1 has a regression that breaks shell command processing, particularly rsync commands with brace expansion (e.g., `file.{d.ts,js,wasm}`). This causes CI failures in clp-ffi-js project (issue #110), so CLP should avoid v3.44.1 and use v3.44.0 instead, which fixes the dynamic variable bug without the shell processing regression.
Learnt from: junhaoliao
PR: y-scope/clp#1242
File: components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh:39-41
Timestamp: 2025-08-25T00:45:05.464Z
Learning: Task v3.44.1 causes issues in the clp-ffi-js project (issue #110), so CLP should avoid upgrading to v3.44.1 and stick with v3.44.0 for now.
📚 Learning: 2025-08-04T18:38:33.130Z
Learnt from: haiqi96
PR: y-scope/clp#0
File: :0-0
Timestamp: 2025-08-04T18:38:33.130Z
Learning: User haiqi96 requested creating a GitHub issue to document a documentation discrepancy where Task version requirements in docs weren't updated after yscope-utils upgrade in PR #1158.
Applied to files:
docs/src/dev-docs/components-core/index.md
docs/src/dev-docs/contributing-linting.md
📚 Learning: 2025-08-25T00:45:05.464Z
Learnt from: junhaoliao
PR: y-scope/clp#1242
File: components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh:39-41
Timestamp: 2025-08-25T00:45:05.464Z
Learning: Task v3.44.1 causes issues in the clp-ffi-js project (issue #110), so CLP should avoid upgrading to v3.44.1 and stick with v3.44.0 for now.
Applied to files:
docs/src/dev-docs/components-core/index.md
docs/src/dev-docs/building-package.md
📚 Learning: 2025-08-25T00:45:05.464Z
Learnt from: junhaoliao
PR: y-scope/clp#1242
File: components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh:39-41
Timestamp: 2025-08-25T00:45:05.464Z
Learning: Task v3.44.1 has a regression that breaks shell command processing, particularly rsync commands with brace expansion (e.g., `file.{d.ts,js,wasm}`). This causes CI failures in clp-ffi-js project (issue #110), so CLP should avoid v3.44.1 and use v3.44.0 instead, which fixes the dynamic variable bug without the shell processing regression.
Applied to files:
docs/src/dev-docs/building-package.md
⏰ 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). (3)
- GitHub Check: manylinux_2_28-x86_64-deps-image
- GitHub Check: lint-check (ubuntu-24.04)
- GitHub Check: build (macos-15)
🔇 Additional comments (2)
docs/src/dev-docs/components-core/index.md (1)
12-12
: All Task version mentions are pinned to v3.44.0; no 3.44.1+ references detected.docs/src/dev-docs/contributing-linting.md (1)
18-18
: Verify all Task invocations in workflows are pinned to 3.44.0
Manually inspect every.github/workflows/*.yml
for anytask
orgo-task
step and confirm the version is set to 3.44.0 to prevent drift.
@@ -13,8 +13,7 @@ prebuilt version instead, check out the [releases](https://github.com/y-scope/cl | |||
* Python 3.9 or newer | |||
* python3-dev | |||
* python3-venv (for the version of Python installed) | |||
* [Task] >= 3.40.0 and < 3.43.0 | |||
* We constrain the version due to unresolved [issues][clp-issue-872]. | |||
* [Task] 3.44.0 |
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.
🧹 Nitpick (assertive)
Pin looks correct; consider adding rationale to prevent accidental bump to 3.44.1.
Add a brief note about the 3.44.1 regression (brace expansion) and why we pin to 3.44.0.
-* [Task] 3.44.0
+* [Task] 3.44.0 <!-- Pinned due to 3.44.1 regression breaking brace expansion; see go-task/task#2401 -->
📝 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.
* [Task] 3.44.0 | |
* [Task] 3.44.0 <!-- Pinned due to 3.44.1 regression breaking brace expansion; see go-task/task#2401 --> |
🤖 Prompt for AI Agents
In docs/src/dev-docs/building-package.md around line 16, the pin entry "* [Task]
3.44.0" lacks context; update this line to append a short rationale explaining
that 3.44.1 contains a brace-expansion regression and therefore we pin to 3.44.0
to avoid that breakage, include a one-sentence summary of the regression and
optionally a reference (issue/PR number) so future readers know why not to
upgrade automatically.
@@ -9,8 +9,7 @@ CLP core is the low-level component that performs compression, decompression, an | |||
* A recent compiler that fully supports C++20 features such as | |||
* std::span | |||
* std::source_location | |||
* [Task] >= 3.40.0 and < 3.43.0 | |||
* We constrain the version due to unresolved [issues][clp-issue-872]. | |||
* [Task] 3.44.0 |
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.
🧹 Nitpick (assertive)
Good pin; add a short caution about 3.44.1 regression.
Helps future editors avoid bumping past the safe version.
-* [Task] 3.44.0
+* [Task] 3.44.0 <!-- Pin: 3.44.1 introduces a shell/brace expansion regression -->
📝 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.
* [Task] 3.44.0 | |
* [Task] 3.44.0 <!-- Pin: 3.44.1 introduces a shell/brace expansion regression --> |
🤖 Prompt for AI Agents
In docs/src/dev-docs/components-core/index.md around line 12, the pinned version
"* [Task] 3.44.0" needs a short caution about a regression in 3.44.1; update the
line or immediately following sentence to note that 3.44.1 contains a known
regression and that editors should keep the pin at 3.44.0 (or explicitly call
out the safe range), using a concise one-sentence warning so future editors
won’t accidentally bump past the safe version.
* [Task] >= 3.40.0 and < 3.43.0 | ||
* Minimum version 3.40.0 is required for [yscope-dev-utils]. | ||
* We constrain the version to < 3.43.0 due to unresolved [issues][clp-issue-872]. | ||
* [Task] 3.44.0 |
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.
🧹 Nitpick (assertive)
LGTM on pin; mirror the rationale inline for clarity.
-* [Task] 3.44.0
+* [Task] 3.44.0 <!-- Pinned: 3.44.1 regression breaks brace expansion -->
📝 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.
* [Task] 3.44.0 | |
* [Task] 3.44.0 <!-- Pinned: 3.44.1 regression breaks brace expansion --> |
🤖 Prompt for AI Agents
In docs/src/dev-docs/contributing-linting.md around line 18, the release pin "*
[Task] 3.44.0" lacks an inline rationale; update that line to keep the version
pin but append a short explanatory phrase (e.g., "– pinned for lint rule
stability/compatibility with X toolchain") so reviewers see why the pin is
required; keep the rationale concise and specific to the stability/compatibility
reason referenced in the PR.
Description
As the title suggests. See #872 (comment) for the details of the "bug".
We previously locked Task to versions < 3.43.0 due to critical issues that caused build failures and hangs. Two specific issues were identified:
task package
hangs with task >= 3.43.1 #872 (reported to Dynamic variables insources
cause glob expansion to hang due to unpopulated placeholders and symlink loops go-task/task#2400): Dynamic variables in sources caused glob expansion to hang due to unpopulated placeholders and symlink loops. This was particularly problematic with patterns like{{.DYNAMIC_VARIABLE}}/**/*
which would expand to/**/*
and cause infinite symlink traversal.{
and}
characters, breaking legitimate shell features likefile.{txt,md}
expansion.This PR makes modification to the
taskfile.yaml
, upgrades the Task tool to version 3.44.0 and removes the previous version constraints that were in place due to critical bugs introduced in Task v3.43.0. The upgrade to 3.44.0 brings in various benefits such as unblocking use cases the leverage globing in sources files: #1119 (comment)Checklist
breaking change.
Validation performed
which task
andtask --version
just to be sure the version was to be used.rm -rf build .task
task
and observed the command to be completed in less than 3 minutes.Summary by CodeRabbit