Skip to content

Commit d0ff81e

Browse files
author
Flor Chacón
authored
Add support for RequireExplicitUpgrade manifest element (#1795)
1 parent a5e202c commit d0ff81e

File tree

10 files changed

+343
-87
lines changed

10 files changed

+343
-87
lines changed

src/AppInstallerCLICore/Commands/UpgradeCommand.cpp

+18-13
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ namespace AppInstaller::CLI
4343
// either for upgrading or for listing available upgrades.
4444
bool HasArgumentsForMultiplePackages(Execution::Args& execArgs)
4545
{
46-
return execArgs.Contains(Args::Type::All);
46+
return execArgs.Contains(Args::Type::All) ||
47+
execArgs.Contains(Args::Type::IncludeUnknown);
4748
}
4849

4950
// Determines whether there are any arguments only used as options during an upgrade,
@@ -102,7 +103,7 @@ namespace AppInstaller::CLI
102103
Argument::ForType(Args::Type::AcceptSourceAgreements),
103104
Argument::ForType(Execution::Args::Type::CustomHeader),
104105
Argument{ "all", Argument::NoAlias, Args::Type::All, Resource::String::UpdateAllArgumentDescription, ArgumentType::Flag },
105-
Argument{ "include-unknown", Argument::NoAlias, Args::Type::IncludeUnknown, Resource::String::IncludeUnknownArgumentDescription, ArgumentType::Flag }
106+
Argument{ "include-unknown", Argument::NoAlias, Args::Type::IncludeUnknown, Resource::String::IncludeUnknownArgumentDescription, ArgumentType::Flag },
106107
};
107108
}
108109

@@ -158,25 +159,29 @@ namespace AppInstaller::CLI
158159

159160
void UpgradeCommand::ValidateArgumentsInternal(Execution::Args& execArgs) const
160161
{
161-
if (execArgs.Contains(Execution::Args::Type::Manifest) &&
162+
if (execArgs.Contains(Execution::Args::Type::Manifest) &&
162163
(HasSearchQueryArguments(execArgs) ||
163-
HasArgumentsForMultiplePackages(execArgs) ||
164-
HasArgumentsForSource(execArgs)))
164+
HasArgumentsForMultiplePackages(execArgs) ||
165+
HasArgumentsForSource(execArgs)))
165166
{
166167
throw CommandException(Resource::String::BothManifestAndSearchQueryProvided);
167168
}
168169

169-
170-
else if (!ShouldListUpgrade(execArgs)
171-
&& !HasSearchQueryArguments(execArgs)
172-
&& (execArgs.Contains(Args::Type::Log) ||
173-
execArgs.Contains(Args::Type::Override) ||
174-
execArgs.Contains(Args::Type::InstallLocation) ||
175-
execArgs.Contains(Args::Type::HashOverride) ||
176-
execArgs.Contains(Args::Type::AcceptPackageAgreements)))
170+
if (!ShouldListUpgrade(execArgs)
171+
&& !HasSearchQueryArguments(execArgs)
172+
&& (execArgs.Contains(Args::Type::Log) ||
173+
execArgs.Contains(Args::Type::Override) ||
174+
execArgs.Contains(Args::Type::InstallLocation) ||
175+
execArgs.Contains(Args::Type::HashOverride) ||
176+
execArgs.Contains(Args::Type::AcceptPackageAgreements)))
177177
{
178178
throw CommandException(Resource::String::InvalidArgumentWithoutQueryError);
179179
}
180+
181+
if (HasArgumentsForSinglePackage(execArgs) && HasArgumentsForMultiplePackages(execArgs))
182+
{
183+
throw CommandException(Resource::String::IncompatibleArgumentsProvided);
184+
}
180185
}
181186

182187
void UpgradeCommand::ExecuteInternal(Execution::Context& context) const

src/AppInstallerCLICore/ExecutionArgs.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,11 @@ namespace AppInstaller::CLI::Execution
7979
AdminSettingEnable,
8080
AdminSettingDisable,
8181

82+
// Upgrade Command
83+
All, // Update all installed packages to latest
84+
IncludeUnknown, // Allow upgrades of packages with unknown versions
85+
8286
// Other
83-
All, // Used in Update command to update all installed packages to latest
8487
ListVersions, // Used in Show command to list all available versions of an app
8588
NoVT, // Disable VirtualTerminal outputs
8689
RetroStyle, // Makes progress display as retro
@@ -91,7 +94,6 @@ namespace AppInstaller::CLI::Execution
9194
DependencySource, // Index source to be queried against for finding dependencies
9295
CustomHeader, // Optional Rest source header
9396
AcceptSourceAgreements, // Accept all source agreements
94-
IncludeUnknown, // Used in Upgrade command to allow upgrades of packages with unknown versions
9597
Wait, // Prompts the user to press any key before exiting
9698

9799
// Used for demonstration purposes

src/AppInstallerCLICore/Resources.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ namespace AppInstaller::CLI::Resource
109109
WINGET_DEFINE_RESOURCE_STRINGID(ImportSearchFailed);
110110
WINGET_DEFINE_RESOURCE_STRINGID(ImportSourceNotInstalled);
111111
WINGET_DEFINE_RESOURCE_STRINGID(IncludeUnknownArgumentDescription);
112+
WINGET_DEFINE_RESOURCE_STRINGID(IncompatibleArgumentsProvided);
112113
WINGET_DEFINE_RESOURCE_STRINGID(InstallAndUpgradeCommandsReportDependencies);
113114
WINGET_DEFINE_RESOURCE_STRINGID(InstallArchitectureArgumentDescription);
114115
WINGET_DEFINE_RESOURCE_STRINGID(InstallationAbandoned);
@@ -373,12 +374,13 @@ namespace AppInstaller::CLI::Resource
373374
WINGET_DEFINE_RESOURCE_STRINGID(UnsupportedArgument);
374375
WINGET_DEFINE_RESOURCE_STRINGID(UpdateAllArgumentDescription);
375376
WINGET_DEFINE_RESOURCE_STRINGID(UpdateNotApplicable);
377+
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeAvailableForPinned);
376378
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeCommandLongDescription);
377379
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeCommandShortDescription);
378380
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeDifferentInstallTechnology);
379381
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeDifferentInstallTechnologyInNewerVersions);
380-
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeUnknownCount);
381-
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeUnknownCountSingle);
382+
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeRequireExplicitCount);
383+
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeUnknownVersionCount);
382384
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeUnknownVersionExplanation);
383385
WINGET_DEFINE_RESOURCE_STRINGID(Usage);
384386
WINGET_DEFINE_RESOURCE_STRINGID(ValidateCommandLongDescription);

src/AppInstallerCLICore/Workflows/UpdateFlow.cpp

+71-38
Original file line numberDiff line numberDiff line change
@@ -44,32 +44,41 @@ namespace AppInstaller::CLI::Workflow
4444
bool updateFound = false;
4545
bool installedTypeInapplicable = false;
4646

