Change the bazel-out structure to avoid busybox symlinks. (#4505)

As described in symlink_helpers.bzl, copied here for visibility:

Symlinking busybox things needs special logic.

This is because Bazel doesn't cache the actual symlink, resulting in
essentially resolved symlinks being produced in place of the expected
tool. As a consequence, we can't rely on the symlink name when dealing
with busybox entries.

An example repro of this using a local build cache is:

    bazel build //toolchain
    bazel clean
    bazel build //toolchain

We could in theory get reasonable behavior with
`ctx.actions.declare_symlink`, but that's disallowed in our `.bazelrc`
for cross-environment compatibility.

The particular approach here uses the Python script as a launching pad
so that the busybox still receives an appropriate location in argv[0],
allowing it to find other files in the lib directory. Arguments are
inserted to get equivalent behavior as if symlink resolution had
occurred.

The underlying bug is noted at:
https://github.com/bazelbuild/bazel/issues/23620

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/bin/carbon
+lldb bazel-bin/toolchain/install/prefix_root/lib/carbon/carbon-busybox
 ```
 
 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_filegroup", "install_symlink", "install_target", "make_install_filegroups")
+load("install_filegroups.bzl", "install_busybox_wrapper", "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")
 
@@ -108,10 +108,9 @@ lld_aliases = [
 # based on the FHS (Filesystem Hierarchy Standard).
 install_dirs = {
     "bin": [
-        install_symlink(
+        install_busybox_wrapper(
             "carbon",
             "../lib/carbon/carbon-busybox",
-            is_driver = True,
         ),
     ],
     "lib/carbon": [
@@ -130,10 +129,13 @@ install_dirs = {
             "@llvm-project//lld:lld",
             executable = True,
         ),
-        install_symlink(
+        install_busybox_wrapper(
             "clang",
             "../../carbon-busybox",
-            is_driver = True,
+            [
+                "clang",
+                "--",
+            ],
         ),
     ] + [install_symlink(name, "lld") for name in lld_aliases],
 }

toolchain/install/install_filegroups.bzl

@@ -5,7 +5,25 @@
 """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", "symlink_file", "symlink_filegroup")
+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,
+    }
 
 def install_filegroup(name, filegroup_target):
     """Adds a filegroup for install.
@@ -22,7 +40,7 @@ def install_filegroup(name, filegroup_target):
         "name": name,
     }
 
-def install_symlink(name, symlink_to, is_driver = False):
+def install_symlink(name, symlink_to):
     """Adds a symlink for install.
 
     Used in the `install_dirs` dict.
@@ -30,11 +48,9 @@ def install_symlink(name, symlink_to, is_driver = False):
     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": is_driver,
+        "is_driver": False,
         "name": name,
         "symlink": symlink_to,
     }
@@ -106,6 +122,19 @@ 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,14 +4,34 @@
 
 """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}"
+
+# Make sure we have the expected structure.
+if not __file__.endswith(_SCRIPT_LOCATION):
+    exit(
+        "Unable to figure out path:\\n"
+        f"  __file__: {{__file__}}\\n"
+        f"  script: {{_SCRIPT_LOCATION}}\\n"
+    )
+
+# Run the tool using the absolute path, forwarding arguments.
+tool_path = __file__.removesuffix(_SCRIPT_LOCATION) + _TOOL_LOCATION
+os.execv(tool_path, [tool_path] + sys.argv[1:])
+"""
+
 def _run_tool_impl(ctx):
-    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(
+    content = _RUN_TOOL_TMPL.format(ctx.outputs.executable.path, ctx.file.tool.path)
+    ctx.actions.write(
         output = ctx.outputs.executable,
-        target_file = tool_files[0],
         is_executable = True,
+        content = content,
     )
     return [
         DefaultInfo(
@@ -30,6 +50,7 @@ run_tool = rule(
             allow_single_file = True,
             executable = True,
             cfg = "target",
+            mandatory = True,
         ),
     },
     executable = True,

toolchain/install/symlink_helpers.bzl

@@ -4,36 +4,66 @@
 
 """Rules for symlinking in ways that assist install_filegroups."""
 
-def _symlink_filegroup_impl(ctx):
-    prefix = ctx.attr.out_prefix
+_SYMLINK_BUSYBOX_TMPL = """#!/usr/bin/env python3
 
-    outputs = []
-    for f in ctx.files.srcs:
-        # We normalize the path to be package-relative in order to ensure
-        # consistent paths across possible repositories.
-        relative_path = f.short_path.removeprefix(f.owner.package)
+from pathlib import Path
+import os
+import sys
 
-        out = ctx.actions.declare_file(prefix + relative_path)
-        outputs.append(out)
-        ctx.actions.symlink(output = out, target_file = f)
+_RELATIVE_PATH = "{0}"
+_BUSYBOX_ARGS = {1}
 
-    if len(ctx.files.srcs) != len(outputs):
-        fail("Output count mismatch!")
+# Run the tool using the absolute path, forwarding arguments.
+tool_path = Path(__file__).parent / _RELATIVE_PATH
+os.execv(tool_path, [tool_path] + _BUSYBOX_ARGS + sys.argv[1:])
+"""
 
-    return [
-        DefaultInfo(
-            files = depset(direct = outputs),
-            default_runfiles = ctx.runfiles(files = outputs),
-        ),
-    ]
+def _busybox_wrapper_impl(ctx):
+    """Symlinking busybox things needs special logic.
 
-symlink_filegroup = rule(
-    doc = "Symlinks an entire filegroup, preserving its structure",
-    implementation = _symlink_filegroup_impl,
+    This is because Bazel doesn't cache the actual symlink, resulting in
+    essentially resolved symlinks being produced in place of the expected tool.
+    As a consequence, we can't rely on the symlink name when dealing with
+    busybox entries.
+
+    An example repro of this using a local build cache is:
+        bazel build //toolchain
+        bazel clean
+        bazel build //toolchain
+
+    We could in theory get reasonable behavior with
+    `ctx.actions.declare_symlink`, but that's disallowed in our `.bazelrc` for
+    cross-environment compatibility.
+
+    The particular approach here uses the Python script as a launching pad so
+    that the busybox still receives an appropriate location in argv[0], allowing
+    it to find other files in the lib directory. Arguments are inserted to get
+    equivalent behavior as if symlink resolution had occurred.
+
+    The underlying bug is noted at:
+    https://github.com/bazelbuild/bazel/issues/23620
+    """
+    content = _SYMLINK_BUSYBOX_TMPL.format(
+        ctx.attr.symlink,
+        ctx.attr.busybox_args,
+    )
+    ctx.actions.write(
+        output = ctx.outputs.executable,
+        is_executable = True,
+        content = content,
+    )
+    return []
+
+busybox_wrapper = rule(
+    doc = "Helper for running a busybox with symlink-like characteristics.",
+    implementation = _busybox_wrapper_impl,
     attrs = {
-        "out_prefix": attr.string(mandatory = True),
-        "srcs": attr.label_list(mandatory = True),
+        "busybox_args": attr.string_list(
+            doc = "Optional arguments to pass for equivalent behavior to a symlink.",
+        ),
+        "symlink": attr.string(mandatory = True),
     },
+    executable = True,
 )
 
 def _symlink_file_impl(ctx):
@@ -75,3 +105,35 @@ symlink_file = rule(
         "symlink_label": attr.label(allow_single_file = True),
     },
 )
+
+def _symlink_filegroup_impl(ctx):
+    prefix = ctx.attr.out_prefix
+
+    outputs = []
+    for f in ctx.files.srcs:
+        # We normalize the path to be package-relative in order to ensure
+        # consistent paths across possible repositories.
+        relative_path = f.short_path.removeprefix(f.owner.package)
+
+        out = ctx.actions.declare_file(prefix + relative_path)
+        outputs.append(out)
+        ctx.actions.symlink(output = out, target_file = f)
+
+    if len(ctx.files.srcs) != len(outputs):
+        fail("Output count mismatch!")
+
+    return [
+        DefaultInfo(
+            files = depset(direct = outputs),
+            default_runfiles = ctx.runfiles(files = outputs),
+        ),
+    ]
+
+symlink_filegroup = rule(
+    doc = "Symlinks an entire filegroup, preserving its structure",
+    implementation = _symlink_filegroup_impl,
+    attrs = {
+        "out_prefix": attr.string(mandatory = True),
+        "srcs": attr.label_list(mandatory = True),
+    },
+)