Repair switch support for V1.7 YAML manifest (microsoft#4041)

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 the Installer & root level field, determines the repair behavior for the source package used to repair a package. - Updated the test manifest (ManifestV1_7-MultiFile-Installer.yaml) and the test code that validate the Repair switch and RepairBehavior fields. **[How Validated:]** - Made the necessary changes to the manifest - Ran the ManifestValidation tests that are part of AppInstallerCLITests and verified that all the tests passed **[Test Result:]** ``` Filters: [ManifestValidation] =============================================================================== All tests passed (4008 assertions in 30 test cases) Filters: [ManifestCreation] =============================================================================== All tests passed (1309 assertions in 7 test cases) ``` **TODO's:** - [ ] Determine the interdependency validation between the "RepairBehavior" and "Repair" fields. Should this be a semantic validation or a validation at the time of workflow execution as an early validation step?  - [x] I have signed the [Contributor License Agreement](https://cla.opensource.microsoft.com/microsoft/winget-pkgs). - [x] This pull request is related to an issue. Related to : * microsoft#148 ----- ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/microsoft/winget-cli/pull/4041)
Madhusudhan-MSFT · Jan 17, 2024 · 4993e4b · 4993e4b · github-actions · Jan 18, 2024
1 parent b4d8c96
commit 4993e4b
Show file tree

Hide file tree

Showing 10 changed files with 194 additions and 3 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
@@ -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."
         }
       }
     },
@@ -581,6 +587,15 @@
       "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",
       "properties": {
@@ -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
@@ -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"
         }
       }
     },
@@ -680,6 +686,15 @@
       "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",
       "properties": {
@@ -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": {

diff --git a/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml b/src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml
@@ -54,10 +54,12 @@ InstallerSwitches:
   Log: /log=<LOGPATH>
   InstallLocation: /dir=<INSTALLPATH>
   Upgrade: /upgrade
+  Repair: /repair
 InstallerSuccessCodes:
   - 1
   - 0x80070005
 UpgradeBehavior: uninstallPrevious
+RepairBehavior: modify
 Commands:
   - makemsix
   - makeappx
@@ -144,6 +146,7 @@ Installers:
       Log: /l=<LOGPATH>
       InstallLocation: /d=<INSTALLPATH>
       Upgrade: /u
+      Repair: /r
     UpgradeBehavior: install
     Commands:
       - makemsixPreview

diff --git a/...AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml b/...AppInstallerCLITests/TestData/MultiFileManifestV1_7/ManifestV1_7-MultiFile-Installer.yaml
@@ -19,10 +19,12 @@ InstallerSwitches:
   Log: /log=<LOGPATH>
   InstallLocation: /dir=<INSTALLPATH>
   Upgrade: /upgrade
+  Repair: /repair
 InstallerSuccessCodes:
   - 1
   - 0x80070005
 UpgradeBehavior: uninstallPrevious
+RepairBehavior: modify
 Commands:
   - makemsix
   - makeappx
@@ -109,6 +111,7 @@ Installers:
       Log: /l=<LOGPATH>
       InstallLocation: /d=<INSTALLPATH>
       Upgrade: /u
+      Repair: /r
     UpgradeBehavior: install
     Commands:
       - makemsixPreview
@@ -161,7 +164,10 @@ Installers:
     InstallerUrl: https://www.microsoft.com/msixsdk/msixsdkx64.exe
     InstallerSha256: 69D84CA8899800A5575CE31798293CD4FEBAB1D734A07C2E51E56A28E0DF8C82
     ProductCode: "{Bar}"
+    InstallerSwitches:
+      Repair: /r
     UpgradeBehavior: deny
+    RepairBehavior: uninstaller
   - Architecture: x86
     InstallerType: portable
     InstallerUrl: https://www.microsoft.com/msixsdk/msixsdkx86.exe
@@ -191,5 +197,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
diff --git 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(defaultSwitches.at(InstallerSwitchType::Repair) == "/repair");
+            REQUIRE(manifest.DefaultInstallerInfo.RepairBehavior == RepairBehaviorEnum::Modify);
+        }
     }
 
     if (isSingleton || isExported)
@@ -535,7 +541,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);
         }
@@ -631,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)
@@ -708,6 +724,21 @@ 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);
+                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);
+            }
         }
 
         // Localization
@@ -792,7 +823,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({
@@ -1121,6 +1152,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;
@@ -1469,7 +1531,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");

diff --git 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)
@@ -962,6 +979,27 @@ namespace AppInstaller::Manifest
         }
     }
 
+    RepairBehaviorEnum ConvertToRepairBehaviorEnum(std::string_view in)
+    {
+        std::string inStrLower = Utility::ToLower(in);
+        RepairBehaviorEnum result = RepairBehaviorEnum::Unknown;
+
+        if (inStrLower == "installer")
+        {
+            result = RepairBehaviorEnum::Installer;
+        }
+        else if (inStrLower == "uninstaller")
+        {
+            result = RepairBehaviorEnum::Uninstaller;
+        }
+        else if (inStrLower == "modify")
+        {
+            result = RepairBehaviorEnum::Modify;
+        }
+
+        return result;
+    }
+
     std::map<DWORD, ExpectedReturnCodeEnum> GetDefaultKnownReturnCodes(InstallerTypeEnum installerType)
     {
         switch (installerType)

diff --git a/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp b/src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp
@@ -329,6 +329,16 @@ 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<FieldProcessInfo> fields_v1_7 =
+                {
+                    { "RepairBehavior", [this](const YAML::Node& value)->ValidationErrors { m_p_installer->RepairBehavior = ConvertToRepairBehaviorEnum(value.as<std::string>()); return {}; } },
+                };
+
+                std::move(fields_v1_7.begin(), fields_v1_7.end(), std::inserter(result, result.end()));
+            }
         }
 
         return result;
@@ -357,6 +367,11 @@ 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<std::string>(); return{}; });
+
+            if (manifestVersion >= ManifestVer{ s_ManifestVersionV1_7 })
+            {
+                result.emplace_back("Repair", [this](const YAML::Node& value)->ValidationErrors { (*m_p_switches)[InstallerSwitchType::Repair] = value.as<std::string>(); return{}; });
+            };
         }
 
         return result;

diff --git 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);