Explorar o código

Switch install to be based on the busybox root (#6579)

Previously, we used the FHS "prefix" concept as the basis of the
install, but this makes it hard to integrate an installed toolchain with
Bazel (or similar) build system where it wants the "root" of the
toolchain to have some specific files (`MODULES.bazel` or
`BUILD.bazel`), and cannot reference anything outside that directory
tree.

An easy solution is to make the `lib/carbon` directory the root of the
install and never walking up from it. Then we simply have a `bin/carbon`
symlink to the busybox that is useful for getting the command into the
PATH, but isn't used for anything else. The FHS-constrained install
paths surround a root we fully control the layout and files within.

While initially motivated by trying to make a single toolchain structure
that works both for installation and for Bazel, it actually makes the
paths we end up using in the toolchain much simpler. We no longer have
awkward `.../lib/carbon/../../lib/carbon/...` sequences in the toolchain
which is cleaner and even a (trivial) efficiency gain.

As I was doing this I noticed several out-of-date comments that I tried
to fix, and I tried to improve some code reuse rather than re-computing
paths.

---------

Co-authored-by: Geoff Romer <gromer@google.com>
Chandler Carruth hai 3 meses
pai
achega
d66b2f899c

+ 3 - 3
bazel/carbon_rules/defs.bzl

@@ -76,7 +76,7 @@ def _carbon_binary_impl(ctx):
             # TODO: This is a hack; replace with something better once the toolchain
             # supports doing so.
             #
-            # TODO: Switch to the `prefix_root` based rule similar to linking when
+            # TODO: Switch to the `prefix` based rule similar to linking when
             # the prelude moves there.
             out = ctx.actions.declare_file("_objs/{0}/{1}o".format(
                 ctx.label.name,
@@ -177,7 +177,7 @@ def carbon_binary(name, srcs, deps = [], flags = [], tags = []):
         # `select` which one we use.
         internal_exec_toolchain_driver = select({
             "//bazel/carbon_rules:use_target_config_carbon_rules_config": None,
-            "//conditions:default": "//toolchain/install:prefix_root/bin/carbon",
+            "//conditions:default": "//toolchain/install:prefix/bin/carbon",
         }),
         internal_exec_toolchain_data = select({
             "//bazel/carbon_rules:use_target_config_carbon_rules_config": None,
@@ -188,7 +188,7 @@ def carbon_binary(name, srcs, deps = [], flags = [], tags = []):
             "//conditions:default": "//toolchain/driver:prebuilt_runtimes",
         }),
         internal_target_toolchain_driver = select({
-            "//bazel/carbon_rules:use_target_config_carbon_rules_config": "//toolchain/install:prefix_root/bin/carbon",
+            "//bazel/carbon_rules:use_target_config_carbon_rules_config": "//toolchain/install:prefix/bin/carbon",
             "//conditions:default": None,
         }),
         internal_target_toolchain_data = select({

+ 1 - 1
toolchain/BUILD

@@ -11,7 +11,7 @@ run_tool(
     name = "carbon",
     data = ["//toolchain/install:install_data"],
     env = cc_env(),
-    tool = "//toolchain/install:prefix_root/bin/carbon",
+    tool = "//toolchain/install:prefix/bin/carbon",
 )
 
 # A convenience target for running the toolchain with the full prelude

+ 44 - 47
toolchain/base/install_paths.cpp

@@ -20,13 +20,12 @@
 
 namespace Carbon {
 
-// The location within our Bazel output tree of the prefix_root.
-static constexpr llvm::StringLiteral PrefixRoot =
-    "carbon/toolchain/install/prefix_root/";
+// The location within our Bazel output tree of the install root.
+static constexpr llvm::StringLiteral BazelRoot =
+    "carbon/toolchain/install/prefix/lib/carbon/";
 
-// Path within an install prefix for our marker of a valid install.
-static constexpr llvm::StringLiteral MarkerPath =
-    "lib/carbon/carbon_install.txt";
+// Path within an install root for our marker of a valid install.
+static constexpr llvm::StringLiteral MarkerPath = "carbon_install.txt";
 
 auto InstallPaths::MakeExeRelative(llvm::StringRef exe_path) -> InstallPaths {
   InstallPaths paths;
@@ -54,22 +53,21 @@ auto InstallPaths::MakeForBazelRunfiles(llvm::StringRef exe_path)
   CARBON_CHECK(runfiles != nullptr, "Failed to find runtimes tree: {0}",
                runtimes_error);
 
-  std::string relative_marker_path = (PrefixRoot.str() + MarkerPath).str();
+  std::string relative_marker_path = (BazelRoot.str() + MarkerPath).str();
   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.
+  // Directly use the marker file's path.
   return MakeFromFile(std::move(runtimes_marker_path));
 }
 
-auto InstallPaths::Make(llvm::StringRef install_prefix) -> InstallPaths {
-  InstallPaths paths(install_prefix.str());
-  auto open_result = Filesystem::Cwd().OpenDir(paths.prefix_);
+auto InstallPaths::Make(llvm::StringRef install_root) -> InstallPaths {
+  InstallPaths paths(install_root.str());
+  auto open_result = Filesystem::Cwd().OpenDir(paths.root_);
   if (!open_result.ok()) {
     paths.SetError(open_result.error().ToString());
   } else {
-    paths.prefix_dir_ = *std::move(open_result);
+    paths.root_dir_ = *std::move(open_result);
     paths.CheckMarkerFile();
   }
   return paths;
@@ -82,7 +80,11 @@ auto InstallPaths::ReadPreludeManifest() const
 
 auto InstallPaths::ReadClangHeadersManifest() const
     -> ErrorOr<llvm::SmallVector<std::string>> {
-  return ReadManifest(prefix_ / "..", "clang_headers_manifest.txt");
+  // TODO: This is the only place where we read from outside of the install
+  // root. Consider whether this manifest should be within the install or
+  // consider moving the code to access it to be separate and specific to the
+  // infrastructure needing it.
+  return ReadManifest(root_ / "../../..", "clang_headers_manifest.txt");
 }
 
 auto InstallPaths::ReadManifest(std::filesystem::path manifest_path,
@@ -93,7 +95,7 @@ auto InstallPaths::ReadManifest(std::filesystem::path manifest_path,
       llvm::SmallVector<std::string>();
 
   // 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
+  // root 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 =
@@ -125,47 +127,42 @@ auto InstallPaths::ReadManifest(std::filesystem::path manifest_path,
 
 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.
+  // TODO: Add any custom logic needed to detect the correct install root on
+  // Windows once we have support 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::move(file_path).remove_filename() / "../..");
-  if (ec) {
-    paths.SetError(ec.message());
-    return paths;
-  }
+  // We assume the provided path is either the marker file itself or the busybox
+  // executable that is adjacent to the marker file. That means the root is just
+  // the directory of this path.
+  InstallPaths paths(std::move(file_path).remove_filename());
 
-  auto open_result = Filesystem::Cwd().OpenDir(paths.prefix_);
+  auto open_result = Filesystem::Cwd().OpenDir(paths.root_);
   if (!open_result.ok()) {
     paths.SetError(open_result.error().ToString());
     return paths;
   }
 
-  paths.prefix_dir_ = *std::move(open_result);
+  paths.root_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
+  // Use an empty root on error as that should use the working directory which
   // is the least likely problematic.
-  prefix_ = "";
-  prefix_dir_ = Filesystem::Dir();
+  root_ = "";
+  root_dir_ = Filesystem::Dir();
   error_ = {message.str()};
 }
 
 auto InstallPaths::CheckMarkerFile() -> void {
-  auto access_result = prefix_dir_.Access(MarkerPath.str());
+  auto access_result = root_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());
+             (root_ / std::string_view(MarkerPath)).native());
     return;
   }
 
@@ -174,73 +171,73 @@ auto InstallPaths::CheckMarkerFile() -> void {
 
 auto InstallPaths::core_package() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/core";
+  return root_ / "core";
 }
 
 auto InstallPaths::llvm_install_bin() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/llvm/bin/";
+  return root_ / "llvm/bin";
 }
 
 auto InstallPaths::clang_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/llvm/bin/clang";
+  return llvm_install_bin() / "clang";
 }
 
 auto InstallPaths::lld_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/llvm/bin/lld";
+  return llvm_install_bin() / "lld";
 }
 
 auto InstallPaths::ld_lld_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/llvm/bin/ld.lld";
+  return llvm_install_bin() / "ld.lld";
 }
 
 auto InstallPaths::ld64_lld_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/llvm/bin/ld64.lld";
+  return llvm_install_bin() / "ld64.lld";
 }
 
 auto InstallPaths::llvm_tool_path(LLVMTool tool) const
     -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/llvm/bin" / std::string_view(tool.bin_name());
+  return llvm_install_bin() / std::string_view(tool.bin_name());
 }
 
 auto InstallPaths::clang_resource_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/llvm/lib/clang/" CLANG_VERSION_MAJOR_STRING;
+  return root_ / "llvm/lib/clang/" CLANG_VERSION_MAJOR_STRING;
 }
 
 auto InstallPaths::runtimes_root() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/runtimes";
+  return root_ / "runtimes";
 }
 
 auto InstallPaths::libunwind_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/runtimes/libunwind";
