Skip to content

Commit 2e509e9

Browse files
authored
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.
1 parent 42d2976 commit 2e509e9

11 files changed

+266
-291
lines changed

toolchain/driver/clang_runner_test.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ TEST(ClangRunnerTest, Version) {
5555
EXPECT_THAT(out, HasSubstr((llvm::Twine("Target: ") + target).str()));
5656
// Clang's install should be our private LLVM install bin directory.
5757
EXPECT_THAT(out, HasSubstr(std::string("InstalledDir: ") +
58-
install_paths.llvm_install_bin()));
58+
install_paths.llvm_install_bin().native()));
5959
}
6060

6161
// It's hard to write a portable and reliable unittest for all the layers of the

toolchain/install/BUILD

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ cc_library(
3131
deps = [
3232
"//common:check",
3333
"//common:error",
34+
"//common:filesystem",
3435
"//toolchain/base:llvm_tools",
3536
"@bazel_tools//tools/cpp/runfiles",
3637
"@llvm-project//llvm:Support",
@@ -55,6 +56,8 @@ cc_test(
5556
deps = [
5657
":install_paths",
5758
"//common:check",
59+
"//common:error_test_helpers",
60+
"//common:filesystem",
5861
"//common:ostream",
5962
"//testing/base:global_exe_path",
6063
"//testing/base:gtest_main",
@@ -83,6 +86,7 @@ cc_library(
8386
deps = [
8487
"//common:error",
8588
"//common:exe_path",
89+
"//common:filesystem",
8690
"@llvm-project//llvm:Support",
8791
],
8892
)
@@ -94,6 +98,7 @@ cc_test(
9498
deps = [
9599
":busybox_info",
96100
"//common:check",
101+
"//common:filesystem",
97102
"//testing/base:gtest_main",
98103
"@googletest//:gtest",
99104
"@llvm-project//llvm:Support",

toolchain/install/busybox_info.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <iterator>
88

99
#include "common/exe_path.h"
10+
#include "common/filesystem.h"
1011
#include "llvm/ADT/StringRef.h"
1112

1213
namespace Carbon {
@@ -46,7 +47,8 @@ auto GetBusyboxInfo(const char* argv0) -> ErrorOr<BusyboxInfo> {
4647
// handled in the other return path.
4748
std::string prefix_root = info.bin_path.parent_path().string() +
4849
"/prefix_root/lib/carbon/carbon-busybox";
49-
if (std::filesystem::exists(prefix_root)) {
50+
if (auto access = Filesystem::Cwd().Access(prefix_root);
51+
access.ok() && *access) {
5052
info.bin_path = prefix_root;
5153
}
5254
return info;
@@ -75,24 +77,23 @@ auto GetBusyboxInfo(const char* argv0) -> ErrorOr<BusyboxInfo> {
7577
? parent_path / ".." / "lib" / "carbon"
7678
: parent_path / ".." / "..";
7779
auto busybox_path = lib_path / "carbon-busybox";
78-
std::error_code ec;
79-
if (std::filesystem::exists(busybox_path, ec)) {
80+
if (auto access = Filesystem::Cwd().Access(busybox_path);
81+
access.ok() && *access) {
8082
info.bin_path = busybox_path;
8183
return info;
8284
}
8385
}
8486

8587
// Try to walk through another layer of symlinks and see if we can find the
8688
// installation there or are linked directly to the busybox.
87-
std::error_code ec;
88-
auto symlink_target = std::filesystem::read_symlink(info.bin_path, ec);
89-
if (ec) {
89+
auto readlink = Filesystem::Cwd().Readlink(info.bin_path);
90+
if (!readlink.ok()) {
9091
return ErrorBuilder()
9192
<< "expected carbon-busybox symlink at `" << info.bin_path << "`";
9293
}
9394

9495
// Do a path join, to handle relative symlinks.
95-
info.bin_path = parent_path / symlink_target;
96+
info.bin_path = parent_path / *readlink;
9697
}
9798
}
9899

0 commit comments

Comments
 (0)