Просмотр исходного кода

Port //toolchain/install to new filesystem library (#5905)

This removes a bunch of manual filesystem helpers and complexity that
are directly provided by the new library.

It also moves all of the install paths detection to use
`std::filesystem::path` instead of the LLVM path library. The goal is to
consolidate all our logic onto a single stack, and the standard one
seems the best for that purpose. This does give up some of the
optimizations of this code to avoid memory allocation, but in practice
that likely isn't a critical issue. And with the new filesystem library
we can likely do more to avoid that by using directory-object-relative
filesystem access. However, that will have to wait for moving more parts
of the toolchain over to use this set of filesystem abstractions. There
is a related TODO left in the manifest handling code.
Chandler Carruth 8 месяцев назад
Родитель
Сommit
2e509e9103

+ 1 - 1
toolchain/driver/clang_runner_test.cpp

@@ -55,7 +55,7 @@ TEST(ClangRunnerTest, Version) {
   EXPECT_THAT(out, HasSubstr((llvm::Twine("Target: ") + target).str()));
   // Clang's install should be our private LLVM install bin directory.
   EXPECT_THAT(out, HasSubstr(std::string("InstalledDir: ") +
-                             install_paths.llvm_install_bin()));
+                             install_paths.llvm_install_bin().native()));
 }
 
 // It's hard to write a portable and reliable unittest for all the layers of the

+ 5 - 0
toolchain/install/BUILD

