From c435cf4478fc6e097edc5dba0e71de6608ab77d8 Mon Sep 17 00:00:00 2001 From: Nick Rebbok Date: Thu, 10 Feb 2022 19:40:02 +0300 Subject: [PATCH] propagate buildscript link search paths to parent crate (#1123) Adds a new `link_search_paths: File` field to `BuildInfo` provider. Resolves #859 I use native Windows bindings through [windows-rs](https://github.com/microsoft/windows-rs) crate and it fails to link without propagating the link search path to a platform-dependent native library. The bindings call for a native library via `#[link(name = "windows")]` and propagate search path by a buildscript from a dependency (i.e. a transitive dependency for a root crate). Propagating link search paths seems to be what is cargo doing. --- cargo/cargo_build_script.bzl | 5 +++- cargo/cargo_build_script_runner/bin.rs | 11 +++++-- cargo/cargo_build_script_runner/lib.rs | 9 ++++-- docs/flatten.md | 5 ++-- docs/providers.md | 5 ++-- rust/private/providers.bzl | 2 ++ rust/private/rustc.bzl | 40 +++++++++++++++++--------- 7 files changed, 54 insertions(+), 23 deletions(-) diff --git a/cargo/cargo_build_script.bzl b/cargo/cargo_build_script.bzl index 688fe31431..80384d7204 100644 --- a/cargo/cargo_build_script.bzl +++ b/cargo/cargo_build_script.bzl @@ -45,6 +45,7 @@ def _build_script_impl(ctx): dep_env_out = ctx.actions.declare_file(ctx.label.name + ".depenv") flags_out = ctx.actions.declare_file(ctx.label.name + ".flags") link_flags = ctx.actions.declare_file(ctx.label.name + ".linkflags") + link_search_paths = ctx.actions.declare_file(ctx.label.name + ".linksearchpaths") # rustc-link-search, propagated from transitive dependencies manifest_dir = "%s.runfiles/%s/%s" % (script.path, ctx.label.workspace_name or ctx.workspace_name, ctx.label.package) compilation_mode_opt_level = get_compilation_mode_opts(ctx, toolchain).opt_level @@ -155,6 +156,7 @@ def _build_script_impl(ctx): env_out.path, flags_out.path, link_flags.path, + link_search_paths.path, dep_env_out.path, streams.stdout.path, streams.stderr.path, @@ -171,7 +173,7 @@ def _build_script_impl(ctx): ctx.actions.run( executable = ctx.executable._cargo_build_script_runner, arguments = [args], - outputs = [out_dir, env_out, flags_out, link_flags, dep_env_out, streams.stdout, streams.stderr], + outputs = [out_dir, env_out, flags_out, link_flags, link_search_paths, dep_env_out, streams.stdout, streams.stderr], tools = tools, inputs = build_script_inputs, mnemonic = "CargoBuildScriptRun", @@ -186,6 +188,7 @@ def _build_script_impl(ctx): dep_env = dep_env_out, flags = flags_out, link_flags = link_flags, + link_search_paths = link_search_paths, ), OutputGroupInfo(streams = depset([streams.stdout, streams.stderr])), ] diff --git a/cargo/cargo_build_script_runner/bin.rs b/cargo/cargo_build_script_runner/bin.rs index e99c102899..2f305e053f 100644 --- a/cargo/cargo_build_script_runner/bin.rs +++ b/cargo/cargo_build_script_runner/bin.rs @@ -41,6 +41,7 @@ fn run_buildrs() -> Result<(), String> { env_file, compile_flags_file, link_flags_file, + link_search_paths_file, output_dep_env_path, stdout_path, stderr_path, @@ -162,12 +163,15 @@ fn run_buildrs() -> Result<(), String> { let CompileAndLinkFlags { compile_flags, link_flags, + link_search_paths, } = BuildScriptOutput::outputs_to_flags(&buildrs_outputs, &exec_root.to_string_lossy()); write(&compile_flags_file, compile_flags.as_bytes()) .unwrap_or_else(|_| panic!("Unable to write file {:?}", compile_flags_file)); write(&link_flags_file, link_flags.as_bytes()) .unwrap_or_else(|_| panic!("Unable to write file {:?}", link_flags_file)); + write(&link_search_paths_file, link_search_paths.as_bytes()) + .unwrap_or_else(|_| panic!("Unable to write file {:?}", link_search_paths_file)); Ok(()) } @@ -179,6 +183,7 @@ struct Options { env_file: String, compile_flags_file: String, link_flags_file: String, + link_search_paths_file: String, output_dep_env_path: String, stdout_path: String, stderr_path: String, @@ -190,7 +195,7 @@ fn parse_args() -> Result { let mut args = env::args().skip(1); // TODO: we should consider an alternative to positional arguments. - match (args.next(), args.next(), args.next(), args.next(), args.next(), args.next(), args.next(), args.next(), args.next()) { + match (args.next(), args.next(), args.next(), args.next(), args.next(), args.next(), args.next(), args.next(), args.next(), args.next()) { ( Some(progname), Some(crate_links), @@ -198,6 +203,7 @@ fn parse_args() -> Result { Some(env_file), Some(compile_flags_file), Some(link_flags_file), + Some(link_search_paths_file), Some(output_dep_env_path), Some(stdout_path), Some(stderr_path), @@ -209,6 +215,7 @@ fn parse_args() -> Result { env_file, compile_flags_file, link_flags_file, + link_search_paths_file, output_dep_env_path, stdout_path, stderr_path, @@ -216,7 +223,7 @@ fn parse_args() -> Result { }) } _ => { - Err(format!("Usage: $0 progname crate_links out_dir env_file compile_flags_file link_flags_file output_dep_env_path stdout_path stderr_path input_dep_env_paths[arg1...argn]\nArguments passed: {:?}", args.collect::>())) + Err(format!("Usage: $0 progname crate_links out_dir env_file compile_flags_file link_flags_file link_search_paths_file output_dep_env_path stdout_path stderr_path input_dep_env_paths[arg1...argn]\nArguments passed: {:?}", args.collect::>())) } } } diff --git a/cargo/cargo_build_script_runner/lib.rs b/cargo/cargo_build_script_runner/lib.rs index 29e8a7caed..3a4e53bf66 100644 --- a/cargo/cargo_build_script_runner/lib.rs +++ b/cargo/cargo_build_script_runner/lib.rs @@ -21,6 +21,7 @@ use std::process::{Command, Output}; pub struct CompileAndLinkFlags { pub compile_flags: String, pub link_flags: String, + pub link_search_paths: String, } /// Enum containing all the considered return value from the script @@ -171,6 +172,7 @@ impl BuildScriptOutput { pub fn outputs_to_flags(outputs: &[BuildScriptOutput], exec_root: &str) -> CompileAndLinkFlags { let mut compile_flags = Vec::new(); let mut link_flags = Vec::new(); + let mut link_search_paths = Vec::new(); for flag in outputs { match flag { @@ -178,13 +180,15 @@ impl BuildScriptOutput { BuildScriptOutput::Flags(e) => compile_flags.push(e.to_owned()), BuildScriptOutput::LinkArg(e) => compile_flags.push(format!("-Clink-arg={}", e)), BuildScriptOutput::LinkLib(e) => link_flags.push(format!("-l{}", e)), - BuildScriptOutput::LinkSearch(e) => link_flags.push(format!("-L{}", e)), + BuildScriptOutput::LinkSearch(e) => link_search_paths.push(format!("-L{}", e)), _ => {} } } + CompileAndLinkFlags { compile_flags: compile_flags.join("\n"), link_flags: Self::redact_exec_root(&link_flags.join("\n"), exec_root), + link_search_paths: Self::redact_exec_root(&link_search_paths.join("\n"), exec_root), } } @@ -279,7 +283,8 @@ cargo:rustc-link-arg=Metal compile_flags: "-Lblah\n--cfg=feature=awesome\n-Clink-arg=-weak_framework\n-Clink-arg=Metal" .to_owned(), - link_flags: "-lsdfsdf\n-L${pwd}/bleh".to_owned(), + link_flags: "-lsdfsdf".to_owned(), + link_search_paths: "-L${pwd}/bleh".to_owned(), } ); } diff --git a/docs/flatten.md b/docs/flatten.md index 0cee286bc1..7c1e7b5bfc 100644 --- a/docs/flatten.md +++ b/docs/flatten.md @@ -1386,8 +1386,8 @@ A provider containing general Crate information. ## DepInfo 
-DepInfo(dep_env, direct_crates, transitive_build_infos, transitive_crate_outputs, transitive_crates,
-        transitive_noncrates)
+DepInfo(dep_env, direct_crates, link_search_path_files, transitive_build_infos,
+        transitive_crate_outputs, transitive_crates, transitive_noncrates)
A provider containing information about a Crate's dependencies. @@ -1399,6 +1399,7 @@ A provider containing information about a Crate's dependencies. | :------------- | :------------- | | dep_env | File: File with environment variables direct dependencies build scripts rely upon. | | direct_crates | depset[AliasableDepInfo] | +| link_search_path_files | depset[File]: All transitive files containing search paths to pass to the linker | | transitive_build_infos | depset[BuildInfo] | | transitive_crate_outputs | depset[File]: All transitive crate outputs. | | transitive_crates | depset[CrateInfo] | diff --git a/docs/providers.md b/docs/providers.md index e810289f91..0a78246809 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -42,8 +42,8 @@ A provider containing general Crate information. ## DepInfo 
-DepInfo(dep_env, direct_crates, transitive_build_infos, transitive_crate_outputs, transitive_crates,
-        transitive_noncrates)
+DepInfo(dep_env, direct_crates, link_search_path_files, transitive_build_infos,
+        transitive_crate_outputs, transitive_crates, transitive_noncrates)
A provider containing information about a Crate's dependencies. @@ -55,6 +55,7 @@ A provider containing information about a Crate's dependencies. | :------------- | :------------- | | dep_env | File: File with environment variables direct dependencies build scripts rely upon. | | direct_crates | depset[AliasableDepInfo] | +| link_search_path_files | depset[File]: All transitive files containing search paths to pass to the linker | | transitive_build_infos | depset[BuildInfo] | | transitive_crate_outputs | depset[File]: All transitive crate outputs. | | transitive_crates | depset[CrateInfo] | diff --git a/rust/private/providers.bzl b/rust/private/providers.bzl index f6b231a892..b471ac029f 100644 --- a/rust/private/providers.bzl +++ b/rust/private/providers.bzl @@ -45,6 +45,7 @@ DepInfo = provider( fields = { "dep_env": "File: File with environment variables direct dependencies build scripts rely upon.", "direct_crates": "depset[AliasableDepInfo]", + "link_search_path_files": "depset[File]: All transitive files containing search paths to pass to the linker", "transitive_build_infos": "depset[BuildInfo]", "transitive_crate_outputs": "depset[File]: All transitive crate outputs.", "transitive_crates": "depset[CrateInfo]", @@ -58,6 +59,7 @@ BuildInfo = provider( "dep_env": "File: extra build script environment varibles to be set to direct dependencies.", "flags": "File: file containing additional flags to pass to rustc", "link_flags": "File: file containing flags to pass to the linker", + "link_search_paths": "File: file containing search paths to pass to the linker", "out_dir": "File: directory containing the result of a build script", "rustc_env": "File: file containing additional environment variables to set for rustc.", }, diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 1fd17dfede..1b5a71f06e 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -141,6 +141,7 @@ def collect_deps( transitive_crates = [] transitive_noncrates = [] transitive_build_infos = [] + transitive_link_search_paths = [] build_info = None linkstamps = [] transitive_crate_outputs = [] @@ -175,6 +176,8 @@ def collect_deps( ) transitive_noncrates.append(dep_info.transitive_noncrates) transitive_build_infos.append(dep_info.transitive_build_infos) + transitive_link_search_paths.append(dep_info.link_search_path_files) + elif cc_info: # This dependency is a cc_library transitive_noncrates.append(cc_info.linking_context.linker_inputs) @@ -184,6 +187,7 @@ def collect_deps( "only one is allowed in the dependencies") build_info = dep_build_info transitive_build_infos.append(depset([build_info])) + transitive_link_search_paths.append(depset([build_info.link_search_paths])) else: fail("rust targets can only depend on rust_library, rust_*_library or cc_library " + "targets.") @@ -200,6 +204,7 @@ def collect_deps( ), transitive_crate_outputs = depset(transitive = transitive_crate_outputs), transitive_build_infos = depset(transitive = transitive_build_infos), + link_search_path_files = depset(transitive = transitive_link_search_paths), dep_env = build_info.dep_env if build_info else None, ), build_info, @@ -298,23 +303,24 @@ def get_linker_and_args(ctx, attr, cc_toolchain, feature_configuration, rpaths): def _process_build_scripts( build_info, + dep_info, compile_inputs): """Gathers the outputs from a target's `cargo_build_script` action. Args: build_info (BuildInfo): The target Build's dependency info. + dep_info (DepInfo): The Depinfo provider form the target Crate's set of inputs. compile_inputs (depset): A set of all files that will participate in the build. Returns: tuple: A tuple: A tuple of the following items: - - (list): A list of all build info `OUT_DIR` File objects + - (depset[File]): A list of all build info `OUT_DIR` File objects - (str): The `OUT_DIR` of the current build info - (File): An optional path to a generated environment file from a `cargo_build_script` target - - (list): All direct and transitive build flags from the current build info. + - (depset[File]): All direct and transitive build flags from the current build info. """ - extra_inputs, out_dir, build_env_file, build_flags_files = _create_extra_input_args(build_info) - if extra_inputs: - compile_inputs = depset(extra_inputs, transitive = [compile_inputs]) + extra_inputs, out_dir, build_env_file, build_flags_files = _create_extra_input_args(build_info, dep_info) + compile_inputs = depset(transitive = [extra_inputs, compile_inputs]) return compile_inputs, out_dir, build_env_file, build_flags_files def collect_inputs( @@ -350,7 +356,7 @@ def collect_inputs( - (list): A list of all build info `OUT_DIR` File objects - (str): The `OUT_DIR` of the current build info - (File): An optional path to a generated environment file from a `cargo_build_script` target - - (list): All direct and transitive build flags from the current build info + - (depset[File]): All direct and transitive build flag files from the current build info - (list[File]): Linkstamp outputs. """ linker_script = getattr(file, "linker_script") if hasattr(file, "linker_script") else None @@ -432,7 +438,7 @@ def collect_inputs( ) build_env_files = getattr(files, "rustc_env_files", []) - compile_inputs, out_dir, build_env_file, build_flags_files = _process_build_scripts(build_info, compile_inputs) + compile_inputs, out_dir, build_env_file, build_flags_files = _process_build_scripts(build_info, dep_info, compile_inputs) if build_env_file: build_env_files = [f for f in build_env_files] + [build_env_file] compile_inputs = depset(build_env_files, transitive = [compile_inputs]) @@ -477,7 +483,7 @@ def construct_arguments( rust_flags (list): Additional flags to pass to rustc out_dir (str): The path to the output directory for the target Crate. build_env_files (list): Files containing rustc environment variables, for instance from `cargo_build_script` actions. - build_flags_files (list): The output files of a `cargo_build_script` actions containing rustc build flags + build_flags_files (depset): The output files of a `cargo_build_script` actions containing rustc build flags emit (list): Values for the --emit flag to rustc. force_all_deps_direct (bool, optional): Whether to pass the transitive rlibs with --extern to the commandline as opposed to -L. @@ -925,18 +931,19 @@ def add_edition_flags(args, crate): if crate.edition != "2015": args.add("--edition={}".format(crate.edition)) -def _create_extra_input_args(build_info): +def _create_extra_input_args(build_info, dep_info): """Gather additional input arguments from transitive dependencies Args: build_info (BuildInfo): The BuildInfo provider from the target Crate's set of inputs. + dep_info (DepInfo): The Depinfo provider form the target Crate's set of inputs. Returns: tuple: A tuple of the following items: - - (list): A list of all build info `OUT_DIR` File objects + - (depset[File]): A list of all build info `OUT_DIR` File objects - (str): The `OUT_DIR` of the current build info - (File): An optional generated environment file from a `cargo_build_script` target - - (list): All direct and transitive build flags from the current build info. + - (depset[File]): All direct and transitive build flag files from the current build info. """ input_files = [] @@ -949,12 +956,17 @@ def _create_extra_input_args(build_info): if build_info: out_dir = build_info.out_dir.path build_env_file = build_info.rustc_env - build_flags_files.append(build_info.flags.path) - build_flags_files.append(build_info.link_flags.path) + build_flags_files.append(build_info.flags) + build_flags_files.append(build_info.link_flags) input_files.append(build_info.out_dir) input_files.append(build_info.link_flags) - return input_files, out_dir, build_env_file, build_flags_files + return ( + depset(input_files, transitive = [dep_info.link_search_path_files]), + out_dir, + build_env_file, + depset(build_flags_files, transitive = [dep_info.link_search_path_files]), + ) def _compute_rpaths(toolchain, output_dir, dep_info): """Determine the artifact's rpaths relative to the bazel root for runtime linking of shared libraries.