Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove zip installer type from 1.0 and 1.1 manifests #3006

Merged
merged 2 commits into from
Feb 24, 2023
Merged

Remove zip installer type from 1.0 and 1.1 manifests #3006

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a10f629
remove zip from all manifests and adjust tests
Feb 23, 2023
a073edc
address comments
Feb 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion schemas/JSON/manifests/v1.0.0/manifest.installer.1.0.0.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
"msi",
"appx",
"exe",
"zip",
"inno",
"nullsoft",
"wix",
Expand Down
1 change: 0 additions & 1 deletion schemas/JSON/manifests/v1.0.0/manifest.singleton.1.0.0.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
"msi",
"appx",
"exe",
"zip",
"inno",
"nullsoft",
"wix",
Expand Down
1 change: 0 additions & 1 deletion schemas/JSON/manifests/v1.1.0/manifest.installer.1.1.0.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
"msi",
"appx",
"exe",
"zip",
"inno",
"nullsoft",
"wix",
Expand Down
1 change: 0 additions & 1 deletion schemas/JSON/manifests/v1.1.0/manifest.singleton.1.1.0.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@
"msi",
"appx",
"exe",
"zip",
"inno",
"nullsoft",
"wix",
Expand Down
6 changes: 1 addition & 5 deletions src/AppInstallerCLITests/TestData/Manifest-Good-SystemReferenceComplex.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@ Version: 1.7.32
Publisher: Microsoft
License: MIT License
Description: A complex system reference test file.
InstallerType: Zip
InstallerType: EXE
Copy link
Member

Choose a reason for hiding this comment

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

Nit: You use inconsistent case (EXE vs exe) on these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed all the installer types in this test manifest to lowercase to match other yamls.

PackageFamilyName: Microsoft.DesktopAppInstaller_8wekyb3d8bbwe
ProductCode: "{Foo}"
Installers:
- InstallerType: Zip
Arch: x86
Url: https://rubengustorage.blob.core.windows.net/publiccontainer/msixsdkx86.zip
Sha256: 69D84CA8899800A5575CE31798293CD4FEBAB1D734A07C2E51E56A28E0DF8C82
Copy link
Member

Choose a reason for hiding this comment

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

I'm assuming you made sure that we don't lose coverage by removing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the v1.4 manifest tests cover this scenario and ensure that fields from the root get passed to the installer if they are not defined.

