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 10 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 @@ -86,6 +86,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 "--name,-alias".
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
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
9 changes: 1 addition & 8 deletions src/AppInstallerCLICore/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,14 +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() << ',';
}
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;
}
}
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
47 changes: 47 additions & 0 deletions src/AppInstallerCLICore/Workflows/InstallFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "Workflows/DependenciesFlow.h"
#include <AppInstallerDeployment.h>
#include <winget/ARPCorrelation.h>
#include <Argument.h>

using namespace winrt::Windows::ApplicationModel::Store::Preview::InstallControl;
using namespace winrt::Windows::Foundation;
Expand Down Expand Up @@ -57,6 +58,30 @@ namespace AppInstaller::CLI::Workflow
}
}

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

AppInstaller::CLI::Argument GetUnsupportedArgument(UnsupportedArgumentEnum arg)
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
{
switch (arg)
{
case UnsupportedArgumentEnum::Log:
return AppInstaller::CLI::Argument::ForType(Execution::Args::Type::Log);
case UnsupportedArgumentEnum::Location:
return AppInstaller::CLI::Argument::ForType(Execution::Args::Type::InstallLocation);
default:
THROW_HR(E_UNEXPECTED);
}
}

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

void CheckForUnsupportedArgs(Execution::Context& context)
{
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& 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;
}
}
}
}

void ShowInstallationDisclaimer(Execution::Context& context)
{
auto installerType = context.Get<Execution::Data::Installer>().value().InstallerType;
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>This argument is not supported for this package:</value>
</data>
</root>
3 changes: 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,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 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
64 changes: 64 additions & 0 deletions src/AppInstallerCLITests/WorkFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,70 @@ TEST_CASE("InstallFlow_InstallationNotes", "[InstallFlow][workflow]")
REQUIRE(installOutput.str().find("testInstallationNotes") != std::string::npos);
}

TEST_CASE("InstallFlow_UnsupportedArguments_Warn", "[InstallFlow][workflow]")
{
TestCommon::TempFile installResultPath("TestExeInstalled.txt");
TestCommon::TempDirectory tempDirectory("TempDirectory", false);

std::ostringstream installOutput;
TestContext context{ installOutput, std::cin };
auto previousThreadGlobals = context.SetForCurrentThread();
OverrideForShellExecute(context);
context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_UnsupportedArguments.yaml").GetPath().u8string());
context.Args.AddArg(Execution::Args::Type::Log, tempDirectory);

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

// Verify unsupported arguments warn message is shown
REQUIRE(context.GetTerminationHR() == S_OK);
REQUIRE(std::filesystem::exists(installResultPath.GetPath()));
REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::UnsupportedArgument).get() + " -o,--log") != std::string::npos);
}

TEST_CASE("InstallFlow_UnsupportedArguments_Error", "[InstallFlow][workflow]")
{
TestCommon::TempFile installResultPath("TestExeInstalled.txt");
TestCommon::TempDirectory tempDirectory("TempDirectory", false);

std::ostringstream installOutput;
TestContext context{ installOutput, std::cin };
auto previousThreadGlobals = context.SetForCurrentThread();
context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_UnsupportedArguments.yaml").GetPath().u8string());
context.Args.AddArg(Execution::Args::Type::InstallLocation, tempDirectory);

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

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

TEST_CASE("InstallFlow_UnsupportedArguments_NotProvided")
{
TestCommon::TempFile installResultPath("TestExeInstalled.txt");

std::ostringstream installOutput;
TestContext context{ installOutput, std::cin };
auto previousThreadGlobals = context.SetForCurrentThread();
OverrideForShellExecute(context);
context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_UnsupportedArguments.yaml").GetPath().u8string());

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

// Verify unsupported arguments error message is not shown when not provided
REQUIRE(context.GetTerminationHR() == S_OK);
REQUIRE(std::filesystem::exists(installResultPath.GetPath()));
REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::UnsupportedArgument).get() + " -o,--log") == std::string::npos);
REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::UnsupportedArgument).get() + " -l,--location") == std::string::npos);
}

TEST_CASE("InstallFlow_ExpectedReturnCodes", "[InstallFlow][workflow]")
{
TestCommon::TempFile installResultPath("TestExeInstalled.txt");
Expand Down
7 changes: 5 additions & 2 deletions src/AppInstallerCLITests/YamlManifest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -466,9 +466,8 @@ void VerifyV1ManifestContent(const Manifest& manifest, bool isSingleton, Manifes
if (manifestVer >= ManifestVer{ s_ManifestVersionV1_2 })
{
REQUIRE(manifest.DefaultInstallerInfo.DisplayInstallWarnings);
REQUIRE(manifest.DefaultInstallerInfo.UnsupportedArguments.size() == 2);
REQUIRE(manifest.DefaultInstallerInfo.UnsupportedArguments.size() == 1);
REQUIRE(manifest.DefaultInstallerInfo.UnsupportedArguments.at(0) == UnsupportedArgumentEnum::Log);
REQUIRE(manifest.DefaultInstallerInfo.UnsupportedArguments.at(1) == UnsupportedArgumentEnum::Location);
}

if (isSingleton)
Expand Down Expand Up @@ -545,6 +544,8 @@ void VerifyV1ManifestContent(const Manifest& manifest, bool isSingleton, Manifes
REQUIRE_FALSE(installer1.DisplayInstallWarnings);
REQUIRE(installer1.ExpectedReturnCodes.at(3).ReturnResponseEnum == ExpectedReturnCodeEnum::Custom);
REQUIRE(installer1.ExpectedReturnCodes.at(3).ReturnResponseUrl == "https://defaultReturnResponseUrl.com");
REQUIRE(installer1.UnsupportedArguments.size() == 1);
REQUIRE(installer1.UnsupportedArguments.at(1) == UnsupportedArgumentEnum::Location);
}

if (!isSingleton)
Expand Down Expand Up @@ -590,6 +591,8 @@ void VerifyV1ManifestContent(const Manifest& manifest, bool isSingleton, Manifes
REQUIRE(installer3.ExpectedReturnCodes.at(11).ReturnResponseEnum == ExpectedReturnCodeEnum::Custom);
REQUIRE(installer3.ExpectedReturnCodes.at(11).ReturnResponseUrl == "https://defaultReturnResponseUrl.com");
REQUIRE_FALSE(installer3.DisplayInstallWarnings);
REQUIRE(installer3.UnsupportedArguments.size() == 1);
REQUIRE(installer3.UnsupportedArguments.at(0) == UnsupportedArgumentEnum::Location);
}

// Localization
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 @@ -99,7 +99,8 @@
#define APPINSTALLER_CLI_ERROR_PORTABLE_PACKAGE_ALREADY_EXISTS ((HRESULT)0x8A150054)
#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_PORTABLE_UNINSTALL_FAILED ((HRESULT)0x8A150057)
#define APPINSTALLER_CLI_ERROR_UNSUPPORTED_ARGUMENT ((HRESULT)0x8A150058)
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved

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