Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cargo_build_script now only prints warnings to stderr. #548

Merged
merged 3 commits into from
Jan 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions cargo/cargo_build_script.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ def _build_script_impl(ctx):
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

streams = struct(
stdout = ctx.actions.declare_file(ctx.label.name + ".stdout.log"),
stderr = ctx.actions.declare_file(ctx.label.name + ".stderr.log"),
)

crate_name = ctx.attr.crate_name

# Derive crate name from the rule label which is <crate_name>_build_script if not provided.
Expand Down Expand Up @@ -134,7 +139,18 @@ def _build_script_impl(ctx):
# See https://doc.rust-lang.org/cargo/reference/build-scripts.html#-sys-packages
# for details.
args = ctx.actions.args()
args.add_all([script.path, crate_name, links, out_dir.path, env_out.path, flags_out.path, link_flags.path, dep_env_out.path])
args.add_all([
script.path,
crate_name,
links,
out_dir.path,
env_out.path,
flags_out.path,
link_flags.path,
dep_env_out.path,
streams.stdout.path,
streams.stderr.path,
])
build_script_inputs = []
for dep in ctx.attr.deps:
if DepInfo in dep and dep[DepInfo].dep_env:
Expand All @@ -147,7 +163,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],
outputs = [out_dir, env_out, flags_out, link_flags, dep_env_out, streams.stdout, streams.stderr],
tools = tools,
inputs = build_script_inputs,
mnemonic = "CargoBuildScriptRun",
Expand All @@ -162,6 +178,7 @@ def _build_script_impl(ctx):
flags = flags_out,
link_flags = link_flags,
),
OutputGroupInfo(streams = depset([streams.stdout, streams.stderr])),
]

