Make toolchain vars available but don't automatically set them

Previously if a target depended on a toolchain which exposed $(FOO), we
would automatically set $FOO as an env var.

This was not correct. Toolchains should make Make variables available
for expansion in env vars, but should not automatically promote them to
action env.

If you want $FOO set, you should set $FOO=$(FOO) in the appropriate env
var attribute.

MODULE.bazel

@@ -184,3 +184,5 @@ use_repo(rust_host_tools, "rust_host_tools")
 
 cargo_bazel_bootstrap = use_extension("//crate_universe/private/module_extensions:cargo_bazel_bootstrap.bzl", "cargo_bazel_bootstrap")
 use_repo(cargo_bazel_bootstrap, "cargo_bazel_bootstrap")
+
+register_toolchains("//test/foreign_toolchain_make_variables:toolchain_for_test", dev_dependency=True)

WORKSPACE.bazel

@@ -91,3 +91,5 @@ http_archive(
 load("//test/3rdparty/crates:crates.bzl", test_crate_repositories = "crate_repositories")
 
 test_crate_repositories()
+
+register_toolchains("//test/foreign_toolchain_make_variables:toolchain_for_test")

cargo/private/cargo_build_script.bzl

@@ -435,6 +435,8 @@ def _cargo_build_script_impl(ctx):
     # Add environment variables from the Rust toolchain.
     env.update(toolchain.env)
 
+    known_variables = {}
+
     # Gather data from the `toolchains` attribute.
     for target in ctx.attr.toolchains:
         if DefaultInfo in target:
@@ -449,7 +451,7 @@ def _cargo_build_script_impl(ctx):
             toolchain_tools.append(all_files)
         if platform_common.TemplateVariableInfo in target:
             variables = getattr(target[platform_common.TemplateVariableInfo], "variables", depset([]))
-            env.update(variables)
+            known_variables.update(variables)
 
     _merge_env_dict(env, expand_dict_value_locations(
         ctx,
@@ -459,6 +461,7 @@ def _cargo_build_script_impl(ctx):
         getattr(ctx.attr, "tools", []) +
         script_info.data +
         script_info.tools,
+        known_variables,
     ))
 
     tools = depset(

rust/private/rust.bzl

@@ -261,6 +261,7 @@ def _rust_binary_impl(ctx):
             ctx,
             ctx.attr.env,
             ctx.attr.data,
+            {},
         ),
     ))
 
@@ -345,6 +346,7 @@ def _rust_test_impl(ctx):
             ctx,
             ctx.attr.rustc_env,
             data_paths,
+            {},
         ))
         aliases = dict(crate.aliases)
         aliases.update(ctx.attr.aliases)
@@ -396,6 +398,7 @@ def _rust_test_impl(ctx):
             ctx,
             ctx.attr.rustc_env,
             data_paths,
+            {},
         )
 
         # Target is a standalone crate. Build the test binary as its own crate.
@@ -432,6 +435,7 @@ def _rust_test_impl(ctx):
         ctx,
         getattr(ctx.attr, "env", {}),
         data,
+        {},
     )
     if toolchain.llvm_cov and ctx.configuration.coverage_enabled:
         if not toolchain.llvm_profdata:

rust/private/rustc.bzl

@@ -991,6 +991,14 @@ def construct_arguments(
     # Deduplicate data paths due to https://github.com/bazelbuild/bazel/issues/14681
     data_paths = depset(direct = getattr(attr, "data", []), transitive = [crate_info.compile_data_targets]).to_list()
 
+    rustc_flags.add_all(
+        expand_list_element_locations(
+            ctx,
+            getattr(attr, "rustc_flags", []),
+            data_paths,
+            {},
+        ),
+    )
     add_edition_flags(rustc_flags, crate_info)
 
     # Link!
@@ -1053,6 +1061,7 @@ def construct_arguments(
             ctx,
             crate_info.rustc_env,
             data_paths,
+            {},
         ))
 
     # Ensure the sysroot is set for the target platform
@@ -1096,6 +1105,7 @@ def construct_arguments(
             ctx,
             getattr(attr, "rustc_flags", []),
             data_paths,
+            {},
         ),
     )
 

rust/private/utils.bzl

@@ -242,7 +242,7 @@ def _deduplicate(xs):
 def concat(xss):
     return [x for xs in xss for x in xs]
 
