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 5 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
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
29 changes: 29 additions & 0 deletions src/AppInstallerCLICore/Workflows/InstallFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,19 @@ namespace AppInstaller::CLI::Workflow
}
}

Execution::Args::Type GetUnsupportedArgumentType(UnsupportedArgumentEnum unsupportedArg)
{
switch (unsupportedArg)
{
case UnsupportedArgumentEnum::Log:
return Execution::Args::Type::Log;
case UnsupportedArgumentEnum::Location:
return 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 +141,22 @@ namespace AppInstaller::CLI::Workflow
}
}

void CheckForUnsupportedArgs(Execution::Context& context)
{
const auto& unsupportedArgs = context.Get<Execution::Data::Installer>().value().UnsupportedArguments;
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved

if (!unsupportedArgs.empty())
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
{
for (auto unsupportedArg : unsupportedArgs)
{
if (context.Args.Contains(GetUnsupportedArgumentType(unsupportedArg)))
{
context.Reporter.Error() << Resource::String::UnsupportedArgument << " --" << UnsupportedArgumentToString(unsupportedArg) << std::endl;
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

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, UnsupportedArguments
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
// 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
23 changes: 23 additions & 0 deletions src/AppInstallerCLITests/WorkFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,29 @@ TEST_CASE("InstallFlow_InstallationNotes", "[InstallFlow][workflow]")
REQUIRE(installOutput.str().find("testInstallationNotes") != std::string::npos);
}

TEST_CASE("InstallFlow_UnsupportedArguments", "[InstallFlow][workflow]")
{
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());
context.Args.AddArg(Execution::Args::Type::Log);
context.Args.AddArg(Execution::Args::Type::InstallLocation);

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

// Verify Unsupported arguments error message is shown
REQUIRE(context.GetTerminationHR() == S_OK);
REQUIRE(std::filesystem::exists(installResultPath.GetPath()));
REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::UnsupportedArgument).get() + " --log") != std::string::npos);
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::UnsupportedArgument).get() + " --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
13 changes: 13 additions & 0 deletions src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,19 @@ namespace AppInstaller::Manifest
return "Unknown"sv;
}

std::string_view UnsupportedArgumentToString(UnsupportedArgumentEnum unsupportedArg)
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
{
switch (unsupportedArg)
{
case UnsupportedArgumentEnum::Log:
return "log"sv;
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
case UnsupportedArgumentEnum::Location:
return "location"sv;
}

return "Unknown"sv;
}

bool DoesInstallerTypeUsePackageFamilyName(InstallerTypeEnum installerType)
{
return (installerType == InstallerTypeEnum::Msix || installerType == InstallerTypeEnum::MSStore);
Expand Down
2 changes: 1 addition & 1 deletion src/AppInstallerCommonCore/Public/AppInstallerErrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
#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)

// Install errors.
#define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE ((HRESULT)0x8A150101)
Expand Down
2 changes: 2 additions & 0 deletions src/AppInstallerCommonCore/Public/winget/ManifestCommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,8 @@ bool HasExtension(std::string_view extension) const;

std::string_view ScopeToString(ScopeEnum scope);

std::string_view UnsupportedArgumentToString(UnsupportedArgumentEnum unsupportedArg);

// Gets a value indicating whether the given installer type uses the PackageFamilyName system reference.
bool DoesInstallerTypeUsePackageFamilyName(InstallerTypeEnum installerType);

Expand Down