47-
if (!installedVersion.IsUnknown() || context.Args.Contains(Execution::Args::Type::IncludeUnknown))
47+
if (installedVersion.IsUnknown() && !context.Args.Contains(Execution::Args::Type::IncludeUnknown))
4848
{
49-
// The version keys should have already been sorted by version
50-
const auto& versionKeys = package->GetAvailableVersionKeys();
51-
for (const auto& key : versionKeys)
49+
// the package has an unknown version and the user did not request to upgrade it anyway.
50+
if (m_reportUpdateNotFound)
5251
{
53-
// Check Update Version
54-
if (IsUpdateVersionApplicable(installedVersion, Utility::Version(key.Version)))
55-
{
56-
auto packageVersion = package->GetAvailableVersion(key);
57-
auto manifest = packageVersion->GetManifest();
52+
context.Reporter.Info() << Resource::String::UpgradeUnknownVersionExplanation << std::endl;
53+
}
54+
55+
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE);
56+
}
57+
58+
// The version keys should have already been sorted by version
59+
const auto& versionKeys = package->GetAvailableVersionKeys();
60+
for (const auto& key : versionKeys)
61+
{
62+
// Check Update Version
63+
if (IsUpdateVersionApplicable(installedVersion, Utility::Version(key.Version)))
64+
{
65+
auto packageVersion = package->GetAvailableVersion(key);
66+
auto manifest = packageVersion->GetManifest();
5867

59-
// Check applicable Installer
60-
auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(manifest);
61-
if (!installer.has_value())
68+
// Check applicable Installer
69+
auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(manifest);
70+
if (!installer.has_value())
71+
{
72+
// If there is at least one installer whose only reason is InstalledType.
73+
auto onlyInstalledType = std::find(inapplicabilities.begin(), inapplicabilities.end(), InapplicabilityFlags::InstalledType);
74+
if (onlyInstalledType != inapplicabilities.end())
6275
{
63-
// If there is at least one installer whose only reason is InstalledType.
64-
auto onlyInstalledType = std::find(inapplicabilities.begin(), inapplicabilities.end(), InapplicabilityFlags::InstalledType);
65-
if (onlyInstalledType != inapplicabilities.end())
66-
{
67-
installedTypeInapplicable = true;
68-
}
69-
70-
continue;
76+
installedTypeInapplicable = true;
7177
}
7278

79+
continue;
80+
}
81+
7382
Logging::Telemetry().LogSelectedInstaller(
7483
static_cast<int>(installer->Arch),
7584
installer->Url,
@@ -88,25 +97,16 @@ namespace AppInstaller::CLI::Workflow
8897
context.Add<Execution::Data::PackageVersion>(std::move(packageVersion));
8998
context.Add<Execution::Data::Installer>(std::move(installer));
9099

91-
updateFound = true;
92-
break;
93-
}
94-
else
95-
{
96-
// Any following versions are not applicable
97-
break;
98-
}
100+
updateFound = true;
101+
break;
99102
}
100-
}
101-
else
102-
{
103-
// the package has an unknown version and the user did not request to upgrade it anyway.
104-
if (m_reportUpdateNotFound)
103+
else
105104
{
106-
context.Reporter.Info() << Resource::String::UpgradeUnknownVersionExplanation << std::endl;
105+
// Any following versions are not applicable
106+
break;
107107
}
108-
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE);
109108
}
109+
110110
if (!updateFound)
111111
{
112112
if (m_reportUpdateNotFound)
@@ -143,7 +143,8 @@ namespace AppInstaller::CLI::Workflow
143143
const auto& matches = context.Get<Execution::Data::SearchResult>().Matches;
144144
std::vector<std::unique_ptr<Execution::Context>> packagesToInstall;
145145
bool updateAllFoundUpdate = false;
146-
int unknownPackagesCount = 0;
146+
int packagesWithUnknownVersionSkipped = 0;
147+
int packagesThatRequireExplicitSkipped = 0;
147148

148149
for (const auto& match : matches)
149150
{
@@ -154,15 +155,17 @@ namespace AppInstaller::CLI::Workflow
154155
auto installedVersion = match.Package->GetInstalledVersion();
155156

156157
updateContext.Add<Execution::Data::Package>(match.Package);
157-
158+
159+
// Filter out packages with unknown installed versions
158160
if (context.Args.Contains(Execution::Args::Type::IncludeUnknown))
159161
{
160162
updateContext.Args.AddArg(Execution::Args::Type::IncludeUnknown);
161163
}
162164
else if (Utility::Version(installedVersion->GetProperty(PackageVersionProperty::Version)).IsUnknown())
163165
{
164166
// we don't know what the package's version is and the user didn't ask to upgrade it anyway.
165-
unknownPackagesCount++;
167+
AICLI_LOG(CLI, Info, << "Skipping " << match.Package->GetProperty(PackageProperty::Id) << " as it has unknown installed version");
168+
++packagesWithUnknownVersionSkipped;
166169
continue;
167170
}
168171

@@ -176,6 +179,24 @@ namespace AppInstaller::CLI::Workflow
176179
continue;
177180
}
178181

182+
// Filter out packages that require explicit upgrades.
183+
// We require explicit upgrades only if the installed version is pinned,
184+
// either because it was manually pinned or because the manifest indicated
185+
// RequireExplicitUpgrade.
186+
// Note that this does not consider whether the update to be installed has
187+
// RequireExplicitUpgrade. While this has the downside of not working with
188+
// packages installed from another source, it ensures consistency with the
189+
// list of available updates (there we don't have the selected installer)
190+
// and at most we will update each package like this once.
191+
auto installedMetadata = updateContext.Get<Execution::Data::InstalledPackageVersion>()->GetMetadata();
192+
auto pinnedState = ConvertToPackagePinnedStateEnum(installedMetadata[PackageVersionMetadata::PinnedState]);
193+
if (pinnedState != PackagePinnedState::NotPinned)
194+
{
195+
AICLI_LOG(CLI, Info, << "Skipping " << match.Package->GetProperty(PackageProperty::Id) << " as it requires explicit upgrade");
196+
++packagesThatRequireExplicitSkipped;
197+
continue;
198+
}
199+
179200
updateAllFoundUpdate = true;
180201

181202
AddToPackagesToInstallIfNotPresent(packagesToInstall, std::move(updateContextPtr));
@@ -191,5 +212,17 @@ namespace AppInstaller::CLI::Workflow
191212
APPINSTALLER_CLI_ERROR_UPDATE_ALL_HAS_FAILURE,
192213
{ APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE });
193214
}
215+
216+
if (packagesWithUnknownVersionSkipped > 0)
217+
{
218+
AICLI_LOG(CLI, Info, << packagesWithUnknownVersionSkipped << " package(s) skipped due to unknown installed version");
219+
context.Reporter.Info() << packagesWithUnknownVersionSkipped << " " << Resource::String::UpgradeUnknownVersionCount << std::endl;
220+
}
221+
222+
if (packagesThatRequireExplicitSkipped > 0)
223+
{
224+
AICLI_LOG(CLI, Info, << packagesThatRequireExplicitSkipped << " package(s) skipped due to requiring explicit upgrade");
225+
context.Reporter.Info() << packagesThatRequireExplicitSkipped << " " << Resource::String::UpgradeRequireExplicitCount << std::endl;
226+
}
194227
}
195228
}

0 commit comments

Comments
 (0)