Skip to content

fix: Ensure utils:curl re-runs when input URLs or the library target's commit hash changes. #24

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Bill-hbrhbr
Copy link
Contributor

@Bill-hbrhbr Bill-hbrhbr commented Feb 14, 2025

Description

Task can't force re-runs by detecting input argument changes. Without this PR, utils:curl will stay ' up-to-date' if we change only the url of the tar source but not its FILE_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 wrong FILE_SHA256.

To solve the problems above:

  • We introduce a file that stores the URL variable as plain text. On subsequent runs, we force the task to be re-run if the supplied URL becomes different and fails the task's status check.
  • We add a seemingly redundant commit hash check at the end of the task to inform users of the commit hash mismatch, otherwise the commit hash check in status simply runs the curl operation repeatedly.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

  • Tested extensively in ystdlib-cpp by stimulating the adjusting of input arguments to utils:curl.
  • Tested that it works well even when embedded in parent tasks such as utils:download-and-extract-tar and utils:cmake-install-remote-tar.

Copy link

coderabbitai bot commented Feb 14, 2025

Walkthrough

The pull request updates the curl task in taskfiles/utils-remote.yaml. A new variable URL_FILE is introduced, derived from OUTPUT_FILE by replacing the period with a hash and appending .url. The task now generates both OUTPUT_FILE and URL_FILE, and it includes commands to echo the URL into URL_FILE and verify its content. Additionally, the file verification process now compares the expected SHA256 hash with the computed hash of OUTPUT_FILE before checking the URL file content.

Changes

File Change Summary
taskfiles/utils-remote.yaml - Added URL_FILE variable (derived from OUTPUT_FILE)
- Updated generates to include both OUTPUT_FILE and URL_FILE
- Added commands to echo URL into URL_FILE and verify its content, along with existing SHA256 verification

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
Loading

Suggested Reviewers

  • davidlion
  • kirkrodrigues

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 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_FILE contains 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

📥 Commits

Reviewing files that changed from the base of the PR and between 368bdf6 and d70fa61.

📒 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>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between d70fa61 and 3c0e7d9.

📒 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.

@Bill-hbrhbr Bill-hbrhbr changed the title fix: Force utils:curl to run when input urls or commit hash for the library target to be downloaded changes. fix: Ensure utils:curl re-runs when input URLs or the library target's commit hash changes. Feb 14, 2025
Copy link
Member

@davidlion davidlion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@Bill-hbrhbr
Copy link
Contributor Author

Bill-hbrhbr commented Feb 18, 2025

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 label works well with sources, but status operates independently, disregarding the label of the task. I can showcase this behavior with a dummy example.
Also what kinds of output are you thinking in status? Do you mean extra logging statements (e.g. on the checks we are performing)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants