Replace #4505 with a different set of workarounds.

This restores the symlinks for the installation, but teaches the busybox
info search to look for a relative path to the busybox binary itself
before walking through symlinks. This let's it find the tree structure
when directly invoking `prefix_root/bin/carbon` or similar, either
inside of a Bazel rule or from the command line, and mirrors how we
expect the installed tree to look. This works even when Bazel resolves
the symlink target fully, and potentially to something nonsensical like
a CAS file.

In order to make a convenient Bazel target that can be used with `bazel
run //toolchain`, this adds an override to explicitly set the desired
argv[0] to use when selecting a mode for the busybox and a busybox
binary. Currently, the workaround uses an environment variable because
that required the least amount of plumbing, and seems a useful override
mechanism generally, but I'm open to other approaches.

This should allow a few things to work a bit more nicely:
- It should handle sibling symlinks like `clang++` to `clang` or
  `ld.lld` to `lld`, where that symlink in turn points at the busybox.
  We want to use *initial* `argv[0]` value to select the mode there.
- It avoids bouncing through Python (or other subprocesses) when
  invoking the `carbon` binary in Bazel rules, which will be nice for
  building the example code and benchmarking.

It does come at a cost of removing one feature: the initial symlink
can't be some unrelated alias like `my_carbon_symlink` -- we expect the
*first* argv[0] name to have the meaningful filename for selecting
a busybox mode.

It also trades the complexity of the Python script for some complexity
in the busybox search in order to look for a relative `carbon-busybox`
binary. On the whole, I think that tradeoff is worthwhile, but it isn't
free.

docs/project/contribution_tools.md

@@ -305,7 +305,7 @@ bazel build -c dbg //toolchain
 Then debugging works with LLDB:
 
 ```shell
-lldb bazel-bin/toolchain/install/prefix_root/lib/carbon/carbon-busybox
+lldb bazel-bin/toolchain/install/prefix_root/bin/carbon
 ```
 
 Any installed version of LLDB at least as recent as the installed Clang used for

toolchain/install/BUILD

@@ -5,7 +5,7 @@
 load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test")
 load("//bazel/cc_toolchains:defs.bzl", "cc_env")
 load("//bazel/manifest:defs.bzl", "manifest")
-load("install_filegroups.bzl", "install_busybox_wrapper", "install_filegroup", "install_symlink", "install_target", "make_install_filegroups")
+load("install_filegroups.bzl", "install_filegroup", "install_symlink", "install_target", "make_install_filegroups")
 load("pkg_helpers.bzl", "pkg_naming_variables", "pkg_tar_and_test")
 load("run_tool.bzl", "run_tool")
 
@@ -88,6 +88,7 @@ cc_test(
         "//common:check",
         "//testing/base:gtest_main",
         "@googletest//:gtest",
+        "@llvm-project//llvm:Support",
     ],
 )
 
