rust_toolchain: expand Make variables in flags (#3640)

This allows a user to say:

```bzl
rust_register_toolchains( # or rust_toolchain(
    extra_rustc_flags = {
        "aarch64-unknown-linux-gnu": [
            "--codegen=link-arg=-B$(RUST_SYSROOT)/lib/rustlib/aarch64-unknown-linux-gnu/bin/gcc-ld",
            "--codegen=link-arg=-fuse-ld=lld",
        ],
        "x86_64-unknown-linux-gnu": [
            "--codegen=link-arg=-B$(RUST_SYSROOT)/lib/rustlib/x86_64-unknown-linux-gnu/bin/gcc-ld",
            "--codegen=link-arg=-fuse-ld=lld",
        ],
    },
)
```

bringing us one step closer to #3602.

This is **technically** an incompatible change: if a user currently sets
a `extra_rustc_flags` containing `$`, they now need to escape `$` to
`$$` pursuant to how [Make variable
expansion](https://bazel.build/reference/be/make-variables) works in
Bazel. However, I would assume this is rare.

Inspired by #3540 – cc
@krasimirgg

Author: timothyg-stripe

rust/repositories.bzl

@@ -652,7 +652,11 @@ def rust_toolchain_repository(
         dev_components (bool, optional): Whether to download the rustc-dev components.
             Requires version to be "nightly". Defaults to False.
         extra_rustc_flags (list, optional): Extra flags to pass to rustc in non-exec configuration.
+            Subject to Make variable expansion with respect to RUST_SYSROOT,
+            RUST_SYSROOT_SHORT, RUSTC, etc.
         extra_exec_rustc_flags (list, optional): Extra flags to pass to rustc in exec configuration.
+            Subject to Make variable expansion with respect to RUST_SYSROOT,
+            RUST_SYSROOT_SHORT, RUSTC, etc.
         opt_level (dict, optional): Optimization level config for this toolchain.
         strip_level (dict, optional): Strip level config for this toolchain.
         sha256s (str, optional): A dict associating tool subdirectories to sha256 hashes. See

rust/toolchain.bzl

@@ -533,11 +533,20 @@ def _generate_sysroot(
 def _experimental_use_cc_common_link(ctx):
     return ctx.attr.experimental_use_cc_common_link[BuildSettingInfo].value
 
-def _expand_flags(ctx, flags, targets):
+def _expand_flags(ctx, attr_name, targets, make_variables):
     targets = deduplicate(targets)
     expanded_flags = []
+    flags = getattr(ctx.attr, attr_name)
     for flag in flags:
-        expanded_flags.append(ctx.expand_location(flag, targets))
+        # Fast-path - both location expansions and make vars have a `$` so we
+        # can short-circuit if $ doesn't exist.
+        if "$" in flag:
+            # The ordering here matters. If we expand Make variables first, then
+            # "$(location //foo)" would have to be written as "$$(location //foo)",
+            # which is inconsistent with how Bazel builtin rules work.
+            flag = ctx.expand_location(flag, targets)
+            flag = ctx.expand_make_variables(attr_name, flag, make_variables)
+        expanded_flags.append(flag)
     return expanded_flags
 
 def _rust_toolchain_impl(ctx):
@@ -594,29 +603,6 @@ def _rust_toolchain_impl(ctx):
         llvm_tools = ctx.attr.llvm_tools,
     )
 
-    expanded_stdlib_linkflags = _expand_flags(ctx, ctx.attr.stdlib_linkflags, rust_std[rust_common.stdlib_info].srcs)
-    expanded_extra_rustc_flags = _expand_flags(ctx, ctx.attr.extra_rustc_flags, rust_std[rust_common.stdlib_info].srcs)
-    expanded_extra_exec_rustc_flags = _expand_flags(ctx, ctx.attr.extra_exec_rustc_flags, rust_std[rust_common.stdlib_info].srcs)
-
-    linking_context = cc_common.create_linking_context(
-        linker_inputs = depset([
-            cc_common.create_linker_input(
-                owner = ctx.label,
-                user_link_flags = depset(expanded_stdlib_linkflags),
-            ),
-        ]),
-    )
-
-    # Contains linker flags needed to link Rust standard library.
-    # These need to be added to linker command lines when the linker is not rustc
-    # (rustc does this automatically). Linker flags wrapped in an otherwise empty
-    # `CcInfo` to provide the flags in a way that doesn't duplicate them per target
-    # providing a `CcInfo`.
-    stdlib_linkflags_cc_info = CcInfo(
-        compilation_context = cc_common.create_compilation_context(),
-        linking_context = linking_context,
-    )
-
     # Determine the path and short_path of the sysroot
     sysroot_path = sysroot.sysroot_anchor.dirname
     sysroot_short_path, _, _ = sysroot.sysroot_anchor.short_path.rpartition("/")
@@ -642,6 +628,29 @@ def _rust_toolchain_impl(ctx):
 
     make_variable_info = platform_common.TemplateVariableInfo(make_variables)
 
+    expanded_stdlib_linkflags = _expand_flags(ctx, "stdlib_linkflags", rust_std[rust_common.stdlib_info].srcs, make_variables)
+    expanded_extra_rustc_flags = _expand_flags(ctx, "extra_rustc_flags", rust_std[rust_common.stdlib_info].srcs, make_variables)
+    expanded_extra_exec_rustc_flags = _expand_flags(ctx, "extra_exec_rustc_flags", rust_std[rust_common.stdlib_info].srcs, make_variables)
+
+    linking_context = cc_common.create_linking_context(
+        linker_inputs = depset([
+            cc_common.create_linker_input(
+                owner = ctx.label,
+                user_link_flags = depset(expanded_stdlib_linkflags),
+            ),
+        ]),
+    )
+
+    # Contains linker flags needed to link Rust standard library.
+    # These need to be added to linker command lines when the linker is not rustc
+    # (rustc does this automatically). Linker flags wrapped in an otherwise empty
+    # `CcInfo` to provide the flags in a way that doesn't duplicate them per target
+    # providing a `CcInfo`.
+    stdlib_linkflags_cc_info = CcInfo(
+        compilation_context = cc_common.create_compilation_context(),
+        linking_context = linking_context,
+    )
+
     exec_triple = triple(ctx.attr.exec_triple)
 
     if not exec_triple.system:
@@ -843,10 +852,10 @@ rust_toolchain = rule(
             doc = "Label to a boolean build setting that controls whether cc_common.link is used to link rust binaries.",
         ),
         "extra_exec_rustc_flags": attr.string_list(
-            doc = "Extra flags to pass to rustc in exec configuration. Subject to location expansion with respect to the srcs of the `rust_std` attribute.",
+            doc = "Extra flags to pass to rustc in exec configuration. Subject to location expansion with respect to the srcs of the `rust_std` attribute. Subject to Make variable expansion with respect to RUST_SYSROOT, RUST_SYSROOT_SHORT, RUSTC, etc.",
         ),
         "extra_rustc_flags": attr.string_list(
-            doc = "Extra flags to pass to rustc in non-exec configuration. Subject to location expansion with respect to the srcs of the `rust_std` attribute.",
+            doc = "Extra flags to pass to rustc in non-exec configuration. Subject to location expansion with respect to the srcs of the `rust_std` attribute. Subject to Make variable expansion with respect to RUST_SYSROOT, RUST_SYSROOT_SHORT, RUSTC, etc.",
         ),
         "extra_rustc_flags_for_crate_types": attr.string_list_dict(
             doc = "Extra flags to pass to rustc based on crate type",
@@ -923,7 +932,8 @@ rust_toolchain = rule(
             doc = (
                 "Additional linker flags to use when Rust standard library is linked by a C++ linker " +
                 "(rustc will deal with these automatically). Subject to location expansion with respect " +
-                "to the srcs of the `rust_std` attribute."
+                "to the srcs of the `rust_std` attribute. Subject to Make variable expansion with respect " +
+                "to RUST_SYSROOT, RUST_SYSROOT_SHORT, RUSTC, etc."
             ),
             mandatory = True,
         ),

test/unit/toolchain/toolchain_test.bzl

@@ -44,6 +44,11 @@ def _toolchain_location_expands_linkflags_impl(ctx):
         "test:test/unit/toolchain/config.txt",
         toolchain_info.stdlib_linkflags.linking_context.linker_inputs.to_list()[0].user_link_flags[0],
     )
+    asserts.equals(
+        env,
+        "test:sysroot=" + toolchain_info.sysroot,
+        toolchain_info.stdlib_linkflags.linking_context.linker_inputs.to_list()[0].user_link_flags[1],
+    )
 
     return analysistest.end(env)
 
@@ -56,6 +61,11 @@ def _toolchain_location_expands_extra_rustc_flags_impl(ctx):
         "extra_rustc_flags:test/unit/toolchain/config.txt",
         toolchain_info.extra_rustc_flags[0],
     )
+    asserts.equals(
+        env,
+        "extra_rustc_flags:sysroot=" + toolchain_info.sysroot,
+        toolchain_info.extra_rustc_flags[1],
+    )
 
     return analysistest.end(env)
 
@@ -173,8 +183,8 @@ def _define_test_targets():
         rust_std = ":std_libs",
         rustc = ":mock_rustc",
         staticlib_ext = ".a",
-        stdlib_linkflags = ["test:$(location :stdlib_srcs)"],
-        extra_rustc_flags = ["extra_rustc_flags:$(location :stdlib_srcs)"],
+        stdlib_linkflags = ["test:$(location :stdlib_srcs)", "test:sysroot=$(RUST_SYSROOT)"],
+        extra_rustc_flags = ["extra_rustc_flags:$(location :stdlib_srcs)", "extra_rustc_flags:sysroot=$(RUST_SYSROOT)"],
         target_json = encoded_target_json,
     )