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

Implementation for Zip Install (Non-Portable) #2320

Merged
merged 8 commits into from
Jul 13, 2022
Merged

Implementation for Zip Install (Non-Portable) #2320

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9da58fa
fix spelling
Jul 8, 2022
bbba7c7
Initial commit for supporting zip install (non-portable) (#5)
ryfu-msft Jul 8, 2022
de2688c
remove itemgroup
Jul 8, 2022
b40196d
fix E2E tests
Jul 11, 2022
516b992
save work
Jul 12, 2022
399367b
address PR comments
Jul 12, 2022
f6e4f88
move using definitions
Jul 13, 2022
e260da9
address second round of comments
Jul 13, 2022
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
13 changes: 13 additions & 0 deletions .github/actions/spelling/expect.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ fintimes
Fixfor
flargle
flexera
FOF
foldc
foldcase
FOLDERID
Expand Down Expand Up @@ -166,7 +167,9 @@ IAttachment
IARP
IConfiguration
idx
IEnum
IFACEMETHODIMP
IFile
IGlobal
IHelp
IHost
Expand All @@ -186,6 +189,7 @@ IPersist
IRead
IService
ISettings
IShell
ishelp
ISQ
issuecomment
Expand Down Expand Up @@ -222,6 +226,7 @@ localhost
localizationpriority
LPBYTE
LPDWORD
LPITEMIDLIST
LPWSTR
LSTATUS
LTDA
Expand Down Expand Up @@ -259,11 +264,13 @@ mylog
mysilent
mysilentwithprogress
mytool
NESTEDINSTALLER
NETFX
netlify
Newtonsoft
NOEXPAND
nonetwork
NONFOLDERS
nonterminated
normer
NOSEPARATOR
Expand Down Expand Up @@ -293,6 +300,8 @@ PEGI
pfn
pfxpath
Pherson
pidl
PIDLIST
pkgmgr
pkindex
PMS
Expand Down Expand Up @@ -346,6 +355,8 @@ seof
serializer
setmetadatabymanifestid
SETTINGMAPPING
SHCONTF
SHGDN
Shlobj
sid
SIGNATUREHASH
Expand All @@ -360,6 +371,7 @@ srs
standalone
startswith
streambuf
STRRET
strtoull
subdir
subkey
Expand Down Expand Up @@ -453,4 +465,5 @@ yamlcreateps
yao
ype
Zanollo
ZIPHASH
zy
2 changes: 2 additions & 0 deletions src/AppInstallerCLICore/AppInstallerCLICore.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@
<ClInclude Include="TableOutput.h" />
<ClInclude Include="VTSupport.h" />
<ClInclude Include="PackageCollection.h" />
<ClInclude Include="Workflows\ArchiveFlow.h" />
<ClInclude Include="Workflows\CompletionFlow.h" />
<ClInclude Include="Workflows\DependencyNodeProcessor.h" />
<ClInclude Include="Workflows\DownloadFlow.h" />
Expand Down Expand Up @@ -327,6 +328,7 @@
</ClCompile>
<ClCompile Include="Resources.cpp" />
<ClCompile Include="VTSupport.cpp" />
<ClCompile Include="Workflows\ArchiveFlow.cpp" />
<ClCompile Include="Workflows\CompletionFlow.cpp" />
<ClCompile Include="Workflows\DependencyNodeProcessor.cpp" />
<ClCompile Include="Workflows\DownloadFlow.cpp" />
Expand Down
6 changes: 6 additions & 0 deletions src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@
<ClInclude Include="Workflows\PortableFlow.h">
<Filter>Workflows</Filter>
</ClInclude>
<ClInclude Include="Workflows\ArchiveFlow.h">
<Filter>Workflows</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<ClCompile Include="pch.cpp">
Expand Down Expand Up @@ -325,6 +328,9 @@
<ClCompile Include="Workflows\PortableFlow.cpp">
<Filter>Workflows</Filter>
</ClCompile>
<ClCompile Include="Workflows\ArchiveFlow.cpp">
<Filter>Workflows</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<None Include="PropertySheet.props" />
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/ExecutionContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ namespace AppInstaller::CLI::Execution
// TODO: Remove when the source interface is refactored.
TreatSourceFailuresAsWarning = 0x10,
ShowSearchResultsOnPartialFailure = 0x20,
InstallerExtractedFromArchive = 0x40,
Copy link
Member

Choose a reason for hiding this comment

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

There are a few options here:

  1. This
  2. Move installer type to be a context data item
  3. Leverage IsArchiveType

Option 3 seems to be the least amount of adding additional context data and the easiest. Is there any case in which we wouldn't use the nested type for an archive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed InstallerExtractedFromArchive context flag.

Added ExecuteInstallerForInstallerType which selects the appropriate install flow based on the installerType to handle whether to use InstallerType or NestedInstallerType

};

DEFINE_ENUM_FLAG_OPERATORS(ContextFlag);
Expand Down
3 changes: 3 additions & 0 deletions src/AppInstallerCLICore/Resources.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(ExportIncludeVersionsArgumentDescription);
WINGET_DEFINE_RESOURCE_STRINGID(ExportSourceArgumentDescription);
WINGET_DEFINE_RESOURCE_STRINGID(ExternalDependencies);
WINGET_DEFINE_RESOURCE_STRINGID(ExtractArchiveFailed);
WINGET_DEFINE_RESOURCE_STRINGID(ExtraPositionalError);
WINGET_DEFINE_RESOURCE_STRINGID(FeatureDisabledByAdminSettingMessage);
WINGET_DEFINE_RESOURCE_STRINGID(FeatureDisabledMessage);
Expand Down Expand Up @@ -188,6 +189,7 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(MultipleInstalledPackagesFound);
WINGET_DEFINE_RESOURCE_STRINGID(MultiplePackagesFound);
WINGET_DEFINE_RESOURCE_STRINGID(NameArgumentDescription);
WINGET_DEFINE_RESOURCE_STRINGID(NestedInstallerNotFound);
WINGET_DEFINE_RESOURCE_STRINGID(NoApplicableInstallers);
WINGET_DEFINE_RESOURCE_STRINGID(NoExperimentalFeaturesMessage);
WINGET_DEFINE_RESOURCE_STRINGID(NoInstalledPackageFound);
Expand Down Expand Up @@ -216,6 +218,7 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(PortableHashMismatchOverridden);
WINGET_DEFINE_RESOURCE_STRINGID(PortableHashMismatchOverrideRequired);
WINGET_DEFINE_RESOURCE_STRINGID(PortableInstallFailed);
WINGET_DEFINE_RESOURCE_STRINGID(PortableInstallFromArchiveNotSupported);
WINGET_DEFINE_RESOURCE_STRINGID(PortableRegistryCollisionOverridden);
WINGET_DEFINE_RESOURCE_STRINGID(PositionArgumentDescription);
WINGET_DEFINE_RESOURCE_STRINGID(PreserveArgumentDescription);
Expand Down
50 changes: 50 additions & 0 deletions src/AppInstallerCLICore/Workflows/ArchiveFlow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
#include "pch.h"
#include "ArchiveFlow.h"
#include "winget/Archive.h"

namespace AppInstaller::CLI::Workflow
{
void ExtractInstallerFromArchive(Execution::Context& context)
{
const auto& installerPath = context.Get<Execution::Data::InstallerPath>();
const auto& installerParentPath = installerPath.parent_path();

HRESULT hr = AppInstaller::Archive::ExtractArchive(installerPath, installerParentPath);

if (SUCCEEDED(hr))
{
AICLI_LOG(CLI, Info, << "Successfully extracted archive to: " << installerParentPath );
context.SetFlags(Execution::ContextFlag::InstallerExtractedFromArchive);
}
else
{
AICLI_LOG(CLI, Info, << "Failed to extract archive to: " << installerParentPath);
context.Reporter.Error() << Resource::String::ExtractArchiveFailed << std::endl;
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_EXTRACT_ARCHIVE_FAILED);
}
}

void VerifyAndSetNestedInstaller(Execution::Context& context)
{
const auto& installer = context.Get<Execution::Data::Installer>().value();
const auto& installerPath = context.Get<Execution::Data::InstallerPath>();
const auto& installerParentPath = installerPath.parent_path();
const auto& relativeFilePath = ConvertToUTF16(installer.NestedInstallerFiles[0].RelativeFilePath);

std::filesystem::path nestedInstallerPath = installerParentPath / relativeFilePath;

if (!std::filesystem::exists(nestedInstallerPath))
{
AICLI_LOG(CLI, Error, << "Unable to locate nested installer at: " << nestedInstallerPath);
context.Reporter.Error() << Resource::String::NestedInstallerNotFound << ' ' << nestedInstallerPath << std::endl;
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NESTEDINSTALLER_NOT_FOUND);
}
else
{
AICLI_LOG(CLI, Info, << "Setting installerPath to: " << nestedInstallerPath);
context.Add<Execution::Data::InstallerPath>(nestedInstallerPath);
}
}
}
11 changes: 11 additions & 0 deletions src/AppInstallerCLICore/Workflows/ArchiveFlow.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
#pragma once
#include "ExecutionContext.h"

