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

Add support for UnsupportedArguments #2216

Merged
merged 14 commits into from
Jun 21, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion src/AppInstallerCLICore/Argument.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ namespace AppInstaller::CLI
// Requires that some form of package selection argument is present
static void ValidatePackageSelectionArgumentSupplied(const Execution::Args& args);

// Gets the argument usage string in the format of "--name,-alias".
// Gets the argument usage string in the format of "-alias,--name".
std::string GetUsageString() const;

// Arguments are not localized at this time.
Expand Down
4 changes: 3 additions & 1 deletion src/AppInstallerCLICore/Commands/UpgradeCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,9 @@ namespace AppInstaller::CLI
context << SelectLatestApplicableUpdate(true);
}

context << InstallSinglePackage;
context <<
CheckForUnsupportedArgs <<
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
InstallSinglePackage;
}
}
}
2 changes: 1 addition & 1 deletion src/AppInstallerCLICore/Core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ namespace AppInstaller::CLI
}

context.UpdateForArgs();

context.SetExecutingCommand(command.get());
command->ValidateArguments(context.Args);
}
// Exceptions specific to parsing the arguments of a command
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/ExecutionContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ namespace AppInstaller::CLI::Execution
{
auto clone = std::make_unique<Context>(Reporter, m_threadGlobals);
clone->m_flags = m_flags;
clone->m_executingCommand = m_executingCommand;
// If the parent is hooked up to the CTRL signal, have the clone be as well
if (m_disableCtrlHandlerOnExit)
{
Expand Down
12 changes: 12 additions & 0 deletions src/AppInstallerCLICore/ExecutionContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@
// Also returns the specified value from the current function.
#define AICLI_TERMINATE_CONTEXT_RETURN(_hr_,_ret_) AICLI_TERMINATE_CONTEXT_ARGS(context,_hr_,_ret_)

namespace AppInstaller::CLI
{
struct Command;
}

namespace AppInstaller::CLI::Workflow
{
struct WorkflowTask;
Expand Down Expand Up @@ -137,6 +142,12 @@ namespace AppInstaller::CLI::Execution

std::unique_ptr<AppInstaller::ThreadLocalStorage::PreviousThreadGlobals> SetForCurrentThread();

// Gets the executing command
AppInstaller::CLI::Command* GetExecutingCommand() { return m_executingCommand; }

// Sets the executing command
void SetExecutingCommand(AppInstaller::CLI::Command* command) { m_executingCommand = command; }

#ifndef AICLI_DISABLE_TEST_HOOKS
// Enable tests to override behavior
bool ShouldExecuteWorkflowTask(const Workflow::WorkflowTask& task);
Expand All @@ -156,5 +167,6 @@ namespace AppInstaller::CLI::Execution
ContextFlag m_flags = ContextFlag::None;
Workflow::ExecutionStage m_executionStage = Workflow::ExecutionStage::Initial;
AppInstaller::ThreadLocalStorage::ThreadGlobals m_threadGlobals;
AppInstaller::CLI::Command* m_executingCommand = nullptr;
};
}
43 changes: 29 additions & 14 deletions src/AppInstallerCLICore/Workflows/InstallFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <AppInstallerDeployment.h>
#include <winget/ARPCorrelation.h>
#include <Argument.h>
#include <Command.h>

using namespace winrt::Windows::ApplicationModel::Store::Preview::InstallControl;
using namespace winrt::Windows::Foundation;
Expand Down Expand Up @@ -69,17 +70,23 @@ namespace AppInstaller::CLI::Workflow
}
}

AppInstaller::CLI::Argument GetUnsupportedArgument(UnsupportedArgumentEnum arg)
Execution::Args::Type GetUnsupportedArgumentType(UnsupportedArgumentEnum unsupportedArgument)
{
switch (arg)
Execution::Args::Type execArg;

switch (unsupportedArgument)
{
case UnsupportedArgumentEnum::Log:
return AppInstaller::CLI::Argument::ForType(Execution::Args::Type::Log);
execArg = Execution::Args::Type::Log;
break;
case UnsupportedArgumentEnum::Location:
return AppInstaller::CLI::Argument::ForType(Execution::Args::Type::InstallLocation);
execArg = Execution::Args::Type::InstallLocation;
break;
default:
THROW_HR(E_UNEXPECTED);
}

return execArg;
}