- InstallerType: MSIX
Arch: x86
Url: https://rubengustorage.blob.core.windows.net/publiccontainer/msixsdkx86.zip
Expand Down
6 changes: 3 additions & 3 deletions src/AppInstallerCLITests/TestData/Manifest-Good.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Protocols: "protocol1,protocol2"
FileExtensions: "appx,appxbundle,msix,msixbundle"
# InstallerType and Switches CAN have a "default" value
# on the root. An installer can override them.
InstallerType: Zip
InstallerType: exe
PackageFamilyName: Microsoft.DesktopAppInstaller_8wekyb3d8bbwe
ProductCode: "{Foo}"
UpdateBehavior: UninstallPrevious
Expand All @@ -34,7 +34,7 @@ Installers:
Url: https://rubengustorage.blob.core.windows.net/publiccontainer/msixsdkx86.zip
Sha256: 69D84CA8899800A5575CE31798293CD4FEBAB1D734A07C2E51E56A28E0DF8C82
Language: en-US
InstallerType: Zip
InstallerType: exe
Scope: user
UpdateBehavior: Install
Switches:
Expand All @@ -50,7 +50,7 @@ Installers:
Url: https://rubengustorage.blob.core.windows.net/publiccontainer/msixsdkx64.zip
Sha256: 69D84CA8899800A5575CE31798293CD4FEBAB1D734A07C2E51E56A28E0DF0000
Language: en-US
InstallerType: Zip
InstallerType: exe
Scope: user
Localization:
- Language: es-MX
Expand Down
2 changes: 1 addition & 1 deletion src/AppInstallerCLITests/TestData/ManifestV1-Singleton.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Platform:
- Windows.Desktop
- Windows.Universal
MinimumOSVersion: 10.0.0.0
InstallerType: zip
InstallerType: exe
Scope: machine
InstallModes:
- interactive
Expand Down
2 changes: 1 addition & 1 deletion src/AppInstallerCLITests/TestData/ManifestV1_1-Singleton.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Platform:
- Windows.Desktop
- Windows.Universal
MinimumOSVersion: 10.0.0.0
InstallerType: zip
InstallerType: exe
Scope: machine
InstallModes:
- interactive
Expand Down
2 changes: 1 addition & 1 deletion src/AppInstallerCLITests/TestData/ManifestV1_2-Singleton.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Platform:
- Windows.Desktop
- Windows.Universal
MinimumOSVersion: 10.0.0.0
InstallerType: zip
InstallerType: exe
Scope: machine
InstallModes:
- interactive
Expand Down
2 changes: 1 addition & 1 deletion src/AppInstallerCLITests/TestData/ManifestV1_4-Singleton.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Platform:
- Windows.Desktop
- Windows.Universal
MinimumOSVersion: 10.0.0.0
InstallerType: zip
InstallerType: exe
Scope: machine
InstallModes:
- interactive
Expand Down
2 changes: 1 addition & 1 deletion src/AppInstallerCLITests/TestData/MultiFileManifestV1/ManifestV1-MultiFile-Installer.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Platform:
- Windows.Desktop
- Windows.Universal
MinimumOSVersion: 10.0.0.0
InstallerType: zip
InstallerType: exe
Scope: machine
InstallModes:
- interactive
Expand Down
2 changes: 1 addition & 1 deletion ...AppInstallerCLITests/TestData/MultiFileManifestV1_1/ManifestV1_1-MultiFile-Installer.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Platform:
- Windows.Desktop
- Windows.Universal
MinimumOSVersion: 10.0.0.0
InstallerType: zip
InstallerType: exe
Scope: machine
InstallModes:
- interactive
Expand Down
2 changes: 1 addition & 1 deletion ...AppInstallerCLITests/TestData/MultiFileManifestV1_2/ManifestV1_2-MultiFile-Installer.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Platform:
- Windows.Desktop
- Windows.Universal
MinimumOSVersion: 10.0.0.0
InstallerType: zip
InstallerType: exe
Scope: machine
InstallModes:
- interactive
Expand Down
2 changes: 1 addition & 1 deletion ...AppInstallerCLITests/TestData/MultiFileManifestV1_4/ManifestV1_4-MultiFile-Installer.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Platform:
- Windows.Desktop
- Windows.Universal
MinimumOSVersion: 10.0.0.0
InstallerType: zip
InstallerType: exe
Scope: machine
InstallModes:
- interactive
Expand Down
51 changes: 23 additions & 28 deletions src/AppInstallerCLITests/YamlManifest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ TEST_CASE("ReadPreviewGoodManifestAndVerifyContents", "[ManifestValidation]")
REQUIRE(manifest.DefaultInstallerInfo.Commands == MultiValue{ "makemsix", "makeappx" });
REQUIRE(manifest.DefaultInstallerInfo.Protocols == MultiValue{ "protocol1", "protocol2" });
REQUIRE(manifest.DefaultInstallerInfo.FileExtensions == MultiValue{ "appx", "appxbundle", "msix", "msixbundle" });
REQUIRE(manifest.DefaultInstallerInfo.BaseInstallerType == InstallerTypeEnum::Zip);
REQUIRE(manifest.DefaultInstallerInfo.BaseInstallerType == InstallerTypeEnum::Exe);
REQUIRE(manifest.DefaultInstallerInfo.PackageFamilyName == "Microsoft.DesktopAppInstaller_8wekyb3d8bbwe");
REQUIRE(manifest.DefaultInstallerInfo.ProductCode == "{Foo}");
REQUIRE(manifest.DefaultInstallerInfo.UpdateBehavior == UpdateBehaviorEnum::UninstallPrevious);
Expand All @@ -93,10 +93,10 @@ TEST_CASE("ReadPreviewGoodManifestAndVerifyContents", "[ManifestValidation]")
REQUIRE(installer1.Url == "https://rubengustorage.blob.core.windows.net/publiccontainer/msixsdkx86.zip");
REQUIRE(installer1.Sha256 == SHA256::ConvertToBytes("69D84CA8899800A5575CE31798293CD4FEBAB1D734A07C2E51E56A28E0DF8C82"));
REQUIRE(installer1.Locale == "en-US");
REQUIRE(installer1.BaseInstallerType == InstallerTypeEnum::Zip);
REQUIRE(installer1.BaseInstallerType == InstallerTypeEnum::Exe);
REQUIRE(installer1.Scope == ScopeEnum::User);
REQUIRE(installer1.PackageFamilyName == "");
REQUIRE(installer1.ProductCode == "");
REQUIRE(installer1.ProductCode == "{Foo}");
Copy link
Member

