Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/spelling/allow.txt
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ IIS
ILogger
IManifest
impl
inapplicabilities
Inet
inheritdoc
inno
Expand Down
2 changes: 2 additions & 0 deletions src/AppInstallerCLICore/Resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,8 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(UpdateNotApplicable);
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeCommandLongDescription);
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeCommandShortDescription);
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeDifferentInstallTechnology);
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeDifferentInstallTechnologyInNewerVersions);
WINGET_DEFINE_RESOURCE_STRINGID(Usage);
WINGET_DEFINE_RESOURCE_STRINGID(ValidateCommandLongDescription);
WINGET_DEFINE_RESOURCE_STRINGID(ValidateCommandReportDependencies);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,14 @@ namespace AppInstaller::CLI::Workflow
return DependencyNodeProcessorResult::Error;
}

std::optional<AppInstaller::Manifest::ManifestInstaller> installer;

IPackageVersion::Metadata installationMetadata;
if (m_nodePackageInstalledVersion)
{
installationMetadata = m_nodePackageInstalledVersion->GetMetadata();
}

ManifestComparator manifestComparator(m_context, installationMetadata);
installer = manifestComparator.GetPreferredInstaller(m_nodeManifest);
auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(m_nodeManifest);

if (!installer.has_value())
{
Expand Down
96 changes: 69 additions & 27 deletions src/AppInstallerCLICore/Workflows/ManifestComparator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,14 @@ namespace AppInstaller::CLI::Workflow
{
OSVersionFilter() : details::FilterField("OS Version") {}

bool IsApplicable(const Manifest::ManifestInstaller& installer) override
InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override
{
return installer.MinOSVersion.empty() || Runtime::IsCurrentOSVersionGreaterThanOrEqual(Utility::Version(installer.MinOSVersion));
if (installer.MinOSVersion.empty() || Runtime::IsCurrentOSVersionGreaterThanOrEqual(Utility::Version(installer.MinOSVersion)))
{
return InapplicabilityFlags::None;
}

return InapplicabilityFlags::OSVersion;
}

std::string ExplainInapplicable(const Manifest::ManifestInstaller& installer) override
Expand Down Expand Up @@ -97,9 +102,14 @@ namespace AppInstaller::CLI::Workflow
return std::make_unique<MachineArchitectureComparator>();
}

bool IsApplicable(const Manifest::ManifestInstaller& installer) override
InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override
{
return CheckAllowedArchitecture(installer.Arch) != Utility::InapplicableArchitecture;
if (CheckAllowedArchitecture(installer.Arch) == Utility::InapplicableArchitecture)
{
return InapplicabilityFlags::MachineArchitecture;
}

return InapplicabilityFlags::None;
}

std::string ExplainInapplicable(const Manifest::ManifestInstaller& installer) override
Expand Down Expand Up @@ -182,9 +192,14 @@ namespace AppInstaller::CLI::Workflow
return {};
}

bool IsApplicable(const Manifest::ManifestInstaller& installer) override
InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override
{
return Manifest::IsInstallerTypeCompatible(installer.InstallerType, m_installedType);
if (Manifest::IsInstallerTypeCompatible(installer.InstallerType, m_installedType))
{
return InapplicabilityFlags::None;
}

return InapplicabilityFlags::InstalledType;
}

std::string ExplainInapplicable(const Manifest::ManifestInstaller& installer) override
Expand Down Expand Up @@ -224,10 +239,15 @@ namespace AppInstaller::CLI::Workflow
return {};
}

bool IsApplicable(const Manifest::ManifestInstaller& installer) override
InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override
{
// We have to assume the unknown scope will match our required scope, or the entire catalog would stop working for upgrade.
return installer.Scope == Manifest::ScopeEnum::Unknown || installer.Scope == m_requirement;
if (installer.Scope == Manifest::ScopeEnum::Unknown || installer.Scope == m_requirement)
{
return InapplicabilityFlags::None;
}

return InapplicabilityFlags::InstalledScope;
}

std::string ExplainInapplicable(const Manifest::ManifestInstaller& installer) override
Expand Down Expand Up @@ -275,9 +295,14 @@ namespace AppInstaller::CLI::Workflow
}
}

bool IsApplicable(const Manifest::ManifestInstaller& installer) override
InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override
{
return m_requirement == Manifest::ScopeEnum::Unknown || installer.Scope == m_requirement;
if (m_requirement == Manifest::ScopeEnum::Unknown || installer.Scope == m_requirement)
{
return InapplicabilityFlags::None;
}

return InapplicabilityFlags::Scope;
}

std::string ExplainInapplicable(const Manifest::ManifestInstaller& installer) override
Expand Down Expand Up @@ -328,10 +353,16 @@ namespace AppInstaller::CLI::Workflow
return {};
}