_build_script_run = rule(
Expand All @@ -174,7 +191,7 @@ _build_script_run = rule(
# The source of truth will be the `cargo_build_script` macro until stardoc
# implements documentation inheritence. See https://github.com/bazelbuild/stardoc/issues/27
"script": attr.label(
doc = "The binary script to run, generally a rust_binary target.",
doc = "The binary script to run, generally a `rust_binary` target.",
executable = True,
allow_files = True,
mandatory = True,
Expand All @@ -187,7 +204,8 @@ _build_script_run = rule(
doc = "The name of the native library this crate links against.",
),
"deps": attr.label_list(
doc = "The dependencies of the crate defined by `crate_name`",
doc = "The Rust dependencies of the crate defined by `crate_name`",
providers = [DepInfo],
damienmg marked this conversation as resolved.
Show resolved Hide resolved
),
"version": attr.string(
doc = "The semantic version (semver) of the crate",
Expand Down
53 changes: 42 additions & 11 deletions cargo/cargo_build_script_runner/bin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use std::fs::{create_dir_all, read_to_string, write};
use std::path::Path;
use std::process::Command;

fn main() -> Result<(), String> {
fn run_buildrs() -> Result<(), String> {
// We use exec_root.join rather than std::fs::canonicalize, to avoid resolving symlinks, as
// some execution strategies and remote execution environments may use symlinks in ways which
// canonicalizing them may break them, e.g. by having input files be symlinks into a /cas
Expand All @@ -44,6 +44,8 @@ fn main() -> Result<(), String> {
flagfile,
linkflags,
output_dep_env_path,
stdout_path,
stderr_path,
input_dep_env_paths,
} = parse_args()?;

Expand Down Expand Up @@ -111,14 +113,16 @@ fn main() -> Result<(), String> {
}
}

let output = BuildScriptOutput::from_command(&mut command).map_err(|exit_code| {
let (buildrs_outputs, process_output) = BuildScriptOutput::from_command(&mut command).map_err(|process_output| {
format!(
"Build script process failed{}",
if let Some(exit_code) = exit_code {
"Build script process failed{}\n--stdout:\n{}\n--stderr:\n{}",
if let Some(exit_code) = process_output.status.code() {
format!(" with exit code {}", exit_code)
} else {
String::new()
}
},
String::from_utf8(process_output.stdout).expect("Failed to parse stdout of child process"),
String::from_utf8(process_output.stderr).expect("Failed to parse stdout of child process"),
)
})?;

Expand All @@ -142,20 +146,30 @@ fn main() -> Result<(), String> {

write(
&envfile,
BuildScriptOutput::to_env(&output, &exec_root.to_string_lossy()).as_bytes(),
BuildScriptOutput::to_env(&buildrs_outputs, &exec_root.to_string_lossy()).as_bytes(),
)
.expect(&format!("Unable to write file {:?}", envfile));
write(
&output_dep_env_path,
BuildScriptOutput::to_dep_env(&output, crate_links, &exec_root.to_string_lossy())
BuildScriptOutput::to_dep_env(&buildrs_outputs, crate_links, &exec_root.to_string_lossy())
.as_bytes(),
)
.expect(&format!("Unable to write file {:?}", output_dep_env_path));
write(
&stdout_path,
process_output.stdout,
)
.expect(&format!("Unable to write file {:?}", stdout_path));
write(
&stderr_path,
process_output.stderr,
)
.expect(&format!("Unable to write file {:?}", stderr_path));

let CompileAndLinkFlags {
compile_flags,
link_flags,
} = BuildScriptOutput::to_flags(&output, &exec_root.to_string_lossy());
} = BuildScriptOutput::to_flags(&buildrs_outputs, &exec_root.to_string_lossy());

write(&flagfile, compile_flags.as_bytes())
.expect(&format!("Unable to write file {:?}", flagfile));
Expand All @@ -174,6 +188,8 @@ struct Options {
flagfile: String,
linkflags: String,
output_dep_env_path: String,
stdout_path: String,
stderr_path: String,
input_dep_env_paths: Vec<String>,
}

Expand All @@ -182,16 +198,18 @@ 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()) {
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_name),
Some(crate_links),
Some(out_dir),
Some(envfile),
Some(flagfile),
Some(linkflags),
Some(output_dep_env_path)
Some(linkflags),
Some(output_dep_env_path),
Some(stdout_path),
Some(stderr_path),
) => {
Ok(Options{
progname,
Expand All @@ -202,6 +220,8 @@ fn parse_args() -> Result<Options, String> {
flagfile,
linkflags,
output_dep_env_path,
stdout_path,
stderr_path,
input_dep_env_paths: args.collect(),
})
}
Expand Down Expand Up @@ -261,3 +281,14 @@ fn absolutify(root: &Path, maybe_relative: OsString) -> OsString {
maybe_relative
}
}

fn main() {
std::process::exit(match run_buildrs() {
Ok(_) => 0,
Err(err) => {
// Neatly print errors
eprintln!("{}", err);
1
}
});
}
26 changes: 11 additions & 15 deletions cargo/cargo_build_script_runner/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
//! Parse the output of a cargo build.rs script and generate a list of flags and
//! environment variable for the build.
use std::io::{BufRead, BufReader, Read};
use std::process::{Command, Stdio};
use std::process::{Command, Output};

#[derive(Debug, PartialEq, Eq)]
pub struct CompileAndLinkFlags {
Expand Down Expand Up @@ -51,14 +51,12 @@ impl BuildScriptOutput {
let split = line.splitn(2, '=').collect::<Vec<_>>();
if split.len() <= 1 {
// Not a cargo directive.
print!("{}", line);
return None;
}
let param = split[1].trim().to_owned();
let key_split = split[0].splitn(2, ':').collect::<Vec<_>>();
if key_split.len() <= 1 || key_split[0] != "cargo" {
// Not a cargo directive.
print!("{}", line);
return None;
}
match key_split[1] {
Expand All @@ -73,12 +71,12 @@ impl BuildScriptOutput {
None
}
"warning" => {
eprintln!("Build Script Warning: {}", split[1]);
eprint!("Build Script Warning: {}", split[1]);
None
}
"rustc-cdylib-link-arg" => {
// cargo:rustc-cdylib-link-arg=FLAG — Passes custom flags to a linker for cdylib crates.
eprintln!(
eprint!(
"Warning: build script returned unsupported directive `{}`",
split[0]
);
Expand Down Expand Up @@ -109,18 +107,16 @@ impl BuildScriptOutput {
}

/// Take a [Command], execute it and converts its input into a vector of [BuildScriptOutput]
pub fn from_command(cmd: &mut Command) -> Result<Vec<BuildScriptOutput>, Option<i32>> {
let mut child = cmd
.stdout(Stdio::piped())
.spawn()
pub fn from_command(cmd: &mut Command) -> Result<(Vec<BuildScriptOutput>, Output), Output> {
let child_output = cmd
.output()
.expect("Unable to start binary");
let reader = BufReader::new(child.stdout.as_mut().expect("Failed to open stdout"));
let output = Self::from_reader(reader);
let ecode = child.wait().expect("failed to wait on child");
if ecode.success() {
Ok(output)
if child_output.status.success() {
let reader = BufReader::new(child_output.stdout.as_slice());
let output = Self::from_reader(reader);
Ok((output, child_output))
} else {
Err(ecode.code())
Err(child_output)
}
}

Expand Down