Choose a reason for hiding this comment

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

Why did this one change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think for the older manifests, fields were not passed to the zip installer but that behavior has since changed. Since I changed the installertype to EXE, the productCode specified from the root is now being passed to the installer node.

REQUIRE(installer1.UpdateBehavior == UpdateBehaviorEnum::Install);

auto installer1Switches = installer1.Switches;
Expand All @@ -114,10 +114,10 @@ TEST_CASE("ReadPreviewGoodManifestAndVerifyContents", "[ManifestValidation]")
REQUIRE(installer2.Url == "https://rubengustorage.blob.core.windows.net/publiccontainer/msixsdkx64.zip");
REQUIRE(installer2.Sha256 == SHA256::ConvertToBytes("69D84CA8899800A5575CE31798293CD4FEBAB1D734A07C2E51E56A28E0DF0000"));
REQUIRE(installer2.Locale == "en-US");
REQUIRE(installer2.BaseInstallerType == InstallerTypeEnum::Zip);
REQUIRE(installer2.BaseInstallerType == InstallerTypeEnum::Exe);
REQUIRE(installer2.Scope == ScopeEnum::User);
REQUIRE(installer2.PackageFamilyName == "");
REQUIRE(installer2.ProductCode == "");
REQUIRE(installer2.ProductCode == "{Foo}");
REQUIRE(installer2.UpdateBehavior == UpdateBehaviorEnum::UninstallPrevious);

// Installer2 does not declare switches, it inherits switches from package default.
Expand Down Expand Up @@ -342,36 +342,31 @@ TEST_CASE("ComplexSystemReference", "[ManifestValidation]")
{
Manifest manifest = YamlParser::CreateFromPath(TestDataFile("Manifest-Good-SystemReferenceComplex.yaml"));

REQUIRE(manifest.Installers.size() == 5);

// Zip installer does not inherit
REQUIRE(manifest.Installers[0].BaseInstallerType == InstallerTypeEnum::Zip);
REQUIRE(manifest.Installers[0].PackageFamilyName == "");
REQUIRE(manifest.Installers[0].ProductCode == "");
REQUIRE(manifest.Installers.size() == 4);

// MSIX installer does inherit
REQUIRE(manifest.Installers[1].BaseInstallerType == InstallerTypeEnum::Msix);
Copy link
Member

Choose a reason for hiding this comment

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

It's probably not worth changing, but we could do something like

auto& installer = manifest.Installer[0];
REQUIRE(installer.BaseInstallerType == InstallerTypeEnum::Msix);
...

so that we don't have to change the index in all places. Though I doubt we'll have to do it again...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to suggested so that we never have to think about this again :)

