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 13 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
6 changes: 6 additions & 0 deletions schemas/JSON/manifests/v1.2.0/manifest.installer.1.2.0.json
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,9 @@
"UnsupportedOSArchitectures": {
"$ref": "#/definitions/UnsupportedOSArchitectures"
},
"UnsupportedArguments": {
"$ref": "#/definitions/UnsupportedArguments"
},
"AppsAndFeaturesEntries": {
"$ref": "#/definitions/AppsAndFeaturesEntries"
},
Expand Down Expand Up @@ -668,6 +671,9 @@
"UnsupportedOSArchitectures": {
"$ref": "#/definitions/UnsupportedOSArchitectures"
},
"UnsupportedArguments": {
"$ref": "#/definitions/UnsupportedArguments"
},
"AppsAndFeaturesEntries": {
"$ref": "#/definitions/AppsAndFeaturesEntries"
},
Expand Down
6 changes: 6 additions & 0 deletions schemas/JSON/manifests/v1.2.0/manifest.singleton.1.2.0.json
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,9 @@
"UnsupportedOSArchitectures": {
"$ref": "#/definitions/UnsupportedOSArchitectures"
},
"UnsupportedArguments": {
"$ref": " #/definitions/UnsupportedArguments"
},
"AppsAndFeaturesEntries": {
"$ref": "#/definitions/AppsAndFeaturesEntries"
},
Expand Down Expand Up @@ -822,6 +825,9 @@
"UnsupportedOSArchitectures": {
"$ref": "#/definitions/UnsupportedOSArchitectures"
},
"UnsupportedArguments": {
"$ref": "#/definitions/UnsupportedArguments"
},
"AppsAndFeaturesEntries": {
"$ref": "#/definitions/AppsAndFeaturesEntries"
},
Expand Down
11 changes: 11 additions & 0 deletions src/AppInstallerCLICore/Argument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,17 @@ namespace AppInstaller::CLI
args.push_back(ForType(Args::Type::VerboseLogs));
}

std::string Argument::GetUsageString() const
{
std::ostringstream strstr;
if (m_alias != Argument::NoAlias)
{
strstr << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << m_alias << ',';
}
strstr << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << m_name;
return strstr.str();
}

void Argument::ValidatePackageSelectionArgumentSupplied(const Execution::Args& args)
{
for (Args::Type type : { Args::Type::Query, Args::Type::Manifest, Args::Type::Id, Args::Type::Name, Args::Type::Moniker, Args::Type::ProductCode, Args::Type::Tag, Args::Type::Command })
Expand Down
3 changes: 3 additions & 0 deletions src/AppInstallerCLICore/Argument.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ 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 "-alias,--name".
std::string GetUsageString() const;

// Arguments are not localized at this time.
Utility::LocIndView Name() const { return Utility::LocIndView{ m_name }; }
char Alias() const { return m_alias; }
Expand Down
12 changes: 1 addition & 11 deletions src/AppInstallerCLICore/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,17 +223,7 @@ namespace AppInstaller::CLI
size_t maxArgNameLength = 0;
for (const auto& arg : arguments)
{
std::ostringstream strstr;
if (arg.Alias() != Argument::NoAlias)
{
strstr << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << arg.Alias() << ',';
}
if (arg.AlternateName() != Argument::NoAlternateName) {
strstr << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << arg.AlternateName() << ',';
}
strstr << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << arg.Name();

argNames.emplace_back(strstr.str());
argNames.emplace_back(arg.GetUsageString());
maxArgNameLength = std::max(maxArgNameLength, argNames.back().length());
}

Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/Commands/InstallCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ namespace AppInstaller::CLI
Workflow::GetManifest <<
Workflow::SelectInstaller <<
Workflow::EnsureApplicableInstaller <<
Workflow::CheckForUnsupportedArgs <<
Workflow::InstallSinglePackage;
}
}
3 changes: 2 additions & 1 deletion src/AppInstallerCLICore/Commands/UpgradeCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ namespace AppInstaller::CLI
context << SelectLatestApplicableUpdate(true);
}

context << InstallSinglePackage;
context <<
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;
};
}
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/Resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(UninstallFlowStartingPackageUninstall);
WINGET_DEFINE_RESOURCE_STRINGID(UninstallFlowUninstallSuccess);
WINGET_DEFINE_RESOURCE_STRINGID(UnrecognizedCommand);
WINGET_DEFINE_RESOURCE_STRINGID(UnsupportedArgument);
WINGET_DEFINE_RESOURCE_STRINGID(UpdateAllArgumentDescription);
WINGET_DEFINE_RESOURCE_STRINGID(UpdateNotApplicable);
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeCommandLongDescription);
Expand Down
74 changes: 74 additions & 0 deletions src/AppInstallerCLICore/Workflows/InstallFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "Workflows/DependenciesFlow.h"
#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 @@ -57,6 +59,36 @@ namespace AppInstaller::CLI::Workflow
}
}

bool ShouldErrorForUnsupportedArgument(UnsupportedArgumentEnum arg)
{
switch (arg)
{
case UnsupportedArgumentEnum::Location:
return true;
default:
return false;
}
}

