Refactor tools.cpp to homogenize system tools policies (#540)

* Refactor tools.cpp to homogenize system tools policies * Fix Windows build * Fix message map and formatting. * PR Comments * clang-format, const& Co-authored-by: Billy Robert O'Neal <bion@microsoft.com>
microsoft · May 19, 2022 · 0e37c3e · 0e37c3e
1 parent fb0fa51
commit 0e37c3e
Show file tree

Hide file tree

Showing 24 changed files with 733 additions and 518 deletions.
diff --git a/azure-pipelines/e2e_assets/fetch/ninja.bat b/azure-pipelines/e2e_assets/fetch/ninja.bat
@@ -0,0 +1 @@
+@echo 0.0.0
diff --git a/azure-pipelines/end-to-end-tests-dir/fetch.ps1 b/azure-pipelines/end-to-end-tests-dir/fetch.ps1
@@ -23,6 +23,13 @@ if (-not $IsMacOS -and -not $IsLinux) {
         <sha512>6004140d92e86afbb17b49c49037ccd0786ce238f340f7d0e62b4b0c29ed0d6ad0bab11feda2094ae849c387d70d63504393714ed0a1f4d3a1f155af7a4f1ba3</sha512>
         <archiveName>ninja-win-1.10.2.zip</archiveName>
     </tool>
+    <tool name="ninja" os="windows">
+        <version>1.10.2</version>
+        <exeRelativePath>ninja.exe</exeRelativePath>
+        <url>https://github.com/ninja-build/ninja/releases/download/v1.10.2/ninja-win.zip</url>
+        <sha512>6004140d92e86afbb17b49c49037ccd0786ce238f340f7d0e62b4b0c29ed0d6ad0bab11feda2094ae849c387d70d63504393714ed0a1f4d3a1f155af7a4f1ba3</sha512>
+        <archiveName>ninja-win-1.10.2.zip</archiveName>
+    </tool>
     <tool name="cmake" os="windows">
         <version>3.22.2</version>
         <exeRelativePath>cmake-3.22.2-windows-i386\bin\cmake.exe</exeRelativePath>
@@ -62,4 +69,30 @@ if (-not $IsMacOS -and -not $IsLinux) {
     Run-Vcpkg -TestArgs ($commonArgs + @("fetch", "ninja-testing", "--vcpkg-root=$TestingRoot"))
     Throw-IfFailed
     Require-FileExists "$TestingRoot/down loads/tools/ninja-testing-1.10.2-windows/ninja.exe"
+
+    $path = $env:PATH
+
+    $env:PATH = "$path;$TestingRoot/down loads/tools/ninja-testing-1.10.2-windows"
+    Run-Vcpkg -TestArgs ($commonArgs + @("fetch", "ninja", "--vcpkg-root=$TestingRoot"))
+    Throw-IfFailed
+    Require-FileNotExists "$TestingRoot/down loads/tools/ninja-1.10.2-windows/ninja.exe"
+
+    $env:VCPKG_FORCE_DOWNLOADED_BINARIES = "1"
+    Run-Vcpkg -TestArgs ($commonArgs + @("fetch", "ninja", "--vcpkg-root=$TestingRoot"))
+    Throw-IfFailed
+    Require-FileExists "$TestingRoot/down loads/tools/ninja-1.10.2-windows/ninja.exe"
+
+    Remove-Item -Recurse -Force "$TestingRoot/down loads/tools/ninja-1.10.2-windows" -ErrorAction SilentlyContinue
+    Remove-Item env:VCPKG_FORCE_DOWNLOADED_BINARIES
+
+    $env:VCPKG_FORCE_SYSTEM_BINARIES = "1"
+    $env:PATH = "$PSScriptRoot\..\e2e_assets\fetch;$path"
+    Run-Vcpkg -TestArgs ($commonArgs + @("fetch", "ninja", "--vcpkg-root=$TestingRoot"))
+    Throw-IfFailed
+    Require-FileNotExists "$TestingRoot/down loads/tools/ninja-1.10.2-windows/ninja.exe"
+
+    Remove-Item env:VCPKG_FORCE_SYSTEM_BINARIES
+    Run-Vcpkg -TestArgs ($commonArgs + @("fetch", "ninja", "--vcpkg-root=$TestingRoot"))
+    Throw-IfFailed
+    Require-FileExists "$TestingRoot/down loads/tools/ninja-1.10.2-windows/ninja.exe"
 }
diff --git a/azure-pipelines/end-to-end-tests.ps1 b/azure-pipelines/end-to-end-tests.ps1
@@ -74,6 +74,8 @@ $n = 1
 $m = $AllTests.Count
 
 $envvars_clear = @(
+    "VCPKG_FORCE_SYSTEM_BINARIES",
+    "VCPKG_FORCE_DOWNLOADED_BINARIES",
     "VCPKG_DEFAULT_HOST_TRIPLET",
     "VCPKG_DEFAULT_TRIPLET",
     "VCPKG_BINARY_SOURCES",
@@ -84,7 +86,7 @@ $envvars_clear = @(
     "VCPKG_FEATURE_FLAGS",
     "VCPKG_DISABLE_METRICS"
 )
-$envvars = $envvars_clear + @("VCPKG_DOWNLOADS", "X_VCPKG_REGISTRIES_CACHE")
+$envvars = $envvars_clear + @("VCPKG_DOWNLOADS", "X_VCPKG_REGISTRIES_CACHE", "PATH")
 
 foreach ($Test in $AllTests)
 {

diff --git a/include/vcpkg-test/util.h b/include/vcpkg-test/util.h
@@ -63,6 +63,12 @@ namespace Catch
     {
         static const std::string convert(const vcpkg::PackageSpec& value) { return value.to_string(); }
     };
+
+    template<>
+    struct StringMaker<vcpkg::Path>
+    {
+        static const std::string convert(const vcpkg::Path& value) { return "\"" + value.native() + "\""; }
+    };
 }
 
 namespace vcpkg

diff --git a/include/vcpkg/archives.h b/include/vcpkg/archives.h
@@ -7,27 +7,29 @@
 
 #include <vcpkg/base/files.h>
 
+#include <vcpkg/tools.h>
+
 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);
+    void extract_archive(Filesystem& fs, const ToolCache& tools, const Path& archive, const Path& to_path);
 
 #ifdef _WIN32
     // Extract the 7z archive part of a self extracting 7z installer
-    void win32_extract_self_extracting_7z(const VcpkgPaths& paths, const Path& archive, const Path& to_path);
+    void win32_extract_self_extracting_7z(Filesystem& fs, const Path& archive, const Path& to_path);
     // Extract `archive` to `to_path`, deleting `to_path` first. `archive` must be a zip file.
     // This function will use potentially less performant tools that are reliably available on any machine.
-    void win32_extract_bootstrap_zip(const VcpkgPaths& paths, const Path& archive, const Path& to_path);
+    void win32_extract_bootstrap_zip(Filesystem& fs, const ToolCache& tools, const Path& archive, const Path& to_path);
 #endif
 
     // Compress the source directory into the destination file.
-    int compress_directory_to_zip(const VcpkgPaths& paths, const Path& source, const Path& destination);
+    int compress_directory_to_zip(Filesystem& fs, const ToolCache& tools, const Path& source, const Path& destination);
 
-    Command decompress_zip_archive_cmd(const VcpkgPaths& paths, const Path& dst, const Path& archive_path);
+    Command decompress_zip_archive_cmd(const ToolCache& tools, const Path& dst, const Path& archive_path);
 
     std::vector<ExitCodeAndOutput> decompress_in_parallel(View<Command> jobs);
 }
diff --git a/include/vcpkg/base/downloads.h b/include/vcpkg/base/downloads.h
@@ -1,6 +1,8 @@
 #pragma once
 
-#include <vcpkg/base/expected.h>
+#include <vcpkg/base/fwd/downloads.h>
+#include <vcpkg/base/fwd/expected.h>
+
 #include <vcpkg/base/files.h>
 #include <vcpkg/base/optional.h>
 #include <vcpkg/base/view.h>

diff --git a/include/vcpkg/base/fwd/downloads.h b/include/vcpkg/base/fwd/downloads.h
@@ -0,0 +1,7 @@
+#pragma once
+
+namespace vcpkg
+{
+    struct DownloadManager;
+    struct DownloadManagerConfig;
+}
diff --git a/include/vcpkg/base/messages.h b/include/vcpkg/base/messages.h
@@ -279,6 +279,7 @@ namespace vcpkg::msg
     DECLARE_MSG_ARG(vendor, "Azure");
     DECLARE_MSG_ARG(version, "1.3.8");
     DECLARE_MSG_ARG(action_index, "340");
+    DECLARE_MSG_ARG(env_var, "VCPKG_DEFAULT_TRIPLET");
 #undef DECLARE_MSG_ARG
 
 // These are `...` instead of

diff --git a/include/vcpkg/tools.h b/include/vcpkg/tools.h
@@ -1,11 +1,11 @@
 #pragma once
 
+#include <vcpkg/base/fwd/downloads.h>
+#include <vcpkg/base/fwd/expected.h>
 #include <vcpkg/base/fwd/files.h>
 
 #include <vcpkg/fwd/tools.h>
-#include <vcpkg/fwd/vcpkgpaths.h>
 
-#include <vcpkg/base/optional.h>
 #include <vcpkg/base/stringview.h>
 
 #include <string>
@@ -31,8 +31,6 @@ namespace vcpkg
         static constexpr StringLiteral ARIA2 = "aria2";
         static constexpr StringLiteral NODE = "node";
         static constexpr StringLiteral IFW_INSTALLER_BASE = "ifw_installerbase";
-        static constexpr StringLiteral IFW_BINARYCREATOR = "ifw_binarycreator";
-        static constexpr StringLiteral IFW_REPOGEN = "ifw_repogen";
         // This duplicate of 7zip uses msiexec to unpack, which is a fallback for Windows 7.
         static constexpr StringLiteral SEVEN_ZIP_MSI = "7zip_msi";
     }
