Add support for installing portables from a zip #2500

ryfu-msft · 2022-09-06T23:47:56Z

Resolves #140
Resolves #2523

This PR adds functionality for supporting the installation of portable(s) from a zip archive.

Changes:

PortableEntry has been removed and replaced with PortableInstaller, which encapsulates the state and logic flow for handling a portable install/uninstall.
Portables from a zip are extracted to a temporary folder. If the NestedInstallerType is a portable, those extracted files are moved to the appropriate install location during the install flow.
The implementation for installing and uninstall portables is now based on the expected and desired state, which are determined prior to the installation phase. The expected state will first be examined to determine which files need to be cleaned up. The desired state will then be examined to place down the correct files. This implementation will be optimized and improved on in a separate PR by determining and applying the differences in the expected and desired state.
Uninstall will now check that all exe and symlinks that are created have not been modified and can be safely removed prior to starting the uninstall flow. This is to ensure that we do not partially uninstall the package if a file does not match what is recorded in the index. This check can be overridden with the --force argument.

Tests:

E2E tests to verify that install, update, and uninstall work as expected for portable in zip.
Unit tests to ensure that all files recorded in the index are correctly verified prior to uninstall.
Added more granular tests for verifying portable installation (moving exe, creating symlink, and adding to PATH).

Microsoft Reviewers: Open in CodeFlow

src/AppInstallerCLICore/PortableInstaller.cpp

src/AppInstallerCLICore/PortableInstaller.h

src/AppInstallerRepositoryCore/Microsoft/PortableIndex.cpp

src/AppInstallerCLICore/PortableInstaller.cpp

src/AppInstallerRepositoryCore/Microsoft/Schema/Portable_1_0/PortableTable.h

src/AppInstallerRepositoryCore/Microsoft/Schema/IPortableIndex.h

src/AppInstallerRepositoryCore/Microsoft/PortableIndex.h

src/AppInstallerCLICore/PortableInstaller.cpp

JohnMcPMS · 2022-09-13T23:35:21Z

src/AppInstallerCLICore/PortableInstaller.cpp

+        }
+    }
+
+    HRESULT PortableInstaller::InstallSingle(const std::filesystem::path& installerPath)


I don't think splitting single/multiple is necessarily a good idea from a maintainability standpoint. It isn't really necessary either given what I'm going to suggest in this rather long comment.

Any action (install/upgrade/uninstall) taken on the portable file(s) can be described by 3 states: { Desired, Expected, Actual }

Desired contains the incoming file(s)

Expected contains the description of the file(s) that we maintain (registry or index)

Actual is the state of the filesystem

