From 2e509e9103d965afaf930a0a1c2b7b17ec182d60 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Mon, 11 Aug 2025 19:20:22 -0700 Subject: [PATCH] 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. --- toolchain/driver/clang_runner_test.cpp | 2 +- toolchain/install/BUILD | 5 + toolchain/install/busybox_info.cpp | 15 +- toolchain/install/busybox_info_test.cpp | 219 +++++++----------- toolchain/install/busybox_main.cpp | 7 +- toolchain/install/install_paths.cpp | 194 ++++++++-------- toolchain/install/install_paths.h | 31 ++- toolchain/install/install_paths_test.cpp | 70 +++--- .../install/install_paths_test_helpers.cpp | 2 +- .../install/install_paths_test_helpers.h | 2 +- toolchain/testing/file_test.cpp | 6 +- 11 files changed, 264 insertions(+), 289 deletions(-) diff --git a/toolchain/driver/clang_runner_test.cpp b/toolchain/driver/clang_runner_test.cpp index b2ab6e72e6ec..162d4ab35d2b 100644 --- a/toolchain/driver/clang_runner_test.cpp +++ b/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 diff --git a/toolchain/install/BUILD b/toolchain/install/BUILD index ef415be49989..c980105b5272 100644 --- a/toolchain/install/BUILD +++ b/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", diff --git a/toolchain/install/busybox_info.cpp b/toolchain/install/busybox_info.cpp index 8b90f56e6490..5169c264413b 100644 --- a/toolchain/install/busybox_info.cpp +++ b/toolchain/install/busybox_info.cpp @@ -7,6 +7,7 @@ #include #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 { // 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 { ? 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 { // 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; } } diff --git a/toolchain/install/busybox_info_test.cpp b/toolchain/install/busybox_info_test.cpp index 188e58ab2cd0..38db0cd1df7d 100644 --- a/toolchain/install/busybox_info_test.cpp +++ b/toolchain/install/busybox_info_test.cpp @@ -8,11 +8,13 @@ #include #include +#include #include #include #include #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 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)); } diff --git a/toolchain/install/busybox_main.cpp b/toolchain/install/busybox_main.cpp index 380f94b25115..5901f8df9776 100644 --- a/toolchain/install/busybox_main.cpp +++ b/toolchain/install/busybox_main.cpp @@ -39,9 +39,10 @@ static auto Main(int argc, char** argv) -> ErrorOr { // 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(); diff --git a/toolchain/install/install_paths.cpp b/toolchain/install/install_paths.cpp index 653d2d381c97..b06e094125c9 100644 --- a/toolchain/install/install_paths.cpp +++ b/toolchain/install/install_paths.cpp @@ -4,10 +4,12 @@ #include "toolchain/install/install_paths.h" +#include #include #include #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::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> { // This is structured to avoid a vector copy on success. ErrorOr> result = llvm::SmallVector(); - 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> 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 diff --git a/toolchain/install/install_paths.h b/toolchain/install/install_paths.h index 96dca2f1609b..9db442c12443 100644 --- a/toolchain/install/install_paths.h +++ b/toolchain/install/install_paths.h @@ -5,7 +5,10 @@ #ifndef CARBON_TOOLCHAIN_INSTALL_INSTALL_PATHS_H_ #define CARBON_TOOLCHAIN_INSTALL_INSTALL_PATHS_H_ +#include + #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>; // 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 error_; }; diff --git a/toolchain/install/install_paths_test.cpp b/toolchain/install/install_paths_test.cpp index fac9f0af6024..fa58a00c13fc 100644 --- a/toolchain/install/install_paths_test.cpp +++ b/toolchain/install/install_paths_test.cpp @@ -7,14 +7,15 @@ #include #include +#include #include #include #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; + EXPECT_THAT( + prefix.Access("bin/carbon", Filesystem::AccessCheckFlags::Execute), + IsSuccess(Eq(true))) + << "path: " << (prefix_path / "bin/carbon"); - 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")) + 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; + 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 (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; + 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); } diff --git a/toolchain/install/install_paths_test_helpers.cpp b/toolchain/install/install_paths_test_helpers.cpp index a963cda5bb74..5c6f67fdfd83 100644 --- a/toolchain/install/install_paths_test_helpers.cpp +++ b/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& vfs) -> void { // Load the prelude into the test VFS. auto real_fs = llvm::vfs::getRealFileSystem(); diff --git a/toolchain/install/install_paths_test_helpers.h b/toolchain/install/install_paths_test_helpers.h index 8b14038ef240..da589ec53e18 100644 --- a/toolchain/install/install_paths_test_helpers.h +++ b/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& vfs) -> void; } // namespace Carbon::Testing diff --git a/toolchain/testing/file_test.cpp b/toolchain/testing/file_test.cpp index 9396b9b54691..741fea72492e 100644 --- a/toolchain/testing/file_test.cpp +++ b/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); }