Skip to content

Commit

Permalink
Use System32\tar.exe if present instead of 7zip for zip archives. (#406)
Browse files Browse the repository at this point in the history
* Use System32\tar.exe if present instead of 7zip for zip archives.

This is a more targeted hotfix version of #404 as there are outstanding code review discussions there. (I do still intend to apply the changes therein but don't want code reviewers to feel like they can't say anything because it's resolving a high priority issue)

* Apply Nicole CR feedback.

* Fix e2e test failure.
  • Loading branch information
BillyONeal authored Mar 3, 2022
1 parent ca46ecb commit 6ef3648
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 13 deletions.
2 changes: 2 additions & 0 deletions include/vcpkg/archives.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ namespace vcpkg
{
// Extract `archive` to `to_path` using `tar_tool`.
void extract_tar(const Path& tar_tool, const Path& archive, const Path& to_path);
// Extract `archive` to `to_path` using `cmake_tool`. (CMake's built in tar)
void extract_tar_cmake(const Path& cmake_tool, const Path& archive, const Path& to_path);
// Extract `archive` to `to_path`, deleting `to_path` first.
void extract_archive(const VcpkgPaths& paths, const Path& archive, const Path& to_path);

Expand Down
4 changes: 4 additions & 0 deletions include/vcpkg/base/system.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ namespace vcpkg

#ifdef _WIN32
const ExpectedS<Path>& get_appdata_local() noexcept;

const ExpectedS<Path>& get_system_root() noexcept;

const ExpectedS<Path>& get_system32() noexcept;
#endif

Optional<std::string> get_registry_string(void* base_hkey, StringView subkey, StringView valuename);
Expand Down
12 changes: 12 additions & 0 deletions include/vcpkg/fwd/tools.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#pragma once

namespace vcpkg
{
enum class RequireExactVersions
{
YES,
NO,
};

struct ToolCache;
}
9 changes: 2 additions & 7 deletions include/vcpkg/tools.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include <vcpkg/fwd/tools.h>
#include <vcpkg/fwd/vcpkgpaths.h>

#include <vcpkg/base/files.h>
Expand Down Expand Up @@ -30,15 +31,9 @@ namespace vcpkg
static const std::string IFW_REPOGEN = "ifw_repogen";
}

enum class RequireExactVersions
{
YES,
NO,
};

struct ToolCache
{
virtual ~ToolCache() { }
virtual ~ToolCache() = default;

virtual const Path& get_tool_path_from_system(const Filesystem& fs, const std::string& tool) const = 0;
virtual const Path& get_tool_path(const VcpkgPaths& paths, const std::string& tool) const = 0;
Expand Down
29 changes: 26 additions & 3 deletions src/vcpkg/archives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,23 @@ namespace
{
win32_extract_msi(archive, to_path);
}
else if (Strings::case_insensitive_ascii_equals(ext, ".zip") ||
Strings::case_insensitive_ascii_equals(ext, ".7z") ||
Strings::case_insensitive_ascii_equals(ext, ".exe"))
else if (Strings::case_insensitive_ascii_equals(ext, ".zip"))
{
const auto tar_path = get_system32().value_or_exit(VCPKG_LINE_INFO) / "tar.exe";
if (paths.get_filesystem().exists(tar_path, IgnoreErrors{}))
{
extract_tar(tar_path, archive, to_path);
}
else
{
win32_extract_with_seven_zip(paths, archive, to_path);
}
}
else if (Strings::case_insensitive_ascii_equals(ext, ".7z"))
{
extract_tar_cmake(paths.get_tool_exe(Tools::CMAKE), archive, to_path);
}
else if (Strings::case_insensitive_ascii_equals(ext, ".exe"))
{
win32_extract_with_seven_zip(paths, archive, to_path);
}
Expand Down Expand Up @@ -183,6 +197,15 @@ namespace vcpkg
Checks::check_exit(VCPKG_LINE_INFO, code == 0, "tar failed while extracting %s", archive);
}

void extract_tar_cmake(const Path& cmake_tool, const Path& archive, const Path& to_path)
{
// Note that CMake's built in tar can extract more archive types than many system tars; e.g. 7z
const auto code =
cmd_execute(Command{cmake_tool}.string_arg("-E").string_arg("tar").string_arg("xzf").path_arg(archive),
WorkingDirectory{to_path});
Checks::check_exit(VCPKG_LINE_INFO, code == 0, "tar failed while extracting %s", archive);
}

void extract_archive(const VcpkgPaths& paths, const Path& archive, const Path& to_path)
{
Filesystem& fs = paths.get_filesystem();
Expand Down
22 changes: 22 additions & 0 deletions src/vcpkg/base/system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,28 @@ namespace vcpkg
}();
return s_home;
}

const ExpectedS<Path>& get_system_root() noexcept
{
static const ExpectedS<Path> s_system_root = []() -> ExpectedS<Path> {
auto env = get_environment_variable("SystemRoot");
if (const auto p = env.get())
{
return Path(std::move(*p));
}
else
{
return std::string("Expected the SystemRoot environment variable to be always set on Windows.");
}
}();
return s_system_root;
}

const ExpectedS<Path>& get_system32() noexcept
{
static const ExpectedS<Path> s_system32 = get_system_root().map([](const Path& p) { return p / "System32"; });
return s_system32;
}
#else
static const ExpectedS<Path>& get_xdg_cache_home() noexcept
{
Expand Down
5 changes: 2 additions & 3 deletions src/vcpkg/base/system.process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,9 +260,8 @@ namespace vcpkg
Environment get_modified_clean_environment(const std::unordered_map<std::string, std::string>& extra_env,
StringView prepend_to_path)
{
static const std::string system_root_env =
get_environment_variable("SystemRoot").value_or_exit(VCPKG_LINE_INFO);
static const std::string system32_env = system_root_env + R"(\system32)";
const std::string& system_root_env = get_system_root().value_or_exit(VCPKG_LINE_INFO).native();
const std::string& system32_env = get_system32().value_or_exit(VCPKG_LINE_INFO).native();
std::string new_path = "PATH=";
if (!prepend_to_path.empty())
{
Expand Down

0 comments on commit 6ef3648

Please sign in to comment.