Every filesystem entry that we want can be described in the desired state, and each one has a related state in the expected and actual sets (even if it isn't in the set in memory). This includes the root install directory for the package, which should not be treated as a special case. Each filesystem entry can also be acted on individually, so long as they are properly ordered.

If you use this model, there is no install/upgrade/uninstall distinction in the core code. Those APIs can still be presented, but they are used to create the desired state, which is then passed to the resolution engine. That code can be clear and consistent because it needs only handle one filesystem entry at a time. Each entry can be properly recorded before it committed to disk, or properly removed before removing the recorded entry.

Additionally, creating an abstraction over the single/multiple file state storage seems like a better way to keep the core code clean and in one place. The desired state can easily be inspected to determine if it meets the criteria for storing in the registry (contains exactly one directory, file, and link) or not. The correct interface implementation can be created and not need to have multiple code paths.

Trenly · 2022-09-17T03:20:18Z

src/AppInstallerCLIE2ETests/TestData/Manifests/TestZipInstaller_Portable.2.0.0.0.yaml

+    InstallerSha256: <ZIPHASH>
+    NestedInstallerType: portable
+    NestedInstallerFiles:
+      - RelativeFilePath: AppInstallerTestExeInstaller.exe


There should be some tests which consider two entries with the same RelativeFilePath, Two entries with the same PortableCommandAlias, and possibly Exact duplicate entries?

I was doing some testing and I don't think that these cases are being validated against at all. (#2523)
Perhaps @denelon can provide his thoughts on if these validations would be required

Trenly · 2022-09-17T14:00:23Z

Is the output of the files removed intentional?

PS D:\Git\winget-pkgs> wingetdev install -m D:\Git\winget-pkgs\manifests\c\Codex\ffmpeg\essentials\5.1.1
This application is licensed to you by its owner.
Microsoft is not responsible for, nor does it grant any licenses to, third-party packages.
Downloading https://www.gyan.dev/ffmpeg/builds/packages/ffmpeg-5.1.1-essentials_build.zip
  ██████████████████████████████  78.9 MB / 78.9 MB
Successfully verified installer hash
Starting package install...
Successfully installed
PS D:\Git\winget-pkgs> winget list ffmpeg
Name              Id                                     Version
----------------------------------------------------------------
ffmpeg essentials Codex.ffmpeg.essentials__DefaultSource 5.1.1
PS D:\Git\winget-pkgs> wingetdev uninstall ffmpeg  
No installed package found matching input criteria.
PS D:\Git\winget-pkgs> wingetdev uninstall 'ffmpeg essentials'
Found ffmpeg essentials [Codex.ffmpeg.essentials__DefaultSource]
Starting package uninstall...
"C:\\Users\\Trenly\\AppData\\Local\\Microsoft\\WinGet\\Packages\\Codex.ffmpeg.essentials__DefaultSource\\ffmpeg-5.1.1-essentials_build"
"C:\\Users\\Trenly\\AppData\\Local\\Microsoft\\WinGet\\Links\\ffmpeg.exe"
"C:\\Users\\Trenly\\AppData\\Local\\Microsoft\\WinGet\\Links\\ffplay.exe"
"C:\\Users\\Trenly\\AppData\\Local\\Microsoft\\WinGet\\Links\\ffprobe.exe"
Successfully uninstalled

vedantmgoyal9 · 2022-09-17T16:24:18Z

Is the output of the files removed intentional?

I don't know if it is there for debugging purposes, but it should be written to log instead of stdout.

WinGet-2022-09-17-21-50-32.732.log

Trenly · 2022-09-20T22:23:16Z

src/AppInstallerCLICore/Workflows/ArchiveFlow.cpp

-        // TODO: For portables, extract portables to final install location and log to local database.
-        HRESULT hr = AppInstaller::Archive::TryExtractArchive(installerPath, installerParentPath);
-        AICLI_LOG(CLI, Info, << "Extracting archive to: " << installerParentPath);
+        AICLI_LOG(CLI, Info, << "Extracting archive to: " << destinationFolder);


In testing this, it feels like perhaps there should be output written to inform the user the archive is being extracted. I noticed with particularly large archives there appeared to be significant delay between Successfully verified installer hash and Starting package install. . .

Agreed; the extraction should be part of the install phase, not the download.

We may also be able to have progress callbacks for the extraction, and certainly can for the progress of moving files to their target location.

src/AppInstallerCLICore/PortableInstaller.h

JohnMcPMS · 2022-09-21T21:47:34Z

src/AppInstallerCLICore/PortableInstaller.cpp

+
+        RemoveInstallDirectory();
+
+        RemoveFromPathVariable();


Why not remove it from path first, to keep the flow fully in the opposite order?

The path should only be removed if the directory added to path is empty. That can change depending on whether the user wants to purge the install directory as checked in RemoveInstallDirectory or if there are leftover files after applying the desired state. Even though the flow is not fully in the opposite order, I think this flow ensures that we don't accidentally remove from path if other portables depend on it.

JohnMcPMS · 2022-09-21T21:50:23Z

src/AppInstallerCLICore/PortableInstaller.cpp

+
+    void PortableInstaller::SetExpectedState()
+    {
+        const auto& indexPath = InstallLocation / GetPortableIndexFileName();


Is the file name now package specific to enable multiple portables to target the same directory?

Yes, GetPortableIndexFileName returns the package specific filename which is now the productCode + ".db" extension

JohnMcPMS · 2022-09-21T22:03:46Z

src/AppInstallerCLICore/PortableInstaller.cpp

+        }
+    }
+
+    void PortableInstaller::ApplyDesiredState()


So while this has the structure of what I was pushing for (a single function that applies the state), this is a very inefficient implementation. I was hoping that we would be able to apply the differences rather than removing everything and putting all the files back. That isn't strictly necessary; we could have a follow-up PR at some point to handle that. It is possible that we will find it necessary to do before shipping for real depending on the performance/size of the archives that people are using.

Additionally, while it also isn't necessary to do it this way, it would be cleaner if the index/no index was decided once by creating an object that implemented the necessary interface. That way you just throw values at it and it decides where to put them, rather than having if blocks sprinkled throughout.

SeanFeldman · 2022-10-11T19:09:21Z

@ryfu-msft, is there documentation on how to prepare WinGet zip-based manifest? Thank you.

Trenly · 2022-10-11T19:21:38Z

@ryfu-msft, is there documentation on how to prepare WinGet zip-based manifest? Thank you.

I have a few examples on my fork - https://github.com/Trenly/winget-pkgs/branches/all?query=zips

YamlCreate.ps1 also supports Zip, but you have to enable developer options and override the manifest version to 1.4.0 using the YamlCreate settings. Just be aware that the community repository won't start accepting zip based manifests until the 1.4 client is in a stable release and has a high percentage of adoption

denelon · 2022-10-11T20:30:23Z

We're going to update wingetcreate as well so it can also create manifests for .zip based packages.

Ryan Fu added 6 commits August 29, 2022 18:31

Refactor install for portable

b0eae01

save work

50b6a49

refactor portable flow

af72152

add support for uninstall from index

7812f05

Add Unit tests for portable installer

e2fc062

add tests for portable in zip

150c41b

