Skip to content

Conversation

niheaven
Copy link
Member

@niheaven niheaven commented Aug 13, 2025

Description

Force sync tags while running scoop update to update Scoop Core.

Motivation and Context

How Has This Been Tested?

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

Summary by CodeRabbit

  • Bug Fixes

    • scoop update now force-syncs tags with the remote, ensuring all tags are fetched and updated during the operation.
  • Documentation

    • Added an Unreleased changelog entry under Bug Fixes noting the tag force-sync behavior in scoop update.

@HUMORCE
Copy link
Member

HUMORCE commented Aug 18, 2025

Should we implement similar changes to bucket updates?

@niheaven
Copy link
Member Author

A bucket should not have tag, imo.

@HUMORCE
Copy link
Member

HUMORCE commented Aug 18, 2025

I mean something like hard reset when conflicting.

@niheaven
Copy link
Member Author

Stash? If someone made their own change.

@z-Fng
Copy link

z-Fng commented Sep 25, 2025

Would it be possible to merge these hotfixes (#6439, #6454) and release a new version soon?

It’s been a month since the last release.

Copy link

coderabbitai bot commented Sep 26, 2025

Walkthrough

The update adjusts scoop-update to run git pull with --tags and --force when no repo/branch change is detected, ensuring tag synchronization. A corresponding Unreleased Bug Fix entry is added to CHANGELOG.md referencing this behavior.

Changes

Cohort / File(s) Summary of changes
Changelog entry
CHANGELOG.md
Added Unreleased Bug Fix note: scoop-update now force-syncs tags with remote during update.
Update logic: force tag sync
libexec/scoop-update.ps1
In Sync-Scoop else branch, changed git pull flags from -q to -q --tags --force to fetch all tags and force updates when no remote/branch change is detected.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant S as scoop-update.ps1
  participant G as Git Remote

  U->>S: Run scoop update
  S->>S: Detect repo/branch change?
  alt Repo/branch changed
    S->>G: git fetch/pull (existing logic)
    G-->>S: Updated refs
  else No change detected
    rect rgba(180,220,255,0.25)
    note over S,G: Changed behavior: force-sync tags
    S->>G: git pull -q --tags --force
    end
    G-->>S: Tags and refs synced
  end
  S-->>U: Update complete or error
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I tug the tags with gentle force,
Hop-hop, I course-correct the source.
No sneaky re-tag blocks my way—
I fetch them all, come what may.
Carrot-commit, parsley-pull,
Now updates bloom, repositories full. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning The pull request introduces forced tag synchronization via git pull flags, satisfying the core requirement to prevent tag mutation from blocking updates and explicitly configuring tag replacement, but it does not add meaningful error messaging for tag conflict failures as requested in issue #6438. Please add or update error handling to provide a clear, descriptive message when tag conflicts occur during scoop update, as outlined in issue #6438, to meet the linked issue expectations.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed Title accurately describes the main change by indicating that the scoop-update command will now force synchronize tags with the remote branch, summarizing the primary fix in a concise and clear manner.
Out of Scope Changes Check ✅ Passed All modifications are confined to the scoop-update script and the changelog entry, directly supporting the objectives of issue #6438, and no unrelated features or files have been altered.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-force-tag

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@niheaven
Copy link
Member Author

I'll check for #6454 soon.

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 (2)
CHANGELOG.md (1)

9-9: Link to PR, not issue, per project convention

Change the link target to the PR instead of the issue to match Scoop’s CHANGELOG convention.

Apply this diff:

- - **scoop-update**: Force sync tags w/ remote branch while scoop update ([#6439](https://github.com/ScoopInstaller/Scoop/issues/6439))
+ - **scoop-update**: Force sync tags w/ remote branch while scoop update ([#6439](https://github.com/ScoopInstaller/Scoop/pull/6439))

Based on learnings

libexec/scoop-update.ps1 (1)

139-139: Good fix; also prune tags to fully align with remote

Fetching with --tags --force addresses retagging. Consider adding --prune-tags so locally-deleted/renamed tags are cleaned up too.

Apply this diff:

-            Invoke-Git -Path $currentdir -ArgumentList @('pull', '--tags', '--force', '-q')
+            Invoke-Git -Path $currentdir -ArgumentList @('pull', '--tags', '--prune-tags', '--force', '-q')
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b592b38 and 628c8d7.

📒 Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • libexec/scoop-update.ps1 (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: z-Fng
PR: ScoopInstaller/Scoop#6471
File: CHANGELOG.md:9-9
Timestamp: 2025-08-31T01:48:00.222Z
Learning: The Scoop project's CHANGELOG.md follows a convention of tracking PR numbers only, not issue numbers, according to the maintainer z-Fng.

@niheaven niheaven merged commit db8d554 into develop Sep 26, 2025
3 checks passed
@niheaven niheaven deleted the feat-force-tag branch September 26, 2025 07:20
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.

3 participants