Print message when install technology is different in upgrade scenari…

…os (#1649)

.github/actions/spelling/allow.txt

@@ -221,6 +221,7 @@ IIS
 ILogger
 IManifest
 impl
+inapplicabilities
 Inet
 inheritdoc
 inno

src/AppInstallerCLICore/Resources.h

@@ -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);

src/AppInstallerCLICore/Workflows/DependencyNodeProcessor.cpp

@@ -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())
         {

src/AppInstallerCLICore/Workflows/ManifestComparator.cpp

@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
+            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(
@@ -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(

src/AppInstallerCLICore/Workflows/ManifestComparator.h

@@ -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.
@@ -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.
@@ -47,16 +62,22 @@ namespace AppInstaller::CLI::Workflow
         };
     }
 
+    struct InstallerAndInapplicabilities
+    {
+        std::optional<Manifest::ManifestInstaller> installer;
+        std::vector<InapplicabilityFlags> inapplicabilitiesInstaller;
+    };
+
     // 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(

src/AppInstallerCLICore/Workflows/UpdateFlow.cpp

@@ -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();
@@ -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;
                 }
 
@@ -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);
         }
     }

src/AppInstallerCLICore/Workflows/WorkflowBase.cpp

@@ -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)

src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw

@@ -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">
+    <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>