namespace AppInstaller::CLI::Workflow
{
void ExtractInstallerFromArchive(Execution::Context& context);

void VerifyAndSetNestedInstaller(Execution::Context& context);
}
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/Workflows/DownloadFlow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ namespace AppInstaller::CLI::Workflow
case InstallerTypeEnum::Nullsoft:
case InstallerTypeEnum::Portable:
case InstallerTypeEnum::Wix:
case InstallerTypeEnum::Zip:
context << DownloadInstallerFile;
break;
case InstallerTypeEnum::Msix:
Expand Down
31 changes: 28 additions & 3 deletions src/AppInstallerCLICore/Workflows/InstallFlow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@
#include "ShellExecuteInstallerHandler.h"
#include "MSStoreInstallerHandler.h"
#include "MsiInstallFlow.h"
#include "ArchiveFlow.h"
#include "PortableFlow.h"
#include "WorkflowBase.h"
#include "Workflows/DependenciesFlow.h"
#include <AppInstallerDeployment.h>
#include <winget/ARPCorrelation.h>
#include <winget/Archive.h>
#include <Argument.h>
#include <Command.h>

Expand Down Expand Up @@ -322,7 +324,18 @@ namespace AppInstaller::CLI::Workflow

bool isUpdate = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate);

switch (installer.InstallerType)
InstallerTypeEnum installerType;

if (WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExtractedFromArchive))
{
installerType = installer.NestedInstallerType;
}
else
{
installerType = installer.InstallerType;
}

switch (installerType)
{
case InstallerTypeEnum::Exe:
case InstallerTypeEnum::Burn:
Expand All @@ -337,7 +350,7 @@ namespace AppInstaller::CLI::Workflow
ExecuteUninstaller;
context.ClearFlags(Execution::ContextFlag::InstallerExecutionUseUpdate);
}
if (ShouldUseDirectMSIInstall(installer.InstallerType, context.Args.Contains(Execution::Args::Type::Silent)))
if (ShouldUseDirectMSIInstall(installerType, context.Args.Contains(Execution::Args::Type::Silent)))
{
context << DirectMSIInstall;
}
Expand All @@ -364,11 +377,22 @@ namespace AppInstaller::CLI::Workflow
}
context << PortableInstall;
break;
case InstallerTypeEnum::Zip:
context << ArchiveInstall;
break;
default:
THROW_HR(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED));
}
}

void ArchiveInstall(Execution::Context& context)
{
context <<
ExtractInstallerFromArchive <<
VerifyAndSetNestedInstaller <<
ExecuteInstaller;
Copy link
Member

Choose a reason for hiding this comment

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

This flow will be less efficient for portables; we should just extract directly to the final location. Not sure if that works great with the database tracking files though. We might need to be less efficient just so any files we could lose track of are in %TEMP% and will be more readily cleaned up.

Copy link
Contributor Author

@ryfu-msft ryfu-msft Jul 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I am thinking that portables will require more tasks than the one I have currently specified.
Here is what I am proposing when we start the implementation for portable(s) in an archive.

  • ExtractInstallerFromArchive (now called ExtractFilesFromArchive) will have a separate flow for portables that will copy the extracted files to the final portable install location
  • TryExtractArchive will have an additional parameter to indicate whether we will append to the database file in the final install location to keep track of the files that are extracted and copied.
  • The install location will need to have the directory created prior to extraction, so this will likely need to be its own step that creates the InstallDirectory and the local db file if it does not exist already. So something like SetupPortableInstallDirectory that we call prior to all of these steps if the NestedInstallerType is Portable

Once we enter the portable install flow, we can then use IsArchiveType to check whether we should install multiple portables for a given archive package.

Copy link
Member

Choose a reason for hiding this comment

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

My gut is to create ExecuteInstallerForType(InstallerType) and then have ExecuteInstaller call it with installer.InstallerType and this call it with installer.NestedInstallerType rather than the extra state management.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added ExecuteInstallerForType(Context, InstallerType) to handle the install flow selection logic based on installerType

}