+  return runtimes_root() / "libunwind";
 }
 
 auto InstallPaths::libcxx_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/runtimes/libcxx";
+  return runtimes_root() / "libcxx";
 }
 
 auto InstallPaths::libcxxabi_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/runtimes/libcxxabi";
+  return runtimes_root() / "libcxxabi";
 }
 
 auto InstallPaths::libc_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/runtimes/libc";
+  return runtimes_root() / "libc";
 }
 
 auto InstallPaths::digest_path() const -> std::filesystem::path {
   // TODO: Adjust this to work equally well on Windows.
-  return prefix_ / "lib/carbon/install_digest.txt";
+  return root_ / "install_digest.txt";
 }
 
 }  // namespace Carbon

+ 41 - 39
toolchain/base/install_paths.h

@@ -18,13 +18,21 @@ namespace Carbon {
 
 // Locates the toolchain installation and provides paths to various components.
 //
-// The Carbon toolchain expects to be installed into some install prefix; see
-// `prefix_` for details. When locating an install, we verify it with
-// `CheckMarkerFile`. When errors occur, `SetError` makes `error()`
-// available for diagnostics and clears the install prefix (leaving things
-// minimally functional).
+// The Carbon toolchain expects to be installed into a tree rooted at `root_`.
+// This root contains the marker file and the busy box binary.
 //
-// The factory methods locate the install prefix based on their use-case:
+// In a Unix-like filesystem environment, the root is typically located as
+// `<some prefix>/lib/carbon`, with symlinks in the other parts of the FHS-based
+// layout back to entries below this tree. However, this class and the toolchain
+// itself should only directly use things below the installation root to support
+// non-FHS usage.
+//
+// When locating an install, we verify it with `CheckMarkerFile`. When errors
+// occur, `SetError` makes `error()` available for diagnostics and clears the
+// install root, leaving things minimally functional but with the installation
+// root of the current working directory.
+//
+// The factory methods locate the install root based on their use-case:
 //
 //   - `MakeExeRelative` for command line tools in an install.
 //   - `MakeForBazelRunfiles` for locating through Bazel's runfile tree.
@@ -46,26 +54,22 @@ namespace Carbon {
 class InstallPaths {
  public:
   // Provide the current executable's path to detect the correct installation
-  // prefix path. This assumes the toolchain to be in its installed layout.
-  //
-  // If detection fails, this reverts to using the current working directory as
-  // the install prefix, and the error detected can be checked with `errors()`.
+  // root. This assumes the toolchain to be in its installed layout.
   static auto MakeExeRelative(llvm::StringRef exe_path) -> InstallPaths;
 
   // Provide the current executable's path, and use that to detect a Bazel or
-  // Bazel-compatible runfiles install prefix path. This should only be used
-  // where it is reasonable to rely on this rather than a fixed install location
-  // such as for internal development purposes or other Bazel users of the
-  // Carbon library.
+  // Bazel-compatible runfiles install root. This should only be used where it
+  // is reasonable to rely on this rather than a fixed install location such as
+  // for internal development purposes or other Bazel users of the Carbon
+  // library.
   //
   // This method of construction also ensures the result is valid. If detection
   // fails for any reason, it will `CARBON_CHECK` fail with the error message.
   static auto MakeForBazelRunfiles(llvm::StringRef exe_path) -> InstallPaths;
 
-  // Provide an explicit install paths prefix, which must be absolute. This is
-  // useful for testing or for using Carbon in an environment with an unusual
-  // path to the installed files.
-  static auto Make(llvm::StringRef install_prefix) -> InstallPaths;
+  // Provide an explicit install paths root. This is useful for testing or for
+  // using Carbon in an environment with an unusual path to the installed files.
+  static auto Make(llvm::StringRef install_root) -> InstallPaths;
 
   // Returns the contents of the prelude manifest file. This is the list of
   // files that define the prelude, and will always be non-empty on success.
@@ -131,18 +135,17 @@ class InstallPaths {
  private:
   friend class InstallPathsTestPeer;
 
-  InstallPaths() { SetError("No prefix provided!"); }
-  explicit InstallPaths(std::filesystem::path prefix)
-      : prefix_(std::move(prefix)) {}
+  InstallPaths() { SetError("No root provided!"); }
+  explicit InstallPaths(std::filesystem::path root) : root_(std::move(root)) {}
 
   static auto MakeFromFile(std::filesystem::path file) -> InstallPaths;
 
-  // Set an error message on the install paths and reset the prefix to empty,
+  // Set an error message on the install paths and reset the `root_` to empty,
   // which should use the current working directory.
   auto SetError(llvm::Twine message) -> void;
 
   // Check that the install paths have a marker file at
-  // `prefix()/lib/carbon/carbon_install.txt". If not, calls `SetError` with the
+  // `root()/lib/carbon/carbon_install.txt". If not, calls `SetError` with the
   // relevant error message.
   auto CheckMarkerFile() -> void;
 
@@ -151,32 +154,31 @@ class InstallPaths {
                     std::filesystem::path manifest_file) const
       -> ErrorOr<llvm::SmallVector<std::string>>;
 
-  // The computed installation prefix. This will be an absolute path. We keep an
-  // absolute path for when the command line uses a relative path
-  // (`./bin/carbon`) and the working directory changes after initialization
-  // (for example, to Bazel's working directory). In the event of an error, this
-  // will be the empty string.
+  // The computed installation root. In the event of an error, this will be the
+  // empty string.
   //
-  // When run from bazel (for example, in unit tests or development binaries)
+  // When run from Bazel (for example, in unit tests or development binaries)
   // this will look like:
-  // `bazel-bin/some/bazel/target.runfiles/_main/toolchain/install/prefix_root`
+  // `bazel-bin/some/bazel/target.runfiles/_main/toolchain/install/prefix/lib/carbon`
   //
-  // When installed, it's expected to be similar to the CMake install prefix:
+  // When installed, it's expected to be similar to the CMake install prefix,
+  // followed by `lib/carbon`:
   //
-  // - `C:/Program Files/Carbon` or similar on Windows.
-  // - `/usr` or `/usr/local` on Linux and most BSDs.
-  // - `/opt/homebrew` or similar on macOS with Homebrew.
+  // - `/usr/lib/carbon` or `/usr/local/lib/carbon` on Linux and most BSDs.
+  // - `/opt/homebrew/lib/carbon` or similar on macOS with Homebrew.
+  // - TODO: Figure out if this is `C:/Program Files/Carbon` or something else
+  //   on Windows.
   //
   // See https://cmake.org/cmake/help/latest/variable/CMAKE_INSTALL_PREFIX.html
   // for more details. While we don't build the toolchain with CMake, we expect
   // our installation to behave in a similar and compatible way.
   //
-  // The hierarchy of files beneath the install prefix can be found in the
-  // BUILD's `install_dirs`.
-  std::filesystem::path prefix_;
+  // The hierarchy of files beneath the install root can be found in the
+  // BUILD's `install_dirs` entry for `lib/carbon`.
+  std::filesystem::path root_;
 
-  // The opened prefix directory, suitable for relative path access.
-  Filesystem::Dir prefix_dir_;
+  // The opened root directory.
+  Filesystem::Dir root_dir_;
 
   std::optional<std::string> error_;
 };

+ 29 - 28
toolchain/base/install_paths_test.cpp

@@ -23,8 +23,8 @@ namespace Carbon {
 
 class InstallPathsTestPeer {
  public:
-  static auto GetPrefix(const InstallPaths& paths) -> std::filesystem::path {
-    return paths.prefix_;
+  static auto GetRoot(const InstallPaths& paths) -> std::filesystem::path {
+    return paths.root_;
   }
 };
 
@@ -32,6 +32,7 @@ namespace {
 
 using ::bazel::tools::cpp::runfiles::Runfiles;
 using ::testing::_;
+using ::testing::EndsWith;
 using ::testing::Eq;
 using ::testing::HasSubstr;
 using Testing::IsSuccess;
@@ -47,35 +48,35 @@ class InstallPathsTest : public ::testing::Test {
   }
 
   // Test the install paths found with the given `exe_path`. Will check that
-  // the detected install prefix path starts with `prefix_startswith`, and then
-  // check that the path accessors point to the right kind of file or
-  // directory.
+  // the detected install root path has the expected location relative to the
+  // FHS layout, and then check that the path accessors point to the right kind
+  // of file or directory.
   auto TestInstallPaths(const InstallPaths& paths) -> void {
-    std::filesystem::path prefix_path = InstallPathsTestPeer::GetPrefix(paths);
+    std::filesystem::path root_path = InstallPathsTestPeer::GetRoot(paths);
 
-    SCOPED_TRACE(llvm::formatv("Install prefix: '{0}'", prefix_path));
+    SCOPED_TRACE(llvm::formatv("Install root: '{0}'", root_path));
 
-    // Open the prefix directory.
-    auto prefix_result = Filesystem::Cwd().OpenDir(prefix_path);
-    ASSERT_THAT(prefix_result, IsSuccess(_));
-    Filesystem::Dir prefix = *std::move(prefix_result);
+    // Open the root directory.
+    auto root_result = Filesystem::Cwd().OpenDir(root_path);
+    ASSERT_THAT(root_result, IsSuccess(_));
+    Filesystem::Dir root = *std::move(root_result);
 
-    // Now check that all the expected parts of the toolchain's install are in
-    // fact found using the API.
+    // Check that the root is located in the expected part of the FHS layout.
     // TODO: Adjust this to work equally well on Windows.
+    EXPECT_THAT(root_path.native(), EndsWith("lib/carbon/"));
     EXPECT_THAT(
-        prefix.Access("bin/carbon", Filesystem::AccessCheckFlags::Execute),
+        root.Access("../../bin/carbon", Filesystem::AccessCheckFlags::Execute),
         IsSuccess(Eq(true)))
-        << "path: " << (prefix_path / "bin/carbon");
+        << "path: " << (root_path / "../../bin/carbon");
 
     std::filesystem::path core_package_path = paths.core_package();
-    ASSERT_THAT(core_package_path, StartsWith(prefix_path));
+    ASSERT_THAT(core_package_path, StartsWith(root_path));
     EXPECT_THAT(Filesystem::Cwd().Access(core_package_path / "prelude.carbon"),
                 IsSuccess(Eq(true)))
         << "path: " << core_package_path;
 
     std::filesystem::path llvm_bin_path = paths.llvm_install_bin();
-    ASSERT_THAT(llvm_bin_path, StartsWith(prefix_path));
+    ASSERT_THAT(llvm_bin_path, StartsWith(root_path));
     auto open_result = Filesystem::Cwd().OpenDir(llvm_bin_path);
     ASSERT_THAT(open_result, IsSuccess(_));
     Filesystem::Dir llvm_bin = *std::move(open_result);
@@ -91,24 +92,24 @@ class InstallPathsTest : public ::testing::Test {
   std::unique_ptr<Runfiles> test_runfiles_;
 };
 
-TEST_F(InstallPathsTest, PrefixRootBusybox) {
-  std::string installed_driver_path = test_runfiles_->Rlocation(
-      "carbon/toolchain/install/prefix_root/lib/carbon/carbon-busybox");
+TEST_F(InstallPathsTest, RootBusybox) {
+  std::string installed_busybox_path = test_runfiles_->Rlocation(
+      "carbon/toolchain/install/prefix/lib/carbon/carbon-busybox");
 
-  auto paths = InstallPaths::MakeExeRelative(installed_driver_path);
+  auto paths = InstallPaths::MakeExeRelative(installed_busybox_path);
   ASSERT_THAT(paths.error(), Eq(std::nullopt)) << *paths.error();
   TestInstallPaths(paths);
 }
 
-TEST_F(InstallPathsTest, PrefixRootExplicit) {
+TEST_F(InstallPathsTest, RootExplicit) {
   std::string marker_path = test_runfiles_->Rlocation(
-      "carbon/toolchain/install/prefix_root/lib/carbon/carbon_install.txt");
+      "carbon/toolchain/install/prefix/lib/carbon/carbon_install.txt");
 
-  llvm::StringRef prefix_path = marker_path;
-  CARBON_CHECK(prefix_path.consume_back("lib/carbon/carbon_install.txt"),
+  llvm::StringRef root_path = marker_path;
+  CARBON_CHECK(root_path.consume_back("carbon_install.txt"),
                "Unexpected suffix of the marker path: {0}", marker_path);
 
-  auto paths = InstallPaths::Make(prefix_path);
+  auto paths = InstallPaths::Make(root_path);
   ASSERT_THAT(paths.error(), Eq(std::nullopt)) << *paths.error();
   TestInstallPaths(paths);
 }
@@ -135,11 +136,11 @@ TEST_F(InstallPathsTest, BinaryRunfiles) {
 TEST_F(InstallPathsTest, Errors) {
   auto paths = InstallPaths::Make("/foo/bar/baz");
   EXPECT_THAT(paths.error(), Optional(HasSubstr("foo/bar/baz")));
-  EXPECT_THAT(InstallPathsTestPeer::GetPrefix(paths), Eq(""));
+  EXPECT_THAT(InstallPathsTestPeer::GetRoot(paths), Eq(""));
 
   paths = InstallPaths::MakeExeRelative("foo/bar/baz");
   EXPECT_THAT(paths.error(), Optional(HasSubstr("foo/bar/baz")));
-  EXPECT_THAT(InstallPathsTestPeer::GetPrefix(paths), Eq(""));
+  EXPECT_THAT(InstallPathsTestPeer::GetRoot(paths), Eq(""));
 
   // Note that we can't test the runfiles code path from within a test because
   // it succeeds some of the time even with a bogus executable name.

+ 2 - 2
toolchain/driver/testdata/compile/fail_clang_args.carbon

@@ -12,7 +12,7 @@
 // TIP: To dump output, run:
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/compile/fail_clang_args.carbon
 // CHECK:STDERR: {{.*}}clang version {{.*}}
-// CHECK:STDERR: InstalledDir: {{.*}}/toolchain/install/prefix_root/lib/carbon/../../lib/carbon/llvm/bin
-// CHECK:STDERR:  "{{.*}}/toolchain/install/prefix_root/lib/carbon/../../lib/carbon/llvm/bin/clang" "-cc1" {{.*}}"-triple" "x86_64-unknown-linux-gnu" {{.*}}"-fsyntax-only" {{.*}} "-resource-dir" {{.*}} "-Wall" "-Wextra" "-Wuninitialized" "-Wno-all" {{.*}}
+// CHECK:STDERR: InstalledDir: {{.*}}/toolchain/install/prefix/lib/carbon/llvm/bin
+// CHECK:STDERR:  "{{.*}}/toolchain/install/prefix/lib/carbon/llvm/bin/clang" "-cc1" {{.*}}"-triple" "x86_64-unknown-linux-gnu" {{.*}}"-fsyntax-only" {{.*}} "-resource-dir" {{.*}} "-Wall" "-Wextra" "-Wuninitialized" "-Wno-all" {{.*}}
 
 // --- foo.carbon

+ 1 - 1
toolchain/driver/testdata/compile/optimize/fail_clang_forward_optimize_size.carbon

@@ -11,6 +11,6 @@
 // TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/compile/optimize/fail_clang_forward_optimize_size.carbon
 // TIP: To dump output, run:
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/compile/optimize/fail_clang_forward_optimize_size.carbon
-// CHECK:STDERR:  "{{.*}}/toolchain/install/prefix_root/lib/carbon/../../lib/carbon/llvm/bin/clang" "-cc1" {{.*}} "-Oz" {{.*}}
+// CHECK:STDERR:  "{{.*}}/toolchain/install/prefix/lib/carbon/llvm/bin/clang" "-cc1" {{.*}} "-Oz" {{.*}}
 
 // --- fail_foo.carbon

+ 1 - 1
toolchain/driver/testdata/compile/optimize/fail_clang_forward_optimize_speed.carbon

@@ -11,6 +11,6 @@
 // TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/compile/optimize/fail_clang_forward_optimize_speed.carbon
 // TIP: To dump output, run:
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/compile/optimize/fail_clang_forward_optimize_speed.carbon
-// CHECK:STDERR:  "{{.*}}/toolchain/install/prefix_root/lib/carbon/../../lib/carbon/llvm/bin/clang" "-cc1" {{.*}} "-O3" {{.*}}
+// CHECK:STDERR:  "{{.*}}/toolchain/install/prefix/lib/carbon/llvm/bin/clang" "-cc1" {{.*}} "-O3" {{.*}}
 
 // --- fail_foo.carbon

+ 1 - 1
toolchain/driver/testdata/compile/optimize/fail_clang_no_optimize_arg.carbon

@@ -11,6 +11,6 @@
 // TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/compile/optimize/fail_clang_no_optimize_arg.carbon
 // TIP: To dump output, run:
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/compile/optimize/fail_clang_no_optimize_arg.carbon
-// CHECK:STDERR:  "{{.*}}/toolchain/install/prefix_root/lib/carbon/../../lib/carbon/llvm/bin/clang" "-cc1" {{.*}} "-O1" {{.*}}
+// CHECK:STDERR:  "{{.*}}/toolchain/install/prefix/lib/carbon/llvm/bin/clang" "-cc1" {{.*}} "-O1" {{.*}}
 
 // --- fail_foo.carbon

+ 1 - 1
toolchain/driver/testdata/compile/optimize/fail_clang_override_optimize_arg.carbon

@@ -11,6 +11,6 @@
 // TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/compile/optimize/fail_clang_override_optimize_arg.carbon
 // TIP: To dump output, run:
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/compile/optimize/fail_clang_override_optimize_arg.carbon
-// CHECK:STDERR:  "{{.*}}/toolchain/install/prefix_root/lib/carbon/../../lib/carbon/llvm/bin/clang" "-cc1" {{.*}} "-O1" {{.*}}
+// CHECK:STDERR:  "{{.*}}/toolchain/install/prefix/lib/carbon/llvm/bin/clang" "-cc1" {{.*}} "-O1" {{.*}}
 
 // --- fail_foo.carbon

+ 11 - 8
toolchain/install/BUILD

@@ -20,7 +20,7 @@ package(default_visibility = ["//visibility:public"])
 # Build rules supporting the install data tree for the Carbon toolchain.
 #
 # This populates a synthetic Carbon toolchain installation under the
-# `prefix_root` directory. For details on its layout, see `install_dirs` below.
+# `prefix` directory. For details on its layout, see `install_dirs` below.
 
 cc_library(
     name = "busybox_info",
@@ -294,15 +294,18 @@ filegroup(
     ],
 )
 
-# Given a root `prefix_root`, the hierarchy looks like:
+# Given a CMake-style install prefix[1], the hierarchy looks like:
 #
-# - prefix_root/bin: Binaries intended for direct use.
-# - prefix_root/lib/carbon: Private data and files.
-# - prefix_root/lib/carbon/core: The `Core` package files.
-# - prefix_root/lib/carbon/llvm/bin: LLVM binaries.
+# - prefix/bin: Binaries intended for direct use.
+# - prefix/lib/carbon: Private data and files.
+# - prefix/lib/carbon/core: The `Core` package files.
+# - prefix/lib/carbon/llvm/bin: LLVM binaries.
 #
 # This will be how installs are provided on Unix-y platforms, and is loosely
-# based on the FHS (Filesystem Hierarchy Standard).
+# based on the FHS (Filesystem Hierarchy Standard). See the CMake install prefix
+# documentation[1] for more details.
+#
+# [1]: https://cmake.org/cmake/help/latest/variable/CMAKE_INSTALL_PREFIX.html
 install_dirs = {
     "bin": [
         install_symlink(
@@ -388,7 +391,7 @@ make_install_filegroups(
     no_digest_name = "install_data.no_digest",
     no_driver_name = "install_data.no_driver",
     pkg_name = "pkg_data",
-    prefix = "prefix_root",
+    prefix = "prefix",
 )
 
 py_test(

+ 4 - 4
toolchain/install/busybox_info.cpp

@@ -45,11 +45,11 @@ auto GetBusyboxInfo(const char* argv0) -> ErrorOr<BusyboxInfo> {
       //
       // This will never occur in a "bin" subdirectory, so doesn't need to be
       // handled in the other return path.
-      std::string prefix_root = info.bin_path.parent_path().string() +
-                                "/prefix_root/lib/carbon/carbon-busybox";
-      if (auto access = Filesystem::Cwd().Access(prefix_root);
+      std::string busybox_path = info.bin_path.parent_path().string() +
+                                 "/prefix/lib/carbon/carbon-busybox";
+      if (auto access = Filesystem::Cwd().Access(busybox_path);
           access.ok() && *access) {
-        info.bin_path = prefix_root;
+        info.bin_path = busybox_path;
       }
       return info;
     }

+ 1 - 1
toolchain/install/llvm_symlinks_test.py

@@ -20,7 +20,7 @@ from bazel_tools.tools.python.runfiles import runfiles
 class LLVMSymlinksTest(unittest.TestCase):
     def setUp(self) -> None:
         # The install root is adjacent to the test script
-        self.install_root = Path(sys.argv[0]).parent / "prefix_root"
+        self.install_root = Path(sys.argv[0]).parent / "prefix"
         self.tmpdir = Path(os.environ["TEST_TMPDIR"])
         self.test_o_file = self.tmpdir / "test.o"
         self.test_o_file.touch()

+ 2 - 2
toolchain/install/make_installation_digest.cpp

@@ -82,7 +82,7 @@ auto DigestProgram::Run(int argc, char** argv) -> ErrorOr<int> {
       continue;
     }
     // Compute the full path and installed path for each file. The installed
-    // path comes from the path components below the `prefix_root` component.
+    // path comes from the path components below the `prefix` component.
     std::filesystem::path full_path = manifest_line.trim().str();
     std::filesystem::path install_path;
     bool append = false;
@@ -92,7 +92,7 @@ auto DigestProgram::Run(int argc, char** argv) -> ErrorOr<int> {
         continue;
       }
 
-      if (component == "prefix_root") {
+      if (component == "prefix") {
         append = true;
       }
     }

+ 2 - 2
toolchain/install/toolchain_tar_test.py

@@ -22,10 +22,10 @@ class ToolchainTarTest(unittest.TestCase):
 
         # Gather install data files.
         with open(install_data_manifest) as manifest:
-            # Remove everything up to and including `prefix_root`.
+            # Remove everything up to and including `prefix`.
             install_files = set(
                 [
-                    re.sub("^.*/prefix_root/", "", entry.strip())
+                    re.sub("^.*/prefix/", "", entry.strip())
                     for entry in manifest.readlines()
                 ]
             )