bool IsApplicable(const Manifest::ManifestInstaller& installer) override
InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override
{
// We have to assume an unknown installer locale will match our installed locale, or the entire catalog would stop working for upgrade.
return installer.Locale.empty() || Locale::GetDistanceOfLanguage(m_installedLocale, installer.Locale) >= Locale::MinimumDistanceScoreAsCompatibleMatch;
if (installer.Locale.empty() ||
Locale::GetDistanceOfLanguage(m_installedLocale, installer.Locale) >= Locale::MinimumDistanceScoreAsCompatibleMatch)
{
return InapplicabilityFlags::None;
}

return InapplicabilityFlags::InstalledLocale;
}

std::string ExplainInapplicable(const Manifest::ManifestInstaller& installer) override
Expand Down Expand Up @@ -397,22 +428,22 @@ namespace AppInstaller::CLI::Workflow
}
}

bool IsApplicable(const Manifest::ManifestInstaller& installer) override
InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override
{
if (m_requirement.empty())
{
return true;
return InapplicabilityFlags::None;
}

for (auto const& requiredLocale : m_requirement)
{
if (Locale::GetDistanceOfLanguage(requiredLocale, installer.Locale) >= Locale::MinimumDistanceScoreAsPerfectMatch)
{
return true;
return InapplicabilityFlags::None;
}
}

return false;
return InapplicabilityFlags::Locale;
}

std::string ExplainInapplicable(const Manifest::ManifestInstaller& installer) override
Expand Down Expand Up @@ -502,24 +533,33 @@ namespace AppInstaller::CLI::Workflow
AddComparator(MachineArchitectureComparator::Create(context, installationMetadata));
}

std::optional<Manifest::ManifestInstaller> ManifestComparator::GetPreferredInstaller(const Manifest::Manifest& manifest)
InstallerAndInapplicabilities ManifestComparator::GetPreferredInstaller(const Manifest::Manifest& manifest)
{
AICLI_LOG(CLI, Info, << "Starting installer selection.");

const Manifest::ManifestInstaller* result = nullptr;
std::vector<InapplicabilityFlags> inapplicabilitiesInstallers;

for (const auto& installer : manifest.Installers)
{
if (IsApplicable(installer) && (!result || IsFirstBetter(installer, *result)))
auto inapplicabilityInstaller = IsApplicable(installer);
Comment thread
msftrubengu marked this conversation as resolved.
if (inapplicabilityInstaller == InapplicabilityFlags::None)
{
AICLI_LOG(CLI, Verbose, << "Installer " << installer << " is current best choice");
result = &installer;
if (!result || IsFirstBetter(installer, *result))
{
AICLI_LOG(CLI, Verbose, << "Installer " << installer << " is current best choice");
result = &installer;
}
}
else
{
inapplicabilitiesInstallers.push_back(inapplicabilityInstaller);
}
}

if (!result)
{
return {};
return { {}, std::move(inapplicabilitiesInstallers) };
}

Logging::Telemetry().LogSelectedInstaller(
Expand All @@ -529,22 +569,24 @@ namespace AppInstaller::CLI::Workflow
Manifest::ScopeToString(result->Scope),
result->Locale);

return *result;
return { *result, std::move(inapplicabilitiesInstallers) };
}

// TODO: Implement a mechanism for better error messaging for no applicable installer scenario
bool ManifestComparator::IsApplicable(const Manifest::ManifestInstaller& installer)
InapplicabilityFlags ManifestComparator::IsApplicable(const Manifest::ManifestInstaller& installer)
{
InapplicabilityFlags inapplicabilityResult = InapplicabilityFlags::None;

for (const auto& filter : m_filters)
{
if (!filter->IsApplicable(installer))
auto inapplicability = filter->IsApplicable(installer);
if (inapplicability != InapplicabilityFlags::None)
{
AICLI_LOG(CLI, Info, << "Installer " << installer << " not applicable: " << filter->ExplainInapplicable(installer));
return false;
WI_SetAllFlags(inapplicabilityResult, inapplicability);
}
}

return true;
return inapplicabilityResult;
}

bool ManifestComparator::IsFirstBetter(
Expand Down
27 changes: 24 additions & 3 deletions src/AppInstallerCLICore/Workflows/ManifestComparator.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,21 @@

namespace AppInstaller::CLI::Workflow
{
// Flags to indicate why an installer was not applicable
enum class InapplicabilityFlags : int
{
None = 0x0,
OSVersion = 0x1,
InstalledScope = 0x2,
InstalledType = 0x4,
InstalledLocale = 0x8,
Locale = 0x10,
Scope = 0x20,
MachineArchitecture = 0x40,
};

DEFINE_ENUM_FLAG_OPERATORS(InapplicabilityFlags);

namespace details
{
// An interface for defining new filters based on user inputs.
Expand All @@ -25,7 +40,7 @@ namespace AppInstaller::CLI::Workflow
std::string_view Name() const { return m_name; }

// Determines if the installer is applicable based on this field alone.
virtual bool IsApplicable(const Manifest::ManifestInstaller& installer) = 0;
virtual InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) = 0;

// Explains why the filter regarded this installer as inapplicable.
// Will only be called when IsApplicable returns false.
Expand All @@ -47,16 +62,22 @@ namespace AppInstaller::CLI::Workflow
};
}

