Skip to content
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.

Sign up for GitHub

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

Jump to bottom

(#2050) Upgrade add parameter to ignore pins #2464

Merged
merged 3 commits into from
Apr 25, 2024
Merged

(#2050) Upgrade add parameter to ignore pins #2464

merged 3 commits into from
Apr 25, 2024

Conversation

TheCakeIsNaOH
Copy link
Member

@TheCakeIsNaOH TheCakeIsNaOH commented Nov 21, 2021 •
edited by gep13
Loading

Description Of Changes

This adds a switch to the upgrade command to bypass/ignore any pinned
packages. It will write out a warning message if a pinned package is
being upgraded.

Motivation and Context

This is useful if you want to upgrade a pinned package and have it remain pinned after the upgrade.

Testing

REM if needed
REM .\choco.exe  pin remove -n iperf2 --allow-unofficial
REM .\choco.exe uninstall iperf2 --allow-unofficial
.\choco.exe install iperf2 --version=2.1.1 --allow-unofficial
.\choco.exe pin add -n iperf2  --allow-unofficial
.\choco.exe pin list --allow-unofficial # Validate iperf2 is pinned
.\choco.exe upgrade iperf2 --allow-unofficial # Validate that without bypass-pins then pinned packages are not upgraded
.\choco.exe upgrade iperf2 --ignore-pin --allow-unofficial # Validate that with bypass-pins then pinned packages are upgraded if upgrade is available 
.\choco.exe pin list --allow-unofficial # Validate that iperf2 is still pinned

Change Types Made

  • Bug fix (non-breaking change)
  • Feature / Enhancement (non-breaking change)
  • Breaking change (fix or feature that could cause existing functionality to change)

Related Issue

Fixes #2050
Depends on #2463 (Now merged)
Depends on #2484 (Now merged)

Change Checklist

  • Requires a change to the documentation
  • Documentation has been updated
  • Tests to cover my changes, have been added
  • All new and existing tests passed.
  • N/A PowerShell v2 compatibility checked.

@TheCakeIsNaOH
Copy link
Member Author

TheCakeIsNaOH commented Nov 21, 2021 •
edited
Loading

If this switch also should be added to the install and uninstall commands, it should be trivial.

For some reason, the upgraded package is not pinned, and I'm not sure why that is.
Edit: Now working.

@TheCakeIsNaOH TheCakeIsNaOH force-pushed the bypass-pin branch 4 times, most recently from 77d8f45 to 387cc64 Compare January 17, 2022 05:17
@TheCakeIsNaOH
Copy link
Member Author

TheCakeIsNaOH commented Jan 17, 2022 •
edited
Loading

For some reason, the upgraded package is not pinned, and I'm not sure why that is.

I figured it out, the configuration does not automatically trickle back up to into ChocolateyPackageService which is needed to set pkgInfo.IsPinned. Thus, I've rebased this on top of #2484, and calling the resetConfigAction if config.IsPinned needs to be set to true.

Edit: Now fixed.

@TheCakeIsNaOH TheCakeIsNaOH marked this pull request as ready for review January 17, 2022 05:22
@coveralls
Copy link

coveralls commented Jun 27, 2022 •
edited
Loading

Coverage Status

Coverage remained the same at 27.57% when pulling 13eea3b on TheCakeIsNaOH:bypass-pin into 3892cfb on chocolatey:develop.

@TheCakeIsNaOH TheCakeIsNaOH marked this pull request as draft September 23, 2022 04:52
@TheCakeIsNaOH TheCakeIsNaOH force-pushed the bypass-pin branch 2 times, most recently from 5865e6c to a2387aa Compare September 23, 2022 05:22
@TheCakeIsNaOH TheCakeIsNaOH marked this pull request as ready for review September 23, 2022 05:23
@TheCakeIsNaOH TheCakeIsNaOH force-pushed the bypass-pin branch 2 times, most recently from 05ddb4a to 13eea3b Compare September 23, 2022 05:26
@TheCakeIsNaOH TheCakeIsNaOH force-pushed the bypass-pin branch 2 times, most recently from 224ea75 to a1b925e Compare January 13, 2023 15:25
@TheCakeIsNaOH TheCakeIsNaOH mentioned this pull request Sep 9, 2023
Closed
2 tasks
@gep13
Copy link
Member

gep13 commented Dec 14, 2023

During a conversation with @pauby the suggestion was made that the new option should be --ignore-pin.

The term bypass is only used on one other option, whereas there are 8 that have ignore and 7 that have skip. To start to bring some consistency, let's stick with ignore in this scenario.

@TheCakeIsNaOH TheCakeIsNaOH force-pushed the bypass-pin branch 3 times, most recently from f67c629 to b391ba3 Compare December 14, 2023 21:52
@TheCakeIsNaOH
Copy link
Member Author

To start to bring some consistency, let's stick with ignore in this scenario.

That sound good, I've fixed that up and rebased the PR.

@gep13 gep13 force-pushed the bypass-pin branch 2 times, most recently from 567c2e2 to 2672457 Compare April 25, 2024 07:41
@gep13 gep13 changed the title (#2050) Upgrade add parameter to bypass pins (#2050) Upgrade add parameter to ignore pins Apr 25, 2024
@gep13
Copy link
Member

gep13 commented Apr 25, 2024

Given that there is already --ignore-pinned on the Outdated command, decision has been made to change to use an --ignore-pinned option in this PR as well. @pauby

TheCakeIsNaOH and others added 3 commits April 25, 2024 05:21
@TheCakeIsNaOH @gep13
(chocolatey#2050) Upgrade add parameter to ignore pins
fb22630 
This adds a switch to the upgrade command to bypass/ignore any pinned
packages. It will write out a warning message if a pinned package is
being upgraded.

The package will remain pinned after the upgrade completes, although
pinned to the new version.
@gep13
(maint) Remove unnecessary whitespace
552ee7a
@gep13
(chocolatey#2050) Add basic unit test for new option
b9005df 
This commit adds a simple unit test to verify that the --ignore-pin
option appears in the OptionSet that is generated.
gep13
gep13 approved these changes Apr 25, 2024
View reviewed changes
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13
Copy link
Member

gep13 commented Apr 25, 2024

@TheCakeIsNaOH thank you very much for taking the lead with getting this feature added!

@gep13 gep13 merged commit 2415117 into chocolatey:develop Apr 25, 2024
5 checks passed
@TheCakeIsNaOH TheCakeIsNaOH deleted the bypass-pin branch April 26, 2024 03:56
@soredake soredake mentioned this pull request Apr 28, 2024
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gep13 gep13 gep13 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow upgrading a pinned package without removing the pin beforehand
3 participants
@TheCakeIsNaOH @coveralls @gep13