From 640643dd0abd7978a3def2eca95dc14e10852274 Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Thu, 2 Jun 2022 14:31:22 -0700 Subject: [PATCH 01/10] initialCommit --- .../v1.2.0/manifest.installer.1.2.0.json | 6 ++++ .../v1.2.0/manifest.singleton.1.2.0.json | 6 ++++ .../Workflows/InstallFlow.cpp | 30 +++++++++++++++++++ .../Public/AppInstallerErrors.h | 1 + 4 files changed, 43 insertions(+) diff --git a/schemas/JSON/manifests/v1.2.0/manifest.installer.1.2.0.json b/schemas/JSON/manifests/v1.2.0/manifest.installer.1.2.0.json index 51f976d3c0..767b4c3550 100644 --- a/schemas/JSON/manifests/v1.2.0/manifest.installer.1.2.0.json +++ b/schemas/JSON/manifests/v1.2.0/manifest.installer.1.2.0.json @@ -571,6 +571,9 @@ "UnsupportedOSArchitectures": { "$ref": "#/definitions/UnsupportedOSArchitectures" }, + "UnsupportedArguments": { + "$ref": "#/definitions/UnsupportedArguments" + }, "AppsAndFeaturesEntries": { "$ref": "#/definitions/AppsAndFeaturesEntries" }, @@ -668,6 +671,9 @@ "UnsupportedOSArchitectures": { "$ref": "#/definitions/UnsupportedOSArchitectures" }, + "UnsupportedArguments": { + "$ref": "#/definitions/UnsupportedArguments" + }, "AppsAndFeaturesEntries": { "$ref": "#/definitions/AppsAndFeaturesEntries" }, diff --git a/schemas/JSON/manifests/v1.2.0/manifest.singleton.1.2.0.json b/schemas/JSON/manifests/v1.2.0/manifest.singleton.1.2.0.json index 216705a623..c683bae591 100644 --- a/schemas/JSON/manifests/v1.2.0/manifest.singleton.1.2.0.json +++ b/schemas/JSON/manifests/v1.2.0/manifest.singleton.1.2.0.json @@ -609,6 +609,9 @@ "UnsupportedOSArchitectures": { "$ref": "#/definitions/UnsupportedOSArchitectures" }, + "UnsupportedArguments": { + "$ref": " #/definitions/UnsupportedArguments" + } "AppsAndFeaturesEntries": { "$ref": "#/definitions/AppsAndFeaturesEntries" }, @@ -822,6 +825,9 @@ "UnsupportedOSArchitectures": { "$ref": "#/definitions/UnsupportedOSArchitectures" }, + "UnsupportedArguments": { + "$ref": "#/definitions/UnsupportedArguments" + }, "AppsAndFeaturesEntries": { "$ref": "#/definitions/AppsAndFeaturesEntries" }, diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index 09ced34096..b7ab384396 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -129,6 +129,36 @@ namespace AppInstaller::CLI::Workflow } } + void CheckForUnsupportedArgs(Execution::Context& context) + { + const auto& manifest = context.Get(); + const auto& unsupportedArgs = manifest.DefaultInstallerInfo.UnsupportedArguments; + bool unsupportedArgFound = false; + + if (!unsupportedArgs.empty()) + { + for (auto unsupportedArg : unsupportedArgs) + { + if (unsupportedArg == UnsupportedArgumentEnum::Log && context.Args.Contains(Execution::Args::Type::Log)) + { + unsupportedArgFound = true; + context.Reporter.Error() << Resource::String::LogArgumentNotSupported << std::endl; + } + + if (unsupportedArg == UnsupportedArgumentEnum::Location && context.Args.Contains(Execution::Args::Type::InstallLocation)) + { + unsupportedArgFound = true; + context.Reporter.Error() << Resource::String::LocationArgumentNotSupported << std::endl; + } + } + } + + if (unsupportedArgFound) + { + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_UNSUPPORTED_ARGUMENT); + } + } + void ShowInstallationDisclaimer(Execution::Context& context) { auto installerType = context.Get().value().InstallerType; diff --git a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h index ae04e2f954..ed7c4049c7 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h @@ -100,6 +100,7 @@ #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) // Install errors. #define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE ((HRESULT)0x8A150101) From 88a20337af74cb49ae17a760eb2b38c3ee9874c0 Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Thu, 2 Jun 2022 16:20:10 -0700 Subject: [PATCH 02/10] add string --- src/AppInstallerCLICore/Resources.h | 2 ++ .../Shared/Strings/en-us/winget.resw | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index cd12634abb..b56e842e8d 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -164,7 +164,9 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(ListCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(LocaleArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(LocationArgumentDescription); + WINGET_DEFINE_RESOURCE_STRINGID(LocationArgumentNotSupported); WINGET_DEFINE_RESOURCE_STRINGID(LogArgumentDescription); + WINGET_DEFINE_RESOURCE_STRINGID(LogArgumentNotSupported); WINGET_DEFINE_RESOURCE_STRINGID(Logs); WINGET_DEFINE_RESOURCE_STRINGID(MainCopyrightNotice); WINGET_DEFINE_RESOURCE_STRINGID(MainHomepage); diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index be5a075a7c..c2fd3cfae7 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1337,4 +1337,12 @@ Please specify one of them using the `--source` option to proceed. Cannot purge install directory, as it was not created by WinGet + + The argument --location is not supported + {Locked="--location"} + + + The argument --log is not supported for this package + {Locked="--log"} + \ No newline at end of file From 9067f56e3b7f3fa56c84d6f5878d284e38fbb010 Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Fri, 3 Jun 2022 11:48:07 -0700 Subject: [PATCH 03/10] fix test --- src/AppInstallerCLITests/YamlManifest.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index 2d644efd86..2e6869b95a 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -591,9 +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() == 2); - REQUIRE(installer3.UnsupportedArguments.at(0) == UnsupportedArgumentEnum::Log); - REQUIRE(installer3.UnsupportedArguments.at(1) == UnsupportedArgumentEnum::Location); + REQUIRE(installer3.UnsupportedArguments.size() == 1); + REQUIRE(installer3.UnsupportedArguments.at(0) == UnsupportedArgumentEnum::Location); } // Localization From 47fb4f406aba8f701c6d2b927e01fbb5f035da11 Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Mon, 6 Jun 2022 12:19:16 -0700 Subject: [PATCH 04/10] address PR comments --- src/AppInstallerCLICore/Resources.h | 3 +- .../Workflows/InstallFlow.cpp | 30 +++++++++---------- .../Workflows/InstallFlow.h | 2 +- .../Shared/Strings/en-us/winget.resw | 9 ++---- .../InstallFlowTest_UnsupportedArguments.yaml | 4 +-- src/AppInstallerCLITests/WorkFlow.cpp | 12 +++++--- .../Manifest/ManifestCommon.cpp | 13 ++++++++ .../Public/AppInstallerErrors.h | 3 +- .../Public/winget/ManifestCommon.h | 2 ++ 9 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index a98e47cd75..ddde7a291e 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -164,9 +164,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(ListCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(LocaleArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(LocationArgumentDescription); - WINGET_DEFINE_RESOURCE_STRINGID(LocationArgumentNotSupported); WINGET_DEFINE_RESOURCE_STRINGID(LogArgumentDescription); - WINGET_DEFINE_RESOURCE_STRINGID(LogArgumentNotSupported); WINGET_DEFINE_RESOURCE_STRINGID(Logs); WINGET_DEFINE_RESOURCE_STRINGID(MainCopyrightNotice); WINGET_DEFINE_RESOURCE_STRINGID(MainHomepage); @@ -362,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); diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index 9ff32d4d84..386de1f00d 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -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) : @@ -131,30 +144,17 @@ namespace AppInstaller::CLI::Workflow void CheckForUnsupportedArgs(Execution::Context& context) { const auto& unsupportedArgs = context.Get().value().UnsupportedArguments; - bool unsupportedArgFound = false; if (!unsupportedArgs.empty()) { for (auto unsupportedArg : unsupportedArgs) { - if (unsupportedArg == UnsupportedArgumentEnum::Log && context.Args.Contains(Execution::Args::Type::Log)) + if (context.Args.Contains(GetUnsupportedArgumentType(unsupportedArg))) { - unsupportedArgFound = true; - context.Reporter.Error() << Resource::String::LogArgumentNotSupported << std::endl; - } - - if (unsupportedArg == UnsupportedArgumentEnum::Location && context.Args.Contains(Execution::Args::Type::InstallLocation)) - { - unsupportedArgFound = true; - context.Reporter.Error() << Resource::String::LocationArgumentNotSupported << std::endl; + context.Reporter.Error() << Resource::String::UnsupportedArgument << " --" << UnsupportedArgumentToString(unsupportedArg) << std::endl; } } } - - if (unsupportedArgFound) - { - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_UNSUPPORTED_ARGUMENT); - } } void ShowInstallationDisclaimer(Execution::Context& context) diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.h b/src/AppInstallerCLICore/Workflows/InstallFlow.h index e28cdbc1c3..4c284be70e 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.h +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.h @@ -31,7 +31,7 @@ namespace AppInstaller::CLI::Workflow // Checks if there are any included arguments that are not supported for the package. // Required Args: None - // Inputs: UnsupportedArguments + // Inputs: Installer, UnsupportedArguments // Outputs: None void CheckForUnsupportedArgs(Execution::Context& context); diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 4a71a191fd..bb02e7108d 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1349,12 +1349,7 @@ Please specify one of them using the `--source` option to proceed. InstallationNotes: - - The argument --location is not supported for this package - {Locked="--location"} - - - The argument --log is not supported for this package - {Locked="--log"} + + This argument is not supported for this package: \ No newline at end of file diff --git a/src/AppInstallerCLITests/TestData/InstallFlowTest_UnsupportedArguments.yaml b/src/AppInstallerCLITests/TestData/InstallFlowTest_UnsupportedArguments.yaml index b8d48d96f4..4aac2d3daa 100644 --- a/src/AppInstallerCLITests/TestData/InstallFlowTest_UnsupportedArguments.yaml +++ b/src/AppInstallerCLITests/TestData/InstallFlowTest_UnsupportedArguments.yaml @@ -7,8 +7,8 @@ Publisher: Microsoft Corporation Moniker: AICLITestExe License: Test UnsupportedArguments: - - log - - location + - log + - location Installers: - Architecture: x86 InstallerUrl: https://ThisIsNotUsed diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp index ae4d6cee5a..9eb7772fbc 100644 --- a/src/AppInstallerCLITests/WorkFlow.cpp +++ b/src/AppInstallerCLITests/WorkFlow.cpp @@ -817,9 +817,12 @@ TEST_CASE("InstallFlow_InstallationNotes", "[InstallFlow][workflow]") 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); @@ -828,10 +831,11 @@ TEST_CASE("InstallFlow_UnsupportedArguments", "[InstallFlow][workflow]") install.Execute(context); INFO(installOutput.str()); - // Verify install failed with unsupported arguments error message - REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_UNSUPPORTED_ARGUMENT); - REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::LogArgumentNotSupported).get()) != std::string::npos); - REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::LocationArgumentNotSupported).get()) != std::string::npos); + // 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); + REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::UnsupportedArgument).get() + " --location") != std::string::npos); } TEST_CASE("InstallFlow_ExpectedReturnCodes", "[InstallFlow][workflow]") diff --git a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp index e49d6ffd7c..f2e4123132 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp @@ -417,6 +417,19 @@ namespace AppInstaller::Manifest return "Unknown"sv; } + std::string_view UnsupportedArgumentToString(UnsupportedArgumentEnum unsupportedArg) + { + switch (unsupportedArg) + { + case UnsupportedArgumentEnum::Log: + return "log"sv; + case UnsupportedArgumentEnum::Location: + return "location"sv; + } + + return "Unknown"sv; + } + bool DoesInstallerTypeUsePackageFamilyName(InstallerTypeEnum installerType) { return (installerType == InstallerTypeEnum::Msix || installerType == InstallerTypeEnum::MSStore); diff --git a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h index ed7c4049c7..88f5cedeb2 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h @@ -99,8 +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_UNSUPPORTED_ARGUMENT ((HRESULT)0x8A150058) +#define APPINSTALLER_CLI_ERROR_PORTABLE_UNINSTALL_FAILED ((HRESULT)0x8A150057) // Install errors. #define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE ((HRESULT)0x8A150101) diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index fd6e99a318..391fb44b44 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -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); From 31d7ae25ac91e936bd6ea74a8e07325a89f2731b Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Mon, 6 Jun 2022 13:40:35 -0700 Subject: [PATCH 05/10] add argument value --- src/AppInstallerCLITests/WorkFlow.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp index 9eb7772fbc..8aa6a4719c 100644 --- a/src/AppInstallerCLITests/WorkFlow.cpp +++ b/src/AppInstallerCLITests/WorkFlow.cpp @@ -818,14 +818,15 @@ TEST_CASE("InstallFlow_InstallationNotes", "[InstallFlow][workflow]") TEST_CASE("InstallFlow_UnsupportedArguments", "[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); - context.Args.AddArg(Execution::Args::Type::InstallLocation); + context.Args.AddArg(Execution::Args::Type::Log, tempDirectory); + context.Args.AddArg(Execution::Args::Type::InstallLocation, tempDirectory); InstallCommand install({}); install.Execute(context); From 7af995b4ce6009e91d2d6a66a699c38804b69f08 Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Mon, 6 Jun 2022 14:27:31 -0700 Subject: [PATCH 06/10] address comments #2 --- src/AppInstallerCLICore/Workflows/InstallFlow.cpp | 12 +++++------- src/AppInstallerCLICore/Workflows/InstallFlow.h | 2 +- src/AppInstallerCLITests/WorkFlow.cpp | 5 ++--- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index 386de1f00d..7e63912243 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -24,6 +24,7 @@ using namespace AppInstaller::Manifest; using namespace AppInstaller::Repository; using namespace AppInstaller::Settings; using namespace AppInstaller::Utility; +using namespace AppInstaller::Utility::literals; namespace AppInstaller::CLI::Workflow { @@ -143,16 +144,13 @@ namespace AppInstaller::CLI::Workflow void CheckForUnsupportedArgs(Execution::Context& context) { - const auto& unsupportedArgs = context.Get().value().UnsupportedArguments; + const auto& unsupportedArgs = context.Get()->UnsupportedArguments; - if (!unsupportedArgs.empty()) + for (auto unsupportedArg : unsupportedArgs) { - for (auto unsupportedArg : unsupportedArgs) + if (context.Args.Contains(GetUnsupportedArgumentType(unsupportedArg))) { - if (context.Args.Contains(GetUnsupportedArgumentType(unsupportedArg))) - { - context.Reporter.Error() << Resource::String::UnsupportedArgument << " --" << UnsupportedArgumentToString(unsupportedArg) << std::endl; - } + context.Reporter.Warn() << Resource::String::UnsupportedArgument << " --"_liv << UnsupportedArgumentToString(unsupportedArg) << std::endl; } } } diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.h b/src/AppInstallerCLICore/Workflows/InstallFlow.h index 4c284be70e..74da14435a 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.h +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.h @@ -31,7 +31,7 @@ namespace AppInstaller::CLI::Workflow // Checks if there are any included arguments that are not supported for the package. // Required Args: None - // Inputs: Installer, UnsupportedArguments + // Inputs: Installer // Outputs: None void CheckForUnsupportedArgs(Execution::Context& context); diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp index 8aa6a4719c..d9a0931f96 100644 --- a/src/AppInstallerCLITests/WorkFlow.cpp +++ b/src/AppInstallerCLITests/WorkFlow.cpp @@ -826,17 +826,16 @@ TEST_CASE("InstallFlow_UnsupportedArguments", "[InstallFlow][workflow]") OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_UnsupportedArguments.yaml").GetPath().u8string()); context.Args.AddArg(Execution::Args::Type::Log, tempDirectory); - context.Args.AddArg(Execution::Args::Type::InstallLocation, tempDirectory); InstallCommand install({}); install.Execute(context); INFO(installOutput.str()); - // Verify Unsupported arguments error message is shown + // Verify unsupported arguments error message is shown only for --log argument 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); - REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::UnsupportedArgument).get() + " --location") != std::string::npos); + REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::UnsupportedArgument).get() + " --location") == std::string::npos); } TEST_CASE("InstallFlow_ExpectedReturnCodes", "[InstallFlow][workflow]") From e25e42233032b1e1afa7d61d25c33caed51d705d Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Mon, 6 Jun 2022 14:36:36 -0700 Subject: [PATCH 07/10] add unit test --- src/AppInstallerCLITests/WorkFlow.cpp | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp index d9a0931f96..9e22fb628c 100644 --- a/src/AppInstallerCLITests/WorkFlow.cpp +++ b/src/AppInstallerCLITests/WorkFlow.cpp @@ -826,15 +826,37 @@ TEST_CASE("InstallFlow_UnsupportedArguments", "[InstallFlow][workflow]") OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_UnsupportedArguments.yaml").GetPath().u8string()); context.Args.AddArg(Execution::Args::Type::Log, tempDirectory); + context.Args.AddArg(Execution::Args::Type::InstallLocation, tempDirectory); InstallCommand install({}); install.Execute(context); INFO(installOutput.str()); - // Verify unsupported arguments error message is shown only for --log argument + // 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); + REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::UnsupportedArgument).get() + " --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() + " --log") == std::string::npos); REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::UnsupportedArgument).get() + " --location") == std::string::npos); } From 7bb86afc08ca49777d63bada1fee7bc0d1071cd5 Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Tue, 7 Jun 2022 15:22:31 -0700 Subject: [PATCH 08/10] resolve PR comments --- src/AppInstallerCLICore/Argument.cpp | 11 ++++++ src/AppInstallerCLICore/Argument.h | 3 ++ src/AppInstallerCLICore/Command.cpp | 9 +---- .../Workflows/InstallFlow.cpp | 36 ++++++++++++++----- src/AppInstallerCLITests/WorkFlow.cpp | 33 +++++++++++++---- .../Public/AppInstallerErrors.h | 1 + 6 files changed, 70 insertions(+), 23 deletions(-) diff --git a/src/AppInstallerCLICore/Argument.cpp b/src/AppInstallerCLICore/Argument.cpp index 4ca1348064..7a57d6777d 100644 --- a/src/AppInstallerCLICore/Argument.cpp +++ b/src/AppInstallerCLICore/Argument.cpp @@ -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 }) diff --git a/src/AppInstallerCLICore/Argument.h b/src/AppInstallerCLICore/Argument.h index 77b6f996b0..68e8f78dd9 100644 --- a/src/AppInstallerCLICore/Argument.h +++ b/src/AppInstallerCLICore/Argument.h @@ -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". + 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; } diff --git a/src/AppInstallerCLICore/Command.cpp b/src/AppInstallerCLICore/Command.cpp index 858ca9198b..3db974be11 100644 --- a/src/AppInstallerCLICore/Command.cpp +++ b/src/AppInstallerCLICore/Command.cpp @@ -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()); } diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index 7e63912243..fdd684752d 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -14,6 +14,7 @@ #include "Workflows/DependenciesFlow.h" #include #include +#include using namespace winrt::Windows::ApplicationModel::Store::Preview::InstallControl; using namespace winrt::Windows::Foundation; @@ -24,7 +25,6 @@ using namespace AppInstaller::Manifest; using namespace AppInstaller::Repository; using namespace AppInstaller::Settings; using namespace AppInstaller::Utility; -using namespace AppInstaller::Utility::literals; namespace AppInstaller::CLI::Workflow { @@ -58,14 +58,25 @@ namespace AppInstaller::CLI::Workflow } } - Execution::Args::Type GetUnsupportedArgumentType(UnsupportedArgumentEnum unsupportedArg) + bool ShouldErrorForUnsupportedArgument(UnsupportedArgumentEnum arg) { - switch (unsupportedArg) + switch (arg) + { + case UnsupportedArgumentEnum::Location: + return true; + default: + return false; + } + } + + AppInstaller::CLI::Argument GetUnsupportedArgument(UnsupportedArgumentEnum arg) + { + switch (arg) { case UnsupportedArgumentEnum::Log: - return Execution::Args::Type::Log; + return AppInstaller::CLI::Argument::ForType(Execution::Args::Type::Log); case UnsupportedArgumentEnum::Location: - return Execution::Args::Type::InstallLocation; + return AppInstaller::CLI::Argument::ForType(Execution::Args::Type::InstallLocation); default: THROW_HR(E_UNEXPECTED); } @@ -145,12 +156,21 @@ namespace AppInstaller::CLI::Workflow void CheckForUnsupportedArgs(Execution::Context& context) { const auto& unsupportedArgs = context.Get()->UnsupportedArguments; - for (auto unsupportedArg : unsupportedArgs) { - if (context.Args.Contains(GetUnsupportedArgumentType(unsupportedArg))) + const auto& argument = GetUnsupportedArgument(unsupportedArg); + if (context.Args.Contains(argument.ExecArgType())) { - context.Reporter.Warn() << Resource::String::UnsupportedArgument << " --"_liv << UnsupportedArgumentToString(unsupportedArg) << std::endl; + 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; + } } } } diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp index 9e22fb628c..a0393812a5 100644 --- a/src/AppInstallerCLITests/WorkFlow.cpp +++ b/src/AppInstallerCLITests/WorkFlow.cpp @@ -815,7 +815,7 @@ TEST_CASE("InstallFlow_InstallationNotes", "[InstallFlow][workflow]") REQUIRE(installOutput.str().find("testInstallationNotes") != std::string::npos); } -TEST_CASE("InstallFlow_UnsupportedArguments", "[InstallFlow][workflow]") +TEST_CASE("InstallFlow_UnsupportedArguments_Warn", "[InstallFlow][workflow]") { TestCommon::TempFile installResultPath("TestExeInstalled.txt"); TestCommon::TempDirectory tempDirectory("TempDirectory", false); @@ -826,17 +826,36 @@ TEST_CASE("InstallFlow_UnsupportedArguments", "[InstallFlow][workflow]") OverrideForShellExecute(context); context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_UnsupportedArguments.yaml").GetPath().u8string()); context.Args.AddArg(Execution::Args::Type::Log, tempDirectory); - context.Args.AddArg(Execution::Args::Type::InstallLocation, tempDirectory); InstallCommand install({}); install.Execute(context); INFO(installOutput.str()); - // Verify unsupported arguments error message is shown + // 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() + " --log") != std::string::npos); - REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::UnsupportedArgument).get() + " --location") != std::string::npos); + 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") @@ -856,8 +875,8 @@ TEST_CASE("InstallFlow_UnsupportedArguments_NotProvided") // 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() + " --log") == std::string::npos); - REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::UnsupportedArgument).get() + " --location") == std::string::npos); + 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]") diff --git a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h index 88f5cedeb2..0a0842d1e8 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h @@ -100,6 +100,7 @@ #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) // Install errors. #define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE ((HRESULT)0x8A150101) From 4273343e28ce14fc00a641d7066fbe943d4db002 Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Tue, 7 Jun 2022 15:31:11 -0700 Subject: [PATCH 09/10] remove toString --- .../Manifest/ManifestCommon.cpp | 13 ------------- .../Public/winget/ManifestCommon.h | 2 -- 2 files changed, 15 deletions(-) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp index f2e4123132..e49d6ffd7c 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp @@ -417,19 +417,6 @@ namespace AppInstaller::Manifest return "Unknown"sv; } - std::string_view UnsupportedArgumentToString(UnsupportedArgumentEnum unsupportedArg) - { - switch (unsupportedArg) - { - case UnsupportedArgumentEnum::Log: - return "log"sv; - case UnsupportedArgumentEnum::Location: - return "location"sv; - } - - return "Unknown"sv; - } - bool DoesInstallerTypeUsePackageFamilyName(InstallerTypeEnum installerType) { return (installerType == InstallerTypeEnum::Msix || installerType == InstallerTypeEnum::MSStore); diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index 391fb44b44..fd6e99a318 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -265,8 +265,6 @@ 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); From f1b65390a06556e9ccb2c55f47ddca06df161d50 Mon Sep 17 00:00:00 2001 From: Ryan Fu Date: Thu, 9 Jun 2022 11:34:40 -0700 Subject: [PATCH 10/10] search for command arguments and add update test --- src/AppInstallerCLICore/Argument.h | 2 +- .../Commands/UpgradeCommand.cpp | 4 +- src/AppInstallerCLICore/Core.cpp | 2 +- src/AppInstallerCLICore/ExecutionContext.cpp | 1 + src/AppInstallerCLICore/ExecutionContext.h | 12 ++++++ .../Workflows/InstallFlow.cpp | 43 +++++++++++++------ .../AppInstallerCLITests.vcxproj | 3 ++ .../AppInstallerCLITests.vcxproj.filters | 3 ++ .../UpdateFlowTest_Exe_UnsupportedArgs.yaml | 23 ++++++++++ src/AppInstallerCLITests/WorkFlow.cpp | 41 ++++++++++++++++++ 10 files changed, 117 insertions(+), 17 deletions(-) create mode 100644 src/AppInstallerCLITests/TestData/UpdateFlowTest_Exe_UnsupportedArgs.yaml diff --git a/src/AppInstallerCLICore/Argument.h b/src/AppInstallerCLICore/Argument.h index 68e8f78dd9..955e01bb43 100644 --- a/src/AppInstallerCLICore/Argument.h +++ b/src/AppInstallerCLICore/Argument.h @@ -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. diff --git a/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp b/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp index 56e7ad45dd..895a09a32a 100644 --- a/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp +++ b/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp @@ -252,7 +252,9 @@ namespace AppInstaller::CLI context << SelectLatestApplicableUpdate(true); } - context << InstallSinglePackage; + context << + CheckForUnsupportedArgs << + InstallSinglePackage; } } } diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp index ca7a47d6a3..e8955b8103 100644 --- a/src/AppInstallerCLICore/Core.cpp +++ b/src/AppInstallerCLICore/Core.cpp @@ -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 diff --git a/src/AppInstallerCLICore/ExecutionContext.cpp b/src/AppInstallerCLICore/ExecutionContext.cpp index 0447b156ee..95de83e041 100644 --- a/src/AppInstallerCLICore/ExecutionContext.cpp +++ b/src/AppInstallerCLICore/ExecutionContext.cpp @@ -122,6 +122,7 @@ namespace AppInstaller::CLI::Execution { auto clone = std::make_unique(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) { diff --git a/src/AppInstallerCLICore/ExecutionContext.h b/src/AppInstallerCLICore/ExecutionContext.h index 35037ae689..d2f0deb301 100644 --- a/src/AppInstallerCLICore/ExecutionContext.h +++ b/src/AppInstallerCLICore/ExecutionContext.h @@ -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; @@ -137,6 +142,12 @@ namespace AppInstaller::CLI::Execution std::unique_ptr 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); @@ -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; }; } diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index fdd684752d..dc684b3a0f 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -15,6 +15,7 @@ #include #include #include +#include using namespace winrt::Windows::ApplicationModel::Store::Preview::InstallControl; using namespace winrt::Windows::Foundation; @@ -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 @@ -158,18 +165,26 @@ namespace AppInstaller::CLI::Workflow const auto& unsupportedArgs = context.Get()->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(); + 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; + } + } } } } diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj index ade6b422a6..3aa45c99ce 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -555,6 +555,9 @@ true + + true + true diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters index 61a7556708..f24053fef0 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -480,6 +480,9 @@ TestData + + TestData + TestData diff --git a/src/AppInstallerCLITests/TestData/UpdateFlowTest_Exe_UnsupportedArgs.yaml b/src/AppInstallerCLITests/TestData/UpdateFlowTest_Exe_UnsupportedArgs.yaml new file mode 100644 index 0000000000..27858d1a31 --- /dev/null +++ b/src/AppInstallerCLITests/TestData/UpdateFlowTest_Exe_UnsupportedArgs.yaml @@ -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 \ No newline at end of file diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp index a0393812a5..ab0306ecf8 100644 --- a/src/AppInstallerCLITests/WorkFlow.cpp +++ b/src/AppInstallerCLITests/WorkFlow.cpp @@ -241,6 +241,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{ manifest2, manifest }, + shared_from_this() + ), + PackageMatchFilter(PackageMatchField::Id, MatchType::Exact, "AppInstallerCliTest.TestExeInstaller"))); + } + if (input == "TestExeInstallerWithNothingInstalled") { auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Exe.yaml")); @@ -828,6 +843,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()); @@ -849,6 +865,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()); @@ -869,6 +886,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()); @@ -1595,6 +1613,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");