microsoft / winget-create

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

`wingetcreate new` fails to parse portable package #302

Closed bradenhilton closed 1 year ago

bradenhilton commented 2 years ago

Brief description of your issue

I am attempting to create a manifest for twpayne/chezmoi and wingetcreate new gives a generic "Failed to parse the package" error.

Steps to reproduce

Run wingetcreate new https://github.com/twpayne/chezmoi/releases/download/v2.22.0/chezmoi-windows-amd64.exe.

Expected behavior

The command should run successfully, or give a more informative error message.

If I am not mistaken, failure to automatically parse information should prompt the user for input.

Actual behavior

❯ wingetcreate new https://github.com/twpayne/chezmoi/releases/download/v2.22.0/chezmoi-windows-amd64.exe
Downloading and parsing: https://github.com/twpayne/chezmoi/releases/download/v2.22.0/chezmoi-windows-amd64.exe...
Failed to parse the package from [https://github.com/twpayne/chezmoi/releases/download/v2.22.0/chezmoi-windows-amd64.exe]

I stepped through with the debugger and it fails in ParsePackageAndGenerateInstallerNodes because parseResult is false.

Environment

❯ winget --info
Windows Package Manager (Preview) v1.4.2161-preview
Copyright (c) Microsoft Corporation. All rights reserved.

Windows: Windows.Desktop v10.0.19044.1889
System Architecture: X64
Package: Microsoft.DesktopAppInstaller v1.19.2161.0

❯ wingetcreate
Windows Package Manager Manifest Creator v1.1.2.0
bradenhilton commented 2 years ago

I stepped through with the debugger and it fails in ParsePackageAndGenerateInstallerNodes because parseResult is false.

I tried returning true instead of parseResult just to see what would happen and was prompted for input, after adding (perhaps incorrect) placeholder values I got the following output:

Generating a preview of your manifests...
Version manifest preview:
# Created using wingetcreate 1.1.0.0
# yaml-language-server: $schema=https://aka.ms/winget-manifest.version.1.2.0.schema.json

PackageIdentifier: chezmoi.chezmoi
PackageVersion: 2.22.1
DefaultLocale: en-US
ManifestType: version
ManifestVersion: 1.2.0

Installer manifest preview:
# Created using wingetcreate 1.1.0.0
# yaml-language-server: $schema=https://aka.ms/winget-manifest.installer.1.2.0.schema.json

PackageIdentifier: chezmoi.chezmoi
PackageVersion: 2.22.1
Installers: []
ManifestType: installer
ManifestVersion: 1.2.0

Default locale manifest preview:
# Created using wingetcreate 1.1.0.0
# yaml-language-server: $schema=https://aka.ms/winget-manifest.defaultLocale.1.2.0.schema.json

PackageIdentifier: chezmoi.chezmoi
PackageVersion: 2.22.1
PackageLocale: en-US
Publisher: chezmoi
PublisherUrl: https://github.com/twpayne
PublisherSupportUrl: https://github.com/twpayne/chezmoi/issues
Author: twpayne
PackageName: chezmoi
PackageUrl: https://chezmoi.io
License: MIT
LicenseUrl: https://github.com/twpayne/chezmoi/blob/master/LICENSE
ShortDescription: Manage your dotfiles across multiple diverse machines, securely.
Tags:
- dotfiles
ReleaseNotesUrl: https://github.com/twpayne/chezmoi/releases/tag/v2.22.1
ManifestType: defaultLocale
ManifestVersion: 1.2.0

Manifest validation succeeded: False
Manifest Error: Schema validation failed.
Error context: <root>[Installers] Description: Value type not permitted by 'type' constraint.
Error context: <root> Description: Failed to validate against schema associated with property name 'Installers'.
File: chezmoi.chezmoi.installer.yaml

I'm not sure why the version is 1.1.0.0, I'm using a fresh clone.

Maybe I broke things by returning true, or adding incorrect values, but it looks like it doesn't detect that the installer type should be portable.

matifali commented 2 years ago

I am facing same issue while working with coder

JasonWei512 commented 1 year ago

Same. I want to create manifests for some portable CLI tools written in Rust, like tokei and genact. But wingetcreate failed to parse these packages.

josh-hemphill commented 1 year ago

Stepping through the code, I think the issue is that if an executable does not contain an MS xml manifest file embedded in the binary, then it just fails, instead of continuing as an exe.

It's non-trivial to add the manifest in rust projects at the moment, and I think there's lots of programs that don't have it; so I think the correct thing to do would be to allow and handle programs that don't have it.

I opened a PR #334 to hopefully fix it

darkvertex commented 1 year ago

Me too I'm getting a "failed to parse" error. In my case trying to package from a zipfile containing a installer with a bunch of data files beside it:

> wingetcreate new https://github.com/NatronGitHub/Natron/releases/download/v2.5.0/Natron-2.5.0-Windows-x86_64.zip
Downloading and parsing: https://github.com/NatronGitHub/Natron/releases/download/v2.5.0/Natron-2.5.0-Windows-x86_64.zip...
Failed to parse the package from [https://github.com/NatronGitHub/Natron/releases/download/v2.5.0/Natron-2.5.0-Windows-x86_64.zip]

Curiously, my zipfile's Setup.exe does in fact have a manifest, according to ResourceHacker:

image

Perhaps it's because it's one folder deep? But I tried rezipping it with the setup files at the root of the zip and uploading it elsewhere and using that url and it still failed to parse, so something's funky somewhere. 🤔

josh-hemphill commented 1 year ago

@darkvertex Just tested Natron you linked, and it seems to parse fine with both the preview version of winget-create as well as after the PR for this.

bradenhilton commented 1 year ago

This should be re-opened IMHO.

334 prevents the vague error, but it does not resolve my issue (failure to parse a particular portable exe).

The InstallerType for https://github.com/twpayne/chezmoi/releases/download/v2.29.1/chezmoi-windows-amd64.exe is returned as Exe, and at no point am I prompted to confirm whether it is an exe installer or portable exe. Instead, I am prompted for installer switches, which are not applicable, and providing no input throws an exception (likely related to https://github.com/microsoft/winget-create/pull/338).

I entered some placeholder installer switches to see if I could change the installer type after the fact and am also unable to do so.

It needs to be possible to override the InstallerType value to Portable when it is incorrectly detected. Some projects are just not able to easily provide an XML manifest as @josh-hemphill stated.

Would it be too disruptive to always prompt the user to confirm if a given exe is an installer or portable app?

mdanish-kh commented 1 year ago

The InstallerType for https://github.com/twpayne/chezmoi/releases/download/v2.29.1/chezmoi-windows-amd64.exe is returned as Exe, and at no point am I prompted to confirm whether it is an exe installer or portable exe.

@bradenhilton This issue got fixed in PR https://github.com/microsoft/winget-create/pull/355, and the fix will probably appear in the next release of winget-create.