void ShellExecuteInstall(Execution::Context& context)
{
context <<
Expand Down Expand Up @@ -524,7 +548,8 @@ namespace AppInstaller::CLI::Workflow
void EnsureSupportForInstall(Execution::Context& context)
{
context <<
Workflow::EnsureSupportForPortableInstall;
Workflow::EnsureSupportForPortableInstall <<
Workflow::EnsureNonPortableTypeForArchiveInstall;
}

void InstallMultiplePackages::operator()(Execution::Context& context) const
Expand Down
6 changes: 6 additions & 0 deletions src/AppInstallerCLICore/Workflows/InstallFlow.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ namespace AppInstaller::CLI::Workflow
// Outputs: None
void PortableInstall(Execution::Context& context);

// Runs the flow for installing a package from an archive.
// Required Args: None
// Inputs: Installer, InstallerPath, Manifest
// Outputs: None
void ArchiveInstall(Execution::Context& context);

// Verifies parameters for install to ensure success.
// Required Args: None
// Inputs:
Expand Down
12 changes: 12 additions & 0 deletions src/AppInstallerCLICore/Workflows/PortableFlow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,4 +621,16 @@ namespace AppInstaller::CLI::Workflow
EnsureVolumeSupportsReparsePoints;
}
}

// TODO: remove this check once support for portable in archive has been implemented
void EnsureNonPortableTypeForArchiveInstall(Execution::Context& context)
{
auto nestedInstallerType = context.Get<Execution::Data::Installer>().value().NestedInstallerType;

if (nestedInstallerType == InstallerTypeEnum::Portable)
{
context.Reporter.Error() << Resource::String::PortableInstallFromArchiveNotSupported << std::endl;
AICLI_TERMINATE_CONTEXT(ERROR_NOT_SUPPORTED);
}
}
}
2 changes: 2 additions & 0 deletions src/AppInstallerCLICore/Workflows/PortableFlow.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,6 @@ namespace AppInstaller::CLI::Workflow
void PortableUninstallImpl(Execution::Context& context);

void EnsureSupportForPortableInstall(Execution::Context& context);

void EnsureNonPortableTypeForArchiveInstall(Execution::Context& context);
}
5 changes: 5 additions & 0 deletions src/AppInstallerCLIE2ETests/Constants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ public class Constants
public const string ExeInstaller = "AppInstallerTestExeInstaller";
public const string MsiInstaller = "AppInstallerTestMsiInstaller";
public const string MsixInstaller = "AppInstallerTestMsixInstaller";
public const string ZipInstaller = "AppInstallerTestZipInstaller";
public const string ExeInstallerFileName = "AppInstallerTestExeInstaller.exe";
public const string MsiInstallerFileName = "AppInstallerTestMsiInstaller.msi";
public const string MsixInstallerFileName = "AppInstallerTestMsixInstaller.msix";
public const string ZipInstallerFileName = "AppInstallerTestZipInstaller.zip";
public const string IndexPackage = "source.msix";
public const string MakeAppx = "makeappx.exe";
public const string SignTool = "signtool.exe";
Expand Down
34 changes: 34 additions & 0 deletions src/AppInstallerCLIE2ETests/InstallCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,40 @@ public void InstallPortableFailsWithCleanup()
TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, false);
}

[Test]
public void InstallZipWithExe()
{
var installDir = TestCommon.GetRandomTestDir();
var result = TestCommon.RunAICLICommand("install", $"AppInstallerTest.TestZipInstallerWithExe --silent -l {installDir}");
Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
Assert.True(result.StdOut.Contains("Successfully installed"));
Assert.True(VerifyTestExeInstalled(installDir, "/execustom"));
}

[Test]
public void InstallZipWithMsi()
{
if (string.IsNullOrEmpty(TestCommon.MsiInstallerPath))
{
Assert.Ignore("MSI installer not available");
}

var installDir = TestCommon.GetRandomTestDir();
var result = TestCommon.RunAICLICommand("install", $"AppInstallerTest.TestZipInstallerWithMsi --silent -l {installDir}");
Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
Assert.True(result.StdOut.Contains("Successfully installed"));
Assert.True(VerifyTestMsiInstalledAndCleanup(installDir));
}

[Test]
public void InstallZipWithMsix()
{
var result = TestCommon.RunAICLICommand("install", $"AppInstallerTest.TestZipInstallerWithMsix");
Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
Assert.True(result.StdOut.Contains("Successfully installed"));
Assert.True(VerifyTestMsixInstalledAndCleanup());
}

private bool VerifyTestExeInstalled(string installDir, string expectedContent = null)
{
if (!File.Exists(Path.Combine(installDir, Constants.TestExeInstalledFileName)))
Expand Down
2 changes: 2 additions & 0 deletions src/AppInstallerCLIE2ETests/TestCommon.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ public class TestCommon
public static string MsiInstallerPath { get; set; }

public static string MsixInstallerPath { get; set; }

public static string ZipInstallerPath { get; set; }

public static string PackageCertificatePath { get; set; }

Expand Down
Loading