fix: Ensure utils:curl re-runs when input URLs or the library target's commit hash changes.#24
fix: Ensure utils:curl re-runs when input URLs or the library target's commit hash changes.#24Bill-hbrhbr wants to merge 3 commits into
utils:curl re-runs when input URLs or the library target's commit hash changes.#24Conversation
WalkthroughThe pull request updates the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CT as Curl Task
participant S as Shell
U ->> CT: Invoke curl task
CT ->> S: Download file (OUTPUT_FILE)
S -->> CT: File downloaded
CT ->> S: Compute & compare SHA256 hash for OUTPUT_FILE
S -->> CT: Hash verification result
CT ->> S: Echo URL into URL_FILE
CT ->> S: Compare expected URL with URL_FILE content
S -->> CT: URL verification result
CT ->> U: Return verification result
Suggested Reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
taskfiles/utils-remote.yaml (2)
17-17: Consider using a more robust path construction method.The current implementation replaces all periods with hash symbols, which could lead to unexpected results if
OUTPUT_FILEcontains multiple periods.Consider using a more targeted approach:
- URL_FILE: "{{.OUTPUT_FILE | replace \".\" \"#\"}}.url" + URL_FILE: "{{dir .OUTPUT_FILE}}/{{base .OUTPUT_FILE | trimSuffix (ext .OUTPUT_FILE)}}#url"
34-34: Consider moving URL file creation after successful download.Creating the URL file before attempting the download could lead to an inconsistent state if the download fails.
Consider moving the URL file creation inside the success branch:
- echo "{{.URL}}" > "{{.URL_FILE}}" max_attempts=3 attempt=1 while [ $attempt -le $max_attempts ]; do if curl \ --fail \ --location \ --show-error \ --connect-timeout 10 \ --max-time 300 \ "{{.URL}}" \ --output "{{.OUTPUT_FILE}}"; then + echo "{{.URL}}" > "{{.URL_FILE}}" break fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
taskfiles/utils-remote.yaml(3 hunks)
🔇 Additional comments (1)
taskfiles/utils-remote.yaml (1)
22-26: LGTM! Well-structured URL verification.The URL verification is implemented consistently with the existing hash verification pattern.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
taskfiles/utils-remote.yaml (1)
56-61: Standardize hash terminology.While the additional hash verification is valuable for user feedback, let's maintain consistent terminology throughout the codebase.
Apply this diff to align with the documentation:
- - "echo Verifying the commit hash of {{.OUTPUT_FILE}}." + - "echo Verifying the SHA256 hash of {{.OUTPUT_FILE}}."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
taskfiles/utils-remote.yaml(3 hunks)
🔇 Additional comments (3)
taskfiles/utils-remote.yaml (3)
17-17: LGTM! Well-structured variable definition and generates section.The variable naming and file path construction are clear and follow best practices. The generates section correctly declares both output files.
Also applies to: 20-20
22-25: LGTM! Effective URL verification in status check.The status check will properly trigger a re-run when the URL changes, addressing the core issue mentioned in the PR objectives.
34-34: LGTM! URL tracking implementation.The URL file creation is properly placed and will maintain a record of the source URL.
utils:curl to run when input urls or commit hash for the library target to be downloaded changes.utils:curl re-runs when input URLs or the library target's commit hash changes.
davidlion
left a comment
There was a problem hiding this comment.
Any reason we can't just add the SHA to the task label rather than storing a separate file?
Also, is it possible to just add some output in status?
I think the |
|
Closing this issue for the following reasons:
Afaik, there shouldn't be a need to re-download the file from a new/different URL if the existing file (
This fix has been separated and moved into #70. |
Description
Taskcan't force re-runs by detecting input argument changes. Without this PR,utils:curlwill stay ' up-to-date' if we change only theurlof the tar source but not itsFILE_SHA256.Also in such situations where the tar source downloaded doesn't match the given
FILE_SHA256, no error message is outputted. The task simply re-runs over and over again without users knowing that they have supplied a wrongFILE_SHA256.To solve the problems above:
URLvariable as plain text. On subsequent runs, we force the task to be re-run if the suppliedURLbecomes different and fails the task'sstatuscheck.statussimply runs the curl operation repeatedly.Checklist
breaking change.
Validation performed
ystdlib-cppby stimulating the adjusting of input arguments toutils:curl.utils:download-and-extract-tarandutils:cmake-install-remote-tar.