Added a common framework for built tools (#559)

Co-authored-by: James Sharpe <james.sharpe@zenotech.com>

foreign_cc/built_tools/BUILD.bazel

@@ -4,4 +4,7 @@ bzl_library(
     name = "bzl_srcs",
     srcs = glob(["**/*.bzl"]),
     visibility = ["//:__subpackages__"],
+    deps = [
+        "//foreign_cc/built_tools/private:bzl_srcs",
+    ],
 )

foreign_cc/built_tools/cmake_build.bzl

@@ -1,46 +1,33 @@
 """ Rule for building CMake from sources. """
 
-load("//foreign_cc/private:detect_root.bzl", "detect_root")
+load(
-load("//foreign_cc/private:shell_script_helper.bzl", "convert_shell_script")
+    "//foreign_cc/built_tools/private:built_tools_framework.bzl",
+    "FOREIGN_CC_BUILT_TOOLS_ATTRS",
+    "FOREIGN_CC_BUILT_TOOLS_HOST_FRAGMENTS",
+    "built_tool_rule_impl",
+)
 
-def _cmake_tool(ctx):
+def _cmake_tool_impl(ctx):
-    root = detect_root(ctx.attr.cmake_srcs)
-
-    cmake = ctx.actions.declare_directory("cmake")
     script = [
-        "export BUILD_DIR=##pwd##",
+        "./bootstrap --prefix=$$INSTALLDIR$$",
-        "export BUILD_TMPDIR=$${BUILD_DIR}$$.build_tmpdir",
+        # TODO: Use make from a toolchain
-        "##copy_dir_contents_to_dir## ./{} $BUILD_TMPDIR".format(root),
+        "make",
-        "##mkdirs## " + cmake.path,
-        "cd $$BUILD_TMPDIR$$",
-        "./bootstrap --prefix=install",
         "make install",
-        "##copy_dir_contents_to_dir## ./install $BUILD_DIR/" + cmake.path,
-        "cd $$BUILD_DIR$$",
     ]
 
-    script_text = convert_shell_script(ctx, script)
+    return built_tool_rule_impl(
-
+        ctx,
-    ctx.actions.run_shell(
+        script,
-        mnemonic = "BootstrapCMake",
+        ctx.actions.declare_directory("cmake"),
-        inputs = ctx.attr.cmake_srcs.files,
+        "BootstrapCMake",
-        outputs = [cmake],
-        tools = [],
-        use_default_shell_env = True,
-        command = script_text,
-        execution_requirements = {"block-network": ""},
     )
 
-    return [DefaultInfo(files = depset([cmake]))]
-
 cmake_tool = rule(
     doc = "Rule for building CMake. Invokes bootstrap script and make install.",
-    attrs = {
+    attrs = FOREIGN_CC_BUILT_TOOLS_ATTRS,
-        "cmake_srcs": attr.label(mandatory = True),
+    host_fragments = FOREIGN_CC_BUILT_TOOLS_HOST_FRAGMENTS,
-    },
-    host_fragments = ["cpp"],
     output_to_genfiles = True,
-    implementation = _cmake_tool,
+    implementation = _cmake_tool_impl,
     toolchains = [
         str(Label("//foreign_cc/private/shell_toolchain/toolchains:shell_commands")),
         "@bazel_tools//tools/cpp:toolchain_type",

foreign_cc/built_tools/make_build.bzl

@@ -1,59 +1,43 @@
 """ Rule for building GNU Make from sources. """
 
-load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")
+load(
-load("//foreign_cc/private:detect_root.bzl", "detect_root")
+    "//foreign_cc/built_tools/private:built_tools_framework.bzl",
-load("//foreign_cc/private:run_shell_file_utils.bzl", "fictive_file_in_genroot")
+    "FOREIGN_CC_BUILT_TOOLS_ATTRS",
-load("//foreign_cc/private:shell_script_helper.bzl", "convert_shell_script")
+    "FOREIGN_CC_BUILT_TOOLS_HOST_FRAGMENTS",
+    "built_tool_rule_impl",
+)
+load("//foreign_cc/private:shell_script_helper.bzl", "os_name")
 
-def _make_tool(ctx):
+def _make_tool_impl(ctx):
-    root = detect_root(ctx.attr.make_srcs)
-
-    cc_toolchain = find_cpp_toolchain(ctx)
-
-    # we need this fictive file in the root to get the path of the root in the script
-    empty = fictive_file_in_genroot(ctx.actions, ctx.label.name)
-
-    make = ctx.actions.declare_directory("make")
     script = [
-        "export EXT_BUILD_ROOT=##pwd##",
+        "./configure --disable-dependency-tracking --prefix=$$INSTALLDIR$$",
-        "export INSTALLDIR=$$EXT_BUILD_ROOT$$/" + empty.file.dirname + "/" + ctx.attr.name,
-        "export BUILD_TMPDIR=$$INSTALLDIR$$.build_tmpdir",
-        "##mkdirs## $$BUILD_TMPDIR$$",
-        "##copy_dir_contents_to_dir## ./{} $BUILD_TMPDIR".format(root),
-        "cd $$BUILD_TMPDIR$$",
-        "./configure --disable-dependency-tracking --prefix=$$EXT_BUILD_ROOT$$/{}".format(make.path),
         "./build.sh",
-        "./make install",
-        empty.script,
     ]
-    script_text = convert_shell_script(ctx, script)
 
-    ctx.actions.run_shell(
+    if "win" in os_name(ctx):
-        mnemonic = "BootstrapMake",
+        script.extend([
-        inputs = ctx.attr.make_srcs.files,
+            "./make.exe install",
-        outputs = [make, empty.file],
+        ])
-        tools = cc_toolchain.all_files,
+    else:
-        use_default_shell_env = True,
+        script.extend([
-        command = script_text,
+            "./make install",
-        execution_requirements = {"block-network": ""},
+        ])
+
+    return built_tool_rule_impl(
+        ctx,
+        script,
+        ctx.actions.declare_directory("make"),
+        "BootstrapGNUMake",
     )
 
-    return [DefaultInfo(files = depset([make]))]
-
 make_tool = rule(
     doc = "Rule for building Make. Invokes configure script and make install.",
-    attrs = {
+    attrs = FOREIGN_CC_BUILT_TOOLS_ATTRS,
-        "make_srcs": attr.label(
+    host_fragments = FOREIGN_CC_BUILT_TOOLS_HOST_FRAGMENTS,
-            doc = "target with the Make sources",
-            mandatory = True,
-        ),
-        "_cc_toolchain": attr.label(default = Label("@bazel_tools//tools/cpp:current_cc_toolchain")),
-    },
-    host_fragments = ["cpp"],
     output_to_genfiles = True,
-    implementation = _make_tool,
+    implementation = _make_tool_impl,
     toolchains = [
-        "@rules_foreign_cc//foreign_cc/private/shell_toolchain/toolchains:shell_commands",
+        str(Label("//foreign_cc/private/shell_toolchain/toolchains:shell_commands")),
         "@bazel_tools//tools/cpp:toolchain_type",
     ],
 )

foreign_cc/built_tools/ninja_build.bzl

@@ -1,42 +1,34 @@
 """ Rule for building Ninja from sources. """
 
-load("//foreign_cc/private:detect_root.bzl", "detect_root")
+load(
-load("//foreign_cc/private:shell_script_helper.bzl", "convert_shell_script")
+    "//foreign_cc/built_tools/private:built_tools_framework.bzl",
+    "FOREIGN_CC_BUILT_TOOLS_ATTRS",
+    "FOREIGN_CC_BUILT_TOOLS_HOST_FRAGMENTS",
+    "built_tool_rule_impl",
+)
 
-def _ninja_tool(ctx):
+def _ninja_tool_impl(ctx):
-    root = detect_root(ctx.attr.ninja_srcs)
-
-    ninja = ctx.actions.declare_directory("ninja")
     script = [
-        "##mkdirs## " + ninja.path,
-        "##copy_dir_contents_to_dir## ./{} {}".format(root, ninja.path),
-        "cd " + ninja.path,
         "./configure.py --bootstrap",
+        # TODO: Reduce unnecessary copys and only keep what's required
+        "##copy_dir_contents_to_dir## $$BUILD_TMPDIR$$ $$INSTALLDIR$$",
     ]
-    script_text = convert_shell_script(ctx, script)
 
-    ctx.actions.run_shell(
+    return built_tool_rule_impl(
-        mnemonic = "BootstrapNinja",
+        ctx,
-        inputs = ctx.attr.ninja_srcs.files,
+        script,
-        outputs = [ninja],
+        ctx.actions.declare_directory("ninja"),
-        tools = [],
+        "BootstrapNinjaBuild",
-        use_default_shell_env = True,
-        command = script_text,
-        execution_requirements = {"block-network": ""},
     )
 
-    return [DefaultInfo(files = depset([ninja]))]
-
 ninja_tool = rule(
-    doc = "Rule for building Ninja. Invokes configure script and make install.",
+    doc = "Rule for building Ninja. Invokes configure script.",
-    attrs = {
+    attrs = FOREIGN_CC_BUILT_TOOLS_ATTRS,
-        "ninja_srcs": attr.label(mandatory = True),
+    host_fragments = FOREIGN_CC_BUILT_TOOLS_HOST_FRAGMENTS,
-    },
-    host_fragments = ["cpp"],
     output_to_genfiles = True,
-    implementation = _ninja_tool,
+    implementation = _ninja_tool_impl,
     toolchains = [
-        "@rules_foreign_cc//foreign_cc/private/shell_toolchain/toolchains:shell_commands",
+        str(Label("//foreign_cc/private/shell_toolchain/toolchains:shell_commands")),
         "@bazel_tools//tools/cpp:toolchain_type",
     ],
 )

foreign_cc/built_tools/private/BUILD.bazel

@@ -0,0 +1,7 @@
+load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
+
+bzl_library(
+    name = "bzl_srcs",
+    srcs = glob(["**/*.bzl"]),
+    visibility = ["//:__subpackages__"],
+)

foreign_cc/built_tools/private/built_tools_framework.bzl

@@ -0,0 +1,93 @@
+"""A module defining a common framework for "built_tools" rules"""
+
+load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")
+load("//foreign_cc/private:detect_root.bzl", "detect_root")
+load("//foreign_cc/private:framework.bzl", "wrap_outputs")
+load("//foreign_cc/private:shell_script_helper.bzl", "convert_shell_script")
+
+# Common attributes for all built_tool rules
+FOREIGN_CC_BUILT_TOOLS_ATTRS = {
+    "srcs": attr.label(
+        doc = "The target containing the build tool's sources",
+        mandatory = True,
+    ),
+    "_cc_toolchain": attr.label(default = Label("@bazel_tools//tools/cpp:current_cc_toolchain")),
+}
+
+# Common host fragments for all built_tool rules
+FOREIGN_CC_BUILT_TOOLS_HOST_FRAGMENTS = [
+    "cpp",
+]
+
+def built_tool_rule_impl(ctx, script_lines, out_dir, mnemonic):
+    """Framework function for bootstrapping C/C++ build tools.
+
+    This macro should be shared by all "built-tool" rules defined in rules_foreign_cc.
+    Any rule implementing this function should ensure that the appropriate artifacts
+    are placed in a directory represented by the `INSTALLDIR` environment variable.
+
+    Args:
+        ctx (ctx): The current rule's context object
+        script_lines (list): A list of lines of a bash script for building the build tool
+        out_dir (File): The output directory of the build tool
+        mnemonic (str): The mnemonic of the build action
+
+    Returns:
+        list: A list of providers
+    """
+
+    root = detect_root(ctx.attr.srcs)
+
+    script = [
+        # TODO: The script prelude should be used but for some reason it fails
+        # on RBE builds.
+        # "##script_prelude##",
+        "export EXT_BUILD_ROOT=##pwd##",
+        "export INSTALLDIR=$$EXT_BUILD_ROOT$$/{}".format(out_dir.path),
+        "export BUILD_TMPDIR=$$INSTALLDIR$$.build_tmpdir",
+        "##mkdirs## $$BUILD_TMPDIR$$",
+        "##copy_dir_contents_to_dir## ./{} $$BUILD_TMPDIR$$".format(root),
+        "cd $$BUILD_TMPDIR$$",
+    ]
+
+    script.append("set -x")
+    script.extend(script_lines)
+    script.append("set +x")
+
+    script_text = "\n".join([
+        "#!/usr/bin/env bash",
+        convert_shell_script(ctx, script),
+        "",
+    ])
+
+    lib_name = ctx.attr.name
+
+    wrapped_outputs = wrap_outputs(ctx, lib_name, mnemonic, script_text)
+    cc_toolchain = find_cpp_toolchain(ctx)
+
+    tools = depset(
+        [wrapped_outputs.wrapper_script_file, wrapped_outputs.script_file],
+        transitive = [cc_toolchain.all_files],
+    )
+
+    # The use of `run_shell` here is intended to ensure bash is correctly setup on windows
+    # environments. This should not be replaced with `run` until a cross platform implementation
+    # is found that guarantees bash exists or appropriately errors out.
+    ctx.actions.run_shell(
+        mnemonic = mnemonic,
+        inputs = ctx.attr.srcs.files,
+        outputs = [out_dir, wrapped_outputs.log_file],
+        tools = tools,
+        use_default_shell_env = True,
+        command = wrapped_outputs.wrapper_script_file.path,
+        execution_requirements = {"block-network": ""},
+    )
+
+    return [
+        DefaultInfo(files = depset([out_dir])),
+        OutputGroupInfo(
+            log_file = depset([wrapped_outputs.log_file]),
+            script_file = depset([wrapped_outputs.script_file]),
+            wrapper_script_file = depset([wrapped_outputs.wrapper_script_file]),
+        ),
+    ]

foreign_cc/private/framework.bzl

@@ -382,25 +382,10 @@ def cc_external_rule_impl(ctx, attrs):
     if "requires-network" in ctx.attr.tags:
         execution_requirements = {"requires-network": ""}
 
-    # We need to create a native batch script on windows to ensure the wrapper
+    # The use of `run_shell` here is intended to ensure bash is correctly setup on windows
-    # script can correctly be envoked with bash.
+    # environments. This should not be replaced with `run` until a cross platform implementation
-    wrapper = wrapped_outputs.wrapper_script_file
+    # is found that guarantees bash exists or appropriately errors out.
-    extra_tools = []
+    ctx.actions.run_shell(
-    if "win" in execution_os_name:
-        batch_wrapper = ctx.actions.declare_file(wrapper.short_path + ".bat")
-        ctx.actions.write(
-            output = batch_wrapper,
-            content = "\n".join([
-                "@ECHO OFF",
-                "START /b /wait bash {}".format(wrapper.path),
-                "",
-            ]),
-            is_executable = True,
-        )
-        extra_tools.append(wrapper)
-        wrapper = batch_wrapper
-
-    ctx.actions.run(
         mnemonic = "Cc" + attrs.configure_name.capitalize() + "MakeRule",
         inputs = depset(inputs.declared_inputs),
         outputs = rule_outputs + [
@@ -408,13 +393,13 @@ def cc_external_rule_impl(ctx, attrs):
             wrapped_outputs.log_file,
         ],
         tools = depset(
-            [wrapped_outputs.script_file] + extra_tools + ctx.files.data + ctx.files.tools_deps + ctx.files.additional_tools,
+            [wrapped_outputs.script_file, wrapped_outputs.wrapper_script_file] + ctx.files.data + ctx.files.tools_deps + ctx.files.additional_tools,
             transitive = [cc_toolchain.all_files] + [data[DefaultInfo].default_runfiles.files for data in data_dependencies],
         ),
         # TODO: Default to never using the default shell environment to make builds more hermetic. For now, every platform
         # but MacOS will take the default PATH passed by Bazel, not that from cc_toolchain.
         use_default_shell_env = execution_os_name != "osx",
-        executable = wrapper,
+        command = wrapped_outputs.wrapper_script_file.path,
         execution_requirements = execution_requirements,
         # this is ignored if use_default_shell_env = True
         env = cc_env,
@@ -467,10 +452,11 @@ WrappedOutputs = provider(
 )
 
 # buildifier: disable=function-docstring
-def wrap_outputs(ctx, lib_name, configure_name, script_text):
+def wrap_outputs(ctx, lib_name, configure_name, script_text, build_script_file = None):
-    build_log_file = ctx.actions.declare_file("{}_logs/{}.log".format(lib_name, configure_name))
+    build_log_file = ctx.actions.declare_file("{}_foreign_cc/{}.log".format(lib_name, configure_name))
-    wrapper_script_file = ctx.actions.declare_file("{}_scripts/{}_wrapper_script.sh".format(lib_name, configure_name))
+    build_script_file = ctx.actions.declare_file("{}_foreign_cc/build_script.sh".format(lib_name))
-    build_script_file = ctx.actions.declare_file("{}_scripts/{}_script.sh".format(lib_name, configure_name))
+    wrapper_script_file = ctx.actions.declare_file("{}_foreign_cc/wrapper_build_script.sh".format(lib_name))
+
     ctx.actions.write(
         output = build_script_file,
         content = script_text,

toolchains/BUILD.bazel

@@ -67,7 +67,7 @@ native_tool_toolchain(
 
 make_tool(
     name = "make_tool",
-    make_srcs = "@gnumake_src//:all_srcs",
+    srcs = "@gnumake_src//:all_srcs",
     tags = ["manual"],
 )
 
@@ -84,7 +84,7 @@ native_tool_toolchain(
 
 cmake_tool(
     name = "cmake_tool",
-    cmake_srcs = "@cmake_src//:all_srcs",
+    srcs = "@cmake_src//:all_srcs",
     tags = ["manual"],
 )
 
@@ -101,7 +101,7 @@ native_tool_toolchain(
 
 ninja_tool(
     name = "ninja_tool",
-    ninja_srcs = "@ninja_build_src//:all_srcs",
+    srcs = "@ninja_build_src//:all_srcs",
     tags = ["manual"],
 )