REQUIRE(manifest.Installers[1].Arch == Architecture::X86);
REQUIRE(manifest.Installers[1].PackageFamilyName == "Microsoft.DesktopAppInstaller_8wekyb3d8bbwe");
REQUIRE(manifest.Installers[1].ProductCode == "");
REQUIRE(manifest.Installers[0].BaseInstallerType == InstallerTypeEnum::Msix);
REQUIRE(manifest.Installers[0].Arch == Architecture::X86);
REQUIRE(manifest.Installers[0].PackageFamilyName == "Microsoft.DesktopAppInstaller_8wekyb3d8bbwe");
REQUIRE(manifest.Installers[0].ProductCode == "");

// MSI installer does inherit
REQUIRE(manifest.Installers[2].BaseInstallerType == InstallerTypeEnum::Msi);
REQUIRE(manifest.Installers[2].Arch == Architecture::X86);
REQUIRE(manifest.Installers[2].PackageFamilyName == "");
REQUIRE(manifest.Installers[2].ProductCode == "{Foo}");
REQUIRE(manifest.Installers[1].BaseInstallerType == InstallerTypeEnum::Msi);
REQUIRE(manifest.Installers[1].Arch == Architecture::X86);
REQUIRE(manifest.Installers[1].PackageFamilyName == "");
REQUIRE(manifest.Installers[1].ProductCode == "{Foo}");

// MSIX installer with override
REQUIRE(manifest.Installers[3].BaseInstallerType == InstallerTypeEnum::Msix);
REQUIRE(manifest.Installers[3].Arch == Architecture::X64);
REQUIRE(manifest.Installers[3].PackageFamilyName == "Override_8wekyb3d8bbwe");
REQUIRE(manifest.Installers[3].ProductCode == "");
REQUIRE(manifest.Installers[2].BaseInstallerType == InstallerTypeEnum::Msix);
REQUIRE(manifest.Installers[2].Arch == Architecture::X64);
REQUIRE(manifest.Installers[2].PackageFamilyName == "Override_8wekyb3d8bbwe");
REQUIRE(manifest.Installers[2].ProductCode == "");

// MSI installer with override
REQUIRE(manifest.Installers[4].BaseInstallerType == InstallerTypeEnum::Msi);
REQUIRE(manifest.Installers[4].Arch == Architecture::X64);
REQUIRE(manifest.Installers[4].PackageFamilyName == "");
REQUIRE(manifest.Installers[4].ProductCode == "Override");
REQUIRE(manifest.Installers[3].BaseInstallerType == InstallerTypeEnum::Msi);
REQUIRE(manifest.Installers[3].Arch == Architecture::X64);
REQUIRE(manifest.Installers[3].PackageFamilyName == "");
REQUIRE(manifest.Installers[3].ProductCode == "Override");
}

TEST_CASE("ManifestVersionExtensions", "[ManifestValidation]")
Expand Down Expand Up @@ -437,7 +432,7 @@ void VerifyV1ManifestContent(const Manifest& manifest, bool isSingleton, Manifes
REQUIRE(manifest.DefaultInstallerInfo.Locale == "en-US");
REQUIRE(manifest.DefaultInstallerInfo.Platform == std::vector<PlatformEnum>{ PlatformEnum::Desktop, PlatformEnum::Universal });
REQUIRE(manifest.DefaultInstallerInfo.MinOSVersion == "10.0.0.0");
REQUIRE(manifest.DefaultInstallerInfo.BaseInstallerType == InstallerTypeEnum::Zip);
REQUIRE(manifest.DefaultInstallerInfo.BaseInstallerType == InstallerTypeEnum::Exe);
REQUIRE(manifest.DefaultInstallerInfo.Scope == ScopeEnum::Machine);
REQUIRE(manifest.DefaultInstallerInfo.InstallModes == std::vector<InstallModeEnum>{ InstallModeEnum::Interactive, InstallModeEnum::Silent, InstallModeEnum::SilentWithProgress });

Expand Down