From d618c95926aefc9a0fad2ec659cd92cb369de0af Mon Sep 17 00:00:00 2001 From: --global Date: Mon, 16 Sep 2024 16:38:57 -0700 Subject: [PATCH 1/5] add manifest field --- .../v1.9.0/manifest.installer.1.9.0.json | 16 ++++++++++--- .../v1.9.0/manifest.singleton.1.9.0.json | 12 +++++++++- .../TestData/ManifestV1_9-Singleton.yaml | 4 +++- .../ManifestV1_9-MultiFile-Installer.yaml | 5 +++- src/AppInstallerCLITests/YamlManifest.cpp | 16 +++++++++++++ .../Manifest/ManifestSchemaValidation.cpp | 3 ++- .../Manifest/ManifestValidation.cpp | 24 +++++++++++++++---- .../Manifest/ManifestYamlPopulator.cpp | 12 +++++++++- .../Manifest/YamlWriter.cpp | 6 +++-- .../Public/winget/ManifestInstaller.h | 4 +++- 10 files changed, 87 insertions(+), 15 deletions(-) diff --git a/schemas/JSON/manifests/v1.9.0/manifest.installer.1.9.0.json b/schemas/JSON/manifests/v1.9.0/manifest.installer.1.9.0.json index 609d1e40a9..a5b00edc5e 100644 --- a/schemas/JSON/manifests/v1.9.0/manifest.installer.1.9.0.json +++ b/schemas/JSON/manifests/v1.9.0/manifest.installer.1.9.0.json @@ -188,7 +188,7 @@ "maxLength": 2048, "description": "Custom switches will be passed directly to the installer by winget" }, - "Repair" : { + "Repair": { "type": [ "string", "null" ], "minLength": 1, "maxLength": 512, @@ -587,7 +587,7 @@ "type": [ "boolean", "null" ], "description": "Indicates whether the installer is prohibited from being downloaded for offline installation." }, - "RepairBehavior": { + "RepairBehavior": { "type": [ "string", "null" ], "enum": [ "modify", @@ -596,6 +596,10 @@ ], "description": "The repair method" }, + "ArchiveBinariesDependOnPath": { + "type": [ "boolean", "null" ], + "description": "Indicates whether the install location should be added directly to the PATH environment variable. Only applies to an archive containing portable packages." + }, "Installer": { "type": "object", "properties": { @@ -716,6 +720,9 @@ }, "RepairBehavior": { "$ref": "#/definitions/RepairBehavior" + }, + "ArchiveBinariesDependOnPath": { + "$ref": "#/definitions/ArchiveBinariesDependOnPath" } }, "required": [ @@ -835,6 +842,9 @@ "RepairBehavior": { "$ref": "#/definitions/RepairBehavior" }, + "ArchiveBinariesDependOnPath": { + "$ref": "#/definitions/ArchiveBinariesDependOnPath" + }, "Installers": { "type": "array", "items": { @@ -863,4 +873,4 @@ "ManifestType", "ManifestVersion" ] -} \ No newline at end of file +} diff --git a/schemas/JSON/manifests/v1.9.0/manifest.singleton.1.9.0.json b/schemas/JSON/manifests/v1.9.0/manifest.singleton.1.9.0.json index 428457fc6b..223ac8343d 100644 --- a/schemas/JSON/manifests/v1.9.0/manifest.singleton.1.9.0.json +++ b/schemas/JSON/manifests/v1.9.0/manifest.singleton.1.9.0.json @@ -695,6 +695,10 @@ ], "description": "The repair method" }, + "ArchiveBinariesDependOnPath": { + "type": [ "boolean", "null" ], + "description": "Indicates whether the install location should be added directly to the PATH environment variable. Only applies to an archive containing portable packages." + }, "Installer": { "type": "object", "properties": { @@ -815,6 +819,9 @@ }, "RepairBehavior": { "$ref": "#/definitions/RepairBehavior" + }, + "ArchiveBinariesDependOnPath": { + "$ref": "#/definitions/ArchiveBinariesDependOnPath" } }, "required": [ @@ -1057,6 +1064,9 @@ "RepairBehavior": { "$ref": "#/definitions/RepairBehavior" }, + "ArchiveBinariesDependOnPath": { + "$ref": "#/definitions/ArchiveBinariesDependOnPath" + }, "Installers": { "type": "array", "items": { @@ -1090,4 +1100,4 @@ "ManifestType", "ManifestVersion" ] -} \ No newline at end of file +} diff --git a/src/AppInstallerCLITests/TestData/ManifestV1_9-Singleton.yaml b/src/AppInstallerCLITests/TestData/ManifestV1_9-Singleton.yaml index 8d10796dea..5c3132ec01 100644 --- a/src/AppInstallerCLITests/TestData/ManifestV1_9-Singleton.yaml +++ b/src/AppInstallerCLITests/TestData/ManifestV1_9-Singleton.yaml @@ -124,6 +124,7 @@ InstallationMetadata: InvocationParameter: "/arg" DisplayName: "DisplayName" DownloadCommandProhibited: true +ArchiveBinariesDependOnPath: true Installers: - Architecture: x86 @@ -194,5 +195,6 @@ Installers: ReturnResponse: custom ReturnResponseUrl: https://defaultReturnResponseUrl.com DownloadCommandProhibited: false + ArchiveBinariesDependOnPath: false ManifestType: singleton -ManifestVersion: 1.9.0 \ No newline at end of file +ManifestVersion: 1.9.0 diff --git a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_9/ManifestV1_9-MultiFile-Installer.yaml b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_9/ManifestV1_9-MultiFile-Installer.yaml index aa57983ec0..3d88f9ec76 100644 --- a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_9/ManifestV1_9-MultiFile-Installer.yaml +++ b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_9/ManifestV1_9-MultiFile-Installer.yaml @@ -89,6 +89,7 @@ InstallationMetadata: InvocationParameter: "/arg" DisplayName: "DisplayName" DownloadCommandProhibited: true +ArchiveBinariesDependOnPath: true Installers: - Architecture: x86 @@ -159,6 +160,7 @@ Installers: UnsupportedArguments: - location DownloadCommandProhibited: false + ArchiveBinariesDependOnPath: false - Architecture: x64 InstallerType: exe InstallerUrl: https://www.microsoft.com/msixsdk/msixsdkx64.exe @@ -197,6 +199,7 @@ Installers: FileType: other InvocationParameter: "/arg2" DisplayName: "DisplayName2" + ArchiveBinariesDependOnPath: true - Architecture: x64 InstallerType: burn InstallerUrl: https://www.microsoft.com/msixsdk/msixsdkx64.exe @@ -205,4 +208,4 @@ Installers: UpgradeBehavior: deny RepairBehavior: modify ManifestType: installer -ManifestVersion: 1.9.0 \ No newline at end of file +ManifestVersion: 1.9.0 diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index 8731cfa4d4..4764a6980a 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -261,6 +261,11 @@ namespace REQUIRE(defaultSwitches.at(InstallerSwitchType::Repair) == "/repair"); REQUIRE(manifest.DefaultInstallerInfo.RepairBehavior == RepairBehaviorEnum::Modify); } + + if (manifestVer >= ManifestVer{ s_ManifestVersionV1_9 }) + { + REQUIRE(manifest.DefaultInstallerInfo.ArchiveBinariesDependOnPath); + } } if (isSingleton || isExported) @@ -375,6 +380,11 @@ namespace REQUIRE(installer1.RepairBehavior == RepairBehaviorEnum::Modify); } + if (manifestVer >= ManifestVer{ s_ManifestVersionV1_9 }) + { + REQUIRE_FALSE(installer1.ArchiveBinariesDependOnPath); + } + if (!isSingleton) { if (!isExported) @@ -467,6 +477,12 @@ namespace REQUIRE(installer5.Switches.at(InstallerSwitchType::Repair) == "/repair"); REQUIRE(installer5.RepairBehavior == RepairBehaviorEnum::Modify); } + + if (manifestVer >= ManifestVer{ s_ManifestVersionV1_9 }) + { + ManifestInstaller installer4 = manifest.Installers.at(3); + REQUIRE(installer4.ArchiveBinariesDependOnPath); + } } // Localization diff --git a/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp index 133969ad58..2f2d064402 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestSchemaValidation.cpp @@ -32,7 +32,8 @@ namespace AppInstaller::Manifest::YamlParser { "RequireExplicitUpgrade"sv, YamlScalarType::Bool }, { "DisplayInstallWarnings"sv, YamlScalarType::Bool }, { "InstallerReturnCode"sv, YamlScalarType::Int }, - { "DownloadCommandProhibited", YamlScalarType::Bool } + { "DownloadCommandProhibited", YamlScalarType::Bool }, + { "ArchiveBinariesDependOnPath", YamlScalarType::Bool } }; YamlScalarType GetManifestScalarValueType(const std::string& key) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp index 1ff1bceaca..31e91a12ce 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -63,7 +63,7 @@ namespace AppInstaller::Manifest { AppInstaller::Manifest::ManifestError::ArpValidationError, "Arp Validation Error."sv }, { AppInstaller::Manifest::ManifestError::SchemaError, "Schema Error."sv }, { AppInstaller::Manifest::ManifestError::MsixSignatureHashFailed, "Failed to calculate MSIX signature hash.Please verify that the input file is a valid, signed MSIX."sv }, - { AppInstaller::Manifest::ManifestError::ShadowManifestNotAllowed, "Shadow manifest is not allowed."} + { AppInstaller::Manifest::ManifestError::ShadowManifestNotAllowed, "Shadow manifest is not allowed." } }; return ErrorIdToMessageMap; @@ -258,9 +258,17 @@ namespace AppInstaller::Manifest { resultErrors.emplace_back(ManifestError::RequiredFieldMissing, "NestedInstallerFiles"); } - if (installer.NestedInstallerType != InstallerTypeEnum::Portable && installer.NestedInstallerFiles.size() != 1) + if (installer.NestedInstallerType != InstallerTypeEnum::Portable) { - resultErrors.emplace_back(ManifestError::ExceededNestedInstallerFilesLimit, "NestedInstallerFiles"); + if (installer.NestedInstallerFiles.size() != 1) + { + resultErrors.emplace_back(ManifestError::ExceededNestedInstallerFilesLimit, "NestedInstallerFiles"); + } + // ArchiveBinariesDependOnPath only applies to an archive containing portable packages. + if (installer.ArchiveBinariesDependOnPath) + { + resultErrors.emplace_back(ManifestError::FieldNotSupported, "ArchiveBinariesDependOnPath", ValidationError::Level::Warning); + } } std::set commandAliasSet; @@ -297,6 +305,14 @@ namespace AppInstaller::Manifest } } } + else + { + // ArchiveBinariesDependOnPath only applies to an archive containing portable packages. + if (installer.ArchiveBinariesDependOnPath) + { + resultErrors.emplace_back(ManifestError::FieldNotSupported, "ArchiveBinariesDependOnPath", ValidationError::Level::Warning); + } + } // Check empty string before calling IsValidUrl to avoid duplicate error reporting. if (!installer.Url.empty() && IsValidURL(NULL, Utility::ConvertToUTF16(installer.Url).c_str(), 0) == S_FALSE) @@ -414,4 +430,4 @@ namespace AppInstaller::Manifest return Utility::ConvertToUTF8(Message); } -} \ No newline at end of file +} diff --git a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp index b1a7ca6ae2..51b9c57e68 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp @@ -376,6 +376,16 @@ namespace AppInstaller::Manifest std::move(fields_v1_7.begin(), fields_v1_7.end(), std::inserter(result, result.end())); } + + if (m_manifestVersion.get() >= ManifestVer{ s_ManifestVersionV1_9 }) + { + std::vector fields_v1_9 = + { + { "ArchiveBinariesDependOnPath", [](const YAML::Node& value, const VariantManifestPtr& v)->ValidationErrors { GetManifestInstallerPtr(v)->ArchiveBinariesDependOnPath = value.as(); return {}; }, true }, + }; + + std::move(fields_v1_9.begin(), fields_v1_9.end(), std::inserter(result, result.end())); + } } return result; @@ -1243,4 +1253,4 @@ namespace AppInstaller::Manifest return errors; } -} \ No newline at end of file +} diff --git a/src/AppInstallerCommonCore/Manifest/YamlWriter.cpp b/src/AppInstallerCommonCore/Manifest/YamlWriter.cpp index 61910e13e9..92f23dcbc1 100644 --- a/src/AppInstallerCommonCore/Manifest/YamlWriter.cpp +++ b/src/AppInstallerCommonCore/Manifest/YamlWriter.cpp @@ -66,7 +66,8 @@ namespace AppInstaller::Manifest::YamlWriter constexpr std::string_view DisplayName = "DisplayName"sv; constexpr std::string_view MinimumOSVersion = "MinimumOSVersion"sv; constexpr std::string_view DownloadCommandProhibited = "DownloadCommandProhibited"sv; - constexpr std::string_view RepairBehavior = "RepairBehavior"sv; + constexpr std::string_view RepairBehavior = "RepairBehavior"sv; + constexpr std::string_view ArchiveBinariesDependOnPath = "ArchiveBinariesDependOnPath"sv; // Installer switches constexpr std::string_view InstallerSwitches = "InstallerSwitches"sv; @@ -568,7 +569,8 @@ namespace AppInstaller::Manifest::YamlWriter WRITE_BOOL_PROPERTY(out, InstallLocationRequired, installer.InstallLocationRequired); WRITE_BOOL_PROPERTY(out, RequireExplicitUpgrade, installer.RequireExplicitUpgrade); WRITE_BOOL_PROPERTY(out, DisplayInstallWarnings, installer.DisplayInstallWarnings); - WRITE_BOOL_PROPERTY(out, DownloadCommandProhibited, installer.DownloadCommandProhibited); + WRITE_BOOL_PROPERTY(out, DownloadCommandProhibited, installer.DownloadCommandProhibited); + WRITE_BOOL_PROPERTY(out, ArchiveBinariesDependOnPath, installer.ArchiveBinariesDependOnPath); WRITE_PROPERTY_IF_EXISTS(out, MinimumOSVersion, installer.MinOSVersion); WRITE_PROPERTY_IF_EXISTS(out, ProductCode, installer.ProductCode); WRITE_PROPERTY_IF_EXISTS(out, UpgradeBehavior, UpdateBehaviorToString(installer.UpdateBehavior)); diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h b/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h index c663271a76..e80d076f9c 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h @@ -115,5 +115,7 @@ namespace AppInstaller::Manifest InstallationMetadataInfo InstallationMetadata; bool DownloadCommandProhibited = false; + + bool ArchiveBinariesDependOnPath = false; }; -} \ No newline at end of file +} From 4208cafd9270fb20191af6bf65e5bc536aba295a Mon Sep 17 00:00:00 2001 From: --global Date: Mon, 16 Sep 2024 17:37:27 -0700 Subject: [PATCH 2/5] add tests --- src/AppInstallerCLICore/PortableInstaller.cpp | 11 ++++++++++- src/AppInstallerCLICore/PortableInstaller.h | 3 ++- .../Workflows/PortableFlow.cpp | 11 ++++++++--- .../Helpers/TestCommon.cs | 8 +++++--- src/AppInstallerCLIE2ETests/InstallCommand.cs | 19 +++++++++++++++++++ ...staller_Portable_BinariesDependOnPath.yaml | 19 +++++++++++++++++++ 6 files changed, 63 insertions(+), 8 deletions(-) create mode 100644 src/AppInstallerCLIE2ETests/TestData/Manifests/TestZipInstaller_Portable_BinariesDependOnPath.yaml diff --git a/src/AppInstallerCLICore/PortableInstaller.cpp b/src/AppInstallerCLICore/PortableInstaller.cpp index e80cfe5de7..810a04b3f8 100644 --- a/src/AppInstallerCLICore/PortableInstaller.cpp +++ b/src/AppInstallerCLICore/PortableInstaller.cpp @@ -121,7 +121,16 @@ namespace AppInstaller::CLI::Portable } else if (entry.FileType == PortableFileType::Symlink) { - if (!InstallDirectoryAddedToPath) + if (BinariesDependOnPath) + { + // Scenario indicated by 'ArchiveBinariesDependOnPath' manifest entry. + // Skip symlink creation for portables dependent on binaries that require the install directory to be added to PATH. + std::filesystem::path installDirectory = std::filesystem::path(entry.SymlinkTarget).parent_path(); + AddToPathVariable(installDirectory); + AICLI_LOG(Core, Info, << "Install directory added to PATH: " << installDirectory); + CommitToARPEntry(PortableValueName::InstallDirectoryAddedToPath, InstallDirectoryAddedToPath = true); + } + else if (!InstallDirectoryAddedToPath) { std::filesystem::file_status status = std::filesystem::status(filePath); if (std::filesystem::is_directory(status)) diff --git a/src/AppInstallerCLICore/PortableInstaller.h b/src/AppInstallerCLICore/PortableInstaller.h index 7ac1a951dd..6a611df7a3 100644 --- a/src/AppInstallerCLICore/PortableInstaller.h +++ b/src/AppInstallerCLICore/PortableInstaller.h @@ -32,6 +32,7 @@ namespace AppInstaller::CLI::Portable std::string WinGetPackageIdentifier; std::string WinGetSourceIdentifier; bool InstallDirectoryCreated = false; + bool BinariesDependOnPath = false; // If we fail to create a symlink, add install directory to PATH variable bool InstallDirectoryAddedToPath = false; @@ -112,4 +113,4 @@ namespace AppInstaller::CLI::Portable void AddToPathVariable(const std::filesystem::path& value); void RemoveFromPathVariable(const std::filesystem::path& value); }; -} \ No newline at end of file +} diff --git a/src/AppInstallerCLICore/Workflows/PortableFlow.cpp b/src/AppInstallerCLICore/Workflows/PortableFlow.cpp index a0cbe166b5..d2df70bd90 100644 --- a/src/AppInstallerCLICore/Workflows/PortableFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PortableFlow.cpp @@ -135,12 +135,18 @@ namespace AppInstaller::CLI::Workflow } } - Utility::Architecture arch = context.Get()->Arch; + const auto& installer = context.Get().value(); + Utility::Architecture arch = installer.Arch; const std::string& productCode = GetPortableProductCode(context); PortableInstaller portableInstaller = PortableInstaller(scope, arch, productCode); portableInstaller.IsUpdate = isUpdate; + if (IsArchiveType(installer.BaseInstallerType) && installer.ArchiveBinariesDependOnPath) + { + portableInstaller.BinariesDependOnPath = true; + } + // Set target install directory std::string_view locationArg = context.Args.GetArg(Execution::Args::Type::InstallLocation); std::filesystem::path targetInstallDirectory; @@ -239,7 +245,6 @@ namespace AppInstaller::CLI::Workflow void PortableInstallImpl(Execution::Context& context) { PortableInstaller& portableInstaller = context.Get(); - try { context.Reporter.Info() << Resource::String::InstallFlowStartingPackageInstall << std::endl; @@ -324,4 +329,4 @@ namespace AppInstaller::CLI::Workflow EnsureVolumeSupportsReparsePoints; } } -} \ No newline at end of file +} diff --git a/src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs b/src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs index 567e2f6953..bd45ff38d7 100644 --- a/src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs +++ b/src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs @@ -354,13 +354,15 @@ public static string GetCheckpointsDirectory() /// Product code. /// Should exists. /// Scope. + /// Install directory added to path instead of the symlink directory. public static void VerifyPortablePackage( string installDir, string commandAlias, string filename, string productCode, bool shouldExist, - Scope scope = Scope.User) + Scope scope = Scope.User, + bool installDirectoryAddedToPath = false) { // When portables are installed, if the exe path is inside a directory it will not be aliased // if the exe path is at the root level, it will be aliased. Therefore, if either exist, the exe exists @@ -387,7 +389,7 @@ public static void VerifyPortablePackage( { string pathName = "Path"; var currentPathValue = (string)environmentRegistryKey.GetValue(pathName); - var portablePathValue = symlinkDirectory + ';'; + var portablePathValue = (installDirectoryAddedToPath ? installDir : symlinkDirectory) + ';'; isAddedToPath = currentPathValue.Contains(portablePathValue); } @@ -399,7 +401,7 @@ public static void VerifyPortablePackage( Assert.AreEqual(shouldExist, exeExists, $"Expected portable exe path: {exePath}"); Assert.AreEqual(shouldExist, symlinkExists, $"Expected portable symlink path: {symlinkPath}"); Assert.AreEqual(shouldExist, portableEntryExists, $"Expected {productCode} subkey in path: {uninstallSubKey}"); - Assert.AreEqual(shouldExist, isAddedToPath, $"Expected path variable: {symlinkDirectory}"); + Assert.AreEqual(shouldExist, isAddedToPath, $"Expected path variable: {(installDirectoryAddedToPath ? installDir : symlinkDirectory)}"); } /// diff --git a/src/AppInstallerCLIE2ETests/InstallCommand.cs b/src/AppInstallerCLIE2ETests/InstallCommand.cs index 54efdef1c8..ef6932a1e4 100644 --- a/src/AppInstallerCLIE2ETests/InstallCommand.cs +++ b/src/AppInstallerCLIE2ETests/InstallCommand.cs @@ -516,6 +516,25 @@ public void InstallZip_Portable() TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, true, TestCommon.Scope.User); } + /// + /// Test install zip portable with binaries that depend on PATH variable. + /// + [Test] + public void InstallZip_PortableWithBinariesDependentOnPath() + { + string installDir = TestCommon.GetPortablePackagesDirectory(); + string packageId, commandAlias, fileName, packageDirName, productCode; + packageId = "AppInstallerTest.ArchiveBinariesDependOnPath"; + packageDirName = productCode = packageId + "_" + Constants.TestSourceIdentifier; + commandAlias = "TestPortable.exe"; + fileName = "AppInstallerTestExeInstaller.exe"; + + var result = TestCommon.RunAICLICommand("install", $"{packageId}"); + Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); + Assert.True(result.StdOut.Contains("Successfully installed")); + TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, true, TestCommon.Scope.User, true); + } + /// /// Test install zip with invalid relative file path. /// diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestZipInstaller_Portable_BinariesDependOnPath.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestZipInstaller_Portable_BinariesDependOnPath.yaml new file mode 100644 index 0000000000..2ce51611f7 --- /dev/null +++ b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestZipInstaller_Portable_BinariesDependOnPath.yaml @@ -0,0 +1,19 @@ +PackageIdentifier: AppInstallerTest.ArchiveBinariesDependOnPath +PackageVersion: 1.0.0.0 +PackageName: TestZipInstallerWithPortable +PackageLocale: en-US +Publisher: AppInstallerTest +License: Test +ShortDescription: E2E test for installing a zip with portable with binaries that depend on path variable. +Installers: + - Architecture: x64 + InstallerUrl: https://localhost:5001/TestKit/AppInstallerTestZipInstaller/AppInstallerTestZipInstaller.zip + InstallerType: zip + InstallerSha256: + NestedInstallerType: portable + NestedInstallerFiles: + - RelativeFilePath: AppInstallerTestExeInstaller.exe + PortableCommandAlias: TestPortable + ArchiveBinariesDependOnPath: true +ManifestType: singleton +ManifestVersion: 1.9.0 From 908e4df6f18d85c478a350720fe2cab15788d686 Mon Sep 17 00:00:00 2001 From: --global Date: Tue, 17 Sep 2024 10:58:55 -0700 Subject: [PATCH 3/5] fix test --- src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs | 2 +- src/AppInstallerCLIE2ETests/InstallCommand.cs | 4 ++-- .../TestZipInstaller_Portable_BinariesDependOnPath.yaml | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs b/src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs index bd45ff38d7..2b314c0e35 100644 --- a/src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs +++ b/src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs @@ -399,7 +399,7 @@ public static void VerifyPortablePackage( } Assert.AreEqual(shouldExist, exeExists, $"Expected portable exe path: {exePath}"); - Assert.AreEqual(shouldExist, symlinkExists, $"Expected portable symlink path: {symlinkPath}"); + Assert.AreEqual(shouldExist && !installDirectoryAddedToPath, symlinkExists, $"Expected portable symlink path: {symlinkPath}"); Assert.AreEqual(shouldExist, portableEntryExists, $"Expected {productCode} subkey in path: {uninstallSubKey}"); Assert.AreEqual(shouldExist, isAddedToPath, $"Expected path variable: {(installDirectoryAddedToPath ? installDir : symlinkDirectory)}"); } diff --git a/src/AppInstallerCLIE2ETests/InstallCommand.cs b/src/AppInstallerCLIE2ETests/InstallCommand.cs index ef6932a1e4..8340407fe4 100644 --- a/src/AppInstallerCLIE2ETests/InstallCommand.cs +++ b/src/AppInstallerCLIE2ETests/InstallCommand.cs @@ -520,11 +520,11 @@ public void InstallZip_Portable() /// Test install zip portable with binaries that depend on PATH variable. /// [Test] - public void InstallZip_PortableWithBinariesDependentOnPath() + public void InstallZip_ArchivePortableWithBinariesDependentOnPath() { string installDir = TestCommon.GetPortablePackagesDirectory(); string packageId, commandAlias, fileName, packageDirName, productCode; - packageId = "AppInstallerTest.ArchiveBinariesDependOnPath"; + packageId = "AppInstallerTest.ArchivePortableWithBinariesDependentOnPath"; packageDirName = productCode = packageId + "_" + Constants.TestSourceIdentifier; commandAlias = "TestPortable.exe"; fileName = "AppInstallerTestExeInstaller.exe"; diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestZipInstaller_Portable_BinariesDependOnPath.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestZipInstaller_Portable_BinariesDependOnPath.yaml index 2ce51611f7..7707a08a8d 100644 --- a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestZipInstaller_Portable_BinariesDependOnPath.yaml +++ b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestZipInstaller_Portable_BinariesDependOnPath.yaml @@ -1,10 +1,10 @@ -PackageIdentifier: AppInstallerTest.ArchiveBinariesDependOnPath +PackageIdentifier: AppInstallerTest.ArchivePortableWithBinariesDependentOnPath PackageVersion: 1.0.0.0 -PackageName: TestZipInstallerWithPortable +PackageName: TestArchivePortableWithBinariesDependentOnPath PackageLocale: en-US Publisher: AppInstallerTest License: Test -ShortDescription: E2E test for installing a zip with portable with binaries that depend on path variable. +ShortDescription: E2E test for installing a zip containing a portable with binaries that depend on the PATH variable. Installers: - Architecture: x64 InstallerUrl: https://localhost:5001/TestKit/AppInstallerTestZipInstaller/AppInstallerTestZipInstaller.zip From 60fac55b8de9f9685d83c4fb7ed7ab565a30f800 Mon Sep 17 00:00:00 2001 From: --global Date: Tue, 17 Sep 2024 11:05:52 -0700 Subject: [PATCH 4/5] fix if statement --- src/AppInstallerCLICore/PortableInstaller.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCLICore/PortableInstaller.cpp b/src/AppInstallerCLICore/PortableInstaller.cpp index 810a04b3f8..8371e04be0 100644 --- a/src/AppInstallerCLICore/PortableInstaller.cpp +++ b/src/AppInstallerCLICore/PortableInstaller.cpp @@ -121,7 +121,7 @@ namespace AppInstaller::CLI::Portable } else if (entry.FileType == PortableFileType::Symlink) { - if (BinariesDependOnPath) + if (BinariesDependOnPath && !InstallDirectoryAddedToPath) { // Scenario indicated by 'ArchiveBinariesDependOnPath' manifest entry. // Skip symlink creation for portables dependent on binaries that require the install directory to be added to PATH. From 2ddd8dcb3eb98805ee5ed5ea0fa50c08b5883476 Mon Sep 17 00:00:00 2001 From: --global Date: Wed, 18 Sep 2024 15:29:00 -0700 Subject: [PATCH 5/5] fix validation --- .../Manifest/ManifestValidation.cpp | 20 ++----------------- .../Manifest/ManifestYamlPopulator.cpp | 2 +- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp index 31e91a12ce..4b1da4fb92 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -258,17 +258,9 @@ namespace AppInstaller::Manifest { resultErrors.emplace_back(ManifestError::RequiredFieldMissing, "NestedInstallerFiles"); } - if (installer.NestedInstallerType != InstallerTypeEnum::Portable) + if (installer.NestedInstallerType != InstallerTypeEnum::Portable && installer.NestedInstallerFiles.size() != 1) { - if (installer.NestedInstallerFiles.size() != 1) - { - resultErrors.emplace_back(ManifestError::ExceededNestedInstallerFilesLimit, "NestedInstallerFiles"); - } - // ArchiveBinariesDependOnPath only applies to an archive containing portable packages. - if (installer.ArchiveBinariesDependOnPath) - { - resultErrors.emplace_back(ManifestError::FieldNotSupported, "ArchiveBinariesDependOnPath", ValidationError::Level::Warning); - } + resultErrors.emplace_back(ManifestError::ExceededNestedInstallerFilesLimit, "NestedInstallerFiles"); } std::set commandAliasSet; @@ -305,14 +297,6 @@ namespace AppInstaller::Manifest } } } - else - { - // ArchiveBinariesDependOnPath only applies to an archive containing portable packages. - if (installer.ArchiveBinariesDependOnPath) - { - resultErrors.emplace_back(ManifestError::FieldNotSupported, "ArchiveBinariesDependOnPath", ValidationError::Level::Warning); - } - } // Check empty string before calling IsValidUrl to avoid duplicate error reporting. if (!installer.Url.empty() && IsValidURL(NULL, Utility::ConvertToUTF16(installer.Url).c_str(), 0) == S_FALSE) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp index 51b9c57e68..51379004da 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp @@ -381,7 +381,7 @@ namespace AppInstaller::Manifest { std::vector fields_v1_9 = { - { "ArchiveBinariesDependOnPath", [](const YAML::Node& value, const VariantManifestPtr& v)->ValidationErrors { GetManifestInstallerPtr(v)->ArchiveBinariesDependOnPath = value.as(); return {}; }, true }, + { "ArchiveBinariesDependOnPath", [](const YAML::Node& value, const VariantManifestPtr& v)->ValidationErrors { GetManifestInstallerPtr(v)->ArchiveBinariesDependOnPath = value.as(); return {}; } }, }; std::move(fields_v1_9.begin(), fields_v1_9.end(), std::inserter(result, result.end()));