ryfu-msft requested a review from a team as a code owner September 6, 2022 23:47

ghost added the Issue-Feature This is a feature request for the Windows Package Manager client. label Sep 6, 2022

fix unit tests

06d3d00

Trenly reviewed Sep 7, 2022

View reviewed changes

Ryan Fu added 4 commits September 7, 2022 16:30

fix hashing

e857f84

fix E2E tests

cdbb8c0

Additional logging and existence check

2b8d33b

get parsing path from extracted files

7dbdab4

ihordyrman mentioned this pull request Sep 11, 2022

Register with WinGet when .zip installation type is supported paolosalvatori/ServiceBusExplorer#455

Open

fix COM test install dir

e0ebb9d

JohnMcPMS reviewed Sep 13, 2022

View reviewed changes

Ryan Fu added 2 commits September 16, 2022 12:37

initial refactor for portable state

8d1af15

save work

dee2a3b

Trenly reviewed Sep 17, 2022

View reviewed changes

szeestraten mentioned this pull request Sep 19, 2022

Unable to install using WinGet Cinchoo/ChoEazyCopy#38

Open

Ryan Fu added 4 commits September 19, 2022 16:34

finish work for archive

d292891

savework

6882f39

clean up

3b7810b

resolve merge conflicts

2c82d07

ryfu-msft mentioned this pull request Sep 20, 2022

[Zip] Nested Installer Files Validation #2523

Closed

Trenly reviewed Sep 20, 2022

View reviewed changes

fix E2e tests

ba5fbcc

fix unit

65bb6f6

JohnMcPMS reviewed Sep 21, 2022

View reviewed changes

Ryan Fu added 2 commits September 26, 2022 12:07

add duplicate command alias check and move resources.pri file

63420e2

change validation check to toLower

13ef536

JohnMcPMS approved these changes Sep 26, 2022

View reviewed changes

fix pipeline, e2e inproc, and yaml validation

7f6e697

ghost added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Sep 27, 2022

ryfu-msft merged commit e0ac965 into microsoft:master Sep 27, 2022

ryfu-msft linked an issue Sep 28, 2022 that may be closed by this pull request

Support for installers inside of a .zip file #859

Closed

MrAdam mentioned this pull request Oct 5, 2022

Support for installation using winget? zyedidia/micro#2339

Open

Haegi mentioned this pull request Oct 11, 2022

Add Helm to Winget helm/helm#8203

Closed

lwbt mentioned this pull request Oct 18, 2022

Publish to winget helix-editor/helix#4344

Closed

mekwall mentioned this pull request Oct 20, 2022

support winget Rem0o/FanControl.Releases#628

Closed

ssrahul96 mentioned this pull request Oct 23, 2022

[Zip] New Package: Helm.Helm 3.10.1 microsoft/winget-pkgs#86104

Merged

5 tasks

rrmistry mentioned this pull request Dec 2, 2022

[Duplicate] Consider publishing act to winget packages repository nektos/act#1489

Closed

rmcdouga mentioned this pull request Jan 25, 2023

Support official winget Windows package manager? jbangdev/jbang#882

Open

MatejKafka mentioned this pull request Sep 12, 2023

Add application installer so x64dbg can be made available on winget-pkgs x64dbg/x64dbg#2865

Open

ryfu-msft deleted the portableInZip branch February 29, 2024 18:22

chuim mentioned this pull request Jun 14, 2024

[Feature Request] Add Cmder to Winget cmderdev/cmder#2552

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for installing portables from a zip #2500

Add support for installing portables from a zip #2500

ryfu-msft commented Sep 6, 2022 •

edited

Loading

JohnMcPMS Sep 13, 2022

Trenly Sep 17, 2022

Trenly commented Sep 17, 2022

vedantmgoyal9 commented Sep 17, 2022 •

edited

Loading

Trenly Sep 20, 2022

JohnMcPMS Sep 21, 2022

JohnMcPMS Sep 21, 2022

JohnMcPMS Sep 21, 2022

ryfu-msft Sep 26, 2022

JohnMcPMS Sep 21, 2022

ryfu-msft Sep 26, 2022

JohnMcPMS Sep 21, 2022

SeanFeldman commented Oct 11, 2022

Trenly commented Oct 11, 2022 •

edited

Loading

denelon commented Oct 11, 2022

Add support for installing portables from a zip #2500

Add support for installing portables from a zip #2500

Conversation

ryfu-msft commented Sep 6, 2022 • edited Loading

Microsoft Reviewers: Open in CodeFlow

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Trenly commented Sep 17, 2022

vedantmgoyal9 commented Sep 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SeanFeldman commented Oct 11, 2022

Trenly commented Oct 11, 2022 • edited Loading

denelon commented Oct 11, 2022

ryfu-msft commented Sep 6, 2022 •

edited

Loading

vedantmgoyal9 commented Sep 17, 2022 •

edited

Loading

Trenly commented Oct 11, 2022 •

edited

Loading