@@ -31,6 +31,7 @@ cc_library(
     deps = [
         "//common:check",
         "//common:error",
+        "//common:filesystem",
         "//toolchain/base:llvm_tools",
         "@bazel_tools//tools/cpp/runfiles",
         "@llvm-project//llvm:Support",
@@ -55,6 +56,8 @@ cc_test(
     deps = [
         ":install_paths",
         "//common:check",
+        "//common:error_test_helpers",
+        "//common:filesystem",
         "//common:ostream",
         "//testing/base:global_exe_path",
         "//testing/base:gtest_main",
@@ -83,6 +86,7 @@ cc_library(
     deps = [
         "//common:error",
         "//common:exe_path",
+        "//common:filesystem",
         "@llvm-project//llvm:Support",
     ],
 )
@@ -94,6 +98,7 @@ cc_test(
     deps = [
         ":busybox_info",
         "//common:check",
+        "//common:filesystem",
         "//testing/base:gtest_main",
         "@googletest//:gtest",
         "@llvm-project//llvm:Support",

+ 8 - 7
toolchain/install/busybox_info.cpp

@@ -7,6 +7,7 @@
 #include <iterator>
 
 #include "common/exe_path.h"
+#include "common/filesystem.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace Carbon {
@@ -46,7 +47,8 @@ auto GetBusyboxInfo(const char* argv0) -> ErrorOr<BusyboxInfo> {
       // handled in the other return path.
       std::string prefix_root = info.bin_path.parent_path().string() +
                                 "/prefix_root/lib/carbon/carbon-busybox";
-      if (std::filesystem::exists(prefix_root)) {
+      if (auto access = Filesystem::Cwd().Access(prefix_root);
+          access.ok() && *access) {
         info.bin_path = prefix_root;
       }
       return info;
@@ -75,8 +77,8 @@ auto GetBusyboxInfo(const char* argv0) -> ErrorOr<BusyboxInfo> {
                           ? parent_path / ".." / "lib" / "carbon"
                           : parent_path / ".." / "..";
       auto busybox_path = lib_path / "carbon-busybox";
-      std::error_code ec;
-      if (std::filesystem::exists(busybox_path, ec)) {
+      if (auto access = Filesystem::Cwd().Access(busybox_path);
+          access.ok() && *access) {
         info.bin_path = busybox_path;
         return info;
       }
@@ -84,15 +86,14 @@ auto GetBusyboxInfo(const char* argv0) -> ErrorOr<BusyboxInfo> {
 
     // Try to walk through another layer of symlinks and see if we can find the
     // installation there or are linked directly to the busybox.
-    std::error_code ec;
-    auto symlink_target = std::filesystem::read_symlink(info.bin_path, ec);
-    if (ec) {
+    auto readlink = Filesystem::Cwd().Readlink(info.bin_path);
+    if (!readlink.ok()) {
       return ErrorBuilder()
              << "expected carbon-busybox symlink at `" << info.bin_path << "`";
     }
 
     // Do a path join, to handle relative symlinks.
-    info.bin_path = parent_path / symlink_target;
+    info.bin_path = parent_path / *readlink;
   }
 }
 

+ 90 - 129
toolchain/install/busybox_info_test.cpp

@@ -8,11 +8,13 @@
 #include <gtest/gtest.h>
 
 #include <cstdlib>
+#include <filesystem>
 #include <fstream>
 #include <optional>
 #include <system_error>
 
 #include "common/check.h"
+#include "common/filesystem.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/FileSystem.h"
 
@@ -23,148 +25,104 @@ using ::testing::Eq;
 
 class BusyboxInfoTest : public ::testing::Test {
  public:
-  explicit BusyboxInfoTest() {
-    // Set up a temp directory for the test case.
-    const char* tmpdir = std::getenv("TEST_TMPDIR");
-    CARBON_CHECK(tmpdir);
-    dir_ = MakeDir(std::filesystem::absolute(
-        std::filesystem::path(tmpdir) /
-        ::testing::UnitTest::GetInstance()->current_test_info()->name()));
-
+  explicit BusyboxInfoTest()
+      : dir_(std::move(*Filesystem::MakeTmpDir())), path_(dir_.abs_path()) {
     // Most tests need the running binary for `MakeBusyboxFile`.
     static int static_for_main_addr;
     running_binary_ = llvm::sys::fs::getMainExecutable("busybox_info_test",
                                                        &static_for_main_addr);
   }
 
-  // Delete the test case's temp directory.
-  ~BusyboxInfoTest() override {
-    std::error_code ec;
-    std::filesystem::remove_all(dir_, ec);
-    CARBON_CHECK(!ec, "error removing {0}: {1}", dir_, ec.message());
-  }
-
-  // Creates a stub file. Returns the input file for easier use.
-  auto MakeFile(std::filesystem::path file) -> std::filesystem::path {
-    std::ofstream out(file.c_str());
-    out << "stub";
-    CARBON_CHECK(out, "error creating {0}", file);
-    return file;
-  }
-
-  // Creates a symlink to the target. Returns the input file for easier use.
-  auto MakeSymlink(std::filesystem::path file, auto target)
-      -> std::filesystem::path {
-    std::error_code ec;
-    std::filesystem::create_symlink(target, file, ec);
-    CARBON_CHECK(!ec, "error creating {0}: {1}", file, ec.message());
-    return file;
-  }
-
-  // Creates a directory, recursively if needed. Returns the input file for
-  // easier use.
-  auto MakeDir(std::filesystem::path dir) -> std::filesystem::path {
-    std::error_code ec;
-    std::filesystem::create_directories(dir, ec);
-    CARBON_CHECK(!ec, "error creating {0}: {1}", dir, ec.message());
-    return dir;
-  }
-
   // Creates a synthetic install tree to test a batch of interactions.
   // Optionally accepts a symlink target for the busybox in the install tree.
   // Returns the input prefix for easy use.
   auto MakeInstallTree(std::filesystem::path prefix,
                        std::optional<std::filesystem::path> busybox_target = {})
       -> std::filesystem::path {
-    MakeDir(prefix / "lib/carbon");
+    Filesystem::Dir prefix_dir = *dir_.CreateDirectories(prefix);
+    Filesystem::Dir lib_carbon = *prefix_dir.CreateDirectories("lib/carbon");
     if (busybox_target) {
-      MakeSymlink(prefix / "lib/carbon/carbon-busybox", *busybox_target);
+      lib_carbon.Symlink("carbon-busybox", busybox_target->native()).Check();
     } else {
-      MakeBusyboxFile(prefix / "lib/carbon/carbon-busybox");
+      lib_carbon.Symlink("carbon-busybox", running_binary_).Check();
     }
-    MakeDir(prefix / "lib/carbon/llvm/bin");
-    MakeSymlink(prefix / "lib/carbon/llvm/bin/clang++", "clang");
-    MakeSymlink(prefix / "lib/carbon/llvm/bin/clang", "../../carbon-busybox");
-    MakeDir(prefix / "bin");
-    MakeSymlink(prefix / "bin/carbon", "../lib/carbon/carbon-busybox");
-    return prefix;
-  }
-
-  // Makes a fake busybox file. This is a symlink to the running binary because
-  // we validate to make sure the running binary is one and the same.
-  auto MakeBusyboxFile(std::filesystem::path file) -> std::filesystem::path {
-    return MakeSymlink(file, running_binary_);
+    Filesystem::Dir llvm_bin = *lib_carbon.CreateDirectories("llvm/bin");
+    llvm_bin.Symlink("clang++", "clang").Check();
+    llvm_bin.Symlink("clang", "../../carbon-busybox").Check();
+    Filesystem::Dir bin = *prefix_dir.OpenDir("bin", Filesystem::CreateNew);
+    bin.Symlink("carbon", "../lib/carbon/carbon-busybox").Check();
+    return path_ / prefix;
   }
 
   // The path to the running binary, `busybox_info_test`. This is provided
   // because `GetExecutablePath` can fall back to it.
   std::string running_binary_;
 
-  // The test's temp directory, deleted on destruction.
-  std::filesystem::path dir_;
+  Filesystem::RemovingDir dir_;
+  std::filesystem::path path_;
 };
 
 TEST_F(BusyboxInfoTest, Direct) {
-  auto busybox = MakeBusyboxFile(dir_ / "carbon-busybox");
+  dir_.Symlink("carbon-busybox", running_binary_).Check();
 
-  auto info = GetBusyboxInfo(busybox.c_str());
+  auto info = GetBusyboxInfo((path_ / "carbon-busybox").c_str());
   ASSERT_TRUE(info.ok()) << info.error();
-  EXPECT_THAT(info->bin_path, Eq(busybox));
+  EXPECT_THAT(info->bin_path, Eq(path_ / "carbon-busybox"));
   EXPECT_THAT(info->mode, Eq(std::nullopt));
 }
 
 TEST_F(BusyboxInfoTest, SymlinkInCurrentDirectory) {
-  MakeBusyboxFile(dir_ / "carbon-busybox");
-  auto target = MakeSymlink(dir_ / "carbon", "carbon-busybox");
+  dir_.Symlink("carbon-busybox", running_binary_).Check();
+  dir_.Symlink("carbon", "carbon-busybox").Check();
 
-  auto info = GetBusyboxInfo(target.c_str());
+  auto info = GetBusyboxInfo((path_ / "carbon").c_str());
   ASSERT_TRUE(info.ok()) << info.error();
-  EXPECT_THAT(info->bin_path, Eq(dir_ / "carbon-busybox"));
+  EXPECT_THAT(info->bin_path, Eq(path_ / "carbon-busybox"));
   EXPECT_THAT(info->mode, Eq(std::nullopt));
 }
 
 TEST_F(BusyboxInfoTest, SymlinkInCurrentDirectoryWithDot) {
-  MakeBusyboxFile(dir_ / "carbon-busybox");
-  auto target = MakeSymlink(dir_ / "carbon", "./carbon-busybox");
+  dir_.Symlink("carbon-busybox", running_binary_).Check();
+  dir_.Symlink("carbon", "./carbon-busybox").Check();
 
-  auto info = GetBusyboxInfo(target.c_str());
+  auto info = GetBusyboxInfo((path_ / "carbon").c_str());
   ASSERT_TRUE(info.ok()) << info.error();
-  EXPECT_THAT(info->bin_path, Eq(dir_ / "./carbon-busybox"));
+  EXPECT_THAT(info->bin_path, Eq(path_ / "./carbon-busybox"));
   EXPECT_THAT(info->mode, Eq(std::nullopt));
 }
 
 TEST_F(BusyboxInfoTest, ExtraSymlink) {
-  MakeBusyboxFile(dir_ / "carbon-busybox");
-  MakeSymlink(dir_ / "c", "carbon-busybox");
-  auto target = MakeSymlink(dir_ / "carbon", "c");
+  dir_.Symlink("carbon-busybox", running_binary_).Check();
+  dir_.Symlink("c", "carbon-busybox").Check();
+  dir_.Symlink("carbon", "c").Check();
 
-  auto info = GetBusyboxInfo(target.c_str());
+  auto info = GetBusyboxInfo((path_ / "carbon").c_str());
   ASSERT_TRUE(info.ok()) << info.error();
-  EXPECT_THAT(info->bin_path, Eq(dir_ / "carbon-busybox"));
+  EXPECT_THAT(info->bin_path, Eq(path_ / "carbon-busybox"));
   EXPECT_THAT(info->mode, Eq(std::nullopt));
 }
 
 TEST_F(BusyboxInfoTest, OriginalSymlinkNameFormsMode) {
-  MakeBusyboxFile(dir_ / "carbon-busybox");
-  MakeSymlink(dir_ / "carbon", "carbon-busybox");
-  auto clang_target = MakeSymlink(dir_ / "clang", "carbon");
-  auto clang_plusplus_target = MakeSymlink(dir_ / "clang++", "clang");
+  dir_.Symlink("carbon-busybox", running_binary_).Check();
+  dir_.Symlink("carbon", "carbon-busybox").Check();
+  dir_.Symlink("clang", "carbon").Check();
+  dir_.Symlink("clang++", "clang").Check();
 
-  auto info = GetBusyboxInfo(clang_target.c_str());
+  auto info = GetBusyboxInfo((path_ / "clang").c_str());
   ASSERT_TRUE(info.ok()) << info.error();
-  EXPECT_THAT(info->bin_path, Eq(dir_ / "carbon-busybox"));
+  EXPECT_THAT(info->bin_path, Eq(path_ / "carbon-busybox"));
   EXPECT_THAT(info->mode, Eq("clang"));
 
-  info = GetBusyboxInfo(clang_plusplus_target.c_str());
+  info = GetBusyboxInfo((path_ / "clang++").c_str());
   ASSERT_TRUE(info.ok()) << info.error();
-  EXPECT_THAT(info->bin_path, Eq(dir_ / "carbon-busybox"));
+  EXPECT_THAT(info->bin_path, Eq(path_ / "carbon-busybox"));
   EXPECT_THAT(info->mode, Eq("clang++"));
 }
 
 TEST_F(BusyboxInfoTest, BusyboxIsSymlinkToNowhere) {
-  auto target = MakeSymlink(dir_ / "carbon-busybox", "nonexistent");
+  dir_.Symlink("carbon-busybox", "nonexistent").Check();
 
-  auto info = GetBusyboxInfo(target.c_str());
+  auto info = GetBusyboxInfo((path_ / "carbon-busybox").c_str());
   ASSERT_FALSE(info.ok());
   EXPECT_THAT(info.error().message(),
               Eq(llvm::formatv("expected carbon-busybox symlink at `{0}`",
@@ -175,9 +133,9 @@ TEST_F(BusyboxInfoTest, BusyboxIsSymlinkToNowhere) {
 TEST_F(BusyboxInfoTest, BusyboxIsWrongFile) {
   // This has the correct name, but it doesn't map back to the running binary
   // and so is ignored.
-  auto target = MakeFile(dir_ / "carbon-busybox");
+  dir_.WriteFileFromString("carbon-busybox", "stub").Check();
 
-  auto info = GetBusyboxInfo(target.c_str());
+  auto info = GetBusyboxInfo((path_ / "carbon-busybox").c_str());
   ASSERT_FALSE(info.ok());
   EXPECT_THAT(info.error().message(),
               Eq(llvm::formatv("expected carbon-busybox symlink at `{0}`",
@@ -186,52 +144,52 @@ TEST_F(BusyboxInfoTest, BusyboxIsWrongFile) {
 }
 
 TEST_F(BusyboxInfoTest, RelativeSymlink) {
-  MakeDir(dir_ / "dir1");
-  MakeBusyboxFile(dir_ / "dir1/carbon-busybox");
-  MakeDir(dir_ / "dir2");
-  auto target = MakeSymlink(dir_ / "dir2/carbon", "../dir1/carbon-busybox");
+  Filesystem::Dir d1 = *dir_.OpenDir("dir1", Filesystem::CreateNew);
+  d1.Symlink("carbon-busybox", running_binary_).Check();
+  Filesystem::Dir d2 = *dir_.OpenDir("dir2", Filesystem::CreateNew);
+  d2.Symlink("carbon", "../dir1/carbon-busybox").Check();
 
-  auto info = GetBusyboxInfo(target.c_str());
+  auto info = GetBusyboxInfo((path_ / "dir2/carbon").c_str());
   ASSERT_TRUE(info.ok()) << info.error();
-  EXPECT_THAT(info->bin_path, Eq(dir_ / "dir2/../dir1/carbon-busybox"));
+  EXPECT_THAT(info->bin_path, Eq(path_ / "dir2/../dir1/carbon-busybox"));
   EXPECT_THAT(info->mode, Eq(std::nullopt));
 }
 
 TEST_F(BusyboxInfoTest, AbsoluteSymlink) {
-  MakeDir(dir_ / "dir1");
-  auto busybox = MakeBusyboxFile(dir_ / "dir1/carbon-busybox");
-  ASSERT_TRUE(busybox.is_absolute());
-  MakeDir(dir_ / "dir2");
-  auto target = MakeSymlink(dir_ / "dir2/carbon", busybox);
+  Filesystem::Dir d1 = *dir_.OpenDir("dir1", Filesystem::CreateNew);
+  d1.Symlink("carbon-busybox", running_binary_).Check();
+  Filesystem::Dir d2 = *dir_.OpenDir("dir2", Filesystem::CreateNew);
+  ASSERT_TRUE(path_.is_absolute());
+  d2.Symlink("carbon", (path_ / "dir1/carbon-busybox")).Check();
 
-  auto info = GetBusyboxInfo(target.c_str());
+  auto info = GetBusyboxInfo((path_ / "dir2/carbon").c_str());
   ASSERT_TRUE(info.ok()) << info.error();
-  EXPECT_THAT(info->bin_path, Eq(busybox));
+  EXPECT_THAT(info->bin_path, Eq(path_ / "dir1/carbon-busybox"));
   EXPECT_THAT(info->mode, Eq(std::nullopt));
 }
 
 TEST_F(BusyboxInfoTest, NotBusyboxFile) {
-  auto target = MakeFile(dir_ / "file");
+  dir_.WriteFileFromString("file", "stub").Check();
 
-  auto info = GetBusyboxInfo(target.c_str());
+  auto info = GetBusyboxInfo((path_ / "file").c_str());
   EXPECT_FALSE(info.ok());
 }
 
 TEST_F(BusyboxInfoTest, NotBusyboxSymlink) {
-  MakeFile(dir_ / "file");
-  auto target = MakeSymlink(dir_ / "carbon", "file");
+  dir_.WriteFileFromString("file", "stub").Check();
+  dir_.Symlink("carbon", "file").Check();
 
-  auto info = GetBusyboxInfo(target.c_str());
+  auto info = GetBusyboxInfo((path_ / "carbon").c_str());
   EXPECT_FALSE(info.ok());
 }
 
 TEST_F(BusyboxInfoTest, LayerSymlinksInstallTree) {
-  auto actual_busybox = MakeBusyboxFile(dir_ / "actual-busybox");
+  dir_.Symlink("actual-busybox", running_binary_).Check();
 
   // Create a facsimile of the install prefix with even the busybox as a
   // symlink. Also include potential relative sibling symlinks like `clang++` to
   // `clang`.
-  auto prefix = MakeInstallTree(dir_ / "test_prefix", actual_busybox);
+  auto prefix = MakeInstallTree("test_prefix", (path_ / "actual-busybox"));
 
   auto info = GetBusyboxInfo((prefix / "bin/carbon").c_str());
   ASSERT_TRUE(info.ok()) << info.error();
@@ -253,34 +211,36 @@ TEST_F(BusyboxInfoTest, LayerSymlinksInstallTree) {
 
 TEST_F(BusyboxInfoTest, StopSearchAtFirstSymlinkWithRelativeBusybox) {
   // Some install of Carbon under `opt`.
-  auto opt_prefix = MakeInstallTree(dir_ / "opt");
+  std::filesystem::path opt_prefix = MakeInstallTree("opt");
 
   // A second install, but with its symlinks pointing into the `opt` tree rather
   // than at its busybox.
-  MakeDir(dir_ / "lib/carbon");
-  MakeBusyboxFile(dir_ / "lib/carbon/carbon-busybox");
-  MakeDir(dir_ / "bin");
-  auto target = MakeSymlink(dir_ / "bin/carbon", "../opt/bin/carbon");
-  MakeDir(dir_ / "lib/carbon/llvm/bin");
-  auto clang_target = MakeSymlink(dir_ / "lib/carbon/llvm/bin/clang",
-                                  opt_prefix / "lib/carbon/llvm/bin/clang");
+  {
+    Filesystem::Dir lib_carbon = *dir_.CreateDirectories("lib/carbon");
+    lib_carbon.Symlink("carbon-busybox", running_binary_).Check();
+    Filesystem::Dir bin = *dir_.OpenDir("bin", Filesystem::CreateNew);
+    bin.Symlink("carbon", "../opt/bin/carbon").Check();
+    Filesystem::Dir llvm_bin = *lib_carbon.CreateDirectories("llvm/bin");
+    llvm_bin.Symlink("clang", (opt_prefix / "lib/carbon/llvm/bin/clang"))
+        .Check();
+  }
 
   // Starting from the second install uses the relative busybox rather than
   // traversing the symlink further.
-  auto info = GetBusyboxInfo(target.c_str());
+  auto info = GetBusyboxInfo((path_ / "bin/carbon").c_str());
   ASSERT_TRUE(info.ok()) << info.error();
-  EXPECT_THAT(info->bin_path, Eq(dir_ / "bin/../lib/carbon/carbon-busybox"));
-  info = GetBusyboxInfo(clang_target.c_str());
+  EXPECT_THAT(info->bin_path, Eq(path_ / "bin/../lib/carbon/carbon-busybox"));
+  info = GetBusyboxInfo((path_ / "lib/carbon/llvm/bin/clang").c_str());
   ASSERT_TRUE(info.ok()) << info.error();
   EXPECT_THAT(info->bin_path,
-              Eq(dir_ / "lib/carbon/llvm/bin/../../carbon-busybox"));
+              Eq(path_ / "lib/carbon/llvm/bin/../../carbon-busybox"));
 }
 
 TEST_F(BusyboxInfoTest, RejectSymlinkInUnrelatedInstall) {
   // Add two installs of Carbon nested inside each other in a realistic
   // scenario: `/usr` and `/usr/local`.
-  MakeInstallTree(dir_ / "usr");
-  MakeInstallTree(dir_ / "usr/local");
+  MakeInstallTree("usr");
+  std::filesystem::path usr_local = MakeInstallTree("usr/local");
 
   // Now add a stray symlink directly in `.../usr/local` to the local install.
   //
@@ -288,24 +248,25 @@ TEST_F(BusyboxInfoTest, RejectSymlinkInUnrelatedInstall) {
   // same busybox but probably wanted to find different ones:
   // - `.../usr/local/../lib/carbon/carbon-busybox`
   // - `.../usr/bin/../lib/carbon/carbon-busybox`
-  auto stray_target = MakeSymlink(dir_ / "usr/local/carbon", "bin/carbon");
+  Filesystem::Dir usr_local_dir = *dir_.OpenDir("usr/local");
+  usr_local_dir.Symlink("carbon", "bin/carbon").Check();
 
   // Check that the busybox doesn't use the relative busybox in this case, and
   // walks the symlink to find the correct installation.
-  auto info = GetBusyboxInfo(stray_target.c_str());
+  auto info = GetBusyboxInfo((usr_local / "carbon").c_str());
   ASSERT_TRUE(info.ok()) << info.error();
   EXPECT_THAT(info->bin_path,
-              Eq(dir_ / "usr/local/bin/../lib/carbon/carbon-busybox"));
+              Eq(usr_local / "bin/../lib/carbon/carbon-busybox"));
 
   // Ensure this works even with intervening `.` directory components.
-  stray_target = MakeSymlink(dir_ / "usr/local/carbon2", "bin/././carbon");
+  usr_local_dir.Symlink("carbon2", "bin/././carbon").Check();
 
   // Check that the busybox doesn't use the relative busybox in this case, and
   // walks the symlink to find the correct installation.
-  info = GetBusyboxInfo(stray_target.c_str());
+  info = GetBusyboxInfo((usr_local / "carbon2").c_str());
   ASSERT_TRUE(info.ok()) << info.error();
   EXPECT_THAT(info->bin_path,
-              Eq(dir_ / "usr/local/bin/../lib/carbon/carbon-busybox"));
+              Eq(usr_local / "bin/../lib/carbon/carbon-busybox"));
 }
 
 TEST_F(BusyboxInfoTest, EnvBinaryPathOverride) {
@@ -313,9 +274,9 @@ TEST_F(BusyboxInfoTest, EnvBinaryPathOverride) {
   ASSERT_THAT(getenv(Argv0OverrideEnv), Eq(nullptr));
 
   // Set the environment to our actual busybox.
-  auto busybox = MakeBusyboxFile(dir_ / "carbon-busybox");
+  dir_.Symlink("carbon-busybox", running_binary_).Check();
 
-  setenv(Argv0OverrideEnv, busybox.c_str(), /*overwrite=*/1);
+  setenv(Argv0OverrideEnv, (path_ / "carbon-busybox").c_str(), /*overwrite=*/1);
   auto info = GetBusyboxInfo("/some/nonexistent/path");
   if (getenv(Argv0OverrideEnv)) {
     unsetenv(Argv0OverrideEnv);
@@ -323,7 +284,7 @@ TEST_F(BusyboxInfoTest, EnvBinaryPathOverride) {
   }
 
   ASSERT_TRUE(info.ok()) << info.error();
-  EXPECT_THAT(info->bin_path, Eq(busybox));
+  EXPECT_THAT(info->bin_path, Eq(path_ / "carbon-busybox"));
   EXPECT_THAT(info->mode, Eq(std::nullopt));
 }
 

+ 4 - 3
toolchain/install/busybox_main.cpp

@@ -39,9 +39,10 @@ static auto Main(int argc, char** argv) -> ErrorOr<int> {
   // If `LLVM_SYMBOLIZER_PATH` is unset, sets it. Signals.cpp would do some more
   // path resolution which this overrides in favor of using the busybox itself
   // for symbolization.
-  setenv("LLVM_SYMBOLIZER_PATH",
-         (install_paths.llvm_install_bin() + "llvm-symbolizer").c_str(),
-         /*overwrite=*/0);
+  setenv(
+      "LLVM_SYMBOLIZER_PATH",
+      (install_paths.llvm_install_bin().native() + "llvm-symbolizer").c_str(),
+      /*overwrite=*/0);
 
   SetWorkingDirForBazel();
 

+ 95 - 99
toolchain/install/install_paths.cpp

@@ -4,10 +4,12 @@
 
 #include "toolchain/install/install_paths.h"
 
+#include <filesystem>
 #include <memory>
 #include <string>
 
 #include "common/check.h"
+#include "common/filesystem.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FileSystem.h"
@@ -28,31 +30,18 @@ static constexpr llvm::StringLiteral MarkerPath =
 auto InstallPaths::MakeExeRelative(llvm::StringRef exe_path) -> InstallPaths {
   InstallPaths paths;
 
-  // Map from the executable path from the executable path to an install
-  // prefix path.
-  if (!llvm::sys::fs::exists(exe_path)) {
-    paths.SetError(llvm::Twine("No file at executable path: ") + exe_path);
+  // Double check the exe was present.
+  auto exe_access_result = Filesystem::Cwd().Access(exe_path.str());
+  if (!exe_access_result.ok()) {
+    paths.SetError(llvm::Twine("Failed to test for access executable: ") +
+                   exe_access_result.error().ToString());
     return paths;
-  }
-  paths = InstallPaths(exe_path);
-
-  // TODO: Detect a Windows executable path and use custom logic to map to the
-  // correct install prefix for that platform.
-
-  // We assume an executable will be in a `bin` directory and this is a
-  // FHS-like install prefix. We remove the filename and walk up to find the
-  // expected install prefix.
-  llvm::sys::path::remove_filename(paths.prefix_);
-  llvm::sys::path::append(paths.prefix_, llvm::sys::path::Style::posix,
-                          "../../");
-
-  if (auto error = llvm::sys::fs::make_absolute(paths.prefix_)) {
-    paths.SetError(error.message());
+  } else if (!*exe_access_result) {
+    paths.SetError(llvm::Twine("Unable to access executable: ") + exe_path);
     return paths;
   }
 
-  paths.CheckMarkerFile();
-  return paths;
+  return MakeFromFile(exe_path.str());
 }
 
 auto InstallPaths::MakeForBazelRunfiles(llvm::StringRef exe_path)
@@ -65,28 +54,23 @@ auto InstallPaths::MakeForBazelRunfiles(llvm::StringRef exe_path)
                runtimes_error);
 
   std::string relative_marker_path = (PrefixRoot.str() + MarkerPath).str();
-  std::string runtimes_marker_path = runfiles->Rlocation(relative_marker_path);
+  std::filesystem::path runtimes_marker_path =
+      runfiles->Rlocation(relative_marker_path);
 
   // Start from the marker, remove that filename, and walk up to find the
   // install prefix.
-  InstallPaths paths(runtimes_marker_path);
-  llvm::sys::path::remove_filename(paths.prefix_);
-  llvm::sys::path::append(paths.prefix_, llvm::sys::path::Style::posix,
-                          "../../");
-
-  if (auto error = llvm::sys::fs::make_absolute(paths.prefix_)) {
-    paths.SetError(error.message());
-    return paths;
-  }
-
-  paths.CheckMarkerFile();
-  CARBON_CHECK(!paths.error(), "{0}", *paths.error());
-  return paths;
+  return MakeFromFile(std::move(runtimes_marker_path));
 }
 
 auto InstallPaths::Make(llvm::StringRef install_prefix) -> InstallPaths {
-  InstallPaths paths(install_prefix);
-  paths.CheckMarkerFile();
+  InstallPaths paths(install_prefix.str());
+  auto open_result = Filesystem::Cwd().OpenDir(paths.prefix_);
+  if (!open_result.ok()) {
+    paths.SetError(open_result.error().ToString());
+  } else {
+    paths.prefix_dir_ = *std::move(open_result);
+    paths.CheckMarkerFile();
+  }
   return paths;
 }
 
@@ -97,124 +81,136 @@ auto InstallPaths::ReadPreludeManifest() const
 
 auto InstallPaths::ReadClangHeadersManifest() const
     -> ErrorOr<llvm::SmallVector<std::string>> {
-  llvm::SmallString<256> manifest_path(prefix_);
-  llvm::sys::path::append(manifest_path, llvm::sys::path::Style::posix, "..");
-  return ReadManifest(manifest_path, "clang_headers_manifest.txt");
+  return ReadManifest(prefix_ / "..", "clang_headers_manifest.txt");
 }
 
-auto InstallPaths::ReadManifest(llvm::StringRef manifest_path,
-                                llvm::StringRef manifest_file) const
+auto InstallPaths::ReadManifest(std::filesystem::path manifest_path,
+                                std::filesystem::path manifest_file) const
     -> ErrorOr<llvm::SmallVector<std::string>> {
   // This is structured to avoid a vector copy on success.
   ErrorOr<llvm::SmallVector<std::string>> result =
       llvm::SmallVector<std::string>();
 
-  llvm::SmallString<256> manifest;
-  llvm::sys::path::append(manifest, llvm::sys::path::Style::posix,
-                          manifest_path, manifest_file);
-
-  auto fs = llvm::vfs::getRealFileSystem();
-  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> file =
-      fs->getBufferForFile(manifest);
-  if (!file) {
-    result = ErrorBuilder() << "Loading manifest `" << manifest
-                            << "`: " << file.getError().message();
+  // TODO: It would be nice to adjust the manifests to be within the install
+  // prefix and use that open directory to access the manifest. Also to update
+  // callers to be able to use the relative paths via an open directory rather
+  // than having to form absolute paths for all the entries.
+  auto read_result =
+      Filesystem::Cwd().ReadFileToString(manifest_path / manifest_file);
+  if (!read_result.ok()) {
+    result = ErrorBuilder()
+             << "Loading manifest `" << (manifest_path / manifest_file)
+             << "`: " << read_result.error();
     return result;
   }
 
   // The manifest should have one file per line.
-  llvm::StringRef buffer = file.get()->getBuffer();
+  llvm::StringRef buffer = *read_result;
   while (true) {
     auto [token, remainder] = llvm::getToken(buffer, "\n");
     if (token.empty()) {
       break;
     }
-    llvm::SmallString<256> path;
-    llvm::sys::path::append(path, llvm::sys::path::Style::posix, manifest_path,
-                            token);
-    result->push_back(path.str().str());
+    result->push_back((manifest_path / std::string_view(token)).native());
     buffer = remainder;
   }
 
   if (result->empty()) {
-    result = ErrorBuilder() << "Manifest `" << manifest << "` is empty";
+    result = ErrorBuilder()
+             << "Manifest `" << (manifest_path / manifest_file) << "` is empty";
   }
   return result;
 }
 
+auto InstallPaths::MakeFromFile(std::filesystem::path file_path)
+    -> InstallPaths {
+  // TODO: Detect a Windows executable path and use custom logic to map to the
+  // correct install prefix for that platform.
+  //
+  // We assume an executable will be in a `bin` directory and this is a
+  // FHS-like install prefix. We remove the filename and walk up to find the
+  // expected install prefix.
+  std::error_code ec;
+  InstallPaths paths(std::filesystem::absolute(
+      std::move(file_path).remove_filename() / "../..", ec));
+  if (ec) {
+    paths.SetError(ec.message());
+    return paths;
+  }
+
+  auto open_result = Filesystem::Cwd().OpenDir(paths.prefix_);
+  if (!open_result.ok()) {
+    paths.SetError(open_result.error().ToString());
+    return paths;
+  }
+
+  paths.prefix_dir_ = *std::move(open_result);
+  paths.CheckMarkerFile();
+  return paths;
+}
+
 auto InstallPaths::SetError(llvm::Twine message) -> void {
   // Use an empty prefix on error as that should use the working directory which
   // is the least likely problematic.
   prefix_ = "";
+  prefix_dir_ = Filesystem::Dir();
   error_ = {message.str()};
 }
 
 auto InstallPaths::CheckMarkerFile() -> void {
-  if (!llvm::sys::path::is_absolute(prefix_)) {
-    SetError(llvm::Twine("Not an absolute path: ") + prefix_);
+  if (!prefix_.is_absolute()) {
+    SetError(llvm::Twine("Not an absolute path: ") + prefix_.native());
+    return;
   }
 
-  llvm::SmallString<256> path(prefix_);
-  llvm::sys::path::append(path, llvm::sys::path::Style::posix, MarkerPath);
-  if (!llvm::sys::fs::exists(path)) {
-    SetError(llvm::Twine("No install marker at path: ") + path);
+  auto access_result = prefix_dir_.Access(MarkerPath.str());
+  if (!access_result.ok()) {
+    SetError(access_result.error().ToString());
+    return;
   }
+  if (!*access_result) {
+    SetError(llvm::Twine("No install marker at path: ") +
+             (prefix_ / std::string_view(MarkerPath)).native());
+    return;
+  }
+
+  // Success!
 }
 
-auto InstallPaths::core_package() const -> std::string {
-  llvm::SmallString<256> path(prefix_);
+auto InstallPaths::core_package() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  llvm::sys::path::append(path, llvm::sys::path::Style::posix,
-                          "lib/carbon/core");
-  return path.str().str();
+  return prefix_ / "lib/carbon/core";
 }
 
-auto InstallPaths::llvm_install_bin() const -> std::string {
-  llvm::SmallString<256> path(prefix_);
+auto InstallPaths::llvm_install_bin() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  llvm::sys::path::append(path, llvm::sys::path::Style::posix,
-                          "lib/carbon/llvm/bin/");
-  return path.str().str();
+  return prefix_ / "lib/carbon/llvm/bin/";
 }
 
-auto InstallPaths::clang_path() const -> std::string {
-  llvm::SmallString<256> path(prefix_);
+auto InstallPaths::clang_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  llvm::sys::path::append(path, llvm::sys::path::Style::posix,
-                          "lib/carbon/llvm/bin/clang");
-  return path.str().str();
+  return prefix_ / "lib/carbon/llvm/bin/clang";
 }
 
-auto InstallPaths::lld_path() const -> std::string {
-  llvm::SmallString<256> path(prefix_);
+auto InstallPaths::lld_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  llvm::sys::path::append(path, llvm::sys::path::Style::posix,
-                          "lib/carbon/llvm/bin/lld");
-  return path.str().str();
+  return prefix_ / "lib/carbon/llvm/bin/lld";
 }
 
-auto InstallPaths::ld_lld_path() const -> std::string {
-  llvm::SmallString<256> path(prefix_);
+auto InstallPaths::ld_lld_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  llvm::sys::path::append(path, llvm::sys::path::Style::posix,
-                          "lib/carbon/llvm/bin/ld.lld");
-  return path.str().str();
+  return prefix_ / "lib/carbon/llvm/bin/ld.lld";
 }
 
-auto InstallPaths::ld64_lld_path() const -> std::string {
-  llvm::SmallString<256> path(prefix_);
+auto InstallPaths::ld64_lld_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  llvm::sys::path::append(path, llvm::sys::path::Style::posix,
-                          "lib/carbon/llvm/bin/ld64.lld");
-  return path.str().str();
+  return prefix_ / "lib/carbon/llvm/bin/ld64.lld";
 }
 
-auto InstallPaths::llvm_tool_path(LLVMTool tool) const -> std::string {
-  llvm::SmallString<256> path(prefix_);
+auto InstallPaths::llvm_tool_path(LLVMTool tool) const
+    -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  llvm::sys::path::append(path, llvm::sys::path::Style::posix,
-                          "lib/carbon/llvm/bin", tool.bin_name());
-  return path.str().str();
+  return prefix_ / "lib/carbon/llvm/bin" / std::string_view(tool.bin_name());
 }
 
 }  // namespace Carbon

+ 20 - 11
toolchain/install/install_paths.h

@@ -5,7 +5,10 @@
 #ifndef CARBON_TOOLCHAIN_INSTALL_INSTALL_PATHS_H_
 #define CARBON_TOOLCHAIN_INSTALL_INSTALL_PATHS_H_
 
+#include <filesystem>
+
 #include "common/error.h"
+#include "common/filesystem.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
@@ -86,27 +89,30 @@ class InstallPaths {
   }
 
   // The directory containing the `Core` package. Computed on demand.
-  auto core_package() const -> std::string;
+  auto core_package() const -> std::filesystem::path;
 
   // The directory containing LLVM install binaries. Computed on demand.
-  auto llvm_install_bin() const -> std::string;
+  auto llvm_install_bin() const -> std::filesystem::path;
 
   // The path to `clang`.
-  auto clang_path() const -> std::string;
+  auto clang_path() const -> std::filesystem::path;
 
   // The path to `lld' and various aliases of `lld`.
-  auto lld_path() const -> std::string;
-  auto ld_lld_path() const -> std::string;
-  auto ld64_lld_path() const -> std::string;
+  auto lld_path() const -> std::filesystem::path;
+  auto ld_lld_path() const -> std::filesystem::path;
+  auto ld64_lld_path() const -> std::filesystem::path;
 
   // The path to any of the LLVM tools.
-  auto llvm_tool_path(LLVMTool tool) const -> std::string;
+  auto llvm_tool_path(LLVMTool tool) const -> std::filesystem::path;
 
  private:
   friend class InstallPathsTestPeer;
 
   InstallPaths() { SetError("No prefix provided!"); }
-  explicit InstallPaths(llvm::StringRef prefix) : prefix_(prefix) {}
+  explicit InstallPaths(std::filesystem::path prefix)
+      : prefix_(std::move(prefix)) {}
+
+  static auto MakeFromFile(std::filesystem::path file) -> InstallPaths;
 
   // Set an error message on the install paths and reset the prefix to empty,
   // which should use the current working directory.
@@ -118,8 +124,8 @@ class InstallPaths {
   auto CheckMarkerFile() -> void;
 
   // Read a manifest file.
-  auto ReadManifest(llvm::StringRef manifest_path,
-                    llvm::StringRef manifest_file) const
+  auto ReadManifest(std::filesystem::path manifest_path,
+                    std::filesystem::path manifest_file) const
       -> ErrorOr<llvm::SmallVector<std::string>>;
 
   // The computed installation prefix. This will be an absolute path. We keep an
@@ -144,7 +150,10 @@ class InstallPaths {
   //
   // The hierarchy of files beneath the install prefix can be found in the
   // BUILD's `install_dirs`.
-  llvm::SmallString<256> prefix_;
+  std::filesystem::path prefix_;
+
+  // The opened prefix directory, suitable for relative path access.
+  Filesystem::Dir prefix_dir_;
 
   std::optional<std::string> error_;
 };

+ 38 - 36
toolchain/install/install_paths_test.cpp

@@ -7,14 +7,15 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <filesystem>
 #include <memory>
 #include <string>
 
 #include "common/check.h"
+#include "common/error_test_helpers.h"
+#include "common/filesystem.h"
 #include "llvm/ADT/SmallString.h"
-#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatVariadic.h"
-#include "llvm/Support/Path.h"
 #include "testing/base/global_exe_path.h"
 #include "tools/cpp/runfiles/runfiles.h"
 
@@ -22,7 +23,7 @@ namespace Carbon {
 
 class InstallPathsTestPeer {
  public:
-  static auto GetPrefix(const InstallPaths& paths) -> llvm::StringRef {
+  static auto GetPrefix(const InstallPaths& paths) -> std::filesystem::path {
     return paths.prefix_;
   }
 };
@@ -30,8 +31,10 @@ class InstallPathsTestPeer {
 namespace {
 
 using ::bazel::tools::cpp::runfiles::Runfiles;
+using ::testing::_;
 using ::testing::Eq;
 using ::testing::HasSubstr;
+using Testing::IsSuccess;
 using ::testing::Optional;
 using ::testing::StartsWith;
 
@@ -48,43 +51,40 @@ class InstallPathsTest : public ::testing::Test {
   // check that the path accessors point to the right kind of file or
   // directory.
   auto TestInstallPaths(const InstallPaths& paths) -> void {
-    auto prefix = InstallPathsTestPeer::GetPrefix(paths);
+    std::filesystem::path prefix_path = InstallPathsTestPeer::GetPrefix(paths);
 
-    SCOPED_TRACE(llvm::formatv("Install prefix: '{0}'", prefix));
+    SCOPED_TRACE(llvm::formatv("Install prefix: '{0}'", prefix_path));
 
-    // Grab a the prefix into a string to make it easier to use in the test.
-    EXPECT_TRUE(llvm::sys::fs::exists(prefix));
-    EXPECT_TRUE(llvm::sys::fs::is_directory(prefix));
+    // Open the prefix directory.
+    auto prefix_result = Filesystem::Cwd().OpenDir(prefix_path);
+    ASSERT_THAT(prefix_result, IsSuccess(_));
+    Filesystem::Dir prefix = *std::move(prefix_result);
 
     // Now check that all the expected parts of the toolchain's install are in
     // fact found using the API.
-    llvm::SmallString<256> driver_path(prefix);
     // TODO: Adjust this to work equally well on Windows.
-    llvm::sys::path::append(driver_path, llvm::sys::path::Style::posix,
-                            "bin/carbon");
-    EXPECT_TRUE(llvm::sys::fs::exists(driver_path)) << "path: " << driver_path;
-    EXPECT_TRUE(llvm::sys::fs::can_execute(driver_path))
-        << "path: " << driver_path;
-
-    std::string core_package_path = paths.core_package();
-    ASSERT_THAT(core_package_path, StartsWith(prefix));
-    EXPECT_TRUE(llvm::sys::fs::exists(core_package_path + "/prelude.carbon"))
+    EXPECT_THAT(
+        prefix.Access("bin/carbon", Filesystem::AccessCheckFlags::Execute),
+        IsSuccess(Eq(true)))
+        << "path: " << (prefix_path / "bin/carbon");
+
+    std::filesystem::path core_package_path = paths.core_package();
+    ASSERT_THAT(core_package_path, StartsWith(prefix_path));
+    EXPECT_THAT(Filesystem::Cwd().Access(core_package_path / "prelude.carbon"),
+                IsSuccess(Eq(true)))
         << "path: " << core_package_path;
 
-    std::string llvm_bin_path = paths.llvm_install_bin();
-    ASSERT_THAT(llvm_bin_path, StartsWith(prefix));
-    EXPECT_TRUE(llvm::sys::fs::exists(llvm_bin_path))
-        << "path: " << llvm_bin_path;
-    EXPECT_TRUE(llvm::sys::fs::is_directory(llvm_bin_path))
-        << "path: " << llvm_bin_path;
-
-    for (llvm::StringRef llvm_bin : {"ld.lld", "ld64.lld"}) {
-      llvm::SmallString<128> bin_path;
-      bin_path.assign(llvm_bin_path);
-      llvm::sys::path::append(bin_path, llvm_bin);
-
-      EXPECT_TRUE(llvm::sys::fs::exists(bin_path)) << "path: " << bin_path;
-      EXPECT_TRUE(llvm::sys::fs::can_execute(bin_path)) << "path: " << bin_path;
+    std::filesystem::path llvm_bin_path = paths.llvm_install_bin();
+    ASSERT_THAT(llvm_bin_path, StartsWith(prefix_path));
+    auto open_result = Filesystem::Cwd().OpenDir(llvm_bin_path);
+    ASSERT_THAT(open_result, IsSuccess(_));
+    Filesystem::Dir llvm_bin = *std::move(open_result);
+
+    for (std::filesystem::path bin_name : {"ld.lld", "ld64.lld"}) {
+      EXPECT_THAT(
+          llvm_bin.Access(bin_name, Filesystem::AccessCheckFlags::Execute),
+          IsSuccess(Eq(true)))
+          << "path: " << (llvm_bin_path / bin_name);
     }
   }
 
@@ -120,12 +120,14 @@ TEST_F(InstallPathsTest, TestRunfiles) {
 }
 
 TEST_F(InstallPathsTest, BinaryRunfiles) {
-  std::string test_binary_path =
+  std::filesystem::path test_binary_path =
       test_runfiles_->Rlocation("carbon/toolchain/install/test_binary");
-  CARBON_CHECK(llvm::sys::fs::can_execute(test_binary_path), "{0}",
-               test_binary_path);
+  ASSERT_THAT(Filesystem::Cwd().Access(test_binary_path,
+                                       Filesystem::AccessCheckFlags::Execute),
+              IsSuccess(Eq(true)))
+      << "path: " << test_binary_path;
 
-  auto paths = InstallPaths::MakeForBazelRunfiles(test_binary_path);
+  auto paths = InstallPaths::MakeForBazelRunfiles(test_binary_path.native());
   ASSERT_THAT(paths.error(), Eq(std::nullopt)) << *paths.error();
   TestInstallPaths(paths);
 }

+ 1 - 1
toolchain/install/install_paths_test_helpers.cpp

@@ -14,7 +14,7 @@ namespace Carbon::Testing {
 // Prepares the VFS with prelude files from the real filesystem. Primarily for
 // tests.
 auto AddPreludeFilesToVfs(
-    InstallPaths install_paths,
+    const InstallPaths& install_paths,
     llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>& vfs) -> void {
   // Load the prelude into the test VFS.
   auto real_fs = llvm::vfs::getRealFileSystem();

+ 1 - 1
toolchain/install/install_paths_test_helpers.h

@@ -12,7 +12,7 @@ namespace Carbon::Testing {
 
 // Prepares the VFS with prelude files from the real filesystem.
 auto AddPreludeFilesToVfs(
-    InstallPaths install_paths,
+    const InstallPaths& install_paths,
     llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>& vfs) -> void;
 
 }  // namespace Carbon::Testing

+ 3 - 3
toolchain/testing/file_test.cpp

@@ -224,7 +224,7 @@ auto ToolchainFileTest::GetDefaultArgs() const
                               "--phase=" + component_.str(),
                               // Use the install path to exclude prelude files.
                               "--exclude-dump-file-prefix=" +
-                                  data_->installation.core_package(),
+                                  data_->installation.core_package().native(),
                           });
 
   if (component_ == "lex") {
@@ -315,8 +315,8 @@ auto ToolchainFileTest::DoExtraCheckReplacements(std::string& check_line) const
     // TODO: Consider adding a content keyword to name the core package, and
     // replace with that instead. Alternatively, consider adding the core
     // package to the VFS with a fixed name.
-    absl::StrReplaceAll({{data_->installation.core_package(), "{{.*}}"}},
-                        &check_line);
+    absl::StrReplaceAll(
+        {{data_->installation.core_package().native(), "{{.*}}"}}, &check_line);
     if (component_ == "check") {
       DoClangASTCheckReplacements(check_line);
     }