Support any architecture for Download and Show command (microsoft#5146)

Also added Complete for Download command. Tested manually and added e2e tests.
JohnMcPMS · Jan 22, 2025 · 7c92b65 · 7c92b65
1 parent e69201b
commit 7c92b65
Show file tree

Hide file tree

Showing 16 changed files with 119 additions and 18 deletions.
diff --git a/src/AppInstallerCLICore/Argument.cpp b/src/AppInstallerCLICore/Argument.cpp
@@ -77,6 +77,8 @@ namespace AppInstaller::CLI
             return { type, "scope"_liv, ArgTypeCategory::InstallerSelection | ArgTypeCategory::CopyValueToSubContext };
         case Execution::Args::Type::InstallArchitecture:
             return { type, "architecture"_liv, 'a', ArgTypeCategory::InstallerSelection | ArgTypeCategory::CopyValueToSubContext };
+        case Execution::Args::Type::InstallerArchitecture: // Used for input architecture that does not need applicability check. E.g. Download, Show.
+            return { type, "architecture"_liv, 'a', ArgTypeCategory::InstallerSelection | ArgTypeCategory::CopyValueToSubContext };
         case Execution::Args::Type::InstallerType:
             return { type, "installer-type"_liv, ArgTypeCategory::InstallerSelection };
         case Execution::Args::Type::HashOverride:
@@ -338,6 +340,8 @@ namespace AppInstaller::CLI
             return Argument{ type, Resource::String::LocaleArgumentDescription, ArgumentType::Standard };
         case Args::Type::InstallArchitecture:
             return Argument{ type, Resource::String::ArchitectureArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help };
+        case Args::Type::InstallerArchitecture:
+            return Argument{ type, Resource::String::ArchitectureArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help };
         case Args::Type::Log:
             return Argument{ type, Resource::String::LogArgumentDescription, ArgumentType::Standard };
         case Args::Type::CustomSwitches:

diff --git a/src/AppInstallerCLICore/Command.cpp b/src/AppInstallerCLICore/Command.cpp
@@ -738,6 +738,22 @@ namespace AppInstaller::CLI
             }
         }
 