@@ -41,12 +39,16 @@ namespace vcpkg
     {
         virtual ~ToolCache() = default;
 
-        virtual const Path& get_tool_path_from_system(const Filesystem& fs, StringView tool) const = 0;
-        virtual const Path& get_tool_path(const VcpkgPaths& paths, StringView tool) const = 0;
-        virtual const std::string& get_tool_version(const VcpkgPaths& paths, StringView tool) const = 0;
+        virtual const Path& get_tool_path(StringView tool) const = 0;
+        virtual const std::string& get_tool_version(StringView tool) const = 0;
     };
 
-    Optional<std::array<int, 3>> parse_tool_version_string(StringView string_version);
+    ExpectedL<Path> find_system_tar(const Filesystem& fs);
 
-    std::unique_ptr<ToolCache> get_tool_cache(RequireExactVersions abiToolVersionHandling);
+    std::unique_ptr<ToolCache> get_tool_cache(Filesystem& fs,
+                                              std::shared_ptr<const DownloadManager> downloader,
+                                              Path downloads,
+                                              Path xml_config,
+                                              Path tools,
+                                              RequireExactVersions abiToolVersionHandling);
 }
diff --git a/include/vcpkg/tools.test.h b/include/vcpkg/tools.test.h
@@ -0,0 +1,35 @@
+#pragma once
+
+#include <vcpkg/base/fwd/expected.h>
+#include <vcpkg/base/fwd/optional.h>
+#include <vcpkg/base/fwd/stringview.h>
+
+#include <vcpkg/base/files.h>
+
+#include <array>
+#include <string>
+
+namespace vcpkg
+{
+    struct ToolData
+    {
+        std::string name;
+
+        std::array<int, 3> version;
+        // relative path inside tool_dir_subpath
+        Path exe_subpath;
+        std::string url;
+        // relative path from paths.downloads
+        Path download_subpath;
+        bool is_archive;
+        // relative path from paths.tools
+        Path tool_dir_subpath;
+        std::string sha512;
+
+        Path exe_path(const Path& tools_base_path) const { return tools_base_path / tool_dir_subpath / exe_subpath; }
+    };
+
+    Optional<ToolData> parse_tool_data_from_xml(StringView XML, StringView XML_PATH, StringView tool, StringView os);
+
+    Optional<std::array<int, 3>> parse_tool_version_string(StringView string_version);
+}
diff --git a/include/vcpkg/vcpkgpaths.h b/include/vcpkg/vcpkgpaths.h
@@ -1,11 +1,13 @@
 #pragma once
 
