From 7c92b65e927f29d54b215e56e13fcaa396a50fda Mon Sep 17 00:00:00 2001 From: yao-msft <50888816+yao-msft@users.noreply.github.com> Date: Tue, 21 Jan 2025 16:20:59 -0800 Subject: [PATCH] Support any architecture for Download and Show command (#5146) Also added Complete for Download command. Tested manually and added e2e tests. --- src/AppInstallerCLICore/Argument.cpp | 4 +++ src/AppInstallerCLICore/Command.cpp | 16 +++++++++ .../Commands/DownloadCommand.cpp | 34 +++++++++++++++++-- .../Commands/DownloadCommand.h | 4 ++- .../Commands/ShowCommand.cpp | 17 ++++++++-- src/AppInstallerCLICore/ExecutionArgs.h | 1 + src/AppInstallerCLICore/ExecutionContext.cpp | 6 ++-- .../Workflows/MSStoreInstallerHandler.cpp | 4 +-- .../Workflows/ManifestComparator.cpp | 9 ++++- .../Workflows/WorkflowBase.cpp | 4 +-- .../DownloadCommand.cs | 14 ++++++++ .../Manifests/TestMultipleInstallers.yaml | 9 ++++- .../MSStoreDownloadFlow.cpp | 4 +-- src/AppInstallerCommonCore/Architecture.cpp | 6 ++++ .../Public/AppInstallerArchitecture.h | 3 ++ .../PackageManager.cpp | 2 +- 16 files changed, 119 insertions(+), 18 deletions(-) diff --git a/src/AppInstallerCLICore/Argument.cpp b/src/AppInstallerCLICore/Argument.cpp index f8490ec1ef..3f2216295d 100644 --- 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 index 066aec0467..1ac2da9597 100644 --- 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 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 index e06254427a..0848ecda55 100644 --- 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 index 06a76cfe9a..a6b8f1806e 100644 --- a/src/AppInstallerCLICore/Commands/DownloadCommand.h +++ b/src/AppInstallerCLICore/Commands/DownloadCommand.h @@ -12,7 +12,9 @@ namespace AppInstaller::CLI std::vector 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 index 76198a9699..853f0afcdc 100644 --- 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 index 7bb2c28628..01ce6637c8 100644 --- 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 index 49789bb11d..99592d56d3 100644 --- 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 index 9b36411334..deab8eecd3 100644 --- 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 index 894badc906..96d617b524 100644 --- a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp +++ b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp @@ -85,6 +85,7 @@ namespace AppInstaller::CLI::Workflow static std::unique_ptr Create(const Execution::Context& context, const Repository::IPackageVersion::Metadata& metadata) { std::vector 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 index 1815d795c7..cdd0699608 100644 --- 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(m_func) - reinterpret_cast(&__ImageBase)) << "]"); + AICLI_LOG(Workflow, Verbose, << "Running task: 0x" << m_func << " [ln 00000001`80000000+" << std::hex << (reinterpret_cast(m_func) - reinterpret_cast(&__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 index c20426e8d4..e18964a515 100644 --- 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); } + + /// + /// Downloads the test installer with Arm64. + /// + [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 + } /// /// 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 index 09e6c4a318..f7f8e8e4e0 100644 --- 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: + Scope: user + InstallerSwitches: + InstallLocation: /InstallDir + - Architecture: arm64 InstallerUrl: https://localhost:5001/TestKit/AppInstallerTestExeInstaller/AppInstallerTestExeInstaller.exe InstallerType: nullsoft InstallerSha256: @@ -37,4 +44,4 @@ Installers: Log: /LogFile InstallLocation: /InstallDir ManifestType: singleton -ManifestVersion: 1.4.0 \ No newline at end of file +ManifestVersion: 1.4.0 diff --git a/src/AppInstallerCLITests/MSStoreDownloadFlow.cpp b/src/AppInstallerCLITests/MSStoreDownloadFlow.cpp index 793e64179d..2e6d33b366 100644 --- 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 index 9370e0351f..95839455a8 100644 --- a/src/AppInstallerCommonCore/Architecture.cpp +++ b/src/AppInstallerCommonCore/Architecture.cpp @@ -259,6 +259,12 @@ namespace AppInstaller::Utility return applicableArchs; } + const std::vector& GetAllArchitectures() + { + static std::vector 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 index c2d9127035..17f3fac897 100644 --- 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& GetApplicableArchitectures(); + // Gets the set of architectures that are supported by winget + const std::vector& 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 index 836a31d0c9..5de62a875f 100644 --- 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())); } }