struct InstallerAndInapplicabilities
{
std::optional<Manifest::ManifestInstaller> installer;
std::vector<InapplicabilityFlags> inapplicabilitiesInstaller;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It probably doesn't matter right now, but it seems odd to not know which installer each goes with.

};

// Class in charge of comparing manifest entries
struct ManifestComparator
{
ManifestComparator(const Execution::Context& context, const Repository::IPackageVersion::Metadata& installationMetadata);

// Gets the best installer from the manifest, if at least one is applicable.
std::optional<Manifest::ManifestInstaller> GetPreferredInstaller(const Manifest::Manifest& manifest);
InstallerAndInapplicabilities GetPreferredInstaller(const Manifest::Manifest& manifest);

// Determines if an installer is applicable.
bool IsApplicable(const Manifest::ManifestInstaller& installer);
InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer);

// Determines if the first installer is a better choice.
bool IsFirstBetter(
Expand Down
20 changes: 18 additions & 2 deletions src/AppInstallerCLICore/Workflows/UpdateFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ namespace AppInstaller::CLI::Workflow
Utility::Version installedVersion = Utility::Version(installedPackage->GetProperty(PackageVersionProperty::Version));
ManifestComparator manifestComparator(context, installedPackage->GetMetadata());
bool updateFound = false;
bool installedTypeInapplicable = false;

// The version keys should have already been sorted by version
const auto& versionKeys = package->GetAvailableVersionKeys();
Expand All @@ -54,9 +55,16 @@ namespace AppInstaller::CLI::Workflow
auto manifest = packageVersion->GetManifest();

// Check applicable Installer
auto installer = manifestComparator.GetPreferredInstaller(manifest);
auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(manifest);
if (!installer.has_value())
{
// If there is at least one installer whose only reason is InstalledType.
auto onlyInstalledType = std::find(inapplicabilities.begin(), inapplicabilities.end(), InapplicabilityFlags::InstalledType);
if (onlyInstalledType != inapplicabilities.end())
{
installedTypeInapplicable = true;
}

continue;
}

Expand All @@ -80,8 +88,16 @@ namespace AppInstaller::CLI::Workflow
{
if (m_reportUpdateNotFound)
{
context.Reporter.Info() << Resource::String::UpdateNotApplicable << std::endl;
if (installedTypeInapplicable)
{
context.Reporter.Info() << Resource::String::UpgradeDifferentInstallTechnologyInNewerVersions << std::endl;
}
else
{
context.Reporter.Info() << Resource::String::UpdateNotApplicable << std::endl;
}
}

AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE);
}
}
Expand Down
14 changes: 13 additions & 1 deletion src/AppInstallerCLICore/Workflows/WorkflowBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,19 @@ namespace AppInstaller::CLI::Workflow
context.Add<Execution::Data::AllowedArchitectures>({ Utility::ConvertToArchitectureEnum(std::string(context.Args.GetArg(Execution::Args::Type::InstallArchitecture))) });
}
ManifestComparator manifestComparator(context, installationMetadata);
context.Add<Execution::Data::Installer>(manifestComparator.GetPreferredInstaller(context.Get<Execution::Data::Manifest>()));
auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(context.Get<Execution::Data::Manifest>());

if (!installer.has_value())
{
auto onlyInstalledType = std::find(inapplicabilities.begin(), inapplicabilities.end(), InapplicabilityFlags::InstalledType);
if (onlyInstalledType != inapplicabilities.end())
{
context.Reporter.Info() << Resource::String::UpgradeDifferentInstallTechnology << std::endl;
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE);
}
}

context.Add<Execution::Data::Installer>(installer);
}

void EnsureRunningAsAdmin(Execution::Context& context)
Expand Down
6 changes: 6 additions & 0 deletions src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw
Original file line number Diff line number Diff line change
Expand Up @@ -1237,6 +1237,12 @@ Please specify one of them using the `--source` option to proceed.</value>
<data name="CountOutOfBoundsError" xml:space="preserve">
<value>The requested number of results must be between 1 and 1000.</value>
</data>
<data name="UpgradeDifferentInstallTechnologyInNewerVersions" xml:space="preserve">
<value>A newer version was found, but the install technology is different from the current version installed. Please uninstall the package and install the newer version.</value>
</data>
<data name="UpgradeDifferentInstallTechnology" xml:space="preserve">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is not clear to me when this would be used over the other string.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

UpgradeDifferentInstallTechnologyInNewerVersions is for winget upgrade <id> and we just look at all versions

UpgradeDifferentInstallTechnology is for specific version winget upgrade <id> -v 2.0

it is different codepaths and i thought it was weird showing "At least a newer package was found..." when a version is specified.

<value>The install technology of the newer version specified is different from the current version installed. Please uninstall the package and install the newer version.</value>
</data>
<data name="WindowsPackageManagerPreview" xml:space="preserve">
<value>Windows Package Manager (Preview)</value>
<comment>The product name plus an indicator that this is a pre-release version.</comment>
Expand Down
Loading