From bba4f8aa20649c249d20048cf2ae512f7c4e8868 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Tue, 24 Sep 2024 10:19:44 -0700 Subject: [PATCH] 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 --- toolchain/driver/BUILD | 4 +- toolchain/install/BUILD | 177 +++++------------------ toolchain/install/carbon_install.txt | 5 + toolchain/install/install_filegroups.bzl | 132 +++++++++++++++++ toolchain/install/symlink_filegroup.bzl | 36 ----- toolchain/install/symlink_helpers.bzl | 84 +++++++++++ 6 files changed, 260 insertions(+), 178 deletions(-) create mode 100644 toolchain/install/carbon_install.txt create mode 100644 toolchain/install/install_filegroups.bzl delete mode 100644 toolchain/install/symlink_filegroup.bzl create mode 100644 toolchain/install/symlink_helpers.bzl diff --git a/toolchain/driver/BUILD b/toolchain/driver/BUILD index 1681c2cedbfdd..64a73d60ccc3a 100644 --- a/toolchain/driver/BUILD +++ b/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 = [ diff --git a/toolchain/install/BUILD b/toolchain/install/BUILD index 773b1436843ec..602138809c9fb 100644 --- a/toolchain/install/BUILD +++ b/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. diff --git a/toolchain/install/carbon_install.txt b/toolchain/install/carbon_install.txt new file mode 100644 index 0000000000000..c716e1b45d413 --- /dev/null +++ b/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. diff --git a/toolchain/install/install_filegroups.bzl b/toolchain/install/install_filegroups.bzl new file mode 100644 index 0000000000000..e0ee1b042a6d8 --- /dev/null +++ b/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) diff --git a/toolchain/install/symlink_filegroup.bzl b/toolchain/install/symlink_filegroup.bzl deleted file mode 100644 index faa4cfc482e8b..0000000000000 --- a/toolchain/install/symlink_filegroup.bzl +++ /dev/null @@ -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), - }, -) diff --git a/toolchain/install/symlink_helpers.bzl b/toolchain/install/symlink_helpers.bzl new file mode 100644 index 0000000000000..d45b9db899203 --- /dev/null +++ b/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(), + }, +)