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.

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

Add support for extracting archives using tar.exe #4541

Merged
merged 8 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions doc/Settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,16 @@ The 'skipDependencies' behavior affects whether dependencies are installed for a
"installBehavior": {
"skipDependencies": true
},
```

### Archive Extraction Method
The 'archiveExtractionMethod' behavior affects how installer archives are extracted. Currently there are two supported values: `Tar` or `ShellApi`.
`Tar` indicates that the archive should be extracted using the tar executable ('tar.exe') while `shellApi` indicates using the Windows Shell API. Defaults to `shellApi` if value is not set or is invalid.

```json
"installBehavior": {
"archiveExtractionMethod": "tar" | "shellApi"
},
```

### Preferences and Requirements
Expand Down
57 changes: 33 additions & 24 deletions src/AppInstallerCLICore/Workflows/ArchiveFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
// Licensed under the MIT License.
#include "pch.h"
#include "ArchiveFlow.h"
#include "PortableFlow.h"
#include "PortableFlow.h"
#include "ShellExecuteInstallerHandler.h"
#include <winget/AdminSettings.h>
#include <winget/Archive.h>
#include <winget/Archive.h>
#include <winget/Filesystem.h>

using namespace AppInstaller::Manifest;
Expand Down Expand Up @@ -42,30 +43,38 @@ namespace AppInstaller::CLI::Workflow
}
}
}
}

void ExtractFilesFromArchive(Execution::Context& context)
{
}
void ExtractFilesFromArchive(Execution::Context& context)
{
const auto& installerPath = context.Get<Execution::Data::InstallerPath>();
std::filesystem::path destinationFolder = installerPath.parent_path() / s_Extracted;
std::filesystem::create_directory(destinationFolder);

std::filesystem::create_directory(destinationFolder);
AICLI_LOG(CLI, Info, << "Extracting archive to: " << destinationFolder);
context.Reporter.Info() << Resource::String::ExtractingArchive << std::endl;
HRESULT result = AppInstaller::Archive::TryExtractArchive(installerPath, destinationFolder);

if (SUCCEEDED(result))
{
AICLI_LOG(CLI, Info, << "Successfully extracted archive");
context.Reporter.Info() << Resource::String::ExtractArchiveSucceeded << std::endl;
}
else
{
AICLI_LOG(CLI, Info, << "Failed to extract archive with code " << result);
context.Reporter.Error() << Resource::String::ExtractArchiveFailed << std::endl;
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_EXTRACT_ARCHIVE_FAILED);
}
}
context.Reporter.Info() << Resource::String::ExtractingArchive << std::endl;

if (Settings::User().Get<Settings::Setting::ArchiveExtractionMethod>() == Archive::ExtractionMethod::Tar)
{
context << ShellExecuteExtractArchive(installerPath, destinationFolder);
}
else
{
HRESULT result = AppInstaller::Archive::TryExtractArchive(installerPath, destinationFolder);

if (SUCCEEDED(result))
{
AICLI_LOG(CLI, Info, << "Successfully extracted archive");
context.Reporter.Info() << Resource::String::ExtractArchiveSucceeded << std::endl;
}
else
{
AICLI_LOG(CLI, Info, << "Failed to extract archive with code " << result);
context.Reporter.Error() << Resource::String::ExtractArchiveFailed << std::endl;
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_EXTRACT_ARCHIVE_FAILED);
}
}
}

void VerifyAndSetNestedInstaller(Execution::Context& context)
{
Expand Down Expand Up @@ -137,4 +146,4 @@ namespace AppInstaller::CLI::Workflow
}
}
}
}
}
8 changes: 4 additions & 4 deletions src/AppInstallerCLICore/Workflows/ArchiveFlow.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ namespace AppInstaller::CLI::Workflow
// Inputs: InstallerPath
// Outputs: None
void ScanArchiveFromLocalManifest(Execution::Context& context);

// Extracts the files from an archive
// Required Args: None
// Inputs: InstallerPath
// Outputs: None
void ExtractFilesFromArchive(Execution::Context& context);
// Outputs: None
void ExtractFilesFromArchive(Execution::Context& context);

// Verifies that the NestedInstaller exists and sets the InstallerPath
// Required Args: None
Expand All @@ -28,4 +28,4 @@ namespace AppInstaller::CLI::Workflow
// Inputs: Installer, InstallerPath
// Outputs: None
void EnsureValidNestedInstallerMetadataForArchiveInstall(Execution::Context& context);
}
}
2 changes: 1 addition & 1 deletion src/AppInstallerCLICore/Workflows/RepairFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -505,4 +505,4 @@ namespace AppInstaller::CLI::Workflow
context.Reporter.Info() << Resource::String::RepairFlowRepairSuccess << std::endl;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -538,4 +538,55 @@ namespace AppInstaller::CLI::Workflow
context.Add<Execution::Data::OperationReturnCode>(enableFeatureResult.value());
}
}
}

#ifndef AICLI_DISABLE_TEST_HOOKS
std::optional<DWORD> s_ExtractArchiveWithTarResult_Override{};

void TestHook_SetExtractArchiveWithTarResult_Override(std::optional<DWORD>&& result)
{
s_ExtractArchiveWithTarResult_Override = std::move(result);
}
#endif

void ShellExecuteExtractArchive::operator()(Execution::Context& context) const
{
auto tarExecPath = AppInstaller::Filesystem::GetExpandedPath("%windir%\\system32\\tar.exe");

std::string args = "-xf \"" + m_archivePath.u8string() + "\" -C \"" + m_destPath.u8string() + "\"";

std::optional<DWORD> extractArchiveResult;
#ifndef AICLI_DISABLE_TEST_HOOKS
if (s_ExtractArchiveWithTarResult_Override)
{
extractArchiveResult = *s_ExtractArchiveWithTarResult_Override;
}
else
#endif
{
extractArchiveResult = context.Reporter.ExecuteWithProgress(
std::bind(InvokeShellExecuteEx,
tarExecPath,
args,
false,
SW_HIDE,
std::placeholders::_1));
}

if (!extractArchiveResult)
{
AICLI_TERMINATE_CONTEXT(E_ABORT);
}

if (extractArchiveResult.value() == ERROR_SUCCESS)
{
AICLI_LOG(CLI, Info, << "Successfully extracted archive");
context.Reporter.Info() << Resource::String::ExtractArchiveSucceeded << std::endl;
}
else
{
AICLI_LOG(CLI, Info, << "Failed to extract archive with exit code " << extractArchiveResult.value());
context.Reporter.Error() << Resource::String::ExtractArchiveFailed << std::endl;
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_EXTRACT_ARCHIVE_FAILED);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,19 @@ namespace AppInstaller::CLI::Workflow
private:
std::string_view m_featureName;
};
}

// Extracts the installer archive using the tar executable.
// Required Args: None
// Inputs: InstallerPath
// Outputs: None
struct ShellExecuteExtractArchive : public WorkflowTask
{
ShellExecuteExtractArchive(const std::filesystem::path& archivePath, const std::filesystem::path& destPath) : WorkflowTask("ShellExecuteExtractArchive"), m_archivePath(archivePath), m_destPath(destPath) {}

void operator()(Execution::Context& context) const override;

private:
std::filesystem::path m_archivePath;
std::filesystem::path m_destPath;
};
}
1 change: 1 addition & 0 deletions src/AppInstallerCLIE2ETests/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public class Constants
public const string PathSubKey_Machine = @"SYSTEM\CurrentControlSet\Control\Session Manager\Environment";

// User settings
public const string ArchiveExtractionMethod = "archiveExtractionMethod";
public const string PortablePackageUserRoot = "portablePackageUserRoot";
public const string PortablePackageMachineRoot = "portablePackageMachineRoot";
public const string InstallBehaviorScope = "scope";
Expand Down
17 changes: 16 additions & 1 deletion src/AppInstallerCLIE2ETests/InstallCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,21 @@ public void InstallZipWithMsix()
Assert.True(TestCommon.VerifyTestMsixInstalledAndCleanup());
}

/// <summary>
/// Test install zip exe by extracting with tar.
/// </summary>
[Test]
public void InstallZip_ExtractWithTar()
{
WinGetSettingsHelper.ConfigureInstallBehavior(Constants.ArchiveExtractionMethod, "tar");
var installDir = TestCommon.GetRandomTestDir();
var result = TestCommon.RunAICLICommand("install", $"AppInstallerTest.TestZipInstallerWithExe --silent -l {installDir}");
WinGetSettingsHelper.ConfigureInstallBehavior(Constants.ArchiveExtractionMethod, string.Empty);
Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
Assert.True(result.StdOut.Contains("Successfully installed"));
Assert.True(TestCommon.VerifyTestExeInstalledAndCleanup(installDir, "/execustom"));
}

/// <summary>
/// Test install an installed package and convert to upgrade.
/// </summary>
Expand Down Expand Up @@ -764,4 +779,4 @@ public void InstallExeThatInstallsMSIX()
TestCommon.RemoveARPEntry(fakeProductCode);
}
}
}
}
2 changes: 1 addition & 1 deletion src/AppInstallerCLITests/Archive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@ TEST_CASE("Scan_ZipArchive", "[archive]")
const auto& testZipPath = testZip.GetPath();
bool result = ScanZipFile(testZipPath);
REQUIRE(result);
}
}
53 changes: 53 additions & 0 deletions src/AppInstallerCLITests/InstallFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,59 @@ TEST_CASE("ExtractInstallerFromArchive_InvalidZip", "[InstallFlow][workflow]")
auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Zip_Exe.yaml"));
context.Add<Data::Manifest>(manifest);
context.Add<Data::Installer>(manifest.Installers.at(0));

// Provide an invalid zip file which should be handled appropriately.
context.Add<Data::InstallerPath>(TestDataFile("AppInstallerTestExeInstaller.exe"));
context << ExtractFilesFromArchive;
REQUIRE_TERMINATED_WITH(context, APPINSTALLER_CLI_ERROR_EXTRACT_ARCHIVE_FAILED);
REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::ExtractArchiveFailed).get()) != std::string::npos);
}

TEST_CASE("ExtractInstallerFromArchiveWithTar", "[InstallFlow][workflow]")
{
TestCommon::TestUserSettings testSettings;
testSettings.Set<Setting::ArchiveExtractionMethod>(AppInstaller::Archive::ExtractionMethod::Tar);

TestCommon::TempFile installResultPath("TestExeInstalled.txt");

std::ostringstream installOutput;
TestContext context{ installOutput, std::cin };
auto previousThreadGlobals = context.SetForCurrentThread();

OverrideForShellExecute(context);
OverrideForVerifyAndSetNestedInstaller(context);
context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Zip_Exe.yaml").GetPath().u8string());

TestHook::SetScanArchiveResult_Override scanArchiveResultOverride(true);
TestHook::SetExtractArchiveWithTarResult_Override setExtractArchiveWithTarResultOverride(ERROR_SUCCESS);

InstallCommand install({});
install.Execute(context);
INFO(installOutput.str());
REQUIRE(installOutput.str().find(Resource::LocString(Resource::String::ExtractArchiveSucceeded).get()) != std::string::npos);

// Verify Installer is called and parameters are passed in.
REQUIRE(std::filesystem::exists(installResultPath.GetPath()));
std::ifstream installResultFile(installResultPath.GetPath());
REQUIRE(installResultFile.is_open());
std::string installResultStr;
std::getline(installResultFile, installResultStr);
REQUIRE(installResultStr.find("/custom") != std::string::npos);
REQUIRE(installResultStr.find("/silentwithprogress") != std::string::npos);
}

TEST_CASE("ExtractInstallerFromArchiveWithTar_InvalidZip", "[InstallFlow][workflow]")
Copy link
Member

Choose a reason for hiding this comment

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

Please have a success case in the unit tests.

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 a success case unit test that uses a test hook to override the result of invoking tar.exe so that it returns success and verifies that the install flow can run successfully with the correct string output.

{
TestCommon::TestUserSettings testSettings;
testSettings.Set<Setting::ArchiveExtractionMethod>(AppInstaller::Archive::ExtractionMethod::Tar);

std::ostringstream installOutput;
TestContext context{ installOutput, std::cin };
auto previousThreadGlobals = context.SetForCurrentThread();
auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Zip_Exe.yaml"));
context.Add<Data::Manifest>(manifest);
context.Add<Data::Installer>(manifest.Installers.at(0));

// Provide an invalid zip file which should be handled appropriately.
context.Add<Data::InstallerPath>(TestDataFile("AppInstallerTestExeInstaller.exe"));
context << ExtractFilesFromArchive;
Expand Down
14 changes: 14 additions & 0 deletions src/AppInstallerCLITests/TestHooks.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ namespace AppInstaller
{
void TestHook_SetEnableWindowsFeatureResult_Override(std::optional<DWORD>&& result);
void TestHook_SetDoesWindowsFeatureExistResult_Override(std::optional<DWORD>&& result);
void TestHook_SetExtractArchiveWithTarResult_Override(std::optional<DWORD>&& result);
}

namespace Reboot
Expand Down Expand Up @@ -183,6 +184,19 @@ namespace TestHook
}
};

struct SetExtractArchiveWithTarResult_Override
{
SetExtractArchiveWithTarResult_Override(DWORD result)
{
AppInstaller::CLI::Workflow::TestHook_SetExtractArchiveWithTarResult_Override(result);
}

~SetExtractArchiveWithTarResult_Override()
{
AppInstaller::CLI::Workflow::TestHook_SetExtractArchiveWithTarResult_Override({});
}
};

struct SetInitiateRebootResult_Override
{
SetInitiateRebootResult_Override(bool status) : m_status(status)
Expand Down
22 changes: 22 additions & 0 deletions src/AppInstallerCLITests/UserSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,28 @@ TEST_CASE("SettingsDownloadDefaultDirectory", "[settings]")
REQUIRE(userSettingTest.Get<Setting::DownloadDefaultDirectory>() == "C:/Foo/Bar");
REQUIRE(userSettingTest.GetWarnings().size() == 0);
}
}

TEST_CASE("SettingsArchiveExtractionMethod", "[settings]")
{
auto again = DeleteUserSettingsFiles();

SECTION("Shell api")
{
std::string_view json = R"({ "installBehavior": { "archiveExtractionMethod": "shellApi" } })";
SetSetting(Stream::PrimaryUserSettings, json);
UserSettingsTest userSettingTest;

REQUIRE(userSettingTest.Get<Setting::ArchiveExtractionMethod>() == AppInstaller::Archive::ExtractionMethod::ShellApi);
}
SECTION("Shell api")
{
std::string_view json = R"({ "installBehavior": { "archiveExtractionMethod": "tar" } })";
SetSetting(Stream::PrimaryUserSettings, json);
UserSettingsTest userSettingTest;

REQUIRE(userSettingTest.Get<Setting::ArchiveExtractionMethod>() == AppInstaller::Archive::ExtractionMethod::Tar);
}
}

TEST_CASE("SettingsInstallScope", "[settings]")
Expand Down
3 changes: 1 addition & 2 deletions src/AppInstallerCommonCore/Archive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ namespace AppInstaller::Archive
return S_OK;
}


#ifndef AICLI_DISABLE_TEST_HOOKS
static bool* s_ScanArchiveResult_TestHook_Override = nullptr;

Expand Down Expand Up @@ -77,4 +76,4 @@ namespace AppInstaller::Archive

return scanResult == 0;
}
}
}
9 changes: 8 additions & 1 deletion src/AppInstallerCommonCore/Public/winget/Archive.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@

namespace AppInstaller::Archive
{
enum class ExtractionMethod
{
// Default archive extraction method is ShellApi.
ShellApi,
Tar,
};

HRESULT TryExtractArchive(const std::filesystem::path& archivePath, const std::filesystem::path& destPath);

bool ScanZipFile(const std::filesystem::path& zipPath);
}
}
Loading
Loading