microsoft / winget-create

The Windows Package Manager Manifest Creator command-line tool (aka wingetcreate)
MIT License
475 stars 83 forks source link

Cannot update MSIXBundle and ZIP with multiple nested installers #392

Open mdanish-kh opened 1 year ago

mdanish-kh commented 1 year ago

Brief description of your issue

When an MSIXBundle or a ZIP contains multiple installers, wingetcreate is unable to update it and fails with The number of installers must match existing installers. This is because in the following code snippet:

https://github.com/microsoft/winget-create/blob/5ec4ad62c6434934c3d630d589bf9c2bf767e264/src/WingetCreateCore/Common/PackageParser.cs#L217

numOfNewInstallers will include all the nested installers inside the msixbundle / ZIP and the condition if (numOfNewInstallers != existingInstallers.Count) will return true throwing the exception with the above quoted message.

Logic needs to be added to match only a single suitable installer from msix / ZIP.

Steps to reproduce

Try to update packages with msixbundles, zip that contain multiple installers within. Examples listed below:

Using an arbitrary version and old urls to simulate an update scenario

1. Microsoft.WingetCreate (Resolved, see comment below)

2. Microsoft.WindowsTerminal (Resolved, see comment below)

  1. Microsoft.Sysinternals.ZoomIt

    wingetcreate update Microsoft.Sysinternals.ZoomIt --version 15.0 --urls "https://download.sysinternals.com/files/ZoomIt.zip|x64" "https://download.sysinternals.com/files/ZoomIt.zip|x86" "https://download.sysinternals.com/files/ZoomIt.zip|arm64"
  2. orhun.git-cliff

    wingetcreate update orhun.git-cliff --version 2.3.6 --urls "https://github.com/orhun/git-cliff/releases/download/v2.1.1/git-cliff-2.1.1-i686-pc-windows-msvc.zip" "https://github.com/orhun/git-cliff/releases/download/v2.1.1/git-cliff-2.1.1-aarch64-pc-windows-msvc.zip" "https://github.com/orhun/git-cliff/releases/download/v2.1.1/git-cliff-2.1.1-x86_64-pc-windows-msvc.zip"
  3. sirredbeard.wslinternals

    wingetcreate update sirredbeard.wslinternals --version 0.0.15 --urls "https://github.com/sirredbeard/wslinternals/releases/download/0.0.14/0.0.14.zip|x86" 
  4. Google.PlatformTools

wingetcreate update Google.PlatformTools -v 35.0.1 --urls "https://dl.google.com/android/repository/platform-tools-latest-windows.zip"
  1. Oven-sh.Bun
wingetcreate update Oven-sh.Bun --version 1.1.1 --urls https://github.com/oven-sh/bun/releases/download/bun-v1.1.1/bun-windows-x64.zip

This also occurs in interactive update scenarios.

Expected behavior

Package updates successfully as number of URLs matches the number of installer nodes in the manifest

Actual behavior

wingetcreate fails with:

The number of new installer packages must match the number of existing installer packages.

Environment

Windows Package Manager Manifest Creator v1.2.6.0
ryfu-msft commented 1 year ago

So I tried out some of these examples. WingetCreate is able to parse all installers from an msixbundle so you only need to provide the installer url once. This is the change I just made in #393 in order to handle both the msix and portable installers in the manifest.

For Microsoft.WindowsTerminal, the following command should work:

 wingetcreate update Microsoft.WindowsTerminal --urls "https://github.com/microsoft/terminal/releases/download/v1.15.3465.0/Microsoft.WindowsTerminal_Win10_1.15.3465.0_8wekyb3d8bbwe.msixbundle"

For the Zoomit example, the real issue is how we handle zip installers. Because we can't know for sure if the user is providing us the same zip for all 3 installers or 3 different zip packages for each installer, it is hard to determine which new installer url should be used to update each installer node. Just wanted to clarify some of your examples, but updating zip installers definitely needs more work/logic.

mdanish-kh commented 1 year ago

So I tried out some of these examples. WingetCreate is able to parse all installers from an msixbundle so you only need to provide the installer url once. This is the change I just made in #393 in order to handle both the msix and portable installers in the manifest.

@ryfu-msft IMO that's really a non-obvious behavior as with majority of duplicate InstallerUrls, we use as many URLs as present in the manifest. But even the solution you described doesn't seem to be applicable everywhere. This will only work if the msixbundle in the manifest is used for as many architectures as it supports (which to be fair is the majority of cases currently). I found that this doesn't seem to work in the example of Microsoft.PowerShell or Microsoft.PowerShell.Preview.

The manifest contains 4 installer nodes, with 2 MSIs and 2 installer nodes having the same msixbundle URL. The following command (using msixbundle URL once) also leads to the same error:

wingetcreate update Microsoft.PowerShell -v 60.0 -u "https://github.com/PowerShell/PowerShell/releases/download/v7.3.4/PowerShell-7.3.4-win-x64.msi|x64" "https://github.com/PowerShell/PowerShell/releases/download/v7.3.4/PowerShell-7.3.4-win-x86.msi|x86" "https://github.com/PowerShell/PowerShell/releases/download/v7.3.4/PowerShell-7.3.4-win.msixbundle"

The reason seems to be that the msixbundle contains installers for all architectures i.e., x64, x86, arm64, arm but the msixbundle is only used for arm and arm64 in the manifest.[^1] This would result in wingetcreate matching 2 MSIs and then 4 installers from msixbundle making total of 6 installers in program logic when in reality they're only 4. image

Although it's possible but admittedly rare [^2] that publishers don't want to use MSIX for certain architectures even if supports it for whatever reason so I would understand if we don't want to handle this case. I'm more worried about the UX and the non-obvious part of the situation because it's different from how we currently handle duplicate URLs in the manifest for other InstallerTypes. Maybe we need to have better error messaging / docs to inform the user?

[^1]: The PowerShell team is temporarily using MSIX as they're working on making MSIs for ARM and ARM64 systems. See https://github.com/microsoft/winget-pkgs/pull/95069#issue-1563027787 [^2]: PowerShell is the only example I could find currently in the repo that doesn't use msixbundle for all supporting arch