@@ -129,9 +130,10 @@ lld_aliases = [
 # based on the FHS (Filesystem Hierarchy Standard).
 install_dirs = {
     "bin": [
-        install_busybox_wrapper(
+        install_symlink(
             "carbon",
             "../lib/carbon/carbon-busybox",
+            is_driver = True,
         ),
     ],
     "lib/carbon": [
@@ -150,13 +152,10 @@ install_dirs = {
             "@llvm-project//lld:lld",
             executable = True,
         ),
-        install_busybox_wrapper(
+        install_symlink(
             "clang",
             "../../carbon-busybox",
-            [
-                "clang",
-                "--",
-            ],
+            is_driver = True,
         ),
     ] + [install_symlink(name, "lld") for name in lld_aliases],
 }

toolchain/install/busybox_info.h

@@ -14,6 +14,8 @@
 
 namespace Carbon {
 
+constexpr const char* Argv0OverrideEnv = "CARBON_ARGV0_OVERRIDE";
+
 struct BusyboxInfo {
   // The path to `carbon-busybox`.
   std::filesystem::path bin_path;
@@ -24,21 +26,58 @@ struct BusyboxInfo {
 // Returns the busybox information, given argv[0]. This primarily handles
 // resolving symlinks that point at the busybox.
 inline auto GetBusyboxInfo(llvm::StringRef argv0) -> ErrorOr<BusyboxInfo> {
-  BusyboxInfo info = BusyboxInfo{argv0.str(), std::nullopt};
+  BusyboxInfo info;
+
+  // Check for an override of `argv[0]` from the environment and apply it.
+  if (const char* argv0_override = getenv(Argv0OverrideEnv)) {
+    argv0 = argv0_override;
+  }
+
+  info.bin_path = argv0.str();
+  std::filesystem::path filename = info.bin_path.filename();
+  // The mode is set to the initial filename used for `argv[0]`.
+  if (filename != "carbon" && filename != "carbon-busybox") {
+    info.mode = filename;
+  }
+
+  // Now search through any symlinks to locate the installed busybox binary.
   while (true) {
-    std::string filename = info.bin_path.filename();
+    filename = info.bin_path.filename();
     if (filename == "carbon-busybox") {
       return info;
     }
+
+    // If we've not already reached the busybox, look for it relative to the
+    // current binary path. This can help more immediately locate an
+    // installation tree, and avoids walking through a final layer of symlinks
+    // which may point to content-addressed storage or other parts of a build
+    // output tree.
+    //
+    // We break this into two cases we need to handle:
+    // - Carbon's CLI will be: `<prefix>/bin/carbon`
+    // - Other tools will be: `<prefix>/lib/carbon/<group>/bin/<tool>`
     std::error_code ec;
+    auto parent_path = info.bin_path.parent_path();
+    auto lib_path = filename == "carbon" ? parent_path / ".." / "lib" / "carbon"
+                                         : parent_path / ".." / "..";
+    auto busybox_path = lib_path / "carbon-busybox";
+    if (std::filesystem::exists(busybox_path, ec)) {
+      info.bin_path = busybox_path;
+      return info;
+    }
+    // Errors for the relative busybox search aren't interesting.
+    ec.clear();
+
+    // Try to walk through another layer of symlinks and see if we can find the
+    // installation there or are linked directly to the busybox.
     auto symlink_target = std::filesystem::read_symlink(info.bin_path, ec);
     if (ec) {
       return ErrorBuilder()
              << "expected carbon-busybox symlink at `" << info.bin_path << "`";
     }
-    info.mode = filename;
+
     // Do a path join, to handle relative symlinks.
-    info.bin_path = info.bin_path.parent_path() / symlink_target;
+    info.bin_path = parent_path / symlink_target;
   }
 }
 

toolchain/install/busybox_info_test.cpp

@@ -11,6 +11,7 @@
 #include <fstream>
 
 #include "common/check.h"
+#include "llvm/ADT/ScopeExit.h"
 
 namespace Carbon {
 namespace {
@@ -80,7 +81,7 @@ TEST_F(BusyboxInfoTest, SymlinkInCurrentDirectory) {
   auto info = GetBusyboxInfo(target.string());
   ASSERT_TRUE(info.ok()) << info.error();
   EXPECT_THAT(info->bin_path, Eq(dir_ / "carbon-busybox"));
-  EXPECT_THAT(info->mode, Eq("carbon"));
+  EXPECT_THAT(info->mode, Eq(std::nullopt));
 }
 
 TEST_F(BusyboxInfoTest, SymlinkInCurrentDirectoryWithDot) {
@@ -90,18 +91,18 @@ TEST_F(BusyboxInfoTest, SymlinkInCurrentDirectoryWithDot) {
   auto info = GetBusyboxInfo(target.string());
   ASSERT_TRUE(info.ok()) << info.error();
   EXPECT_THAT(info->bin_path, Eq(dir_ / "./carbon-busybox"));
-  EXPECT_THAT(info->mode, Eq("carbon"));
+  EXPECT_THAT(info->mode, Eq(std::nullopt));
 }
 
 TEST_F(BusyboxInfoTest, ExtraSymlink) {
   MakeFile(dir_ / "carbon-busybox");
-  MakeSymlink(dir_ / "carbon", "carbon-busybox");
-  auto target = MakeSymlink(dir_ / "c", "carbon");
+  MakeSymlink(dir_ / "c", "carbon-busybox");
+  auto target = MakeSymlink(dir_ / "carbon", "c");
 
   auto info = GetBusyboxInfo(target.string());
   ASSERT_TRUE(info.ok()) << info.error();
   EXPECT_THAT(info->bin_path, Eq(dir_ / "carbon-busybox"));
-  EXPECT_THAT(info->mode, Eq("carbon"));
+  EXPECT_THAT(info->mode, Eq(std::nullopt));
 }
 
 TEST_F(BusyboxInfoTest, BusyboxIsSymlink) {
@@ -134,7 +135,7 @@ TEST_F(BusyboxInfoTest, RelativeSymlink) {
   auto info = GetBusyboxInfo(target.string());
   ASSERT_TRUE(info.ok()) << info.error();
   EXPECT_THAT(info->bin_path, Eq(dir_ / "bin/../lib/carbon/carbon-busybox"));
-  EXPECT_THAT(info->mode, Eq("carbon"));
+  EXPECT_THAT(info->mode, Eq(std::nullopt));
 }
 
 TEST_F(BusyboxInfoTest, AbsoluteSymlink) {
@@ -147,8 +148,8 @@ TEST_F(BusyboxInfoTest, AbsoluteSymlink) {
 
   auto info = GetBusyboxInfo(target.string());
   ASSERT_TRUE(info.ok()) << info.error();
-  EXPECT_THAT(info->bin_path, Eq(busybox));
-  EXPECT_THAT(info->mode, Eq("carbon"));
+  EXPECT_TRUE(info->bin_path.is_absolute());
+  EXPECT_THAT(info->mode, Eq(std::nullopt));
 }
 
 TEST_F(BusyboxInfoTest, NotBusyboxFile) {
@@ -166,5 +167,58 @@ TEST_F(BusyboxInfoTest, NotBusyboxSymlink) {
   EXPECT_FALSE(info.ok());
 }
 
+TEST_F(BusyboxInfoTest, LayerSymlinksInstallTree) {
+  auto actual_busybox = MakeFile(dir_ / "actual-busybox");
+
+  // 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 = dir_ / "test_prefix";
+  MakeDir(prefix);
+  MakeDir(prefix / "lib");
+  MakeDir(prefix / "lib/carbon");
+  MakeSymlink(prefix / "lib/carbon/carbon-busybox", actual_busybox);
+  MakeDir(prefix / "lib/llvm");
+  MakeDir(prefix / "lib/llvm/bin");
+  auto clangplusplus_target =
+      MakeSymlink(prefix / "lib/llvm/bin/clang++", "clang");
+  auto clang_target =
+      MakeSymlink(prefix / "lib/llvm/bin/clang", "../../carbon-busybox");
+  MakeDir(prefix / "bin");
+  auto carbon_target =
+      MakeSymlink(prefix / "bin/carbon", "../lib/carbon/carbon-busybox");
+
+  auto info = GetBusyboxInfo(carbon_target.string());
+  ASSERT_TRUE(info.ok()) << info.error();
+  EXPECT_TRUE(info->bin_path.is_absolute());
+  EXPECT_THAT(info->mode, Eq(std::nullopt));
+
+  info = GetBusyboxInfo(clang_target.string());
+  ASSERT_TRUE(info.ok()) << info.error();
+  EXPECT_TRUE(info->bin_path.is_absolute());
+  EXPECT_THAT(info->mode, Eq("clang"));
+
+  info = GetBusyboxInfo(clangplusplus_target.string());
+  ASSERT_TRUE(info.ok()) << info.error();
+  EXPECT_TRUE(info->bin_path.is_absolute());
+  EXPECT_THAT(info->mode, Eq("clang++"));
+}
+
+TEST_F(BusyboxInfoTest, EnvBinaryPathOverride) {
+  // The test should not have this environment variable set.
+  ASSERT_THAT(getenv(Argv0OverrideEnv), Eq(nullptr));
+  // Clean up this environment variable when this test finishes.
+  auto _ = llvm::make_scope_exit([] { unsetenv(Argv0OverrideEnv); });
+
+  // Set the environment to our actual busybox.
+  auto busybox = MakeFile(dir_ / "carbon-busybox");
+  setenv(Argv0OverrideEnv, busybox.c_str(), /*overwrite=*/1);
+
+  auto info = GetBusyboxInfo("/some/nonexistent/path");
+  ASSERT_TRUE(info.ok()) << info.error();
+  EXPECT_THAT(info->bin_path, Eq(busybox));
+  EXPECT_THAT(info->mode, Eq(std::nullopt));
+}
+
 }  // namespace
 }  // namespace Carbon

toolchain/install/busybox_main.cpp

@@ -40,7 +40,7 @@ static auto Main(int argc, char** argv) -> ErrorOr<int> {
 
   llvm::SmallVector<llvm::StringRef> args;
   args.reserve(argc + 1);
-  if (busybox_info.mode && busybox_info.mode != "carbon") {
+  if (busybox_info.mode) {
     args.append({*busybox_info.mode, "--"});
   }
   args.append(argv + 1, argv + argc);

toolchain/install/install_filegroups.bzl

@@ -5,25 +5,7 @@
 """Rules for constructing install information."""
 
 load("@rules_pkg//pkg:mappings.bzl", "pkg_attributes", "pkg_filegroup", "pkg_files", "pkg_mklink", "strip_prefix")
-load("symlink_helpers.bzl", "busybox_wrapper", "symlink_file", "symlink_filegroup")
-
-def install_busybox_wrapper(name, busybox, busybox_args = []):
-    """Adds a busybox wrapper for install.
-
-    Used in the `install_dirs` dict.
-
-    Args:
-      name: The filename to use.
-      busybox: A relative path for the busybox.
-      busybox_args: Arguments needed to simulate busybox when a symlink isn't
-        actually used.
-    """
-    return {
-        "busybox": busybox,
-        "busybox_args": busybox_args,
-        "is_driver": True,
-        "name": name,
-    }
+load("symlink_helpers.bzl", "symlink_file", "symlink_filegroup")
 
 def install_filegroup(name, filegroup_target):
     """Adds a filegroup for install.
@@ -40,17 +22,19 @@ def install_filegroup(name, filegroup_target):
         "name": name,
     }
 
-def install_symlink(name, symlink_to):
+def install_symlink(name, symlink_to, is_driver = False):
     """Adds a symlink for install.
 
     Used in the `install_dirs` dict.
 
     Args:
       name: The filename to use.
       symlink_to: A relative path for the symlink.
+      is_driver: False if it should be included in the `no_driver_name`
+        filegroup.
     """
     return {
-        "is_driver": False,
+        "is_driver": is_driver,
         "name": name,
         "symlink": symlink_to,
     }
@@ -122,19 +106,6 @@ def make_install_filegroups(name, no_driver_name, pkg_name, install_dirs, prefix
                     attributes = pkg_attributes(mode = mode),
                     renames = {entry["target"]: path},
                 )
-            elif "busybox" in entry:
-                busybox_wrapper(
-                    name = prefixed_path,
-                    symlink = entry["busybox"],
-                    busybox_args = entry["busybox_args"],
-                )
-
-                # For the distributed package, we retain relative symlinks.
-                pkg_mklink(
-                    name = pkg_path,
-                    link_name = path,
-                    target = entry["busybox"],
-                )
             elif "filegroup" in entry:
                 symlink_filegroup(
                     name = prefixed_path,

toolchain/install/run_tool.bzl

@@ -4,50 +4,23 @@
 
 """Supports running a tool from the install filegroup."""
 
-_RUN_TOOL_TMPL = """#!/usr/bin/env python3
-
-import os
-import sys
-
-# These will be relative locations in bazel-out.
-_SCRIPT_LOCATION = "{0}"
-_TOOL_LOCATION = "{1}"
-
-# Drop shared parent portions so that we're working with a minimum suffix.
-# This is so that both `bazel-out` and manual `bazel-bin` invocations work.
-script_parts = _SCRIPT_LOCATION.split('/')
-tool_parts = _TOOL_LOCATION.split('/')
-while script_parts and tool_parts and script_parts[0] == tool_parts[0]:
-    del script_parts[0]
-    del tool_parts[0]
-script_suffix = '/' + '/'.join(script_parts)
-tool_suffix = '/' + '/'.join(tool_parts)
-
-# Make sure we have the expected structure.
-if not __file__.endswith(script_suffix):
-    exit(
-        "Unable to figure out path:\\n"
-        f"  __file__: {{__file__}}\\n"
-        f"  script: {{script_suffix}}\\n"
-    )
-
-# Run the tool using the absolute path, forwarding arguments.
-tool_path = __file__.removesuffix(script_suffix) + tool_suffix
-os.execv(tool_path, [tool_path] + sys.argv[1:])
-"""
-
 def _run_tool_impl(ctx):
-    content = _RUN_TOOL_TMPL.format(ctx.outputs.executable.path, ctx.file.tool.path)
-    ctx.actions.write(
+    tool_files = ctx.attr.tool.files.to_list()
+    if len(tool_files) != 1:
+        fail("Expected 1 tool file, found {0}".format(len(tool_files)))
+    ctx.actions.symlink(
         output = ctx.outputs.executable,
+        target_file = tool_files[0],
         is_executable = True,
-        content = content,
     )
     return [
         DefaultInfo(
             runfiles = ctx.runfiles(files = ctx.files.data),
         ),
-        RunEnvironmentInfo(environment = ctx.attr.env),
+        RunEnvironmentInfo(
+            environment = ctx.attr.env |
+                          {"CARBON_ARGV0_OVERRIDE": tool_files[0].short_path},
+        ),
     ]
 
 run_tool = rule(