From 7b8cc91d6459943831e76595b519ad9db598b150 Mon Sep 17 00:00:00 2001 From: Ivan Kalchev Date: Tue, 2 Nov 2021 01:47:48 -0700 Subject: [PATCH 1/2] Forward CXX env and arguments from cargo_build_script We need to forward the C++ command line arguments and env to ``bin.rs`` to make sure cargo_build libraries are compiled with the same options as `cc_*` targets. Fixes https://github.com/bazelbuild/rules_rust/issues/1003 --- AUTHORS | 1 + CONTRIBUTORS | 1 + cargo/cargo_build_script.bzl | 48 ++++++++++++++++++++++++++- test/cargo_build_script/BUILD.bazel | 5 ++- test/cargo_build_script/build.rs | 4 +++ test/cargo_build_script/tools_exec.rs | 10 ++++++ 6 files changed, 67 insertions(+), 2 deletions(-) diff --git a/AUTHORS b/AUTHORS index 9da09a6e37..15cde4ff97 100644 --- a/AUTHORS +++ b/AUTHORS @@ -8,6 +8,7 @@ Google Inc. Spotify AB +VMware Inc. Damien Martin-Guillerez David Chen Florian Weikert diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 491738bfed..ad83aac805 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -20,3 +20,4 @@ Philipp Wollermann Ulf Adams Justine Alexandra Roberts Tunney John Edmonds +Ivan Kalchev \ No newline at end of file diff --git a/cargo/cargo_build_script.bzl b/cargo/cargo_build_script.bzl index a9970e992e..aeaf050613 100644 --- a/cargo/cargo_build_script.bzl +++ b/cargo/cargo_build_script.bzl @@ -1,5 +1,9 @@ # buildifier: disable=module-docstring -load("@bazel_tools//tools/build_defs/cc:action_names.bzl", "C_COMPILE_ACTION_NAME") +load( + "@bazel_tools//tools/build_defs/cc:action_names.bzl", + "C_COMPILE_ACTION_NAME", + "CPP_COMPILE_ACTION_NAME" +) load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") load("//rust:defs.bzl", "rust_binary", "rust_common") @@ -29,6 +33,37 @@ def get_cc_compile_env(cc_toolchain, feature_configuration): variables = compile_variables, ) +def get_cxx_compile_opts(cc_toolchain, feature_configuration): + """Gather command line arguments and env for the ``CPP_COMPILE_ACTION_NAME`` + action from the given cc_toolchain. + + Args: + cc_toolchain (cc_toolchain): The current rule's `cc_toolchain`. + feature_configuration (FeatureConfiguration): Class used to construct command lines from CROSSTOOL features. + + Returns: + tuple: Returns the command line arguments as the first element and the + env as the second element. + """ + compile_variables = cc_common.create_compile_variables( + cc_toolchain = cc_toolchain, + feature_configuration = feature_configuration, + ) + + compile_args_from_toolchain = cc_common.get_memory_inefficient_command_line( + feature_configuration = feature_configuration, + action_name = CPP_COMPILE_ACTION_NAME, + variables = compile_variables, + ) + + cc_compile_env = cc_common.get_environment_variables( + feature_configuration = feature_configuration, + action_name = CPP_COMPILE_ACTION_NAME, + variables = compile_variables, + ) + + return (compile_args_from_toolchain, cc_compile_env) + def _build_script_impl(ctx): """The implementation for the `_build_script_run` rule. @@ -101,6 +136,17 @@ def _build_script_impl(ctx): if include: env["INCLUDE"] = include + compile_args_from_toolchain, _ = get_cxx_compile_opts(cc_toolchain, feature_configuration) + # XXX: Skip adding the CXX env since we already added the C env. Should we stick to just CXX? + # Remove --sysroot since we pass it in the SYSROOT env var (we need to + # use the absolute path for it). See ``cargo_build_script_runner/bin.rs`` + # for how this is handled. + compile_args_from_toolchain = [ + arg for arg in compile_args_from_toolchain + if not arg.startswith("--sysroot") + ] + env["CXXFLAGS"] = " ".join(compile_args_from_toolchain) + if cc_toolchain: toolchain_tools.append(cc_toolchain.all_files) diff --git a/test/cargo_build_script/BUILD.bazel b/test/cargo_build_script/BUILD.bazel index 84fc960be2..8494d66a1f 100644 --- a/test/cargo_build_script/BUILD.bazel +++ b/test/cargo_build_script/BUILD.bazel @@ -7,7 +7,10 @@ load("//rust:defs.bzl", "rust_test") cargo_build_script( name = "tools_exec_build_rs", srcs = ["build.rs"], - build_script_env = {"TOOL": "$(execpath :tool)"}, + build_script_env = { + "TOOL": "$(execpath :tool)", + "CXXFLAGS": "-DMY_DEFINE" + }, tools = [":tool"], ) diff --git a/test/cargo_build_script/build.rs b/test/cargo_build_script/build.rs index 68cef72321..0b14950252 100644 --- a/test/cargo_build_script/build.rs +++ b/test/cargo_build_script/build.rs @@ -4,6 +4,10 @@ fn main() { "cargo:rustc-env=TOOL_PATH={}", std::env::var("TOOL").unwrap() ); + println!( + "cargo:rustc-env=CXXFLAGS={}", + std::env::var("CXXFLAGS").unwrap() + ); // Assert that the CC and CXX env vars existed and were executable. // We don't assert what happens when they're executed (in particular, we don't check for a diff --git a/test/cargo_build_script/tools_exec.rs b/test/cargo_build_script/tools_exec.rs index 6b06920825..11c77713ad 100644 --- a/test/cargo_build_script/tools_exec.rs +++ b/test/cargo_build_script/tools_exec.rs @@ -7,3 +7,13 @@ pub fn test_tool_exec() { tool_path ); } + +#[test] +pub fn test_cxxflags() { + let cxxflags = env!("CXXFLAGS"); + assert!( + cxxflags.contains("-DMY_DEFINE"), + "CXXFLAGS did not contain '-DMY_DEFINE', {}", + cxxflags + ); +} From 1e5f51aff057b2a5972b55fd2841d0785b8e2865 Mon Sep 17 00:00:00 2001 From: Ivan Kalchev Date: Tue, 2 Nov 2021 05:24:51 -0700 Subject: [PATCH 2/2] Fix lint issues --- cargo/cargo_build_script.bzl | 9 +++++---- test/cargo_build_script/BUILD.bazel | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cargo/cargo_build_script.bzl b/cargo/cargo_build_script.bzl index aeaf050613..6ebf36f2fd 100644 --- a/cargo/cargo_build_script.bzl +++ b/cargo/cargo_build_script.bzl @@ -1,8 +1,8 @@ # buildifier: disable=module-docstring load( "@bazel_tools//tools/build_defs/cc:action_names.bzl", + "CPP_COMPILE_ACTION_NAME", "C_COMPILE_ACTION_NAME", - "CPP_COMPILE_ACTION_NAME" ) load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") load("//rust:defs.bzl", "rust_binary", "rust_common") @@ -34,8 +34,7 @@ def get_cc_compile_env(cc_toolchain, feature_configuration): ) def get_cxx_compile_opts(cc_toolchain, feature_configuration): - """Gather command line arguments and env for the ``CPP_COMPILE_ACTION_NAME`` - action from the given cc_toolchain. + """Gather command line arguments and env from the given ``cc_toolchain``. Args: cc_toolchain (cc_toolchain): The current rule's `cc_toolchain`. @@ -137,12 +136,14 @@ def _build_script_impl(ctx): env["INCLUDE"] = include compile_args_from_toolchain, _ = get_cxx_compile_opts(cc_toolchain, feature_configuration) + # XXX: Skip adding the CXX env since we already added the C env. Should we stick to just CXX? # Remove --sysroot since we pass it in the SYSROOT env var (we need to # use the absolute path for it). See ``cargo_build_script_runner/bin.rs`` # for how this is handled. compile_args_from_toolchain = [ - arg for arg in compile_args_from_toolchain + arg + for arg in compile_args_from_toolchain if not arg.startswith("--sysroot") ] env["CXXFLAGS"] = " ".join(compile_args_from_toolchain) diff --git a/test/cargo_build_script/BUILD.bazel b/test/cargo_build_script/BUILD.bazel index 8494d66a1f..0cd57b78b3 100644 --- a/test/cargo_build_script/BUILD.bazel +++ b/test/cargo_build_script/BUILD.bazel @@ -8,8 +8,8 @@ cargo_build_script( name = "tools_exec_build_rs", srcs = ["build.rs"], build_script_env = { + "CXXFLAGS": "-DMY_DEFINE", "TOOL": "$(execpath :tool)", - "CXXFLAGS": "-DMY_DEFINE" }, tools = [":tool"], )