-
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
Implement package pinning #2813
Conversation
Co-authored-by: Kaleb Luedtke <trenlymc@gmail.com>
# Conflicts: # src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters # src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj # src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj.filters # src/AppInstallerCommonCore/Public/AppInstallerRuntime.h
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Kaleb Luedtke <trenlymc@gmail.com>
I moved the logic to filter out pinned versions out of CompositePackage's GetAvailableVersionKeys() and GetAvailableVersion(). Instead, now the version keys returned from GetAvailableVersionKeys() include the pinned state for that version. Versions from different sources may have different pin types, and versions that match a gating pin are not marked as pinned. Callers can the use that info to decide what to do. I added another function like GetAvailableVersion() that also returns the pinned state. I kept the PinBehavior argument for GetLatestAvailableVersion() because the logic there is more complicated and so it is harder to have each caller do it. I'm still missing using the --force argument, and the messages that I'm exposing could probably be more specific. |
/azp run |
Pull request contains merge conflicts. |
# Conflicts: # src/AppInstallerCLICore/Commands/UpgradeCommand.cpp # src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw # src/AppInstallerSharedLib/Errors.cpp # src/AppInstallerSharedLib/Public/AppInstallerErrors.h
src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h
Outdated
Show resolved
Hide resolved
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 great news! I can't wait to test it :) |
We're checking to see if any other PRs are super close to done, and we'll get a preview build out quickly™️. |
This PR builds on top of #2769. I can close that PR in favor of this one, or we can merge that one first to make the changes smaller and easier to review :)
Changes in this PR (not in #2769):
CompositePackage
to be able to hold more than one available package, each from a different source. TheGetAvailableVersionKeys()
function now can return versions from multiple sources, so the source field ofPackageVersionKey
is more relevant now. If the same version is available from multiple sources, they are returned in the order the available packages were added, which matches the previous behavior of just adding the first available package found.PinBehavior
enum which is passed as an argument to anIPackage
's functions for getting available versions. Only theCompositePackage
uses it (as it needs both an installed and an available package) and it is used to determine whether we ignore pinned versions when listing the available versions or not. For example, forlist
we don't care about pins, forupgrade
we do consider them, and forupgrade --include-pinned
we care about them unless they have Pinning type.CompositeSource
tests now need to ensure that the test packages have a reference to their source as it is used to determine if they are pinned.RequiresExplicitUpgrade
manifest field. This change is mostly independent of that one, as in my mind the pinning fromRequiresExplicitUpgrade
is a property of the installed package, whereas user-defined pins are properties of the available packages from each source, so they are handled at different places.Closes #476
Related to #2611
Microsoft Reviewers: Open in CodeFlow