Skip to content

Stop saved versions from pinning updates and report Chocolatey no-op updates - #5700

Open
Gabriel Dufresne (GabrielDuf) wants to merge 3 commits into
mainfrom
worktree-fix-5672-choco-version-override
Open

Gabriel Dufresne (GabrielDuf) wants to merge 3 commits into
mainfrom
worktree-fix-5672-choco-version-override

Conversation

@GabrielDuf

Copy link
Copy Markdown
Contributor

Fixes #5672

Problem

A per-package Version to install saved from the options dialog was also applied to updates. Chocolatey turned it into choco upgrade <id> --version=<saved> --allow-downgrade. When the saved version is the installed one, that upgrades nothing, exits 0, and UniGetUI reported "updated successfully" while the update kept coming back. .NET Tool, Pip and Cargo applied a saved version to updates the same way.

Changes

  • A saved version is install-only. Both options dialogs (Avalonia and TUI) only allow setting it for Install. An update operation now drops it unless it was explicitly requested through the new transient InstallOptions.VersionIsExplicit, which the CLI/IPC path sets. History, the update list and reinstall-on-update follow the same rule. The saved options on disk are not modified.
  • Chocolatey no-op updates are judged correctly. Chocolatey exits 0 even when it upgrades nothing, so the last upgraded X/Y summary decides. When nothing was upgraded:
    • if Chocolatey's newest available version already reaches the offered one, the update is a success (the package was already current);
    • otherwise it is a failure with a reason.
  • Failure explanations. The reason comes from a new IPackageOperationHelper.GetFailureExplanation hook. It returns nothing by default, so other managers are unchanged. Two new English strings were added to lang_en.json.
  • Automatic retries are judged on their own output. The AutoRetry loop never wrote the separator that resets captured output, so every manager also judged a retried attempt on the previous attempt's lines. In the live test, a successful elevated retry was read as a failure because of the unelevated attempt's upgraded 0/0. The loop now writes the same separator a manual retry does.
  • Pinned Chocolatey packages are not offered as updates. choco upgrade always skips them, so they would otherwise fail on every run. They are read from the Pinned column of choco outdated.

The #5686 reinstall test, which kept a saved version on update, is now a theory: only an explicit version on a manager that supports it is reused.

Testing

  • Unit tests: new tests cover the saved and explicit version rules, reinstall-on-update, history, Chocolatey no-op classification and explanations, pinned parsing, and AutoRetry output isolation. With the operations changes reverted, exactly the new dependent cases fail.
  • Targeted suites: Chocolatey, operations, reinstall, history, install options, Cargo, Pip, .NET Tool and IPC tests pass (606/606).
  • Build and checks: UniGetUI.Windows.slnx builds with no new warnings. dotnet format whitespace, dotnet format style and Verify-Translations.ps1 all pass.
  • Live on Chocolatey 2.7.4, through a headless debug build and uniget package update, with a saved Version: 8.17.0 for curl:
    • Without the fix: arguments --version=8.17.0 --allow-downgrade, upgraded 0/1, reported "Curl was updated successfully", curl still at 8.17.0. This reproduces the issue.
    • With the fix: no --version was sent, and curl was upgraded 8.17.0 → 8.23.0 through the elevated retry. Reported as a success.
    • Already-current package: reported as a success.
    • Explicit --version 8.23.0: still passed to choco.

Not covered

  • Pinned packages were not tested on a real package, because pinning needs admin rights. The column layout was confirmed from Chocolatey's own log.
  • Retrying from operation history does not reuse an explicit update version, because the flag is not saved with the history record.

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Explicit-version no-op updates are misclassified as failures, while standalone update commands retain saved version pins and behave inconsistently with queued updates.

2 open issues
What changed in this PR

This PR addresses #5672 by preventing saved installation versions from pinning queued updates and improving Chocolatey’s reporting when an update changes nothing.

Changes:

  • Distinguishes explicitly requested versions from saved installation preferences, including reinstall and history behavior.
  • Classifies Chocolatey no-op updates, provides failure explanations, and filters pinned packages from available updates.
  • Isolates automatic-retry output and adds regression tests.
File Description
src/​UniGetUI.PackageEngine.Tests/​PackageOperationsTests.cs Tests version selection, reinstall behavior, and retry-output isolation.
src/​UniGetUI.PackageEngine.Tests/​OperationHistoryTests.cs Tests history versions for saved and explicit targets.
src/​UniGetUI.PackageEngine.Tests/​ChocolateyManagerTests.cs Tests no-op results, explanations, explicit versions, and pinned filtering.
src/​UniGetUI.PackageEngine.Serializable/​InstallOptions.cs Adds and copies the transient explicit-version flag.
src/​UniGetUI.PackageEngine.PackageManagerClasses/​Manager/​Helpers/​BasePkgOperationHelper.cs Provides the default failure-explanation hook.
src/​UniGetUI.PackageEngine.PackageManagerClasses/​Manager/​Classes/​NullPackageManager.cs Implements the new hook for the null manager.
src/​UniGetUI.PackageEngine.Operations/​PackageOperations.cs Clears saved update versions and consumes manager failure explanations.
src/​UniGetUI.PackageEngine.Operations/​AbstractOperation.cs Separates captured output between automatic retries.
src/​UniGetUI.PackageEngine.Managers.Chocolatey/​Helpers/​ChocolateyPkgOperationHelper.cs Classifies no-op updates and explains failures.
src/​UniGetUI.PackageEngine.Managers.Chocolatey/​Chocolatey.cs Excludes pinned packages from available updates.
src/​UniGetUI.PackageEngine.Interfaces/​ManagerHelpers/​IPackageOperationHelper.cs Defines the failure-explanation contract.
src/​UniGetUI.Interface.IpcApi/​IpcPackageApi.cs Marks requested versions as explicit.
src/​Languages/​lang_en.json Adds English Chocolatey failure messages.

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Concurrent Chocolatey updates can overwrite shared target-version state and misclassify results, while retries can display stale failure explanations.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

Medium severity Reset failure explanations before each retry

src/​UniGetUI.PackageEngine.Operations/​PackageOperations.cs:1221

This explanation persists across retries of the same operation. If a Chocolatey no-op first fails because its sources only offer the old version, then a manual retry fails for a different reason (such as a network error with a nonzero exit code), GetFailureExplanation returns null and the previous source/version explanation is still displayed as the retry's failure message. Restore the generic failure message at the start of each attempt before applying any attempt-specific explanations.

2 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues block approval of the update-version handling and Chocolatey result-reporting changes.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@randy-but-a-ro randy-but-a-ro Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 Pull request was approved automatically: the AI review is complete and all its review threads are resolved. 🎉

Integration Details
{
	"deliveryId": "307f2c20-da22-11f1-9c70-a10c8d189d35",
	"headSha": "8204175f2b3645a26c88c964464932c54c7f6b3b",
	"reviewer": "copilot-pull-request-reviewer[bot]"
}

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working chocolatey

Development

Successfully merging this pull request may close these issues.

[BUG] Chocolatey update reports success without upgrading when a saved version override targets the installed version

2 participants