-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add spec for package pinning. #1894
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
f2bbdae
to
1ab51e8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic! This is almost exactly what we were thinking :)
I know the PowerShell cmdlets aren't ready for prime time, but I do want to treat them as first-class citizens in our specifications. We still need to wire up testing and a build pipeline for them.
Thanks so much for taking the time to write this!!!
|
||
## Solution Design | ||
|
||
A table of packages (by Name, Version and Product Code if available) that are currently pinned will be kept in the local tracking catalog. To be repository independent, these values should be correlated from local sources (Add and Remove Programs, Appx, etc). This also means that the user should be able to pin packages that were not installed via the Windows Package Manager, so that they won't be upgraded via `winget upgrade --all`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the traditional Microsoft Store packages, the Microsoft Store automatically updates programs. Users would not be able to pin those packages. I need to see if there is a way to pin packages from being automatically updated by the Microsoft Store.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to be as repo neutral as possible, but the Microsoft Store will definitely need special treatment. We need to be able to pin packages even if they are coming from "more normal" REST sources or Intune or whatever.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we define "pinning" as in this spec to "not be automatically upgraded via winget upgrade --all
", then anything that upgrades itself (or is upgraded by something else) automatically will be outside of our scope. We can't control most of them anyway, so it is the best thing we can do.
Now if it turns out that Store MSIX upgrades can be put on hold on a per-package basis, we can certainly integrate that control. But that seems like icing rather than cake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least with chocolatey, I ended up pinning for two reasons:
- I really want a specific version
- The application has it's own update mechanism
So the behavior of "the package may still auto update, and is just excluded from winget upgrade --all
makes a lot of sense to me.
doc/specs/#476 - Pinned packages.md
Outdated
``` | ||
|
||
``` | ||
winget pin --set <package> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should cover the intent for the PowerShell cmdlets also. I'd expect several cmdlets. One to show what's pinned, one to pin, and one to "unpin".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went through and added sample PowerShell cmdlet names (or arguments to existing cmdlets, where applicable), if you want to take a peek.
doc/specs/#476 - Pinned packages.md
Outdated
|
||
|
||
``` | ||
winget <upgrade/install> --include-pinned <package> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're working towards a slightly different behavior for "install" when a version is already on the system. We've been discussing essentially switching to "upgrade" unless the user does something like winget install <package> --force
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a really good idea, but in that case I think the argument should still be present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At line 90, you've written winget install --pin <package>
but here, it has been written winget <upgrade/install> --include-pinned <package>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--pin
is to create a new pin, --include-pinned
is acknowledging that you want to do something to a package that you've already pinned. Perhaps the distinction isn't relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could just standardize the same way the powershell one is and use --pinned
|
||
|
||
|
||
In addition to commands, there should be a new `Pinned` key in exported package lists, allowing users to move their pins to different systems: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have several other enhancements to add to the packages.json schema. We've been discussing supporting essentially all the install switches in the schema so running winget import packages.json
would have as much flexibility as though the user was performing discrete installs. The export command likely will be somewhat limited on what it can detect in terms of switches used during an earlier install or upgrade.
Microsoft.Bob, version 2.39.0. | ||
|
||
Microsoft Bob is currently pinned at version 2.32.0, but Rover the Dog 5.1 requires at least version 2.39.0. | ||
To upgrade Microsoft Bob and other pinned dependencies, add the argument --include-pinned to your previous command. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've gone back and forth between just "informing" users and giving an interactive [Y/N] sort of dialog to see if the user wants to perform suggested actions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A [Y/N] prompt would be more user-friendly, but it would make it harder to use winget in automation scenarios. Is there a way on Windows to detect if a user is interactively using a shell vs running a script? I know the way to tell apt on linux not to ask questions is via an environment variable, but that seems kludgy.
I guess we could also say that the way forward for automating winget is via the PowerShell library or directly using COM, but since that's not complete yet...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any of the [Y/N] style prompts are likely suitable for a setting to specify a default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know the way to tell apt on linux not to ask questions is via an environment variable, but that seems kludgy.
Sorry for being a little off topic. Just want to make sure you're talking about export DEBIAN_FRONTEND=noninteractive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You got it :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've requested that @lechacon do more extensive interactivity controls for another review. The goal would be to have a common option that disables interactivity, as well as a setting. Interactivity in general is not a good way to handle these cases, because the COM (and thus PowerShell eventually) entrypoints cannot use that pattern. But with a more generic and extensive set of controls and thoughts about non-interactive defaults, we can use them more (within reason).
|
||
``` | ||
winget pin | ||
PowerShell: Get-WinGetPinnedPackage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@denelon Should this be a separate cmdlet or an argument for Get-WinGetPackage
? I couldn't decide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be voting for Get-WinGetPackage -Pinned
😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PowerShell convention is more likely to be discrete cmdlets for pinning and unpinning. Looking at approved PowerShell verbs: https://docs.microsoft.com/en-us/powershell/scripting/developer/cmdlet/approved-verbs-for-windows-powershell-commands?view=powershell-7.2
We may also consider Lock and Unlock, but that might be confusing when compared with the "pin" name.
I've thrown a few suggestions below. I'll also mention @RDMacLachlan to get some input here:
Get-WinGetPinnedPackage - shows packages that have been pinned.
Set-WinGetPinnedPackage - pins a package
Clear-WinGetPinnedPackage - unpins a package
We may also want to consider packages that upgrade themselves as not pinnable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a few other suggestions below earlier; I like Clear
more than Remove
. I know you already mentioned lock/unlock, but I think different nouns could be used.
Lock-WinGetPackageVersion
and Unlock-WinGetPackageVersion
- From Common verbs
Disable-WinGetPackageUpgrade
and Enable-WinGetPackageUpgrade
- From Lifecycle verbs
Suspend-WingetPackageUpgrade
and Resume-WinGetPackageUpgrade
- From Lifecycle verbs
Note: After more consideration, I do like Set and Clear the best, since it maintains consistency
doc/specs/#476 - Pinned packages.md
Outdated
|
||
``` | ||
winget <upgrade/install> --include-pinned <package> | ||
PowerShell: Install-WinGetPackage -Pinned / Upgrade-WinGetPackage -Pinned |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With Install-WinGetPackage
the switch should be Pin
? Since the package "will be" pinned rather than "already being pinned".
Upgrade-WinGetPackage -Pinned
: Will the argument look for updates for "only" pinned packages or "including" pinned packages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would try to remain consistent here, and suggest using -Pinned
for both
doc/specs/#476 - Pinned packages.md
Outdated
|
||
|
||
``` | ||
winget <upgrade/install> --include-pinned <package> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At line 90, you've written winget install --pin <package>
but here, it has been written winget <upgrade/install> --include-pinned <package>
.
|
||
## Solution Design | ||
|
||
A table of packages (by Name, Version and Product Code if available) that are currently pinned will be kept in the local tracking catalog. To be repository independent, these values should be correlated from local sources (Add and Remove Programs, Appx, etc). This also means that the user should be able to pin packages that were not installed via the Windows Package Manager, so that they won't be upgraded via `winget upgrade --all`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we define "pinning" as in this spec to "not be automatically upgraded via winget upgrade --all
", then anything that upgrades itself (or is upgraded by something else) automatically will be outside of our scope. We can't control most of them anyway, so it is the best thing we can do.
Now if it turns out that Store MSIX upgrades can be put on hold on a per-package basis, we can certainly integrate that control. But that seems like icing rather than cake.
doc/specs/#476 - Pinned packages.md
Outdated
``` | ||
|
||
``` | ||
winget pin --set <package> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might make sense for --set
to be the default behavior, so instead it would be:
winget pin Foo.Bar
winget pin Foo.Bar --clear
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What command would you want to use to list pinned packages? I used winget pin
for that, since it was reminiscent of winget upgrade
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That does seem consistent with upgrade.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it would have to be a specific command; I think it can function exactly like upgrade, where winget pin
would list the pinned packages and winget pin <package>
would set the pin on that package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. winget pin
with no args shows the table, winget pin <package query>
sets the pin, and winget pin --clear <package query>
unpins. That would mean you wouldn't be able to filter the pin table, which probably doesn't happen that often but would make it inconsistent with install/list/upgrade.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see.
winget pin
with no args shows the table,winget pin <package query>
sets the pin, andwinget pin --clear <package query>
unpins. That would mean you wouldn't be able to filter the pin table, which probably doesn't happen that often but would make it inconsistent with install/list/upgrade.
That would be consistent with upgrade
, but not list
. An option to allow filtering would be winget pin <query> --list
. Considering it is a modifying command, I think it makes sense to remain consistent with upgrade
. --list
could even be added to upgrade
, although that behavior is already equivalently exposed via list
itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in latest commit. winget pin
with no args shows the list, winget pin <package>
pins, and winget pin --clear <package>
removes the pin.
doc/specs/#476 - Pinned packages.md
Outdated
PowerShell: Remove-WinGetPinnedPackage | ||
``` | ||
|
||
It functions similarly to `winget pin --set`, but it clears the pin. This should automatically happen on `winget uninstall`, or if it is detected the package is no longer present on the system. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it is detected the package is no longer present on the system.
A nice goal, but difficult for us to achieve. The actual implementation might rely on future installs detecting an abandoned tracking catalog entry and erasing it before installation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe some kind of updating will be necessary of those values regardless, as if a package is pinned because it manages updates itself, whenever the update happens the pinned version in the table will be out of sync with reality.
I'm happy to remove it for now, but it is something to think about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd put it in future considerations. When the user displays what is pinned or an individual package that is pinned, we could perform some kind of check, but I agree it's likely future work.
doc/specs/#476 - Pinned packages.md
Outdated
PowerShell: Install-WinGetPackage -Pinned / Upgrade-WinGetPackage -Pinned | ||
``` | ||
|
||
Upgrade and Install will now need to use the same argument to bypass the pin for a package temporarily, in which case the pin is updated to pin the new version after the installation completes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that this is necessary. The goal is that the standard upgrade --all
ignores these, but specifically targeted packages are how it should be done. Placing yet another hurdle seems likely to just trip people rather than actually preventing accidental upgrades.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved in latest commit. I said that it should warn that you are modifying a pinned package, but continue with the usual behavior.
|
||
Lastly, there should be good error messaging throughout the Windows Package Manager in order to explain to users when something doesn't go quite right. | ||
|
||
As an example, consider the situation where a user has pinned their favorite package, Microsoft Bob: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Microsoft Bob
😎
Microsoft.Bob, version 2.39.0. | ||
|
||
Microsoft Bob is currently pinned at version 2.32.0, but Rover the Dog 5.1 requires at least version 2.39.0. | ||
To upgrade Microsoft Bob and other pinned dependencies, add the argument --include-pinned to your previous command. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've requested that @lechacon do more extensive interactivity controls for another review. The goal would be to have a common option that disables interactivity, as well as a setting. Interactivity in general is not a good way to handle these cases, because the COM (and thus PowerShell eventually) entrypoints cannot use that pattern. But with a more generic and extensive set of controls and thoughts about non-interactive defaults, we can use them more (within reason).
If there is no package directly available for software a user wants to pin, but it is [included as part of another package](https://github.com/microsoft/winget-cli/issues/1073), the package which includes said piece of software should be treated as pinned unless a newer version doesn't change the version of the pinned software. | ||
|
||
``` | ||
winget pin --clear <package> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec should include discussion of how user driven pinning and RequiresExplicitUpgrade
interact. I assert that these are effectively the same thing, with the manifest field driving a default pinned state. The user should also be able to clear that default pin if they so desire.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this is an opportunity to look at what exactly the RequiresExplicitUpgrade field is supposed to show. If it is intended to show the default pinned state, I would suggest we consider renaming the field to something like Pinning:
and make it an enum rather than a boolean so that packages could be marked as unable to be pinned without requiring a separate field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unable to be pinned
That may not be necessary, since winget can't guarantee that a package won't be upgraded anyway, it can only guarantee that winget upgrade
won't upgrade the package without being explicitly asked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unable to be pinned
That may not be necessary, since winget can't guarantee that a package won't be upgraded anyway, it can only guarantee that
winget upgrade
won't upgrade the package without being explicitly asked.
True, but it would be an opportunity to keep the user informed if we know of specific packages that auto-upgrade. Just an idea, but probably outside the realm of this spec anyways
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RequiresExplicitUpgrade was about apps that update themselves and was something added to the manifest to somewhat declutter winget upgrade
for packages that handle upgrades themselves or for ISVs to prefer their own upgrade mechanism. #1163. The description in the schema probably shouldn't use the word "pinned" as pinning is more of a user driven behavior. Although logically it causes apps to be excluded from winget upgrade --all
which pinning also does.
Pinning a package is more about a user saying, "I don't want this upgraded". It's often the case for enterprises who have to support specific versions of applications, and developers wanting to have specific versions of packages for their needs.
We may also need to consider the case where packages won't be able to support pinning. Some packages are going to update no matter what "we" want. This could help us to inform users when we can't control the software upgrade mechanism.
|
||
``` | ||
winget pin --clear <package> | ||
PowerShell: Remove-WinGetPinnedPackage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this may be very opinion based, but to me, this powershell cmdlet naming makes me think it will uninstall the package that has been pinned.
I would suggest any of the followng (and encourage discussion)-
-Suggestions moved to discussion above-
doc/specs/#476 - Pinned packages.md
Outdated
|
||
``` | ||
winget <upgrade/install> --include-pinned <package> | ||
PowerShell: Install-WinGetPackage -Pinned / Upgrade-WinGetPackage -Pinned |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would try to remain consistent here, and suggest using -Pinned
for both
|
||
Outside of missing security updates and having possible dependency resolution failures due to pins, there should not be any other impacts. Considering the opt-in nature of pinning, it should not affect users unless they want to use it. | ||
|
||
## Future considerations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another consideration. As verified publishers start adding their packages, they may want to force override pins / require an update. Do we need a provision for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My opinion would be that publishers shouldn't be able to override pins via the manifests, as the main reason a user would want to pin a package is because they disagree with the ISV's patching schedule. If they wanted to get around it they could add an automatic updater to their piece of software (as many have chosen to do).
|
||
- Group Policy or MDM control of pinned packages would be a natural addition to the defined functionality. Administrators may want to restrict upgrades for certain pieces of conflicting software if it will cause issues for some or all of their users. | ||
|
||
- Packages may want to define pins for their dependencies, if they can't use anything above a certain version of a dependency. These pins could be merged into the user's current pins, making sure that the packages currently installed keep working. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great call out. We've been discussing this scenario with the dependency section. I'll be sure to add this kind of a concept. We've got the concept of a "minimum supported version" but were going back and forth on a "maximum supported version".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be in favor of a maximum version, since if a package requires a specific version then the minimum and maximum could be set to the same thing. This isn't a rare scenario, but it certainly isn't as common as a minimum version.
@Trenly and I were discussing another scenario in #1513 Pinning may need to be a bit more sophisticated in light of the possibility of redirecting manifests. I might be OK with pinning a part of a version. If I'm on 3.x and I'm happy with upgrades, but I wouldn't want to get redirected to an upgrade for 4.x I'd like to be able to pin part of a version. If we use semver as an example, I might want to pin to 3.x or even 3.8.x, but nothing higer. |
Speaking of this - and I'm just thought dumping here - What if a package is side-by-side compatible? Would pinning also prevent the installation of other versions if a package is required as a dependency? The Visual C++ Redistributables are a good example of this. I could pin I think we have considered the idea of a package which upgrades in place thoroughly, but I don't know if the cases for side-by-side apps have been considered, or if those would even need pinning as I believe the upgrade behavior is still being defined? |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
||
A table of packages (by Name, Version and Product Code if available) that are currently pinned will be kept in the local tracking catalog. To be repository independent, these values should be correlated from local sources (Add and Remove Programs, Appx, etc). This also means that the user should be able to pin packages that were not installed via the Windows Package Manager, so that they won't be upgraded via `winget upgrade --all`. | ||
|
||
Some applications would also like to opt-out of `winget upgrade --all` if they manage their own upgrades or often make breaking changes. As such, the `RequiresExplicitUpgrade` key was added in manifest schema v1.1. Given that this creates the same end result for the user, packages that opt-out of upgrades in this manner should behave like pinned packages to the user, and they should be able to override the behavior listed in the manifest if they choose. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does everyone like this RequiresExplicitUpgrade behavior? I believe it continues the trend of "sane defaults, but we'll let you break it".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That aligns with my desire to have safe and rational defaults, but to let "power users" drive the behavior they want with arguments and/or settings. Developers know what they want, and we should let them determine how the Windows Package Manger should behave. We should call out some of the concerns like unintended consequences when updating a package that may be breaking to other things installed on their system.
@jedieaston would you mind converting this to a draft while we iterate? |
Yep! Should’ve done that to start with, sorry. |
Closing in favor of #2611 :) |
Related to #476.
I have written a spec for package pinning, since I thought it was one of the big features where I understood how it had to work. Please rip it to shreds, since this is my first spec here and I didn't know how much detail to go into. I tried to include examples where necessary, but if you want more I can definitely write some!
Microsoft Reviewers: Open in CodeFlow