commit 7c92b65e927f29d54b215e56e13fcaa396a50fda parent e69201b0c8f49002e9961b460ca009c313e5c67b Author: yao-msft <50888816+yao-msft@users.noreply.github.com> Date: Tue, 21 Jan 2025 16:20:59 -0800 Support any architecture for Download and Show command (#5146) Also added Complete for Download command. Tested manually and added e2e tests. Diffstat:
16 files changed, 119 insertions(+), 19 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 @@ namespace AppInstallerCLIE2ETests 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 @@ -12,6 +12,13 @@ Installers: InstallerSha256: <EXEHASH> Scope: user InstallerSwitches: + InstallLocation: /InstallDir <INSTALLPATH> + - Architecture: arm64 + InstallerUrl: https://localhost:5001/TestKit/AppInstallerTestExeInstaller/AppInstallerTestExeInstaller.exe + InstallerType: nullsoft + InstallerSha256: <EXEHASH> + Scope: user + InstallerSwitches: InstallLocation: /InstallDir <INSTALLPATH> - Architecture: x86 InstallerUrl: https://localhost:5001/TestKit/AppInstallerTestMsiInstaller/AppInstallerTestMsiInstaller.msi @@ -37,4 +44,4 @@ Installers: Log: /LogFile <LOGPATH> InstallLocation: /InstallDir <INSTALLPATH> 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 @@ -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())); } }