Execution::Args::Type GetUnsupportedArgumentType(UnsupportedArgumentEnum unsupportedArgument)
{
Execution::Args::Type execArg;

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

return execArg;
}

struct ExpectedReturnCode
{
ExpectedReturnCode(ExpectedReturnCodeEnum installerReturnCode, HRESULT hr, Resource::StringId message) :
Expand Down Expand Up @@ -128,6 +160,47 @@ namespace AppInstaller::CLI::Workflow
}
}

void CheckForUnsupportedArgs(Execution::Context& context)
{
bool messageDisplayed = false;
const auto& unsupportedArgs = context.Get<Execution::Data::Installer>()->UnsupportedArguments;
for (auto unsupportedArg : unsupportedArgs)
{
const auto& unsupportedArgType = GetUnsupportedArgumentType(unsupportedArg);
if (context.Args.Contains(unsupportedArgType))
{
if (!messageDisplayed)
{
context.Reporter.Warn() << Resource::String::UnsupportedArgument << std::endl;
messageDisplayed = true;
}

const auto& executingCommand = context.GetExecutingCommand();
if (executingCommand != nullptr)
{
const auto& commandArguments = executingCommand->GetArguments();
for (const auto& argument : commandArguments)
{
if (unsupportedArgType == argument.ExecArgType())
{
const auto& usageString = argument.GetUsageString();
if (ShouldErrorForUnsupportedArgument(unsupportedArg))
{
context.Reporter.Error() << usageString << std::endl;
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_UNSUPPORTED_ARGUMENT);
}
else
{
context.Reporter.Warn() << usageString << std::endl;
break;
}
}
}
}
}
}
}

void ShowInstallationDisclaimer(Execution::Context& context)
{
auto installerType = context.Get<Execution::Data::Installer>().value().InstallerType;
Expand Down Expand Up @@ -443,6 +516,7 @@ namespace AppInstaller::CLI::Workflow
void InstallSinglePackage(Execution::Context& context)
{
context <<
Workflow::CheckForUnsupportedArgs <<
Workflow::DownloadSinglePackage <<
Workflow::InstallPackageInstaller;
}
Expand Down
6 changes: 6 additions & 0 deletions src/AppInstallerCLICore/Workflows/InstallFlow.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ namespace AppInstaller::CLI::Workflow
// Inputs: InstallationNotes
// Outputs: None
void DisplayInstallationNotes(Execution::Context& context);

// Checks if there are any included arguments that are not supported for the package.
// Required Args: None
// Inputs: Installer
// Outputs: None
void CheckForUnsupportedArgs(Execution::Context& context);

// Shows the license agreements if the application has them.
// Required Args: None
Expand Down
3 changes: 3 additions & 0 deletions src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw
Original file line number Diff line number Diff line change
Expand Up @@ -1349,4 +1349,7 @@ Please specify one of them using the `--source` option to proceed.</value>
<data name="ShowLabelInstallationNotes" xml:space="preserve">
<value>InstallationNotes:</value>
</data>
<data name="UnsupportedArgument" xml:space="preserve">
<value>A provided argument is not supported for this package</value>
</data>
</root>
6 changes: 6 additions & 0 deletions src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,9 @@
<CopyFileToFolders Include="TestData\InstallFlowTest_Portable_WithCommand.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\InstallFlowTest_UnsupportedArguments.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\ImportFile-Bad-Invalid.json">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
Expand Down Expand Up @@ -567,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 @@ -468,6 +468,9 @@
<CopyFileToFolders Include="TestData\InstallFlowTest_Portable_WithCommand.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\InstallFlowTest_UnsupportedArguments.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\InstallerArgTest_Msi_WithSwitches.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
Expand All @@ -492,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,18 @@
PackageIdentifier: AppInstallerCliTest.TestInstaller
PackageVersion: 1.0.0.0
PackageLocale: en-US
PackageName: AppInstaller Test Installer
ShortDescription: AppInstaller Test Installer
Publisher: Microsoft Corporation
Moniker: AICLITestExe
License: Test
UnsupportedArguments:
- log
- location
Installers:
- Architecture: x86
InstallerUrl: https://ThisIsNotUsed
InstallerType: exe
InstallerSha256: 65DB2F2AC2686C7F2FD69D4A4C6683B888DC55BFA20A0E32CA9F838B51689A3B
ManifestType: singleton
ManifestVersion: 1.2.0
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ ExpectedReturnCodes:
ReturnResponseUrl: https://DefaultReturnResponseUrl.com
UnsupportedArguments:
- log
- location

Installers:
- Architecture: x86
Expand Down Expand Up @@ -159,7 +158,6 @@ Installers:
DisplayInstallWarnings: false
ElevationRequirement: elevationRequired
UnsupportedArguments:
- log
- location
UnsupportedOSArchitectures:
- arm64
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ ExpectedReturnCodes:
ReturnResponseUrl: https://DefaultReturnResponseUrl.com
UnsupportedArguments:
- log
- location

Installers:
- Architecture: x86
Expand Down Expand Up @@ -156,5 +155,7 @@ Installers:
- InstallerReturnCode: 11
ReturnResponse: custom
ReturnResponseUrl: https://defaultReturnResponseUrl.com
UnsupportedArguments:
- location
ManifestType: installer
ManifestVersion: 1.2.0
Loading