Skip to content

Commit

Permalink
Repair switch support for V1.7 YAML manifest (#4041)
Browse files Browse the repository at this point in the history
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?



<!-- To check a checkbox place an "x" between the brackets. e.g: [x] -->

- [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 : 

*  #148 

-----

###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/microsoft/winget-cli/pull/4041)
  • Loading branch information
Madhusudhan-MSFT committed Jan 17, 2024
1 parent b4d8c96 commit 4993e4b
Show file tree
Hide file tree
Showing 10 changed files with 194 additions and 3 deletions.
21 changes: 21 additions & 0 deletions schemas/JSON/manifests/v1.7.0/manifest.installer.1.7.0.json
Expand Up @@ -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."
}
}
},
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -698,6 +713,9 @@
},
"DownloadCommandProhibited": {
"$ref": "#/definitions/DownloadCommandProhibited"
},
"RepairBehavior": {
"$ref": "#/definitions/RepairBehavior"
}
},
"required": [
Expand Down Expand Up @@ -814,6 +832,9 @@
"DownloadCommandProhibited": {
"$ref": "#/definitions/DownloadCommandProhibited"
},
"RepairBehavior": {
"$ref": "#/definitions/RepairBehavior"
},
"Installers": {
"type": "array",
"items": {
Expand Down
21 changes: 21 additions & 0 deletions schemas/JSON/manifests/v1.7.0/manifest.singleton.1.7.0.json
Expand Up @@ -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"
}
}
},
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -797,6 +812,9 @@
},
"DownloadCommandProhibited": {
"$ref": "#/definitions/DownloadCommandProhibited"
},
"RepairBehavior": {
"$ref": "#/definitions/RepairBehavior"
}
},
"required": [
Expand Down Expand Up @@ -1036,6 +1054,9 @@
"DownloadCommandProhibited": {
"$ref": "#/definitions/DownloadCommandProhibited"
},
"RepairBehavior": {
"$ref": "#/definitions/RepairBehavior"
},
"Installers": {
"type": "array",
"items": {
Expand Down
3 changes: 3 additions & 0 deletions src/AppInstallerCLITests/TestData/ManifestV1_7-Singleton.yaml
Expand Up @@ -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
Expand Down Expand Up @@ -144,6 +146,7 @@ Installers:
Log: /l=<LOGPATH>
InstallLocation: /d=<INSTALLPATH>
Upgrade: /u
Repair: /r
UpgradeBehavior: install
Commands:
- makemsixPreview
Expand Down
Expand Up @@ -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
Expand Down Expand Up @@ -109,6 +111,7 @@ Installers:
Log: /l=<LOGPATH>
InstallLocation: /d=<INSTALLPATH>
Upgrade: /u
Repair: /r
UpgradeBehavior: install
Commands:
- makemsixPreview
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
68 changes: 65 additions & 3 deletions src/AppInstallerCLITests/YamlManifest.cpp
Expand Up @@ -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)
Expand All @@ -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);
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
38 changes: 38 additions & 0 deletions src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions src/AppInstallerCommonCore/Manifest/ManifestYamlPopulator.cpp
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions src/AppInstallerCommonCore/Manifest/YamlWriter.cpp
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

1 comment on commit 4993e4b

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@check-spelling-bot Report

🔴 Please review

See the 📜action log for details.

❌ Errors Count
❌ forbidden-pattern 1

See ❌ Event descriptions for more information.

Previously acknowledged words that are now absent bitspace Mta PFM testdata :arrow_right:
To accept ✔️ these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands

... in a clone of the git@github.com:microsoft/winget-cli.git repository
on the master branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/winget-cli/actions/runs/7549064235/attempts/1'
If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

Please sign in to comment.