Refactor install structure to make changes easier. (#4332)

Instead of separately constructing file information for `filegroup` and
`pkg_filegroup`, this instead creates a single structure which is used
to generate both. Additionally, I'm unifying the `llvm_link_data` and
`install_lib_data` targets (though the `llvm_link_data` target is
problematic for busyboxing, I don't think it can keep working as it does
right now).

Note I'm also stopping reuse of llvm's binary_alias. We need to be able
to symlink non-binary files, so I'm going to just share logic there.
(plus, I admit I find the name "binary_alias" confusing since it's not
an alias in bazel terms)

---------

Co-authored-by: Chandler Carruth <chandlerc@gmail.com>

toolchain/driver/BUILD

@@ -18,7 +18,7 @@ cc_library(
     srcs = ["clang_runner.cpp"],
     hdrs = ["clang_runner.h"],
     data = [
-        "//toolchain/install:llvm_link_data",
+        "//toolchain/install:install_data.no_driver",
     ],
     deps = [
         "//common:command_line",
@@ -94,7 +94,7 @@ cc_library(
         "driver_subcommand.h",
     ],
     data = [
-        "//toolchain/install:install_lib_data",
+        "//toolchain/install:install_data.no_driver",
     ],
     textual_hdrs = ["flags.def"],
     deps = [

toolchain/install/BUILD

@@ -2,13 +2,10 @@
 # Exceptions. See /LICENSE for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-load("@bazel_skylib//rules:write_file.bzl", "write_file")
-load("@llvm-project//llvm:binary_alias.bzl", "binary_alias")
 load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test")
-load("@rules_pkg//pkg:mappings.bzl", "pkg_attributes", "pkg_filegroup", "pkg_files", "pkg_mklink", "strip_prefix")
+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")
-load("symlink_filegroup.bzl", "symlink_filegroup")
 
 package(default_visibility = ["//visibility:public"])
 
@@ -17,99 +14,6 @@ package(default_visibility = ["//visibility:public"])
 # This populates a synthetic Carbon toolchain installation under the
 # `prefix_root` directory. For details on its layout, see `install_paths.h`.
 
-# A marker of a valid Carbon install. All filegroups in the install should
-# include this one.
-write_file(
-    name = "install_marker",
-    out = "prefix_root/lib/carbon/carbon_install.txt",
-    content = [
-        "// Part of the Carbon Language project, under the Apache License v2.0 with LLVM",
-        "// Exceptions. See /LICENSE for license information.",
-        "// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception",
-        "",
-        "This marks a valid Carbon install tree.",
-    ],
-)
-
-# Create symlinks for the core library. Note that this should *not* be depended
-# on directly, use `:core_data` instead.
-symlink_filegroup(
-    name = "symlink_core",
-    srcs = ["//core:prelude"],
-    out_prefix = "prefix_root/lib/carbon/core/",
-    visibility = ["//visibility:private"],
-)
-
-# The filegroup to get the core library.
-filegroup(
-    name = "core_data",
-    srcs = [
-        ":install_marker",
-        ":symlink_core",
-    ],
-)
-
-# Copy Clang and LLVM toolchain files into a synthetic LLVM installation under
-# `prefix_root/lib/carbon/llvm` so that parts of Clang that expect to find an LLVM
-# installation at relative paths work correctly without exposing these in an
-# installed 'bin' directory where it might get added to a user's PATH.
-binary_alias(
-    name = "prefix_root/lib/carbon/llvm/bin/lld",
-    binary = "@llvm-project//lld:lld",
-)
-
-lld_bin_names = [
-    "ld.lld",
-    "ld64.lld",
-    "lld-link",
-    "wasm-ld",
-]
-
-[
-    binary_alias(
-        name = "prefix_root/lib/carbon/llvm/bin/" + bin_name,
-        binary = "@llvm-project//lld:lld",
-    )
-    for bin_name in lld_bin_names
-]
-
-filegroup(
-    name = "llvm_link_data",
-    srcs = [
-        "prefix_root/lib/carbon/llvm/bin/lld",
-        ":install_marker",
-    ] + [
-        "prefix_root/lib/carbon/llvm/bin/" + bin_name
-        for bin_name in lld_bin_names
-    ],
-)
-
-# All of the install data except for the user-facing binaries. This is typically
-# a reasonable data dependency for libraries and the user-facing binaries
-# without creating a cycle.
-filegroup(
-    name = "install_lib_data",
-    srcs = [
-        ":core_data",
-        ":install_marker",
-        ":llvm_link_data",
-    ],
-)
-
-binary_alias(
-    name = "prefix_root/bin/carbon",
-    binary = "//toolchain/driver:carbon",
-)
-
-filegroup(
-    name = "install_data",
-    srcs = [
-        "prefix_root/bin/carbon",
-        ":install_lib_data",
-        ":install_marker",
-    ],
-)
-
 # A library for computing install paths for the toolchain. Note that this
 # library does *not* include the data itself, as that would form a dependency
 # cycle. Each part of the toolchain should add the narrow data file groups to
@@ -165,51 +69,41 @@ cc_library(
     ],
 )
 
-# Build rules to construct packaged versions of the toolchain's install.
-
-pkg_files(
-    name = "packaging_exe_files",
-    # We break out the driver and LLD here because we can't easily use file
-    # groups that contain symlinks, so those are manually handled below. Other
-    # file groups should be directly included.
-    srcs = [
-        "prefix_root/bin/carbon",
-        "prefix_root/lib/carbon/llvm/bin/lld",
-    ],
-    attributes = pkg_attributes(mode = "0755"),
-    strip_prefix = strip_prefix.from_pkg("prefix_root"),
-)
-
-# Currently, `rules_pkg` can't replicate symlinks from the main tree. To an
-# extent, this is reasonable because we want to be much more explicit about the
-# symlink structure in the package where as for the filegroups we're comfortable
-# with whatever "just works" for development and testing.
-[
-    pkg_mklink(
-        name = "packaging_link_lld_alias_" + bin_name,
-        link_name = "lib/carbon/llvm/bin/" + bin_name,
-        target = "lld",
-    )
-    for bin_name in lld_bin_names
+lld_aliases = [
+    "ld.lld",
+    "ld64.lld",
+    "lld-link",
+    "wasm-ld",
 ]
 
-pkg_files(
-    name = "packaging_data_files",
-    srcs = [
-        ":core_data",
+install_dirs = {
+    "bin": [
+        install_target(
+            "carbon",
+            "//toolchain/driver:carbon",
+            executable = True,
+            is_driver = True,
+        ),
     ],
-    strip_prefix = strip_prefix.from_pkg("prefix_root"),
-)
-
-pkg_filegroup(
-    name = "packaging_files",
-    srcs = [
-        ":packaging_data_files",
-        ":packaging_exe_files",
-    ] + [
-        ":packaging_link_lld_alias_" + bin_name
-        for bin_name in lld_bin_names
+    "lib/carbon": [
+        install_target("carbon_install.txt", "carbon_install.txt"),
+        install_filegroup("core", "//core:prelude"),
     ],
+    "lib/carbon/llvm/bin": [
+        install_target(
+            "lld",
+            "@llvm-project//lld:lld",
+            executable = True,
+        ),
+    ] + [install_symlink(name, "lld") for name in lld_aliases],
+}
+
+make_install_filegroups(
+    name = "install_data",
+    install_dirs = install_dirs,
+    no_driver_name = "install_data.no_driver",
+    pkg_name = "pkg_data",
+    prefix = "prefix_root",
 )
 
 pkg_naming_variables(
@@ -220,7 +114,7 @@ pkg_naming_variables(
 # This lets us use the tar file in testing as it is fast to create, but ship the
 # compressed version as a release.
 pkg_tar_and_test(
-    srcs = [":packaging_files"],
+    srcs = [":pkg_data"],
     name_base = "carbon_toolchain",
     package_dir = "carbon_toolchain-$(version)",
     package_file_name_base = "carbon_toolchain-$(version)",
@@ -229,7 +123,10 @@ pkg_tar_and_test(
     test_data = [
         ":install_data",
     ],
-    test_install_marker = ":install_marker",
+    # TODO: This is used to make sure that tar files are in install_data (one
+    # direction). Replace with a check that the files in install_data and tar
+    # match (bidirectional).
+    test_install_marker = "prefix_root/lib/carbon/carbon_install.txt",
 )
 
 # Support `bazel run` on specific binaries.

toolchain/install/carbon_install.txt

@@ -0,0 +1,5 @@
+# Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+# Exceptions. See /LICENSE for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+This marks a valid Carbon install tree.

toolchain/install/install_filegroups.bzl

@@ -0,0 +1,132 @@
+# Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+# Exceptions. See /LICENSE for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+"""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")
+
+def install_filegroup(name, filegroup_target):
+    """Adds a filegroup for install.
+
+    Used in the `install_dirs` dict.
+
+    Args:
+      name: The base directory for the filegroup.
+      filegroup_target: The bazel filegroup target to install.
+    """
+    return {
+        "filegroup": filegroup_target,
+        "is_driver": False,
+        "name": name,
+    }
+
+def install_symlink(name, symlink_to):
+    """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.
+    """
+    return {
+        "is_driver": False,
+        "name": name,
+        "symlink": symlink_to,
+    }
+
+def install_target(name, target, executable = False, is_driver = False):
+    """Adds a target for install.
+
+    Used in the `install_dirs` dict.
+
+    Args:
+      name: The filename to use.
+      target: The bazel target being installed.
+      executable: True if executable.
+      is_driver: False if it should be included in the `no_driver_name`
+        filegroup.
+    """
+    return {
+        "executable": executable,
+        "is_driver": is_driver,
+        "name": name,
+        "target": target,
+    }
+
+def make_install_filegroups(name, no_driver_name, pkg_name, install_dirs, prefix):
+    """Makes filegroups of install data.
+
+    Args:
+      name: The name of the main filegroup, that contains all install_data.
+      no_driver_name: The name of a filegroup which excludes the driver. This is
+        for the driver to depend on and get other files, without a circular
+        dependency.
+      pkg_name: The name of a pkg_filegroup for tar.
+      install_dirs: A dict of {directory: [install_* rules]}. This is used to
+        structure files to be installed.
+      prefix: A prefix for files in the native (non-pkg) filegroups.
+    """
+    all_srcs = []
+    no_driver_srcs = []
+    pkg_srcs = []
+
+    for dir, entries in install_dirs.items():
+        for entry in entries:
+            path = "{0}/{1}".format(dir, entry["name"])
+
+            prefixed_path = "{0}/{1}".format(prefix, path)
+            all_srcs.append(prefixed_path)
+            if not entry["is_driver"]:
+                no_driver_srcs.append(prefixed_path)
+
+            pkg_path = path + ".pkg"
+            pkg_srcs.append(pkg_path)
+
+            if "target" in entry:
+                if entry["executable"]:
+                    symlink_file(
+                        name = prefixed_path,
+                        symlink_binary = entry["target"],
+                    )
+                    mode = "0755"
+                else:
+                    symlink_file(
+                        name = prefixed_path,
+                        symlink_label = entry["target"],
+                    )
+                    mode = "0644"
+                pkg_files(
+                    name = pkg_path,
+                    srcs = [entry["target"]],
+                    attributes = pkg_attributes(mode = mode),
+                    renames = {entry["target"]: path},
+                )
+            elif "filegroup" in entry:
+                symlink_filegroup(
+                    name = prefixed_path,
+                    out_prefix = prefixed_path,
+                    srcs = [entry["filegroup"]],
+                )
+                pkg_files(
+                    name = pkg_path,
+                    srcs = [prefixed_path],
+                    strip_prefix = strip_prefix.from_pkg(prefix),
+                )
+            elif "symlink" in entry:
+                symlink_file(
+                    name = prefixed_path,
+                    symlink_relative = entry["symlink"],
+                )
+                pkg_mklink(
+                    name = pkg_path,
+                    link_name = path,
+                    target = entry["symlink"],
+                )
+            else:
+                fail("Unrecognized structure: {0}".format(entry))
+    native.filegroup(name = name, srcs = all_srcs)
+    native.filegroup(name = no_driver_name, srcs = no_driver_srcs)
+    pkg_filegroup(name = pkg_name, srcs = pkg_srcs)

toolchain/install/symlink_filegroup.bzl

@@ -1,36 +0,0 @@
-# Part of the Carbon Language project, under the Apache License v2.0 with LLVM
-# Exceptions. See /LICENSE for license information.
-# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-"""Rule for symlinking an entire filegroup, preserving its structure."""
-
-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(
-    implementation = _symlink_filegroup_impl,
-    attrs = {
-        "out_prefix": attr.string(mandatory = True),
-        "srcs": attr.label_list(mandatory = True),
-    },
-)

toolchain/install/symlink_helpers.bzl

@@ -0,0 +1,84 @@
+# Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+# Exceptions. See /LICENSE for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+"""Rules for symlinking in ways that assist install_filegroups."""
+
+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),
+    },
+)
+
+def _symlink_file_impl(ctx):
+    executable = None
+    if ctx.attr.symlink_binary:
+        out = ctx.actions.declare_file(ctx.label.name)
+        ctx.actions.symlink(
+            output = out,
+            target_file = ctx.file.symlink_binary,
+            is_executable = True,
+        )
+        executable = out
+    elif ctx.attr.symlink_label:
+        out = ctx.actions.declare_file(ctx.label.name)
+        ctx.actions.symlink(
+            output = out,
+            target_file = ctx.file.symlink_label,
+        )
+    elif ctx.attr.symlink_relative:
+        out = ctx.actions.declare_symlink(ctx.label.name)
+        ctx.actions.symlink(
+            output = out,
+            target_path = ctx.attr.symlink_relative,
+        )
+    else:
+        fail("Missing symlink target")
+
+    return [
+        DefaultInfo(
+            executable = executable,
+            files = depset(direct = [out]),
+            default_runfiles = ctx.runfiles(files = [out]),
+        ),
+    ]
+
+symlink_file = rule(
+    doc = "Symlinks a single file, with support for multiple approaches.",
+    implementation = _symlink_file_impl,
+    attrs = {
+        "symlink_binary": attr.label(
+            allow_single_file = True,
+            executable = True,
+            cfg = "target",
+        ),
+        "symlink_label": attr.label(allow_single_file = True),
+        "symlink_relative": attr.string(),
+    },
+)