Skip to content

Commit

Permalink
Merge cc toolchain flags into build script env (bazelbuild#1675)
Browse files Browse the repository at this point in the history
Right now, setting build_script_env will _overwrite_ the values from the
cc_toolchain, which is almost certainly ~always wrong.

Instead, allow _appending_ values, which means that if overrides are
necessary people can add them (e.g. disabling specific warnings).

Note: This is technically a breaking change, but one which should
improve faithfulness of the build.
  • Loading branch information
illicitonion authored Nov 29, 2022
1 parent b7c36c0 commit 025bf7d
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 2 deletions.
9 changes: 8 additions & 1 deletion cargo/private/cargo_build_script.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def _cargo_build_script_impl(ctx):
# Add environment variables from the Rust toolchain.
env.update(toolchain.env)

env.update(expand_dict_value_locations(
_merge_env_dict(env, expand_dict_value_locations(
ctx,
ctx.attr.build_script_env,
getattr(ctx.attr, "data", []) +
Expand Down Expand Up @@ -316,6 +316,13 @@ cargo_build_script = rule(
incompatible_use_toolchain_transition = True,
)

def _merge_env_dict(prefix_dict, suffix_dict):
"""Merges suffix_dict into prefix_dict, appending rather than replacing certain env vars."""
for key in ["CFLAGS", "CXXFLAGS", "LDFLAGS"]:
if key in prefix_dict and key in suffix_dict and prefix_dict[key]:
prefix_dict[key] += " " + suffix_dict.pop(key)
prefix_dict.update(suffix_dict)

def name_to_pkg_name(name):
"""Sanitize the name of cargo_build_script targets.
Expand Down
13 changes: 12 additions & 1 deletion examples/zig_cross_compiling/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
load("@aspect_bazel_lib//lib:transitions.bzl", "platform_transition_filegroup")
load("@crate_index//:defs.bzl", "aliases", "all_crate_deps")
load("@rules_rust//cargo:defs.bzl", "cargo_build_script")
load("@rules_rust//rust:defs.bzl", "rust_binary")

rust_binary(
Expand All @@ -8,7 +9,17 @@ rust_binary(
aliases = aliases(),
# We don't care about building this for our own platform, this just exists for the transition
tags = ["manual"],
deps = all_crate_deps(normal = True),
deps = all_crate_deps(normal = True) + [":check_merged_flags"],
)

cargo_build_script(
name = "check_merged_flags",
srcs = ["src/check_merged_flags.rs"],
build_script_env = {
"CFLAGS": "cbeep",
"CXXFLAGS": "cxxbeep",
"LDFLAGS": "ldbeep",
},
)

platform(
Expand Down
13 changes: 13 additions & 0 deletions examples/zig_cross_compiling/src/check_merged_flags.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
fn main() {
let cflags = std::env::var("CFLAGS").unwrap();
assert!(cflags.contains("-target aarch64-linux-gnu.2.28"), "Expected CFLAGS to contain `-target aarch64-linux-gnu.2.28` because of zig toolchain but was `{}`", cflags);
assert!(cflags.contains(" cbeep"), "Expected CFLAGS to contain ` cbeep` because of build_script_env but was `{}`", cflags);

let cxxflags = std::env::var("CXXFLAGS").unwrap();
assert!(cxxflags.contains("-target aarch64-linux-gnu.2.28"), "Expected CXXFLAGS to contain `-target aarch64-linux-gnu.2.28` because of zig toolchain but was `{}`", cxxflags);
assert!(cxxflags.contains(" cxxbeep"), "Expected CXXFLAGS to contain ` cxxbeep` because of build_script_env but was `{}`", cxxflags);

let ldflags = std::env::var("LDFLAGS").unwrap();
assert!(ldflags.contains("-target aarch64-linux-gnu.2.28"), "Expected LDFLAGS to contain `-target aarch64-linux-gnu.2.28` because of zig toolchain but was `{}`", ldflags);
assert!(ldflags.contains(" ldbeep"), "Expected LDFLAGS to contain ` ldbeep` because of build_script_env but was `{}`", ldflags);
}

0 comments on commit 025bf7d

Please sign in to comment.