From 08543c671bbae74f476ebbdc023bac993db0b7b8 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Tue, 19 Dec 2023 17:12:59 -0800 Subject: [PATCH 01/14] Add YAML manifest entries for winget repair behavior Manifest Entries includes: - The Repair switch, within InstallerSwitch, allows for custom repair of a package. - The RepairBehavior enum field, present in both the Installer and root nodes, determines the repair behavior for the source package used to repair a package. --- .../v1.7.0/manifest.installer.1.7.0.json | 25 +++++++++++++++++-- .../v1.7.0/manifest.singleton.1.7.0.json | 25 +++++++++++++++++-- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json b/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json index 27bb1caf3f..dd73833915 100644 --- a/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json +++ b/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json @@ -187,6 +187,12 @@ "minLength": 1, "maxLength": 2048, "description": "Custom switches will be passed directly to the installer by winget" + }, + "Repair" : { + "type": [ "string", "null" ], + "minLength": 1, + "maxLength": 512, + "description": "The ‘Repair’ value must be passed to the installer, ModifyPath ARP command, or uninstaller ARP command when the user opts for a repair." } } }, @@ -578,8 +584,17 @@ "description": "Details about the installation. Used for deeper installation detection." }, "DownloadCommandProhibited": { - "type": [ "boolean", "null" ], - "description": "Indicates whether the installer is prohibited from being downloaded for offline installation." + "type": [ "boolean", "null" ], + "description": "Indicates whether the installer is prohibited from being downloaded for offline installation." + }, + "RepairBehavior": { + "type": [ "string", "null" ], + "enum": [ + "modify", + "uninstaller", + "installer" + ], + "description": "The repair method" }, "Installer": { "type": "object", @@ -698,6 +713,9 @@ }, "DownloadCommandProhibited": { "$ref": "#/definitions/DownloadCommandProhibited" + }, + "RepairBehavior": { + "$ref": "#/definitions/RepairBehavior" } }, "required": [ @@ -814,6 +832,9 @@ "DownloadCommandProhibited": { "$ref": "#/definitions/DownloadCommandProhibited" }, + "RepairBehavior": { + "$ref": "#/definitions/RepairBehavior" + }, "Installers": { "type": "array", "items": { diff --git a/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json b/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json index 9e1b25fe0b..d2ad0f7c67 100644 --- a/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json +++ b/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json @@ -287,6 +287,12 @@ "minLength": 1, "maxLength": 2048, "description": "Custom switches will be passed directly to the installer by winget" + }, + "Repair": { + "type": [ "string", "null" ], + "minLength": 1, + "maxLength": 512, + "description": "The ‘Repair’ value must be passed to the installer, ModifyPath ARP command, or uninstaller ARP command when the user opts for a repair" } } }, @@ -677,8 +683,17 @@ "description": "Details about the installation. Used for deeper installation detection." }, "DownloadCommandProhibited": { - "type": [ "boolean", "null" ], - "description": "Indicates whether the installer is prohibited from being downloaded for offline installation." + "type": [ "boolean", "null" ], + "description": "Indicates whether the installer is prohibited from being downloaded for offline installation." + }, + "RepairBehavior": { + "type": [ "string", "null" ], + "enum": [ + "modify", + "uninstaller", + "installer" + ], + "description": "The repair method" }, "Installer": { "type": "object", @@ -797,6 +812,9 @@ }, "DownloadCommandProhibited": { "$ref": "#/definitions/DownloadCommandProhibited" + }, + "RepairBehavior": { + "$ref": "#/definitions/RepairBehavior" } }, "required": [ @@ -1036,6 +1054,9 @@ "DownloadCommandProhibited": { "$ref": "#/definitions/DownloadCommandProhibited" }, + "RepairBehavior": { + "$ref": "#/definitions/RepairBehavior" + }, "Installers": { "type": "array", "items": { From a412c2009a3d8f8cf8203f3caedb2046c1a75e7c Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Tue, 19 Dec 2023 17:29:48 -0800 Subject: [PATCH 02/14] Parse the repair manifest entries and populate them according to the v1_7 manifest schema. --- .../Manifest/ManifestCommon.cpp | 20 +++++++++++++++++++ .../Manifest/ManifestYamlPopulator.cpp | 13 ++++++++++++ .../Public/winget/ManifestCommon.h | 11 ++++++++++ .../Public/winget/ManifestInstaller.h | 2 ++ 4 files changed, 46 insertions(+) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp index 68206ffe28..d545e0ee28 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp @@ -962,6 +962,26 @@ namespace AppInstaller::Manifest } } + RepairBehaviorEnum ConvertToRepairBehaviorEnum(const std::string& in) + { + RepairBehaviorEnum result = RepairBehaviorEnum::Unknown; + + if (Utility::CaseInsensitiveEquals(in, "install")) + { + result = RepairBehaviorEnum::Install; + } + else if (Utility::CaseInsensitiveEquals(in, "uninstall")) + { + result = RepairBehaviorEnum::Uninstall; + } + else if (Utility::CaseInsensitiveEquals(in, "modify")) + { + result = RepairBehaviorEnum::Modify; + } + + return result; + } + std::map GetDefaultKnownReturnCodes(InstallerTypeEnum installerType) { switch (installerType) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp index c3dad85c2f..cdb1ba98f3 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp @@ -329,6 +329,14 @@ namespace AppInstaller::Manifest std::move(fields_v1_6.begin(), fields_v1_6.end(), std::inserter(result, result.end())); } + + if (manifestVersion >= ManifestVer{ s_ManifestVersionV1_7 }) + { + std::vector fields_v1_7 = + { + { "RepairBehavior", [this](const YAML::Node& value)->ValidationErrors { m_p_installer->RepairBehavior = ConvertToRepairBehaviorEnum(value.as()); return {}; } }, + }; + } } return result; @@ -359,6 +367,11 @@ namespace AppInstaller::Manifest result.emplace_back("Upgrade", [this](const YAML::Node& value)->ValidationErrors { (*m_p_switches)[InstallerSwitchType::Update] = value.as(); return{}; }); } + if (manifestVersion >= ManifestVer{ s_ManifestVersionV1_7 }) + { + result.emplace_back("Repair", [this](const YAML::Node& value)->ValidationErrors { (*m_p_switches)[InstallerSwitchType::Repair] = value.as(); return{}; }); + }; + return result; } diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index 6e8212f7f2..10cb2d9f06 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -112,6 +112,15 @@ namespace AppInstaller::Manifest Log, InstallLocation, Update, + Repair, + }; + + enum class RepairBehaviorEnum + { + Unknown, + Modify, + Install, + Uninstall, }; enum class ScopeEnum @@ -370,6 +379,8 @@ namespace AppInstaller::Manifest IconResolutionEnum ConvertToIconResolutionEnum(std::string_view in); + RepairBehaviorEnum ConvertToRepairBehaviorEnum(const std::string& in); + std::string_view InstallerTypeToString(InstallerTypeEnum installerType); std::string_view InstallerSwitchTypeToString(InstallerSwitchType installerSwitchType); diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h b/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h index f8b2eb3ad0..efaad4b0c8 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h @@ -70,6 +70,8 @@ namespace AppInstaller::Manifest UpdateBehaviorEnum UpdateBehavior = UpdateBehaviorEnum::Install; + RepairBehaviorEnum RepairBehavior = RepairBehaviorEnum::Unknown; // TODO: change to RepairBehaviorEnum::Modify ? + std::vector Commands; std::vector Protocols; From 74eed30f8e4a5fa826c5598402bf045483458513 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Tue, 2 Jan 2024 17:51:31 -0800 Subject: [PATCH 03/14] Fix manifest parsing by adding RepairBehavior field to schema 1.7 --- src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp index cdb1ba98f3..2f0d5be45e 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp @@ -336,6 +336,8 @@ namespace AppInstaller::Manifest { { "RepairBehavior", [this](const YAML::Node& value)->ValidationErrors { m_p_installer->RepairBehavior = ConvertToRepairBehaviorEnum(value.as()); return {}; } }, }; + + std::move(fields_v1_7.begin(), fields_v1_7.end(), std::inserter(result, result.end())); } } From 1488788589013988e319849f6e8765a67635c8f7 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Tue, 2 Jan 2024 18:45:21 -0800 Subject: [PATCH 04/14] Align RepairBehaviorEnum values with manifest schema specifications --- src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp | 8 ++++---- src/AppInstallerCommonCore/Public/winget/ManifestCommon.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp index d545e0ee28..3b6102e331 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp @@ -966,13 +966,13 @@ namespace AppInstaller::Manifest { RepairBehaviorEnum result = RepairBehaviorEnum::Unknown; - if (Utility::CaseInsensitiveEquals(in, "install")) + if (Utility::CaseInsensitiveEquals(in, "installer")) { - result = RepairBehaviorEnum::Install; + result = RepairBehaviorEnum::Installer; } - else if (Utility::CaseInsensitiveEquals(in, "uninstall")) + else if (Utility::CaseInsensitiveEquals(in, "uninstaller")) { - result = RepairBehaviorEnum::Uninstall; + result = RepairBehaviorEnum::Uninstaller; } else if (Utility::CaseInsensitiveEquals(in, "modify")) { diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index 10cb2d9f06..9e7d551f68 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -119,8 +119,8 @@ namespace AppInstaller::Manifest { Unknown, Modify, - Install, - Uninstall, + Installer, + Uninstaller, }; enum class ScopeEnum From 5ac8de3bc8cf3bf5b344494aec96254d1f6b7cb4 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Tue, 2 Jan 2024 19:10:29 -0800 Subject: [PATCH 05/14] [Tests:] Add tests for manifest entries Repair installerswitch and RepairBehavior --- .../TestData/ManifestV1_7-Singleton.yaml | 2 ++ .../ManifestV1_7-MultiFile-Installer.yaml | 3 +++ src/AppInstallerCLITests/YamlManifest.cpp | 15 +++++++++++++-- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml b/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml index fbc5b5c915..8072036ccb 100644 --- a/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml +++ b/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml @@ -54,10 +54,12 @@ InstallerSwitches: Log: /log= InstallLocation: /dir= Upgrade: /upgrade + Repair: /repair InstallerSuccessCodes: - 1 - 0x80070005 UpgradeBehavior: uninstallPrevious +RepairBehavior: modify Commands: - makemsix - makeappx diff --git a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml index 5ce606b274..da643afbc4 100644 --- a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml +++ b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml @@ -19,10 +19,12 @@ InstallerSwitches: Log: /log= InstallLocation: /dir= Upgrade: /upgrade + Repair: /repair InstallerSuccessCodes: - 1 - 0x80070005 UpgradeBehavior: uninstallPrevious +RepairBehavior: modify Commands: - makemsix - makeappx @@ -162,6 +164,7 @@ Installers: InstallerSha256: 69D84CA8899800A5575CE31798293CD4FEBAB1D734A07C2E51E56A28E0DF8C82 ProductCode: "{Bar}" UpgradeBehavior: deny + RepairBehavior: uninstaller - Architecture: x86 InstallerType: portable InstallerUrl: https://www.microsoft.com/msixsdk/msixsdkx86.exe diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index 8a27fc3758..aa4e7c8cfc 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -527,6 +527,12 @@ void VerifyV1ManifestContent(const Manifest& manifest, bool isSingleton, Manifes { REQUIRE(manifest.DefaultInstallerInfo.DownloadCommandProhibited); } + + if (manifestVer >= ManifestVer{ s_ManifestVersionV1_7 }) + { + REQUIRE(manifest.DefaultInstallerInfo.RepairBehavior == RepairBehaviorEnum::Modify); + REQUIRE(defaultSwitches.at(InstallerSwitchType::Repair) == "/repair"); + } } if (isSingleton || isExported) @@ -708,6 +714,11 @@ void VerifyV1ManifestContent(const Manifest& manifest, bool isSingleton, Manifes REQUIRE(installer2.DownloadCommandProhibited); REQUIRE(installer2.UpdateBehavior == UpdateBehaviorEnum::Deny); } + + if (manifestVer >= ManifestVer{ s_ManifestVersionV1_7 }) + { + REQUIRE(installer2.RepairBehavior == RepairBehaviorEnum::Uninstaller); + } } // Localization @@ -792,7 +803,7 @@ TEST_CASE("ValidateV1_1GoodManifestAndVerifyContents", "[ManifestValidation]") TempDirectory singletonDirectory{ "SingletonManifest" }; CopyTestDataFilesToFolder({ "ManifestV1_1-Singleton.yaml" }, singletonDirectory); Manifest singletonManifest = YamlParser::CreateFromPath(singletonDirectory, validateOption); - VerifyV1ManifestContent(singletonManifest, true, ManifestVer{s_ManifestVersionV1_1}); + VerifyV1ManifestContent(singletonManifest, true, ManifestVer{ s_ManifestVersionV1_1 }); TempDirectory multiFileDirectory{ "MultiFileManifest" }; CopyTestDataFilesToFolder({ @@ -1469,7 +1480,7 @@ TEST_CASE("ManifestArpVersionRange", "[ManifestValidation]") { Manifest manifestNoArp = YamlParser::CreateFromPath(TestDataFile("Manifest-Good-NoArpVersionDeclared.yaml")); REQUIRE(manifestNoArp.GetArpVersionRange().IsEmpty()); - + Manifest manifestSingleArp = YamlParser::CreateFromPath(TestDataFile("Manifest-Good-SingleArpVersionDeclared.yaml")); auto arpRangeSingleArp = manifestSingleArp.GetArpVersionRange(); REQUIRE(arpRangeSingleArp.GetMinVersion().ToString() == "11.0"); From 5bc2fcf08a52df8da112737fcb8f16788f628df2 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Mon, 8 Jan 2024 10:26:32 -0800 Subject: [PATCH 06/14] Update RepairBehavior field scope to apply to Installers object, ensuring only supported installers can specify repair behavior. [Why] This change is to prevent incorrect mapping for unsupported installers by disallowing RepairBehavior field at root level --- schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json | 3 --- schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json | 3 --- 2 files changed, 6 deletions(-) diff --git a/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json b/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json index dd73833915..f640f9fe44 100644 --- a/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json +++ b/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json @@ -832,9 +832,6 @@ "DownloadCommandProhibited": { "$ref": "#/definitions/DownloadCommandProhibited" }, - "RepairBehavior": { - "$ref": "#/definitions/RepairBehavior" - }, "Installers": { "type": "array", "items": { diff --git a/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json b/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json index d2ad0f7c67..14cdf62bcb 100644 --- a/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json +++ b/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json @@ -1054,9 +1054,6 @@ "DownloadCommandProhibited": { "$ref": "#/definitions/DownloadCommandProhibited" }, - "RepairBehavior": { - "$ref": "#/definitions/RepairBehavior" - }, "Installers": { "type": "array", "items": { From e059aee1b651d61ad9faa8c26df621439ae570af Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Mon, 8 Jan 2024 10:38:57 -0800 Subject: [PATCH 07/14] Modify V1.7 manifest validation tests to align RepairBehavior field scope with Installer Object. --- .../TestData/ManifestV1_7-Singleton.yaml | 2 -- .../ManifestV1_7-MultiFile-Installer.yaml | 10 ++++++++- src/AppInstallerCLITests/YamlManifest.cpp | 22 ++++++++++++++++--- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml b/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml index 8072036ccb..fbc5b5c915 100644 --- a/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml +++ b/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml @@ -54,12 +54,10 @@ InstallerSwitches: Log: /log= InstallLocation: /dir= Upgrade: /upgrade - Repair: /repair InstallerSuccessCodes: - 1 - 0x80070005 UpgradeBehavior: uninstallPrevious -RepairBehavior: modify Commands: - makemsix - makeappx diff --git a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml index da643afbc4..d0872853a5 100644 --- a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml +++ b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml @@ -24,7 +24,6 @@ InstallerSuccessCodes: - 1 - 0x80070005 UpgradeBehavior: uninstallPrevious -RepairBehavior: modify Commands: - makemsix - makeappx @@ -163,6 +162,8 @@ Installers: InstallerUrl: https://www.microsoft.com/msixsdk/msixsdkx64.exe InstallerSha256: 69D84CA8899800A5575CE31798293CD4FEBAB1D734A07C2E51E56A28E0DF8C82 ProductCode: "{Bar}" + InstallerSwitches: + Repair: /r UpgradeBehavior: deny RepairBehavior: uninstaller - Architecture: x86 @@ -194,5 +195,12 @@ Installers: FileType: other InvocationParameter: "/arg2" DisplayName: "DisplayName2" + - Architecture: x64 + InstallerType: burn + InstallerUrl: https://www.microsoft.com/msixsdk/msixsdkx64.exe + InstallerSha256: 69D84CA8899800A5575CE31798293CD4FEBAB1D734A07C2E51E56A28E0DF8C82 + ProductCode: "{Bar}" + UpgradeBehavior: deny + RepairBehavior: modify ManifestType: installer ManifestVersion: 1.7.0 \ No newline at end of file diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index aa4e7c8cfc..b7163a0e8f 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -530,8 +530,10 @@ void VerifyV1ManifestContent(const Manifest& manifest, bool isSingleton, Manifes if (manifestVer >= ManifestVer{ s_ManifestVersionV1_7 }) { - REQUIRE(manifest.DefaultInstallerInfo.RepairBehavior == RepairBehaviorEnum::Modify); - REQUIRE(defaultSwitches.at(InstallerSwitchType::Repair) == "/repair"); + if (!isSingleton) + { + REQUIRE(defaultSwitches.at(InstallerSwitchType::Repair) == "/repair"); + } } } @@ -541,7 +543,11 @@ void VerifyV1ManifestContent(const Manifest& manifest, bool isSingleton, Manifes } else { - if (manifestVer >= ManifestVer{ s_ManifestVersionV1_4 }) + if (manifestVer >= ManifestVer{ s_ManifestVersionV1_7 }) + { + REQUIRE(manifest.Installers.size() == 5); + } + else if (manifestVer >= ManifestVer{ s_ManifestVersionV1_4 }) { REQUIRE(manifest.Installers.size() == 4); } @@ -718,6 +724,16 @@ void VerifyV1ManifestContent(const Manifest& manifest, bool isSingleton, Manifes if (manifestVer >= ManifestVer{ s_ManifestVersionV1_7 }) { REQUIRE(installer2.RepairBehavior == RepairBehaviorEnum::Uninstaller); + REQUIRE(installer2.Switches.at(InstallerSwitchType::Repair) == "/r"); + + ManifestInstaller installer5 = manifest.Installers.at(4); + REQUIRE(installer5.BaseInstallerType == InstallerTypeEnum::Burn); + REQUIRE(installer5.Arch == Architecture::X64); + REQUIRE(installer5.Url == "https://www.microsoft.com/msixsdk/msixsdkx64.exe"); + REQUIRE(installer5.Sha256 == SHA256::ConvertToBytes("69D84CA8899800A5575CE31798293CD4FEBAB1D734A07C2E51E56A28E0DF8C82")); + REQUIRE(installer5.ProductCode == "{Bar}"); + REQUIRE(installer5.Switches.at(InstallerSwitchType::Repair) == "/repair"); + REQUIRE(installer5.RepairBehavior == RepairBehaviorEnum::Modify); } } From 6fc3b06030cc229db7d7992646c5c04d0e62626a Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Wed, 10 Jan 2024 15:42:11 -0800 Subject: [PATCH 08/14] Review comment fix: Remove inline TODO comment --- src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h b/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h index efaad4b0c8..c663271a76 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestInstaller.h @@ -70,7 +70,7 @@ namespace AppInstaller::Manifest UpdateBehaviorEnum UpdateBehavior = UpdateBehaviorEnum::Install; - RepairBehaviorEnum RepairBehavior = RepairBehaviorEnum::Unknown; // TODO: change to RepairBehaviorEnum::Modify ? + RepairBehaviorEnum RepairBehavior = RepairBehaviorEnum::Unknown; std::vector Commands; From dfa4595c051b5d406ca25d3159a7f3ac7b7f8c27 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Thu, 11 Jan 2024 10:08:41 -0800 Subject: [PATCH 09/14] Add RepairBehavior field support to YAML manifest root level to align with the manifest design. This commit address two spacing alignment issues in manifest through this commit. --- .../JSON/manifests/v1.7.0/manifest.installer.1.7.0.json | 7 +++++-- .../JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json b/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json index f640f9fe44..08cb9925ce 100644 --- a/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json +++ b/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json @@ -584,8 +584,8 @@ "description": "Details about the installation. Used for deeper installation detection." }, "DownloadCommandProhibited": { - "type": [ "boolean", "null" ], - "description": "Indicates whether the installer is prohibited from being downloaded for offline installation." + "type": [ "boolean", "null" ], + "description": "Indicates whether the installer is prohibited from being downloaded for offline installation." }, "RepairBehavior": { "type": [ "string", "null" ], @@ -832,6 +832,9 @@ "DownloadCommandProhibited": { "$ref": "#/definitions/DownloadCommandProhibited" }, + "RepairBehavior": { + "$ref": "#/definitions/RepairBehavior" + }, "Installers": { "type": "array", "items": { diff --git a/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json b/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json index 14cdf62bcb..c5a4651c7d 100644 --- a/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json +++ b/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json @@ -683,8 +683,8 @@ "description": "Details about the installation. Used for deeper installation detection." }, "DownloadCommandProhibited": { - "type": [ "boolean", "null" ], - "description": "Indicates whether the installer is prohibited from being downloaded for offline installation." + "type": [ "boolean", "null" ], + "description": "Indicates whether the installer is prohibited from being downloaded for offline installation." }, "RepairBehavior": { "type": [ "string", "null" ], @@ -1054,6 +1054,9 @@ "DownloadCommandProhibited": { "$ref": "#/definitions/DownloadCommandProhibited" }, + "RepairBehavior": { + "$ref": "#/definitions/RepairBehavior" + }, "Installers": { "type": "array", "items": { From 42a4ced626211216482e6c562f76f978f874d6f1 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Thu, 11 Jan 2024 10:58:44 -0800 Subject: [PATCH 10/14] =?UTF-8?q?Update=20ConvertToRepairBehaviorEnum=20to?= =?UTF-8?q?=20utilize=20string=5Fview=20and=20move=20=C2=91Repair=C2=92=20?= =?UTF-8?q?InstallerSwitch=20manifest=20read/parsing=20logic=20within=20ma?= =?UTF-8?q?nifest=20major=20version=201?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp | 9 +++++---- .../Manifest/ManifestYamlPopulator.cpp | 10 +++++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp index 3b6102e331..b774bece1c 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp @@ -962,19 +962,20 @@ namespace AppInstaller::Manifest } } - RepairBehaviorEnum ConvertToRepairBehaviorEnum(const std::string& in) + RepairBehaviorEnum ConvertToRepairBehaviorEnum(std::string_view in) { + std::string inStrLower = Utility::ToLower(in); RepairBehaviorEnum result = RepairBehaviorEnum::Unknown; - if (Utility::CaseInsensitiveEquals(in, "installer")) + if (inStrLower == "installer") { result = RepairBehaviorEnum::Installer; } - else if (Utility::CaseInsensitiveEquals(in, "uninstaller")) + else if (inStrLower == "uninstaller") { result = RepairBehaviorEnum::Uninstaller; } - else if (Utility::CaseInsensitiveEquals(in, "modify")) + else if (inStrLower == "modify") { result = RepairBehaviorEnum::Modify; } diff --git a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp index 2f0d5be45e..9b829b9c2c 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp @@ -367,12 +367,12 @@ namespace AppInstaller::Manifest else if (manifestVersion.Major() == 1) { result.emplace_back("Upgrade", [this](const YAML::Node& value)->ValidationErrors { (*m_p_switches)[InstallerSwitchType::Update] = value.as(); return{}; }); - } - if (manifestVersion >= ManifestVer{ s_ManifestVersionV1_7 }) - { - result.emplace_back("Repair", [this](const YAML::Node& value)->ValidationErrors { (*m_p_switches)[InstallerSwitchType::Repair] = value.as(); return{}; }); - }; + if (manifestVersion >= ManifestVer{ s_ManifestVersionV1_7 }) + { + result.emplace_back("Repair", [this](const YAML::Node& value)->ValidationErrors { (*m_p_switches)[InstallerSwitchType::Repair] = value.as(); return{}; }); + }; + } return result; } From 01c93870e65acedd85bbbb13390139d737dee4c0 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Thu, 11 Jan 2024 11:19:50 -0800 Subject: [PATCH 11/14] Resolve missed header file declaration update for ConvertToRepairBehaviorEnum to utilize string_view from last commit. --- src/AppInstallerCommonCore/Public/winget/ManifestCommon.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index 9e7d551f68..8e83cb0f99 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -379,7 +379,7 @@ namespace AppInstaller::Manifest IconResolutionEnum ConvertToIconResolutionEnum(std::string_view in); - RepairBehaviorEnum ConvertToRepairBehaviorEnum(const std::string& in); + RepairBehaviorEnum ConvertToRepairBehaviorEnum(std::string_view in); std::string_view InstallerTypeToString(InstallerTypeEnum installerType); From 260376688d0daadb6af742bcf4d8903bfa393d53 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Thu, 11 Jan 2024 14:49:18 -0800 Subject: [PATCH 12/14] Updated YamlWriter.cpp to include RepairBehavior & Repair Switch field and added corresponding test --- src/AppInstallerCLITests/YamlManifest.cpp | 31 +++++++++++++++++++ .../Manifest/ManifestCommon.cpp | 17 ++++++++++ .../Manifest/YamlWriter.cpp | 3 ++ .../Public/winget/ManifestCommon.h | 2 ++ 4 files changed, 53 insertions(+) diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index b7163a0e8f..56eaf27544 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -1148,6 +1148,37 @@ TEST_CASE("WriteV1_6SingletonManifestAndVerifyContents", "[ManifestCreation]") VerifyV1ManifestContent(generatedMultiFileManifest, false, ManifestVer{ s_ManifestVersionV1_6 }, true); } +TEST_CASE("WriteV1_7SingletonManifestAndVerifyContents", "[ManifestCreation]") +{ + TempDirectory singletonDirectory{ "SingletonManifest" }; + CopyTestDataFilesToFolder({ "ManifestV1_7-Singleton.yaml" }, singletonDirectory); + Manifest singletonManifest = YamlParser::CreateFromPath(singletonDirectory); + + TempDirectory exportedSingletonDirectory{ "exportedSingleton" }; + std::filesystem::path generatedSingletonManifestPath = exportedSingletonDirectory.GetPath() / "testSingletonManifest.yaml"; + YamlWriter::OutputYamlFile(singletonManifest, singletonManifest.Installers[0], generatedSingletonManifestPath); + + REQUIRE(std::filesystem::exists(generatedSingletonManifestPath)); + Manifest generatedSingletonManifest = YamlParser::CreateFromPath(exportedSingletonDirectory); + VerifyV1ManifestContent(generatedSingletonManifest, true, ManifestVer{ s_ManifestVersionV1_7 }, true); + + TempDirectory multiFileDirectory{ "MultiFileManifest" }; + CopyTestDataFilesToFolder({ + "ManifestV1_7-MultiFile-Version.yaml", + "ManifestV1_7-MultiFile-Installer.yaml", + "ManifestV1_7-MultiFile-DefaultLocale.yaml", + "ManifestV1_7-MultiFile-Locale.yaml" }, multiFileDirectory); + + Manifest multiFileManifest = YamlParser::CreateFromPath(multiFileDirectory); + TempDirectory exportedMultiFileDirectory{ "exportedMultiFile" }; + std::filesystem::path generatedMultiFileManifestPath = exportedMultiFileDirectory.GetPath() / "testMultiFileManifest.yaml"; + YamlWriter::OutputYamlFile(multiFileManifest, multiFileManifest.Installers[0], generatedMultiFileManifestPath); + + REQUIRE(std::filesystem::exists(generatedMultiFileManifestPath)); + Manifest generatedMultiFileManifest = YamlParser::CreateFromPath(exportedMultiFileDirectory); + VerifyV1ManifestContent(generatedMultiFileManifest, false, ManifestVer{ s_ManifestVersionV1_7 }, true); +} + YamlManifestInfo CreateYamlManifestInfo(std::string testDataFile) { YamlManifestInfo result; diff --git a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp index b774bece1c..46adefd1ac 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp @@ -576,6 +576,8 @@ namespace AppInstaller::Manifest return "InstallLocation"sv; case InstallerSwitchType::Update: return "Upgrade"sv; + case InstallerSwitchType::Repair: + return "Repair"sv; } return "Unknown"sv; @@ -652,6 +654,21 @@ namespace AppInstaller::Manifest return "unknown"sv; } + std::string_view RepairBehaviorToString(RepairBehaviorEnum repairBehavior) + { + switch (repairBehavior) + { + case AppInstaller::Manifest::RepairBehaviorEnum::Modify: + return "modify"sv; + case AppInstaller::Manifest::RepairBehaviorEnum::Installer: + return "installer"sv; + case AppInstaller::Manifest::RepairBehaviorEnum::Uninstaller: + return "uninstaller"sv; + } + + return "unknown"sv; + } + std::string_view ScopeToString(ScopeEnum scope) { switch (scope) diff --git a/src/AppInstallerCommonCore/Manifest/YamlWriter.cpp b/src/AppInstallerCommonCore/Manifest/YamlWriter.cpp index 09eee32c74..33d35249a4 100644 --- a/src/AppInstallerCommonCore/Manifest/YamlWriter.cpp +++ b/src/AppInstallerCommonCore/Manifest/YamlWriter.cpp @@ -66,6 +66,7 @@ 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; // Installer switches constexpr std::string_view InstallerSwitches = "InstallerSwitches"sv; @@ -76,6 +77,7 @@ namespace AppInstaller::Manifest::YamlWriter constexpr std::string_view Log = "Log"sv; constexpr std::string_view Upgrade = "Upgrade"sv; constexpr std::string_view Custom = "Custom"sv; + constexpr std::string_view Repair = "Repair"sv; constexpr std::string_view InstallerSuccessCodes = "InstallerSuccessCodes"sv; constexpr std::string_view UpgradeBehavior = "UpgradeBehavior"sv; @@ -570,6 +572,7 @@ namespace AppInstaller::Manifest::YamlWriter 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)); + WRITE_PROPERTY_IF_EXISTS(out, RepairBehavior, RepairBehaviorToString(installer.RepairBehavior)); ProcessSequence(out, Capabilities, installer.Capabilities); ProcessSequence(out, Commands, installer.Commands); diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h index 8e83cb0f99..daded8b23f 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h @@ -393,6 +393,8 @@ namespace AppInstaller::Manifest std::string_view UpdateBehaviorToString(UpdateBehaviorEnum updateBehavior); + std::string_view RepairBehaviorToString(RepairBehaviorEnum repairBehavior); + std::string_view PlatformToString(PlatformEnum platform); std::string_view ScopeToString(ScopeEnum scope); From 283eb8e55564bb5565d53a3f0ce3a560db7a683d Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Thu, 11 Jan 2024 15:22:02 -0800 Subject: [PATCH 13/14] Updated VerifyV1ManifestContent test to validate root level RepairBehavior and modified V1_7 test manifests. NOTE:Please note that RepairBehavior, Repair reference, and values in test manifest are for demonstration and validation only, not real samples. --- .../TestData/ManifestV1_7-Singleton.yaml | 3 +++ .../ManifestV1_7-MultiFile-Installer.yaml | 2 ++ src/AppInstallerCLITests/YamlManifest.cpp | 12 ++++++++---- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml b/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml index fbc5b5c915..83190f748a 100644 --- a/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml +++ b/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml @@ -54,10 +54,12 @@ InstallerSwitches: Log: /log= InstallLocation: /dir= Upgrade: /upgrade + Repair: /repair InstallerSuccessCodes: - 1 - 0x80070005 UpgradeBehavior: uninstallPrevious +RepairBehavior: modify Commands: - makemsix - makeappx @@ -144,6 +146,7 @@ Installers: Log: /l= InstallLocation: /d= Upgrade: /u + Repair: /r UpgradeBehavior: install Commands: - makemsixPreview diff --git a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml index d0872853a5..a693ed4cde 100644 --- a/src/AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml +++ b/src/AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml @@ -24,6 +24,7 @@ InstallerSuccessCodes: - 1 - 0x80070005 UpgradeBehavior: uninstallPrevious +RepairBehavior: modify Commands: - makemsix - makeappx @@ -110,6 +111,7 @@ Installers: Log: /l= InstallLocation: /d= Upgrade: /u + Repair: /r UpgradeBehavior: install Commands: - makemsixPreview diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index 56eaf27544..c888aacbfd 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -530,10 +530,8 @@ void VerifyV1ManifestContent(const Manifest& manifest, bool isSingleton, Manifes if (manifestVer >= ManifestVer{ s_ManifestVersionV1_7 }) { - if (!isSingleton) - { - REQUIRE(defaultSwitches.at(InstallerSwitchType::Repair) == "/repair"); - } + REQUIRE(defaultSwitches.at(InstallerSwitchType::Repair) == "/repair"); + REQUIRE(manifest.DefaultInstallerInfo.RepairBehavior == RepairBehaviorEnum::Modify); } } @@ -643,6 +641,12 @@ void VerifyV1ManifestContent(const Manifest& manifest, bool isSingleton, Manifes REQUIRE_FALSE(installer1.DownloadCommandProhibited); } + if (manifestVer >= ManifestVer { s_ManifestVersionV1_7}) + { + REQUIRE(installer1.Switches.at(InstallerSwitchType::Repair) == "/r"); + REQUIRE(installer1.RepairBehavior == RepairBehaviorEnum::Modify); + } + if (!isSingleton) { if (!isExported) From 7ef33c6988456f9cda13ea68c1b7f1562e6eb0ff Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Tue, 16 Jan 2024 13:57:26 -0800 Subject: [PATCH 14/14] Fixing PR comment by addressing single quotes characters in the schema manifest to ensure consistency with other usage in the schema manifest. --- schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json | 2 +- schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json b/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json index 08cb9925ce..cfbf745ea6 100644 --- a/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json +++ b/schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json @@ -192,7 +192,7 @@ "type": [ "string", "null" ], "minLength": 1, "maxLength": 512, - "description": "The ‘Repair’ value must be passed to the installer, ModifyPath ARP command, or uninstaller ARP command when the user opts for a repair." + "description": "The 'Repair' value must be passed to the installer, ModifyPath ARP command, or uninstaller ARP command when the user opts for a repair." } } }, diff --git a/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json b/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json index c5a4651c7d..7e1678e1b0 100644 --- a/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json +++ b/schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json @@ -292,7 +292,7 @@ "type": [ "string", "null" ], "minLength": 1, "maxLength": 512, - "description": "The ‘Repair’ value must be passed to the installer, ModifyPath ARP command, or uninstaller ARP command when the user opts for a repair" + "description": "The 'Repair' value must be passed to the installer, ModifyPath ARP command, or uninstaller ARP command when the user opts for a repair" } } },