Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support any architecture for Download and Show command #5146

Merged
merged 2 commits into from
Jan 22, 2025
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
4 changes: 4 additions & 0 deletions src/AppInstallerCLICore/Argument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
16 changes: 16 additions & 0 deletions src/AppInstallerCLICore/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down
34 changes: 32 additions & 2 deletions src/AppInstallerCLICore/Commands/DownloadCommand.cpp
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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),
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion src/AppInstallerCLICore/Commands/DownloadCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
17 changes: 14 additions & 3 deletions src/AppInstallerCLICore/Commands/ShowCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/ExecutionArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ namespace AppInstaller::CLI::Execution
InstallLocation,
InstallScope,
InstallArchitecture,
InstallerArchitecture,
InstallerType,
HashOverride, // Ignore hash mismatches
SkipDependencies, // Skip dependencies
Expand Down
6 changes: 3 additions & 3 deletions src/AppInstallerCLICore/ExecutionContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
9 changes: 8 additions & 1 deletion src/AppInstallerCLICore/Workflows/ManifestComparator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/AppInstallerCLICore/Workflows/WorkflowBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
14 changes: 14 additions & 0 deletions src/AppInstallerCLIE2ETests/DownloadCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -37,4 +44,4 @@ Installers:
Log: /LogFile <LOGPATH>
InstallLocation: /InstallDir <INSTALLPATH>
ManifestType: singleton
ManifestVersion: 1.4.0
ManifestVersion: 1.4.0
4 changes: 2 additions & 2 deletions src/AppInstallerCLITests/MSStoreDownloadFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions src/AppInstallerCommonCore/Architecture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Management.Deployment/PackageManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
}

Expand Down
Loading