+#include <vcpkg/base/fwd/downloads.h>
 #include <vcpkg/base/fwd/json.h>
 #include <vcpkg/base/fwd/system.process.h>
 
 #include <vcpkg/fwd/configuration.h>
 #include <vcpkg/fwd/installedpaths.h>
 #include <vcpkg/fwd/registries.h>
+#include <vcpkg/fwd/tools.h>
 #include <vcpkg/fwd/vcpkgcmdarguments.h>
 #include <vcpkg/fwd/vcpkgpaths.h>
 
@@ -40,8 +42,6 @@ namespace vcpkg
         std::vector<ToolsetArchOption> supported_architectures;
     };
 
-    struct DownloadManager;
-
     namespace Build
     {
         struct PreBuildInfo;
@@ -114,20 +114,21 @@ namespace vcpkg
         const std::unique_ptr<details::VcpkgPathsImpl> m_pimpl;
 
     public:
+        const Path& scripts;
+        const Path& downloads;
+        const Path& tools;
         const Path builtin_registry_versions;
-        const Path scripts;
         const Path prefab;
         const Path buildsystems;
         const Path buildsystems_msbuild_targets;
         const Path buildsystems_msbuild_props;
-        const Path downloads;
-        const Path tools;
         const Path ports_cmake;
         const Path triplets;
         const Path community_triplets;
 
         std::string get_toolver_diagnostics() const;
 
+        const ToolCache& get_tool_cache() const;
         const Path& get_tool_exe(StringView tool) const;
         const std::string& get_tool_version(StringView tool) const;
 

diff --git a/locales/messages.en.json b/locales/messages.en.json
@@ -66,9 +66,10 @@
   "CiBaselineRegressionHeader": "REGRESSIONS:",
   "CiBaselineUnexpectedPass": "PASSING, REMOVE FROM FAIL LIST: {spec} ({path}).",
   "CmakeTargetsExcluded": "note: {count} additional targets are not displayed.",
-  "CommandFailed": "command:\n{command_line}\nfailed with the following results:\n{list}\n",
+  "CommandFailed": "command:\n{command_line}\nfailed with the following results:\n",
   "CouldNotDeduceNugetIdAndVersion": "Could not deduce nuget id and version from filename: {path}",
   "CurlReportedUnexpectedResults": "curl has reported unexpected results to vcpkg and vcpkg cannot continue.\nPlease review the following text for sensitive information and open an issue on the Microsoft/vcpkg GitHub to help fix this problem!\ncmd: {command_line}\n=== curl output ===\n{actual}\n=== end curl output ===\n",
+  "DownloadAvailable": "A downloadable copy of this tool is available and can be used by unsetting {env_var}.",
   "DownloadedSources": "Downloaded sources for {spec}",
   "DownloadingVcpkgCeBundle": "Downloading vcpkg-ce bundle {version}...",
   "DownloadingVcpkgCeBundleLatest": "Downloading latest vcpkg-ce bundle...",
@@ -111,6 +112,9 @@
   "HeaderOnlyUsage": "{package_name} is header-only and can be used from CMake via:",
   "IllegalFeatures": "error: List of features is not allowed in this context",
   "IllegalPlatformSpec": "error: Platform qualifier is not allowed in this context",
+  "InstallWithSystemManager": "You may be able to install this tool via your system package manager.",
+  "InstallWithSystemManagerMono": "Ubuntu 18.04 users may need a newer version of mono, available at {url}.",
+  "InstallWithSystemManagerPkg": "You may be able to install this tool via your system package manager ({command_line}).",
   "InstallingPackage": "Installing {action_index}/{count} {spec}...",
   "InternalErrorMessage": "internal error: ",
   "InternalErrorMessageContact": "Please open an issue at https://github.com/microsoft/vcpkg/issues/new?template=other-type-of-bug-report.md&labels=category:vcpkg-bug with detailed steps to reproduce the problem.",
@@ -159,9 +163,12 @@
   "ResultsHeader": "RESULTS",
   "SeeURL": "See {url} for more information.",
   "SourceFieldPortNameMismatch": "The 'Source' field inside the CONTROL file, or \"name\" field inside the vcpkg.json file has the name {package_name} and does not match the port directory {path}.",
+  "ToolFetchFailed": "Could not fetch {tool_name}.",
+  "ToolInWin10": "This utility is bundled with Windows 10 or later.",
   "UnexpectedErrorDuringBulkDownload": "an unexpected error occurred during bulk download.",
   "UnknownBaselineFileContent": "unrecognizable baseline entry; expected 'port:triplet=(fail|skip)'",
   "UnknownBinaryProviderType": "unknown binary provider type: valid providers are 'clear', 'default', 'nuget', 'nugetconfig', 'interactive', and 'files'",
+  "UnknownTool": "vcpkg does not have a definition of this tool for this platform.",
   "UnsupportedSystemName": "Error: Could not map VCPKG_CMAKE_SYSTEM_NAME '{system_name}' to a vcvarsall platform. Supported system names are '', 'Windows' and 'WindowsStore'.",
   "UnsupportedToolchain": "Error: in triplet {triplet}: Unable to find a valid toolchain combination.\n    The requested target architecture was {arch}\n    The selected Visual Studio instance is at {path}\n    The available toolchain combinations are {list}\n",
   "UpdateBaselineAddBaselineNoManifest": "the --{option} switch was passed, but there is no manifest file to add a `builtin-baseline` field to.",

diff --git a/locales/messages.json b/locales/messages.json
@@ -121,10 +121,14 @@
   "_CiBaselineUnexpectedPass.comment": "example of {spec} is 'zlib:x64-windows'.\nexample of {path} is '/foo/bar'.\n",
   "CmakeTargetsExcluded": "note: {count} additional targets are not displayed.",
   "_CmakeTargetsExcluded.comment": "example of {count} is '42'.\n",
+  "CommandFailed": "command:\n{command_line}\nfailed with the following results:\n",
+  "_CommandFailed.comment": "example of {command_line} is 'vcpkg install zlib'.\n",
   "CouldNotDeduceNugetIdAndVersion": "Could not deduce nuget id and version from filename: {path}",
   "_CouldNotDeduceNugetIdAndVersion.comment": "example of {path} is '/foo/bar'.\n",
   "CurlReportedUnexpectedResults": "curl has reported unexpected results to vcpkg and vcpkg cannot continue.\nPlease review the following text for sensitive information and open an issue on the Microsoft/vcpkg GitHub to help fix this problem!\ncmd: {command_line}\n=== curl output ===\n{actual}\n=== end curl output ===\n",
   "_CurlReportedUnexpectedResults.comment": "{command_line} is the command line to call curl.exe, {actual} is the console output of curl.exe locale-invariant download results.\nexample of {command_line} is 'vcpkg install zlib'.\n",
+  "DownloadAvailable": "A downloadable copy of this tool is available and can be used by unsetting {env_var}.",
+  "_DownloadAvailable.comment": "example of {env_var} is 'VCPKG_DEFAULT_TRIPLET'.\n",
   "DownloadedSources": "Downloaded sources for {spec}",
   "_DownloadedSources.comment": "example of {spec} is 'zlib:x64-windows'.\n",
   "DownloadingVcpkgCeBundle": "Downloading vcpkg-ce bundle {version}...",
@@ -191,6 +195,11 @@
   "_HeaderOnlyUsage.comment": "'header' refers to C/C++ .h files\nexample of {package_name} is 'zlib'.\n",
   "IllegalFeatures": "error: List of features is not allowed in this context",
   "IllegalPlatformSpec": "error: Platform qualifier is not allowed in this context",
+  "InstallWithSystemManager": "You may be able to install this tool via your system package manager.",
+  "InstallWithSystemManagerMono": "Ubuntu 18.04 users may need a newer version of mono, available at {url}.",
+  "_InstallWithSystemManagerMono.comment": "example of {url} is 'https://github.com/microsoft/vcpkg'.\n",
+  "InstallWithSystemManagerPkg": "You may be able to install this tool via your system package manager ({command_line}).",
+  "_InstallWithSystemManagerPkg.comment": "example of {command_line} is 'vcpkg install zlib'.\n",
   "InstallingPackage": "Installing {action_index}/{count} {spec}...",
   "_InstallingPackage.comment": "example of {action_index} is '340'.\nexample of {count} is '42'.\nexample of {spec} is 'zlib:x64-windows'.\n",
   "InternalErrorMessage": "internal error: ",
@@ -273,9 +282,13 @@
   "_SeeURL.comment": "example of {url} is 'https://github.com/microsoft/vcpkg'.\n",
   "SourceFieldPortNameMismatch": "The 'Source' field inside the CONTROL file, or \"name\" field inside the vcpkg.json file has the name {package_name} and does not match the port directory {path}.",
   "_SourceFieldPortNameMismatch.comment": "{package_name} and {path} are both names of installable ports/packages. 'Source', 'CONTROL', 'vcpkg.json', and 'name' references are locale-invariant.\nexample of {package_name} is 'zlib'.\nexample of {path} is '/foo/bar'.\n",
+  "ToolFetchFailed": "Could not fetch {tool_name}.",
+  "_ToolFetchFailed.comment": "example of {tool_name} is 'aria2'.\n",
+  "ToolInWin10": "This utility is bundled with Windows 10 or later.",
   "UnexpectedErrorDuringBulkDownload": "an unexpected error occurred during bulk download.",
   "UnknownBaselineFileContent": "unrecognizable baseline entry; expected 'port:triplet=(fail|skip)'",
   "UnknownBinaryProviderType": "unknown binary provider type: valid providers are 'clear', 'default', 'nuget', 'nugetconfig', 'interactive', and 'files'",
+  "UnknownTool": "vcpkg does not have a definition of this tool for this platform.",
   "UnsupportedSystemName": "Error: Could not map VCPKG_CMAKE_SYSTEM_NAME '{system_name}' to a vcvarsall platform. Supported system names are '', 'Windows' and 'WindowsStore'.",
   "_UnsupportedSystemName.comment": "example of {system_name} is 'Darwin'.\n",
   "UnsupportedToolchain": "Error: in triplet {triplet}: Unable to find a valid toolchain combination.\n    The requested target architecture was {arch}\n    The selected Visual Studio instance is at {path}\n    The available toolchain combinations are {list}\n",

diff --git a/src/vcpkg-test/downloads.cpp b/src/vcpkg-test/downloads.cpp
@@ -1,6 +1,7 @@
 #include <catch2/catch.hpp>
 
 #include <vcpkg/base/downloads.h>
+#include <vcpkg/base/expected.h>
 
 using namespace vcpkg;