struct ExpectedReturnCode
Expand Down Expand Up @@ -158,18 +165,26 @@ namespace AppInstaller::CLI::Workflow
const auto& unsupportedArgs = context.Get<Execution::Data::Installer>()->UnsupportedArguments;
for (auto unsupportedArg : unsupportedArgs)
{
const auto& argument = GetUnsupportedArgument(unsupportedArg);
if (context.Args.Contains(argument.ExecArgType()))
const auto& unsupportedArgType = GetUnsupportedArgumentType(unsupportedArg);
if (context.Args.Contains(unsupportedArgType))
{
const auto& usageString = argument.GetUsageString();
if (ShouldErrorForUnsupportedArgument(unsupportedArg))
{
context.Reporter.Error() << Resource::String::UnsupportedArgument << ' ' << usageString << std::endl;
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_UNSUPPORTED_ARGUMENT);
}
else
const auto& commandArguments = context.GetExecutingCommand()->GetArguments();
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
for (const auto& argument : commandArguments)
{
context.Reporter.Warn() << Resource::String::UnsupportedArgument << ' ' << usageString << std::endl;
if (unsupportedArgType == argument.ExecArgType())
{
const auto& usageString = argument.GetUsageString();
if (ShouldErrorForUnsupportedArgument(unsupportedArg))
{
context.Reporter.Error() << Resource::String::UnsupportedArgument << ' ' << usageString << std::endl;
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_UNSUPPORTED_ARGUMENT);
}
else
{
context.Reporter.Warn() << Resource::String::UnsupportedArgument << ' ' << usageString << std::endl;
break;
}
}
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,9 @@
<CopyFileToFolders Include="TestData\UpdateFlowTest_Exe_2_LicenseAgreement.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\UpdateFlowTest_Exe_UnsupportedArgs.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\UpdateFlowTest_ExeDependencies.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,9 @@
<CopyFileToFolders Include="TestData\UpdateFlowTest_Exe_2_LicenseAgreement.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\UpdateFlowTest_Exe_UnsupportedArgs.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\UpdateFlowTest_ExeDependencies.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Same content with UpdateFlowTest_Exe.yaml but with higher version and UnsupportedArguments
PackageIdentifier: AppInstallerCliTest.TestExeInstaller
PackageVersion: 2.0.0.0
PackageLocale: en-US
PackageName: AppInstaller Test Installer
Publisher: Microsoft Corporation
AppMoniker: AICLITestExe
License: Test
Switches:
Custom: /custom /ver3.0.0.0
SilentWithProgress: /silentwithprogress
Silent: /silence
Update: /update
UnsupportedArguments:
- log
- location
Installers:
- Architecture: x64
InstallerUrl: https://ThisIsNotUsed
InstallerType: exe
InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B
ManifestType: singleton
ManifestVersion: 1.2.0
41 changes: 41 additions & 0 deletions src/AppInstallerCLITests/WorkFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,21 @@ namespace
PackageMatchFilter(PackageMatchField::Id, MatchType::Exact, "AppInstallerCliTest.TestExeInstaller")));
}

if (input == "TestExeInstallerWithUnsupportedArguments")
{
auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Exe.yaml"));
auto manifest2 = YamlParser::CreateFromPath(TestDataFile("UpdateFlowTest_Exe_UnsupportedArgs.yaml"));
result.Matches.emplace_back(
ResultMatch(
TestPackage::Make(
manifest,
TestPackage::MetadataMap{ { PackageVersionMetadata::InstalledType, "Exe" } },
std::vector<Manifest>{ manifest2, manifest },
shared_from_this()
),
PackageMatchFilter(PackageMatchField::Id, MatchType::Exact, "AppInstallerCliTest.TestExeInstaller")));
}

