Support passing arbitrary extra flags to rustc (#566)

djmarcin · web-flow · commit 52e3ba4c9969 · 2021-09-01T17:28:12.000-07:00
* Support passing arbitrary extra codegen args to rustc

* Update defs.bzl

* buildifier fixes

* Add doc string

* Regenerate documentation

* fix lint

* PR feedback

* Regenerate documentation

* Fix link for codegen

* Regenerate documentation

* Rename to extra_rustc_flags

* Regenerate documentation

* buildifier

* Better method to exclude from the exec configuration

* Regenerate documentation

* Refactor is_exec_configuration utility function

* buildifier

* Regenerate documentation

* Add test

* Revert extraneous changes

* Regenerate documentation
diff --git a/BUILD.bazel b/BUILD.bazel
@@ -1,5 +1,5 @@
 load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
-load("//rust:rust.bzl", "error_format")
+load("//rust:rust.bzl", "error_format", "extra_rustc_flags")
 
 exports_files(["LICENSE"])
 
@@ -18,6 +18,15 @@ error_format(
     visibility = ["//visibility:public"],
 )
 
+# This setting may be used to pass extra options to rustc from the command line.
+# It applies across all targets whereas the rustc_flags option on targets applies only
+# to that target. This can be useful for passing build-wide options such as LTO.
+extra_rustc_flags(
+    name = "extra_rustc_flags",
+    build_setting_default = [],
+    visibility = ["//visibility:public"],
+)
+
 # This setting is used by the clippy rules. See https://bazelbuild.github.io/rules_rust/rust_clippy.html
 label_flag(
     name = "clippy.toml",
diff --git a/docs/BUILD.bazel b/docs/BUILD.bazel
@@ -59,6 +59,7 @@ PAGES = dict([
             "rust_benchmark",
             "rust_test",
             "rust_test_suite",
+            "extra_rustc_flags",
         ],
     ),
     page(
diff --git a/docs/defs.md b/docs/defs.md
@@ -9,6 +9,25 @@
 * [rust_benchmark](#rust_benchmark)
 * [rust_test](#rust_test)
 * [rust_test_suite](#rust_test_suite)
+* [extra_rustc_flags](#extra_rustc_flags)
+
+<a id="#extra_rustc_flags"></a>
+
+## extra_rustc_flags
+
+<pre>
+extra_rustc_flags(<a href="#extra_rustc_flags-name">name</a>)
+</pre>
+
+Add additional rustc_flags from the command line with `--@rules_rust//:extra_rustc_flags`. This flag should only be used for flags that need to be applied across the entire build. For options that apply to individual crates, use the rustc_flags attribute on the individual crate's rule instead. NOTE: These flags are currently excluded from the exec configuration (proc-macros, cargo_build_script, etc).
+
+**ATTRIBUTES**
+
+
+| Name  | Description | Type | Mandatory | Default |
+| :------------- | :------------- | :------------- | :------------- | :------------- |
+| <a id="extra_rustc_flags-name"></a>name |  A unique name for this target.   | <a href="https://bazel.build/docs/build-ref.html#name">Name</a> | required |  |
+
 
 <a id="#rust_benchmark"></a>
 
diff --git a/docs/flatten.md b/docs/flatten.md
@@ -7,6 +7,7 @@
 * [cargo_build_script](#cargo_build_script)
 * [crate](#crate)
 * [crate_universe](#crate_universe)
+* [extra_rustc_flags](#extra_rustc_flags)
 * [fail_when_enabled](#fail_when_enabled)
 * [incompatible_flag](#incompatible_flag)
 * [rust_analyzer](#rust_analyzer)
@@ -117,6 +118,24 @@ Environment Variables:
 | <a id="crate_universe-version"></a>version |  The version of cargo the resolver should use   | String | optional | "1.54.0" |
 
 
+<a id="#extra_rustc_flags"></a>
+
+## extra_rustc_flags
+
+<pre>
+extra_rustc_flags(<a href="#extra_rustc_flags-name">name</a>)
+</pre>
+
+Add additional rustc_flags from the command line with `--@rules_rust//:extra_rustc_flags`. This flag should only be used for flags that need to be applied across the entire build. For options that apply to individual crates, use the rustc_flags attribute on the individual crate's rule instead. NOTE: These flags are currently excluded from the exec configuration (proc-macros, cargo_build_script, etc).
+
+**ATTRIBUTES**
+
+
+| Name  | Description | Type | Mandatory | Default |
+| :------------- | :------------- | :------------- | :------------- | :------------- |
+| <a id="extra_rustc_flags-name"></a>name |  A unique name for this target.   | <a href="https://bazel.build/docs/build-ref.html#name">Name</a> | required |  |
+
+
 <a id="#fail_when_enabled"></a>
 
 ## fail_when_enabled
diff --git a/docs/symbols.bzl b/docs/symbols.bzl
@@ -41,6 +41,7 @@ load(
 )
 load(
     "@rules_rust//rust:defs.bzl",
+    _extra_rustc_flags = "extra_rustc_flags",
     _rust_analyzer = "rust_analyzer",
     _rust_analyzer_aspect = "rust_analyzer_aspect",
     _rust_benchmark = "rust_benchmark",
@@ -141,6 +142,7 @@ crate = _crate
 rustfmt_aspect = _rustfmt_aspect
 rustfmt_test = _rustfmt_test
 
+extra_rustc_flags = _extra_rustc_flags
 incompatible_flag = _incompatible_flag
 fail_when_enabled = _fail_when_enabled
 
diff --git a/rust/defs.bzl b/rust/defs.bzl
@@ -43,6 +43,7 @@ load(
 load(
     "//rust/private:rustc.bzl",
     _error_format = "error_format",
+    _extra_rustc_flags = "extra_rustc_flags",
 )
 load(
     "//rust/private:rustdoc.bzl",
@@ -97,6 +98,9 @@ rust_clippy = _rust_clippy
 error_format = _error_format
 # See @rules_rust//rust/private:rustc.bzl for a complete description.
 
+extra_rustc_flags = _extra_rustc_flags
+# See @rules_rust//rust/private:rustc.bzl for a complete description.
+
 rust_common = _rust_common
 # See @rules_rust//rust/private:common.bzl for a complete description.
 
diff --git a/rust/private/clippy.bzl b/rust/private/clippy.bzl
@@ -162,6 +162,7 @@ rust_clippy_aspect = aspect(
             doc = "The desired `--error-format` flags for clippy",
             default = "//:error_format",
         ),
+        "_extra_rustc_flags": attr.label(default = "//:extra_rustc_flags"),
         "_process_wrapper": attr.label(
             doc = "A process wrapper for running clippy on all platforms",
             default = Label("//util/process_wrapper"),
diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl
@@ -663,6 +663,7 @@ _common_attrs = {
         default = "@bazel_tools//tools/cpp:current_cc_toolchain",
     ),
     "_error_format": attr.label(default = "//:error_format"),
+    "_extra_rustc_flags": attr.label(default = "//:extra_rustc_flags"),
     "_process_wrapper": attr.label(
         default = Label("//util/process_wrapper"),
         executable = True,
diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl
@@ -27,6 +27,7 @@ load(
     "find_cc_toolchain",
     "get_lib_name",
     "get_preferred_artifact",
+    "is_exec_configuration",
     "make_static_lib_symlink",
     "relativize",
 )
@@ -57,6 +58,11 @@ ErrorFormatInfo = provider(
     fields = {"error_format": "(string) [" + ", ".join(_error_format_values) + "]"},
 )
 
+ExtraRustcFlagsInfo = provider(
+    doc = "Pass each value as an additional flag to rustc invocations",
+    fields = {"extra_rustc_flags": "List[string] Extra flags to pass to rustc"},
+)
+
 def _get_rustc_env(attr, toolchain):
     """Gathers rustc environment variables
 
@@ -519,6 +525,10 @@ def construct_arguments(
     # Set the SYSROOT to the directory of the rust_lib files passed to the toolchain
     env["SYSROOT"] = paths.dirname(toolchain.rust_lib.files.to_list()[0].short_path)
 
+    # extra_rustc_flags apply to the target configuration, not the exec configuration.
+    if hasattr(ctx.attr, "_extra_rustc_flags") and is_exec_configuration(ctx):
+        rustc_flags.add_all(ctx.attr._extra_rustc_flags[ExtraRustcFlagsInfo].extra_rustc_flags)
+
     # Create a struct which keeps the arguments separate so each may be tuned or
     # replaced where necessary
     args = struct(
@@ -1015,10 +1025,23 @@ def _error_format_impl(ctx):
 
 error_format = rule(
     doc = (
-        "A helper rule for controlling the rustc " +
-        "[--error-format](https://doc.rust-lang.org/rustc/command-line-arguments.html#option-error-format) " +
-        "flag."
+        "Change the [--error-format](https://doc.rust-lang.org/rustc/command-line-arguments.html#option-error-format) " +
+        "flag from the command line with `--@rules_rust//:error_format`. See rustc documentation for valid values."
     ),
     implementation = _error_format_impl,
     build_setting = config.string(flag = True),
 )
+
+def _extra_rustc_flags_impl(ctx):
+    return ExtraRustcFlagsInfo(extra_rustc_flags = ctx.build_setting_value)
+
+extra_rustc_flags = rule(
+    doc = (
+        "Add additional rustc_flags from the command line with `--@rules_rust//:extra_rustc_flags`. " +
+        "This flag should only be used for flags that need to be applied across the entire build. For options that " +
+        "apply to individual crates, use the rustc_flags attribute on the individual crate's rule instead. NOTE: " +
+        "These flags are currently excluded from the exec configuration (proc-macros, cargo_build_script, etc)."
+    ),
+    implementation = _extra_rustc_flags_impl,
+    build_setting = config.string_list(flag = True),
+)
diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl
@@ -323,3 +323,19 @@ def make_static_lib_symlink(actions, rlib_file):
     dot_a = actions.declare_file(basename + ".a", sibling = rlib_file)
     actions.symlink(output = dot_a, target_file = rlib_file)
     return dot_a
+
+def is_exec_configuration(ctx):
+    """Determine if a context is building for the exec configuration.
+
+    This is helpful when processing command line flags that should apply
+    to the target configuration but not the exec configuration.
+
+    Args:
+        ctx (ctx): The ctx object for the current target.
+
+    Returns:
+        True if the exec configuration is detected, False otherwise.
+    """
+
+    # TODO(djmarcin): Is there any better way to determine cfg=exec?
+    return ctx.genfiles_dir.path.find("-exec-") == -1
diff --git a/rust/rust.bzl b/rust/rust.bzl
@@ -17,6 +17,7 @@
 load(
     "//rust:defs.bzl",
     _error_format = "error_format",
+    _extra_rustc_flags = "extra_rustc_flags",
     _rust_analyzer = "rust_analyzer",
     _rust_analyzer_aspect = "rust_analyzer_aspect",
     _rust_benchmark = "rust_benchmark",
@@ -93,5 +94,8 @@ rust_analyzer = _rust_analyzer
 error_format = _error_format
 # See @rules_rust//rust/private:rustc.bzl for a complete description.
 
+extra_rustc_flags = _extra_rustc_flags
+# See @rules_rust//rust/private:rustc.bzl for a complete description.
+
 rust_common = _rust_common
 # See @rules_rust//rust/private:common.bzl for a complete description.
diff --git a/test/unit/extra_rustc_flags/BUILD.bazel b/test/unit/extra_rustc_flags/BUILD.bazel
@@ -0,0 +1,5 @@
+load(":extra_rustc_flags_test.bzl", "extra_rustc_flags_test_suite")
+
+extra_rustc_flags_test_suite(
+    name = "extra_rustc_flags_test_suite",
+)
diff --git a/test/unit/extra_rustc_flags/extra_rustc_flags_test.bzl b/test/unit/extra_rustc_flags/extra_rustc_flags_test.bzl
@@ -0,0 +1,94 @@
+"""Unittest to verify compile_data (attribute) propagation"""
+
+load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
+load("//rust:defs.bzl", "rust_library")
+load("//test/unit:common.bzl", "assert_argv_contains", "assert_argv_contains_not")
+
+EXTRA_FLAG = "--codegen=linker-plugin-lto"
+
+def target_action_contains_not_flag(env, target):
+    action = target.actions[0]
+    asserts.equals(env, "Rustc", action.mnemonic)
+
+    assert_argv_contains_not(
+        env = env,
+        action = action,
+        flag = EXTRA_FLAG,
+    )
+
+def target_action_contains_flag(env, target):
+    action = target.actions[0]
+    asserts.equals(env, "Rustc", action.mnemonic)
+
+    assert_argv_contains(
+        env = env,
+        action = action,
+        flag = EXTRA_FLAG,
+    )
+
+def _extra_rustc_flags_not_present_test(ctx):
+    env = analysistest.begin(ctx)
+    target = analysistest.target_under_test(env)
+    target_action_contains_not_flag(env, target)
+
+    return analysistest.end(env)
+
+def _extra_rustc_flags_present_test(ctx):
+    env = analysistest.begin(ctx)
+    target = analysistest.target_under_test(env)
+    target_action_contains_flag(env, target)
+
+    # Check the exec configuration target does NOT contain.
+    target = ctx.attr.lib_exec
+    target_action_contains_not_flag(env, target)
+
+    return analysistest.end(env)
+
+extra_rustc_flags_not_present_test = analysistest.make(_extra_rustc_flags_not_present_test)
+extra_rustc_flags_present_test = analysistest.make(
+    _extra_rustc_flags_present_test,
+    attrs = {
+        "lib_exec": attr.label(
+            mandatory = True,
+            cfg = "exec",
+        ),
+    },
+    config_settings = {
+        "//:extra_rustc_flags": [EXTRA_FLAG],
+    },
+)
+
+def _define_test_targets():
+    rust_library(
+        name = "lib",
+        srcs = ["lib.rs"],
+        edition = "2018",
+    )
+
+def extra_rustc_flags_test_suite(name):
+    """Entry-point macro called from the BUILD file.
+
+    Args:
+        name (str): Name of the macro.
+    """
+
+    _define_test_targets()
+
+    extra_rustc_flags_not_present_test(
+        name = "extra_rustc_flags_not_present_test",
+        target_under_test = ":lib",
+    )
+
+    extra_rustc_flags_present_test(
+        name = "extra_rustc_flags_present_test",
+        target_under_test = ":lib",
+        lib_exec = ":lib",
+    )
+
+    native.test_suite(
+        name = name,
+        tests = [
+            ":extra_rustc_flags_not_present_test",
+            ":extra_rustc_flags_present_test",
+        ],
+    )
diff --git a/test/unit/extra_rustc_flags/lib.rs b/test/unit/extra_rustc_flags/lib.rs
@@ -0,0 +1 @@
+pub fn call() {}
