Skip to content

Commit

Permalink
propagate buildscript link search paths to parent crate (#1123)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
iRebbok committed Feb 10, 2022
1 parent bbccf77 commit c435cf4
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 23 deletions.
5 changes: 4 additions & 1 deletion cargo/cargo_build_script.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand All @@ -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",
Expand All @@ -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])),
]
Expand Down
11 changes: 9 additions & 2 deletions cargo/cargo_build_script_runner/bin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(())
}

Expand All @@ -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,
Expand All @@ -190,14 +195,15 @@ fn parse_args() -> Result<Options, String> {
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),
Some(out_dir),
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),
Expand All @@ -209,14 +215,15 @@ fn parse_args() -> Result<Options, String> {
env_file,
compile_flags_file,
link_flags_file,
link_search_paths_file,
output_dep_env_path,
stdout_path,
stderr_path,
input_dep_env_paths: args.collect(),
})
}
_ => {
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::<Vec<String>>()))
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::<Vec<String>>()))
}
}
}
Expand Down
9 changes: 7 additions & 2 deletions cargo/cargo_build_script_runner/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -171,20 +172,23 @@ 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 {
BuildScriptOutput::Cfg(e) => compile_flags.push(format!("--cfg={}", e)),
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),
}
}

Expand Down Expand Up @@ -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(),
}
);
}
Expand Down
5 changes: 3 additions & 2 deletions docs/flatten.md
Original file line number Diff line number Diff line change
Expand Up @@ -1386,8 +1386,8 @@ A provider containing general Crate information.
## DepInfo

<pre>
DepInfo(<a href="#DepInfo-dep_env">dep_env</a>, <a href="#DepInfo-direct_crates">direct_crates</a>, <a href="#DepInfo-transitive_build_infos">transitive_build_infos</a>, <a href="#DepInfo-transitive_crate_outputs">transitive_crate_outputs</a>, <a href="#DepInfo-transitive_crates">transitive_crates</a>,
<a href="#DepInfo-transitive_noncrates">transitive_noncrates</a>)
DepInfo(<a href="#DepInfo-dep_env">dep_env</a>, <a href="#DepInfo-direct_crates">direct_crates</a>, <a href="#DepInfo-link_search_path_files">link_search_path_files</a>, <a href="#DepInfo-transitive_build_infos">transitive_build_infos</a>,
<a href="#DepInfo-transitive_crate_outputs">transitive_crate_outputs</a>, <a href="#DepInfo-transitive_crates">transitive_crates</a>, <a href="#DepInfo-transitive_noncrates">transitive_noncrates</a>)
</pre>

A provider containing information about a Crate's dependencies.
Expand All @@ -1399,6 +1399,7 @@ A provider containing information about a Crate's dependencies.
| :------------- | :------------- |
| <a id="DepInfo-dep_env"></a>dep_env | File: File with environment variables direct dependencies build scripts rely upon. |
| <a id="DepInfo-direct_crates"></a>direct_crates | depset[AliasableDepInfo] |
| <a id="DepInfo-link_search_path_files"></a>link_search_path_files | depset[File]: All transitive files containing search paths to pass to the linker |
| <a id="DepInfo-transitive_build_infos"></a>transitive_build_infos | depset[BuildInfo] |
| <a id="DepInfo-transitive_crate_outputs"></a>transitive_crate_outputs | depset[File]: All transitive crate outputs. |
| <a id="DepInfo-transitive_crates"></a>transitive_crates | depset[CrateInfo] |
Expand Down
5 changes: 3 additions & 2 deletions docs/providers.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ A provider containing general Crate information.
## DepInfo

<pre>
DepInfo(<a href="#DepInfo-dep_env">dep_env</a>, <a href="#DepInfo-direct_crates">direct_crates</a>, <a href="#DepInfo-transitive_build_infos">transitive_build_infos</a>, <a href="#DepInfo-transitive_crate_outputs">transitive_crate_outputs</a>, <a href="#DepInfo-transitive_crates">transitive_crates</a>,
<a href="#DepInfo-transitive_noncrates">transitive_noncrates</a>)
DepInfo(<a href="#DepInfo-dep_env">dep_env</a>, <a href="#DepInfo-direct_crates">direct_crates</a>, <a href="#DepInfo-link_search_path_files">link_search_path_files</a>, <a href="#DepInfo-transitive_build_infos">transitive_build_infos</a>,
<a href="#DepInfo-transitive_crate_outputs">transitive_crate_outputs</a>, <a href="#DepInfo-transitive_crates">transitive_crates</a>, <a href="#DepInfo-transitive_noncrates">transitive_noncrates</a>)
</pre>

A provider containing information about a Crate's dependencies.
Expand All @@ -55,6 +55,7 @@ A provider containing information about a Crate's dependencies.
| :------------- | :------------- |
| <a id="DepInfo-dep_env"></a>dep_env | File: File with environment variables direct dependencies build scripts rely upon. |
| <a id="DepInfo-direct_crates"></a>direct_crates | depset[AliasableDepInfo] |
| <a id="DepInfo-link_search_path_files"></a>link_search_path_files | depset[File]: All transitive files containing search paths to pass to the linker |
| <a id="DepInfo-transitive_build_infos"></a>transitive_build_infos | depset[BuildInfo] |
| <a id="DepInfo-transitive_crate_outputs"></a>transitive_crate_outputs | depset[File]: All transitive crate outputs. |
| <a id="DepInfo-transitive_crates"></a>transitive_crates | depset[CrateInfo] |
Expand Down
2 changes: 2 additions & 0 deletions rust/private/providers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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]",
Expand All @@ -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.",
},
Expand Down
40 changes: 26 additions & 14 deletions rust/private/rustc.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand Down Expand Up @@ -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)
Expand All @@ -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.")
Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 = []

Expand All @@ -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.
Expand Down

0 comments on commit c435cf4

Please sign in to comment.