-def _expand_location_for_build_script_runner(ctx, env, data):
+def _expand_location_for_build_script_runner(ctx, env, data, known_variables):
     """A trivial helper for `expand_dict_value_locations` and `expand_list_element_locations`
 
     Args:
@@ -260,10 +260,10 @@ def _expand_location_for_build_script_runner(ctx, env, data):
     return ctx.expand_make_variables(
         env,
         dedup_expand_location(ctx, env, data),
-        {},
+        known_variables,
     )
 
-def expand_dict_value_locations(ctx, env, data):
+def expand_dict_value_locations(ctx, env, data, known_variables):
     """Performs location-macro expansion on string values.
 
     $(execroot ...) and $(location ...) are prefixed with ${pwd},
@@ -292,9 +292,9 @@ def expand_dict_value_locations(ctx, env, data):
     Returns:
         dict: A dict of environment variables with expanded location macros
     """
-    return dict([(k, _expand_location_for_build_script_runner(ctx, v, data)) for (k, v) in env.items()])
+    return dict([(k, _expand_location_for_build_script_runner(ctx, v, data, known_variables)) for (k, v) in env.items()])
 
-def expand_list_element_locations(ctx, args, data):
+def expand_list_element_locations(ctx, args, data, known_variables):
     """Performs location-macro expansion on a list of string values.
 
     $(execroot ...) and $(location ...) are prefixed with ${pwd},
@@ -315,7 +315,7 @@ def expand_list_element_locations(ctx, args, data):
     Returns:
         list: A list of arguments with expanded location macros
     """
-    return [_expand_location_for_build_script_runner(ctx, arg, data) for arg in args]
+    return [_expand_location_for_build_script_runner(ctx, arg, data, known_variables) for arg in args]
 
 def name_to_crate_name(name):
     """Converts a build target's name into the name of its associated crate.

test/foreign_toolchain_make_variables/BUILD.bazel

@@ -0,0 +1,25 @@
+load("//cargo:defs.bzl", "cargo_build_script")
+load("//test/foreign_toolchain_make_variables:toolchain.bzl", "current_dummy_env_var_toolchain_toolchain", "dummy_env_var_toolchain")
+
+cargo_build_script(
+    name = "bs",
+    srcs = ["build.rs"],
+    build_script_env = {
+        "FROM_TOOLCHAIN": "$(FROM_TOOLCHAIN)",
+        "MODIFIED_FROM_TOOLCHAIN": "modified$(FROM_TOOLCHAIN)",
+    },
+    edition = "2021",
+    toolchains = [":current_dummy_env_var_toolchain_toolchain"],
+)
+
+toolchain_type(name = "toolchain_type_for_test")
+
+toolchain(
+    name = "toolchain_for_test",
+    toolchain = ":dummy_env_var_toolchain",
+    toolchain_type = ":toolchain_type_for_test",
+)
+
+dummy_env_var_toolchain(name = "dummy_env_var_toolchain")
+
+current_dummy_env_var_toolchain_toolchain(name = "current_dummy_env_var_toolchain_toolchain")

test/foreign_toolchain_make_variables/build.rs

@@ -0,0 +1,9 @@
+fn main() {
+    assert_eq!(std::env::var("FROM_TOOLCHAIN").unwrap(), "present");
+    assert_eq!(
+        std::env::var("MODIFIED_FROM_TOOLCHAIN").unwrap(),
+        "modifiedpresent"
+    );
+    // This was not explicitly forwarded by the cargo_build_script target, so should not be present.
+    assert!(std::env::var_os("ALSO_FROM_TOOLCHAIN").is_none());
+}

test/foreign_toolchain_make_variables/toolchain.bzl

@@ -0,0 +1,32 @@
+def _dummy_env_var_toolchain_impl(ctx):
+    make_variables = platform_common.TemplateVariableInfo({
+        "ALSO_FROM_TOOLCHAIN": "absent",
+        "FROM_TOOLCHAIN": "present",
+    })
+
+    return [
+        platform_common.ToolchainInfo(
+            make_variables = make_variables,
+        ),
+        make_variables,
+    ]
+
+dummy_env_var_toolchain = rule(
+    implementation = _dummy_env_var_toolchain_impl,
+)
+
+def _current_dummy_env_var_toolchain_impl(ctx):
+    toolchain = ctx.toolchains[str(Label("@rules_rust//test/foreign_toolchain_make_variables:toolchain_type_for_test"))]
+
+    return [
+        toolchain,
+        toolchain.make_variables,
+    ]
+
+current_dummy_env_var_toolchain_toolchain = rule(
+    doc = "A rule for exposing the current registered `dummy_env_var_toolchain`.",
+    implementation = _current_dummy_env_var_toolchain_impl,
+    toolchains = [
+        str(Label("@rules_rust//test/foreign_toolchain_make_variables:toolchain_type_for_test")),
+    ],
+)