-
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 experimental support for multiple installed versions #4282
Conversation
Will this also resolve this issue once out of experimental? I only ask because I don't see anywhere in the uninstall flow where |
if (context.Args.Contains(Execution::Args::Type::TargetVersion)) | ||
{ | ||
Repository::PackageVersionKey versionKey{ "", context.Args.GetArg(Execution::Args::Type::TargetVersion) , "" }; | ||
std::shared_ptr<IPackageVersion> installedVersion = installed->GetVersion(versionKey); | ||
|
||
if (!installedVersion) | ||
{ | ||
context.Reporter.Error() << Resource::String::GetManifestResultVersionNotFound(Utility::LocIndView{ versionKey.Version }) << std::endl; | ||
// This error maintains consistency with passing an available version to commands | ||
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NO_MANIFEST_FOUND); | ||
} | ||
|
||
context.Add<Execution::Data::InstalledPackageVersion>(std::move(installedVersion)); | ||
} |
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.
@Trenly , selection of target 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.
Thanks! I must have missed that
Yes, I believe it would resolve that issue. |
src/AppInstallerCLI.sln
Outdated
@@ -180,6 +180,11 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "WinGetSourceCreator", "WinG | |||
EndProject | |||
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.WinGet.SharedLib", "PowerShell\Microsoft.WinGet.SharedLib\Microsoft.WinGet.SharedLib.csproj", "{272B2B0E-40D4-4F0F-B187-519A6EF89B10}" | |||
EndProject | |||
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "schema", "schema", "{46E419FD-C77F-4EFC-A0FC-2BAD93E60D12}" | |||
ProjectSection(SolutionItems) = preProject | |||
..\schemas\JSON\settings\settings.schema.0.2.json = ..\schemas\JSON\settings\settings.schema.0.2.json |
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.
Nit: settings.schema.0.2.json is already listed under WingetSchemas in the solution unless we wanted to explicitly listed here
@@ -346,45 +343,74 @@ namespace AppInstaller::Repository | |||
{ |
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.
nit: the comment "Supports only a single version of a single package at this time." could be removed
continue; | ||
} | ||
|
||
// We use the `SourceId` field to store the installed package identifier so that we can disambiguate keys is they have the same 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.
// We use the `SourceId` field to store the installed package identifier so that we can disambiguate keys is they have the same version. | |
// We use the `SourceId` field to store the installed package identifier so that we can disambiguate keys if they have the same version. | |
m_versionKeyData.emplace_back(std::move(keyData)); | ||
} | ||
|
||
m_packages.emplace_back(std::move(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.
In rare cases that all the versions of the package get dropped, do we want to drop the package as well?
|
||
for (const auto& availablePackage : currentMatch.Package->GetAvailablePackages()) | ||
{ | ||
auto& packageFoldData = foldData.at(availablePackage); |
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.
Is the availablePackage converted to InstalledResultFoldKey automatically here?
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.
Yes, there is a non-explicit constructor from a std::shared_ptr<IPackage>
.
Movement toward north star #2129
Change
This change introduces a new experimental feature flag called
sideBySide
that enables multiple instances of an installed package to be found by search results. TheCompositeSource
will fold these all together into a single package result through their common association with available packages.Warning
This experimental feature affects the search results that the rest of the Windows Package Manager code relies on. Enabling it may change the behavior of downstream components.
Important
Only the commands in
winget.exe
have been updated to account for this change. Other interfaces will get reasonable defaults, but may not be able to interact properly in all cases.Installed identifier change
The most visible effect of this feature being enabled is that the identifier generated for installed items is no longer simply the registry key name or package family name but is instead constructed to make it fully unique. This is mostly relevant to packages without an available counterpart. For packages that are registered with the system through the registry, the identifier is now of the form
ARP\User\x64\ProductCode
. For MSIX, the form isMSIX\Package_Full_Name
. The previous identifiers are still searchable data though, so it is hopefully not more difficult tolist
installed items.Installed item correlations
The implementation in
CompositeSource
tries to separate the steps into:In an effort to prevent overly broad correlations from available to installed items, there is an additional attempt to ensure that there are no stronger correlations from installed to available. As an example, if a package is split by version while still sharing a name that is reasonably close, searching for one of the version subsets should not attempt to associate all of the installed items so long as the other versions have stronger correlations.
A1
has versions 1.0 - 1.5 with names likeA Package (1.0)
A2
has versions 2.0 - 2.2 with names likeA Package (2.0)
A2
would correlate with version1.0
because the normalized names are the sameA1
Once the basic results are discovered, the results are folded together. A comment on that function indicates the methodology and possible future improvements:
Command changes
list
list
has been updated to simply output all versions contained within a single search result as independent lines. This keeps the resulting output basically the same, but that has the downside of not explicitly indicating the correlation between the items.repair
repair
was updated internally so that passing in a--version
would cause that specific version to be targeted for repair.uninstall
uninstall
should behave as before if only a single installed version is present. When multiple installed versions exist, an error is produced unless one of two new arguments is supplied:--all-versions
/--all
will uninstall all installed versions--version
/-v
will uninstall the targeted version onlyDiscussion
I have created a discussion for this experimental feature: #4281
The goal is to allow feedback to be better collected and threaded, although I expect that there will be issues created independently as well.
Additionally...
I added a script in the
tools
that launches the sandbox with the dev package registered: Launch-DevPackageInSandbox.ps1Since it is dev registered in the sandbox, all you need to do to update it is build and deploy from Visual Studio and the sandbox version will just use those latest files.
Validation
Manual validation of the target scenario (multiple installed versions).
Unit tests added for core folding behavior.
More tests should be added before / as part of moving out of experimental.
Microsoft Reviewers: Open in CodeFlow