-
Notifications
You must be signed in to change notification settings - Fork 9
feat(taskfiles): Add support for CHECKSUM_EXCLUDE_PATTERNS in remote tasks to exclude paths in checksum calculations (resolves #68). #69
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
base: main
Are you sure you want to change the base?
Conversation
…tasks to exclude paths in checksum calculations.
WalkthroughA new optional parameter, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Task (download-and-extract-tar/zip)
participant Checksum Validator
User->>Task (download-and-extract-tar/zip): Provide URL, OUTPUT_DIR, (optional) CHECKSUM_EXCLUDE_PATTERNS
Task (download-and-extract-tar/zip)->>Task (download-and-extract-tar/zip): Download and extract archive
Task (download-and-extract-tar/zip)->>Checksum Validator: Compute/validate checksum, excluding patterns in CHECKSUM_EXCLUDE_PATTERNS
Checksum Validator-->>Task (download-and-extract-tar/zip): Return checksum result
Task (download-and-extract-tar/zip)-->>User: Extraction and checksum status
Possibly related PRs
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind adding a test to taskfiles/remote/tests.yaml
?
# @param {string[]} [CHECKSUM_EXCLUDE_PATTERNS] Path wildcard patterns, relative to `OUTPUT_DIR`, | ||
# to exclude from the checksum. |
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.
Hm, are they relative to OUTPUT_DIR
?
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.
Yes, CHECKSUM_EXCLUDE_PATTERNS
is relative to OUTPUT_DIR
. By looking at the checksum:validate
task's docstring:
# @param {string[]} [EXCLUDE_PATTERNS] Path wildcard patterns, relative to any `INCLUDE_PATTERNS`,
# to exclude from the checksum.
Since OUTPUT_DIR
is passed as one of the INCLUDE_PATTERNS
to the checksum tasks, and the docstring states that EXCLUDE_PATTERNS
are "relative to any INCLUDE_PATTERNS", the CHECKSUM_EXCLUDE_PATTERNS
are indeed relative to OUTPUT_DIR
.
or are you saying that this can be better expressed?
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.
Sorry, no, I just got confused with INCLUDE_PATTERNS.
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
📒 Files selected for processing (1)
taskfiles/remote/tests.yaml
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: unit-tests (ubuntu-22.04)
- GitHub Check: unit-tests (ubuntu-24.04)
🔇 Additional comments (2)
taskfiles/remote/tests.yaml (2)
86-94
: Well-structured new test task for checksum exclusion.The
download-and-extract-zip-test-checksum-exclude
task is clearly defined with its ownOUTPUT_DIR
andCHECKSUM_FILE
vars, matching the pattern of the other zip tests.
106-121
: Checksum comparison logic is clear and robust.Moving the first checksum file to
.ref
and then diffing against the second run is a simple, effective validation of the newCHECKSUM_EXCLUDE_PATTERNS
feature.
# Download the zip file, extract all files, and compute the checksum with some exclude | ||
# patterns. | ||
- task: "remote:download-and-extract-zip" | ||
vars: | ||
CHECKSUM_EXCLUDE_PATTERNS: &checksum_exclude_patterns | ||
- ".github/CODEOWNERS" | ||
- ".github/PULL_REQUEST_TEMPLATE.md" | ||
OUTPUT_DIR: "{{.OUTPUT_DIR}}" | ||
URL: "{{.G_TEST_ZIP_FILE_URL}}" | ||
FILE_SHA256: "{{.G_TEST_ZIP_FILE_SHA256}}" | ||
- "mv '{{.CHECKSUM_FILE}}' '{{.CHECKSUM_FILE}}.ref'" |
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.
❓ Verification inconclusive
Ensure checksum exclude patterns align with extracted file paths.
The anchored patterns ".github/CODEOWNERS"
and ".github/PULL_REQUEST_TEMPLATE.md"
may not match the top-level folder prefix (e.g. yscope-dev-utils-main/.github/...
). To guarantee the exclude rules apply both during extraction and in checksum computation, update the anchor to use a wildcard or reference the existing vars:
vars:
- CHECKSUM_EXCLUDE_PATTERNS: &checksum_exclude_patterns
- - ".github/CODEOWNERS"
- - ".github/PULL_REQUEST_TEMPLATE.md"
+ CHECKSUM_EXCLUDE_PATTERNS: &checksum_exclude_patterns
+ - "*/.github/CODEOWNERS"
+ - "*/.github/PULL_REQUEST_TEMPLATE.md"
Run a quick grep on the extracted directory name to confirm your anchors match:
🏁 Script executed:
rg -A1 'yscope-dev-utils-main' -g '*.md'
Length of output: 40
Ensure checksum exclude patterns align with extracted file paths.
The anchored patterns ".github/CODEOWNERS"
and ".github/PULL_REQUEST_TEMPLATE.md"
may not match the top-level folder prefix (e.g. yscope-dev-utils-main/.github/...
). To guarantee the exclude rules apply both during extraction and in checksum computation, update the anchor to use a wildcard or reference the existing vars:
vars:
- CHECKSUM_EXCLUDE_PATTERNS: &checksum_exclude_patterns
- - ".github/CODEOWNERS"
- - ".github/PULL_REQUEST_TEMPLATE.md"
+ CHECKSUM_EXCLUDE_PATTERNS: &checksum_exclude_patterns
+ - "*/.github/CODEOWNERS"
+ - "*/.github/PULL_REQUEST_TEMPLATE.md"
Run a quick grep on the extracted directory name to confirm your anchors match:
rg -A1 'yscope-dev-utils-main' -g '*.md'
🤖 Prompt for AI Agents
In taskfiles/remote/tests.yaml around lines 95 to 105, the anchored checksum
exclude patterns do not account for the top-level extracted folder prefix,
causing mismatches during checksum computation. Update the
CHECKSUM_EXCLUDE_PATTERNS anchor to include a wildcard prefix (e.g.,
"yscope-dev-utils-main/.github/CODEOWNERS") or adjust the patterns to match the
actual extracted file paths. Verify the updated patterns by running a grep or
ripgrep command on the extracted directory to ensure they correctly exclude the
intended files.
Description
As the title suggests
Checklist
breaking change.
Validation performed
Repeated the reproduction steps in #68 with the intended checksum exclude path passed in an array to
CHECKSUM_EXCLUDE_PATTERNS
and observed the task core logic did not get re-run.Summary by CodeRabbit