if (input == "TestExeInstallerWithNothingInstalled")
{
auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Exe.yaml"));
Expand Down Expand Up @@ -831,6 +846,7 @@ TEST_CASE("InstallFlow_UnsupportedArguments_Warn", "[InstallFlow][workflow]")
context.Args.AddArg(Execution::Args::Type::Log, tempDirectory);

InstallCommand install({});
context.SetExecutingCommand(&install);
install.Execute(context);
INFO(installOutput.str());

Expand All @@ -852,6 +868,7 @@ TEST_CASE("InstallFlow_UnsupportedArguments_Error", "[InstallFlow][workflow]")
context.Args.AddArg(Execution::Args::Type::InstallLocation, tempDirectory);

InstallCommand install({});
context.SetExecutingCommand(&install);
install.Execute(context);
INFO(installOutput.str());

Expand All @@ -872,6 +889,7 @@ TEST_CASE("InstallFlow_UnsupportedArguments_NotProvided")
context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_UnsupportedArguments.yaml").GetPath().u8string());

InstallCommand install({});
context.SetExecutingCommand(&install);
install.Execute(context);
INFO(installOutput.str());

Expand Down Expand Up @@ -1597,6 +1615,29 @@ TEST_CASE("UpdateFlow_UpdatePortable", "[UpdateFlow][workflow]")
REQUIRE(std::filesystem::exists(updateResultPath.GetPath()));
}

TEST_CASE("UpdateFlow_UpdateExeWithUnsupportedArgs", "[UpdateFlow][workflow]")
{
TestCommon::TempFile updateResultPath("TestExeInstalled.txt");
TestCommon::TempDirectory tempDirectory("TempDirectory", false);

std::ostringstream updateOutput;
TestContext context{ updateOutput, std::cin };
auto previousThreadGlobals = context.SetForCurrentThread();
OverrideForCompositeInstalledSource(context);
context.Args.AddArg(Execution::Args::Type::Query, "TestExeInstallerWithUnsupportedArguments"sv);
context.Args.AddArg(Execution::Args::Type::InstallLocation, tempDirectory);

UpgradeCommand update({});
context.SetExecutingCommand(&update);
update.Execute(context);
INFO(updateOutput.str());

// Verify unsupported arguments error message is shown
REQUIRE(context.GetTerminationHR() == APPINSTALLER_CLI_ERROR_UNSUPPORTED_ARGUMENT);
REQUIRE(!std::filesystem::exists(updateResultPath.GetPath()));
REQUIRE(updateOutput.str().find(Resource::LocString(Resource::String::UnsupportedArgument).get() + " -l,--location") != std::string::npos);
}

TEST_CASE("UpdateFlow_UpdatePortableWithManifest", "[UpdateFlow][workflow]")
{
TestCommon::TempFile updateResultPath("TestPortableInstalled.txt");
Expand Down
3 changes: 2 additions & 1 deletion src/AppInstallerCommonCore/Public/AppInstallerErrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@
#define APPINSTALLER_CLI_ERROR_PORTABLE_SYMLINK_PATH_IS_DIRECTORY ((HRESULT)0x8A150055)
#define APPINSTALLER_CLI_ERROR_INSTALLER_PROHIBITS_ELEVATION ((HRESULT)0x8A150056)
#define APPINSTALLER_CLI_ERROR_PORTABLE_UNINSTALL_FAILED ((HRESULT)0x8A150057)
#define APPINSTALLER_CLI_ERROR_UNSUPPORTED_ARGUMENT ((HRESULT)0x8A150058)
#define APPINSTALLER_CLI_ERROR_ARP_VERSION_VALIDATION_FAILED ((HRESULT)0x8A150058)
#define APPINSTALLER_CLI_ERROR_UNSUPPORTED_ARGUMENT ((HRESULT)0x8A150059)

// Install errors.
#define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE ((HRESULT)0x8A150101)
Expand Down