+        if (execArgs.Contains(Execution::Args::Type::InstallerArchitecture))
+        {
+            Utility::Architecture selectedArch = Utility::ConvertToArchitectureEnum(std::string(execArgs.GetArg(Execution::Args::Type::InstallerArchitecture)));
+            if (selectedArch == Utility::Architecture::Unknown)
+            {
+                std::vector<Utility::LocIndString> applicableArchitectures;
+                for (Utility::Architecture i : Utility::GetAllArchitectures())
+                {
+                    applicableArchitectures.emplace_back(Utility::ToString(i));
+                }
+
+                auto validOptions = Utility::Join(", "_liv, applicableArchitectures);
+                throw CommandException(Resource::String::InvalidArgumentValueError(Argument::ForType(Execution::Args::Type::InstallerArchitecture).Name(), validOptions));
+            }
+        }
+
         if (execArgs.Contains(Execution::Args::Type::Locale))
         {
             if (!Locale::IsWellFormedBcp47Tag(execArgs.GetArg(Execution::Args::Type::Locale)))

diff --git a/src/AppInstallerCLICore/Commands/DownloadCommand.cpp b/src/AppInstallerCLICore/Commands/DownloadCommand.cpp
@@ -1,7 +1,8 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 #include "pch.h"
-#include "DownloadCommand.h"
+#include "DownloadCommand.h"
+#include "Workflows/CompletionFlow.h"
 #include "Workflows/DownloadFlow.h"
 #include "Workflows/InstallFlow.h"
 #include "Workflows/PromptFlow.h"
@@ -28,7 +29,7 @@ namespace AppInstaller::CLI
             Argument::ForType(Args::Type::Channel),
             Argument::ForType(Args::Type::Source),
             Argument{ Args::Type::InstallScope, Resource::String::InstallScopeDescription, ArgumentType::Standard, Argument::Visibility::Help },
-            Argument::ForType(Args::Type::InstallArchitecture),
+            Argument::ForType(Args::Type::InstallerArchitecture),
             Argument::ForType(Args::Type::InstallerType),
             Argument::ForType(Args::Type::Exact),
             Argument::ForType(Args::Type::Locale),
@@ -52,6 +53,35 @@ namespace AppInstaller::CLI
     Resource::LocString DownloadCommand::LongDescription() const
     {
         return { Resource::String::DownloadCommandLongDescription };
+    }
+
+    void DownloadCommand::Complete(Context& context, Args::Type valueType) const
+    {
+        switch (valueType)
+        {
+        case Args::Type::Query:
+        case Args::Type::Manifest:
+        case Args::Type::Id:
+        case Args::Type::Name:
+        case Args::Type::Moniker:
+        case Args::Type::Version:
+        case Args::Type::Channel:
+        case Args::Type::Source:
+            context <<
+                Workflow::CompleteWithSingleSemanticsForValue(valueType);
+            break;
+        case Args::Type::InstallerArchitecture:
+        case Args::Type::Locale:
+            // May well move to CompleteWithSingleSemanticsForValue,
+            // but for now output nothing.
+            context <<
+                Workflow::CompleteWithEmptySet;
+            break;
+        case Args::Type::Log:
+        case Args::Type::DownloadDirectory:
+            // Intentionally output nothing to allow pass through to filesystem.
+            break;
+        }
     }
 
     Utility::LocIndView DownloadCommand::HelpLink() const

diff --git a/src/AppInstallerCLICore/Commands/DownloadCommand.h b/src/AppInstallerCLICore/Commands/DownloadCommand.h
@@ -12,7 +12,9 @@ namespace AppInstaller::CLI
         std::vector<Argument> GetArguments() const override;
 
         Resource::LocString ShortDescription() const override;
-        Resource::LocString LongDescription() const override;
+        Resource::LocString LongDescription() const override;
+
+        void Complete(Execution::Context& context, Execution::Args::Type valueType) const override;
 
         Utility::LocIndView HelpLink() const override;
 

diff --git a/src/AppInstallerCLICore/Commands/ShowCommand.cpp b/src/AppInstallerCLICore/Commands/ShowCommand.cpp
@@ -26,7 +26,7 @@ namespace AppInstaller::CLI
             Argument::ForType(Execution::Args::Type::Source),
             Argument::ForType(Execution::Args::Type::Exact),
             Argument{ Args::Type::InstallScope, Resource::String::InstallScopeDescription, ArgumentType::Standard, Argument::Visibility::Help },
-            Argument::ForType(Execution::Args::Type::InstallArchitecture),
+            Argument::ForType(Execution::Args::Type::InstallerArchitecture),
             Argument::ForType(Execution::Args::Type::InstallerType),
             Argument::ForType(Execution::Args::Type::Locale),
             Argument::ForType(Execution::Args::Type::ListVersions),
@@ -49,8 +49,19 @@ namespace AppInstaller::CLI
 
     void ShowCommand::Complete(Execution::Context& context, Execution::Args::Type valueType) const
     {
-        context <<
-            Workflow::CompleteWithSingleSemanticsForValue(valueType);
+        switch (valueType)
+        {
+        case Args::Type::InstallerArchitecture:
+        case Args::Type::Locale:
+            // May well move to CompleteWithSingleSemanticsForValue,
+            // but for now output nothing.
+            context <<
+                Workflow::CompleteWithEmptySet;
+            break;
+        default:
+            context <<
+                Workflow::CompleteWithSingleSemanticsForValue(valueType);
+        }
     }
 
     Utility::LocIndView ShowCommand::HelpLink() const

diff --git a/src/AppInstallerCLICore/ExecutionArgs.h b/src/AppInstallerCLICore/ExecutionArgs.h
@@ -41,6 +41,7 @@ namespace AppInstaller::CLI::Execution
             InstallLocation,
             InstallScope,
             InstallArchitecture,
+            InstallerArchitecture,
             InstallerType,
             HashOverride, // Ignore hash mismatches
             SkipDependencies, // Skip dependencies

diff --git a/src/AppInstallerCLICore/ExecutionContext.cpp b/src/AppInstallerCLICore/ExecutionContext.cpp
@@ -504,13 +504,13 @@ namespace AppInstaller::CLI::Execution
         switch (action)
         {
         case EnumBasedVariantMapAction::Add:
-            AICLI_LOG(Workflow, Info, << "Setting data item: " << data);
+            AICLI_LOG(Workflow, Verbose, << "Setting data item: " << data);
             break;
         case EnumBasedVariantMapAction::Contains:
-            AICLI_LOG(Workflow, Info, << "Checking data item: " << data);
+            AICLI_LOG(Workflow, Verbose, << "Checking data item: " << data);
             break;
         case EnumBasedVariantMapAction::Get:
-            AICLI_LOG(Workflow, Info, << "Getting data item: " << data);
+            AICLI_LOG(Workflow, Verbose, << "Getting data item: " << data);
             break;
         }
 

diff --git a/src/AppInstallerCLICore/Workflows/MSStoreInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/MSStoreInstallerHandler.cpp
@@ -278,9 +278,9 @@ namespace AppInstaller::CLI::Workflow
         Utility::Architecture requiredArchitecture = Utility::Architecture::Unknown;
         Manifest::PlatformEnum requiredPlatform = Manifest::PlatformEnum::Unknown;
         std::string requiredLocale;
-        if (context.Args.Contains(Execution::Args::Type::InstallArchitecture))
+        if (context.Args.Contains(Execution::Args::Type::InstallerArchitecture))
         {
-            requiredArchitecture = Utility::ConvertToArchitectureEnum(context.Args.GetArg(Execution::Args::Type::InstallArchitecture));
+            requiredArchitecture = Utility::ConvertToArchitectureEnum(context.Args.GetArg(Execution::Args::Type::InstallerArchitecture));
         }
         if (context.Args.Contains(Execution::Args::Type::Platform))
         {

diff --git a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp
@@ -85,6 +85,7 @@ namespace AppInstaller::CLI::Workflow
             static std::unique_ptr<MachineArchitectureComparator> Create(const Execution::Context& context, const Repository::IPackageVersion::Metadata& metadata)
             {
                 std::vector<Utility::Architecture> allowedArchitectures;
+                bool skipApplicabilityCheck = false;
 
                 if (context.Contains(Execution::Data::AllowedArchitectures))
                 {
@@ -96,6 +97,12 @@ namespace AppInstaller::CLI::Workflow
                     // Arguments provided in command line
                     allowedArchitectures.emplace_back(Utility::ConvertToArchitectureEnum(context.Args.GetArg(Execution::Args::Type::InstallArchitecture)));
                 }
+                else if (context.Args.Contains(Execution::Args::Type::InstallerArchitecture))
+                {
+                    // Arguments provided in command line. Also skips applicability check.
+                    allowedArchitectures.emplace_back(Utility::ConvertToArchitectureEnum(context.Args.GetArg(Execution::Args::Type::InstallerArchitecture)));
+                    skipApplicabilityCheck = true;
+                }
                 else
                 {
                     auto userIntentItr = metadata.find(Repository::PackageVersionMetadata::UserIntentArchitecture);
@@ -152,7 +159,7 @@ namespace AppInstaller::CLI::Workflow
                         }
 
                         // If the architecture is applicable and not already in our result set...
-                        if (Utility::IsApplicableArchitecture(architecture) != Utility::InapplicableArchitecture &&
+                        if ((skipApplicabilityCheck || Utility::IsApplicableArchitecture(architecture) != Utility::InapplicableArchitecture) &&
                             Utility::IsApplicableArchitecture(architecture, result) == Utility::InapplicableArchitecture)
                         {
                             result.push_back(architecture);

diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp
@@ -340,11 +340,11 @@ namespace AppInstaller::CLI::Workflow
         if (m_isFunc)
         {
             // Using `00000001`80000000` as base address default when loading dll into windbg as dump file.
-            AICLI_LOG(Workflow, Info, << "Running task: 0x" << m_func << " [ln 00000001`80000000+" << std::hex << (reinterpret_cast<char*>(m_func) - reinterpret_cast<char*>(&__ImageBase)) << "]");
+            AICLI_LOG(Workflow, Verbose, << "Running task: 0x" << m_func << " [ln 00000001`80000000+" << std::hex << (reinterpret_cast<char*>(m_func) - reinterpret_cast<char*>(&__ImageBase)) << "]");
         }
         else
         {
-            AICLI_LOG(Workflow, Info, << "Running task: " << m_name);
+            AICLI_LOG(Workflow, Verbose, << "Running task: " << m_name);
         }
     }
 

diff --git a/src/AppInstallerCLIE2ETests/DownloadCommand.cs b/src/AppInstallerCLIE2ETests/DownloadCommand.cs
@@ -69,6 +69,20 @@ public void DownloadToDirectory()
             Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
             TestCommon.AssertInstallerDownload(downloadDir, "TestExeInstaller", "2.0.0.0", ProcessorArchitecture.X86, TestCommon.Scope.Unknown, PackageInstallerType.Exe);
         }
+
+        /// <summary>
+        /// Downloads the test installer with Arm64.
+        /// </summary>
+        [Test]
+        public void DownloadWithArm64()
+        {
+            var downloadDir = TestCommon.GetRandomTestDir();
+            var result = TestCommon.RunAICLICommand("download", $"AppInstallerTest.TestMultipleInstallers --scope user --download-directory {downloadDir} --architecture Arm64");
+            Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
+#pragma warning disable CA1416 // Validate platform compatibility. Arm64 is not reachable.
+            TestCommon.AssertInstallerDownload(downloadDir, "TestMultipleInstallers", "1.0.0.0", ProcessorArchitecture.Arm64, TestCommon.Scope.User, PackageInstallerType.Nullsoft, "en-US");
+#pragma warning restore CA1416
+        }
 
         /// <summary>
         /// Downloads the test installer using the user scope argument.

diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestMultipleInstallers.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestMultipleInstallers.yaml
@@ -7,6 +7,13 @@ License: Test
 ShortDescription: E2E test manifest with multiple installers
 Installers:
   - Architecture: x64
+    InstallerUrl: https://localhost:5001/TestKit/AppInstallerTestExeInstaller/AppInstallerTestExeInstaller.exe
+    InstallerType: nullsoft
+    InstallerSha256: <EXEHASH>
+    Scope: user
+    InstallerSwitches:
+        InstallLocation: /InstallDir <INSTALLPATH>
+  - Architecture: arm64
     InstallerUrl: https://localhost:5001/TestKit/AppInstallerTestExeInstaller/AppInstallerTestExeInstaller.exe
     InstallerType: nullsoft
     InstallerSha256: <EXEHASH>
@@ -37,4 +44,4 @@ Installers:
       Log: /LogFile <LOGPATH>
       InstallLocation: /InstallDir <INSTALLPATH>
 ManifestType: singleton
-ManifestVersion: 1.4.0
+ManifestVersion: 1.4.0
diff --git a/src/AppInstallerCLITests/MSStoreDownloadFlow.cpp b/src/AppInstallerCLITests/MSStoreDownloadFlow.cpp
@@ -408,7 +408,7 @@ TEST_CASE("MSStoreDownloadFlow_Success_SpecificArchitecture", "[MSStoreDownloadF
     context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("DownloadFlowTest_MSStore.yaml").GetPath().u8string());
     context.Args.AddArg(Execution::Args::Type::DownloadDirectory, tempDirectory);
     context.Args.AddArg(Execution::Args::Type::Locale, "en-US"sv);
-    context.Args.AddArg(Execution::Args::Type::InstallArchitecture, "x64"sv);
+    context.Args.AddArg(Execution::Args::Type::InstallerArchitecture, "x64"sv);
 
     DownloadCommand download({});
     download.Execute(context);
@@ -525,7 +525,7 @@ TEST_CASE("MSStoreDownloadFlow_Fail_ArchitectureNotApplicable", "[MSStoreDownloa
     context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("DownloadFlowTest_MSStore.yaml").GetPath().u8string());
     context.Args.AddArg(Execution::Args::Type::DownloadDirectory, tempDirectory);
     context.Args.AddArg(Execution::Args::Type::Locale, "en-US"sv);
-    context.Args.AddArg(Execution::Args::Type::InstallArchitecture, "arm64"sv);
+    context.Args.AddArg(Execution::Args::Type::InstallerArchitecture, "arm64"sv);
 
     DownloadCommand download({});
     download.Execute(context);

diff --git a/src/AppInstallerCommonCore/Architecture.cpp b/src/AppInstallerCommonCore/Architecture.cpp
@@ -259,6 +259,12 @@ namespace AppInstaller::Utility
         return applicableArchs;
     }
 
+    const std::vector<Architecture>& GetAllArchitectures()
+    {
+        static std::vector<Architecture> allArchs = { Architecture::Neutral, Architecture::X86, Architecture::X64, Architecture::Arm, Architecture::Arm64 };
+        return allArchs;
+    }
+
     int IsApplicableArchitecture(Architecture arch)
     {
         return IsApplicableArchitecture(arch, GetApplicableArchitectures());

diff --git a/src/AppInstallerCommonCore/Public/AppInstallerArchitecture.h b/src/AppInstallerCommonCore/Public/AppInstallerArchitecture.h
@@ -35,6 +35,9 @@ namespace AppInstaller::Utility
     // Gets the set of architectures that are applicable to the current system
     const std::vector<Architecture>& GetApplicableArchitectures();
 
+    // Gets the set of architectures that are supported by winget
+    const std::vector<Architecture>& GetAllArchitectures();
+
     // Gets if an architecture is applicable to the system
     // Returns the priority in the applicable architecture list if the architecture is applicable. 0 has lowest priority.
     // Returns -1 if the architecture is not applicable.

diff --git a/src/Microsoft.Management.Deployment/PackageManager.cpp b/src/Microsoft.Management.Deployment/PackageManager.cpp
@@ -658,7 +658,7 @@ namespace winrt::Microsoft::Management::Deployment::implementation
                 auto convertedArchitecture = GetUtilityArchitecture(architecture);
                 if (convertedArchitecture)
                 {
-                    context->Args.AddArg(Execution::Args::Type::InstallArchitecture, ToString(convertedArchitecture.value()));
+                    context->Args.AddArg(Execution::Args::Type::InstallerArchitecture, ToString(convertedArchitecture.value()));
                 }
             }