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

Allow cargo:rustc-env in build scripts #3929

Merged
merged 4 commits into from
May 15, 2017
Merged
Show file tree
Hide file tree
Changes from 2 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
7 changes: 7 additions & 0 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,7 @@ fn scrape_target_config(config: &Config, triple: &str)
library_paths: Vec::new(),
library_links: Vec::new(),
cfgs: Vec::new(),
env: Vec::new(),
metadata: Vec::new(),
rerun_if_changed: Vec::new(),
warnings: Vec::new(),
Expand Down Expand Up @@ -708,6 +709,12 @@ fn scrape_target_config(config: &Config, triple: &str)
let list = value.list(&k)?;
output.cfgs.extend(list.iter().map(|v| v.0.clone()));
}
"rustc-env" => {
for (name, val) in value.table(&k)?.0 {
let val = val.string(name)?.0;
output.env.push((name.clone(), val.to_string()));
}
}
"warning" | "rerun-if-changed" => {
bail!("`{}` is not supported in build script overrides", k);
}
Expand Down
17 changes: 17 additions & 0 deletions src/cargo/ops/cargo_rustc/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ pub struct BuildOutput {
pub library_links: Vec<String>,
/// Various `--cfg` flags to pass to the compiler
pub cfgs: Vec<String>,
/// Additional environment variables to run the compiler with.
pub env: Vec<(String, String)>,
/// Metadata to pass to the immediate dependencies
pub metadata: Vec<(String, String)>,
/// Paths to trigger a rerun of this build script.
Expand Down Expand Up @@ -255,6 +257,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
linked_libs: &parsed_output.library_links,
linked_paths: &library_paths,
cfgs: &parsed_output.cfgs,
env: &parsed_output.env,
});
}

Expand Down Expand Up @@ -321,6 +324,7 @@ impl BuildOutput {
let mut library_paths = Vec::new();
let mut library_links = Vec::new();
let mut cfgs = Vec::new();
let mut env = Vec::new();
let mut metadata = Vec::new();
let mut rerun_if_changed = Vec::new();
let mut warnings = Vec::new();
Expand Down Expand Up @@ -361,6 +365,7 @@ impl BuildOutput {
"rustc-link-lib" => library_links.push(value.to_string()),
"rustc-link-search" => library_paths.push(PathBuf::from(value)),
"rustc-cfg" => cfgs.push(value.to_string()),
"rustc-env" => env.push(BuildOutput::parse_rustc_env(value, &whence)?),
"warning" => warnings.push(value.to_string()),
"rerun-if-changed" => rerun_if_changed.push(value.to_string()),
_ => metadata.push((key.to_string(), value.to_string())),
Expand All @@ -371,6 +376,7 @@ impl BuildOutput {
library_paths: library_paths,
library_links: library_links,
cfgs: cfgs,
env: env,
metadata: metadata,
rerun_if_changed: rerun_if_changed,
warnings: warnings,
Expand Down Expand Up @@ -407,6 +413,17 @@ impl BuildOutput {
}
Ok((library_paths, library_links))
}

pub fn parse_rustc_env(value: &str, whence: &str)
-> CargoResult<(String, String)> {
let mut iter = value.splitn(2, '=');
let name = iter.next();
let val = iter.next();
match (name, val) {
(Some(n), Some(v)) => Ok((n.to_owned(), v.to_owned())),
_ => bail!("Variable rustc-env has no value in {}: {}", whence, value),
}
}
}

/// Compute the `build_scripts` map in the `Context` which tracks what build
Expand Down
18 changes: 18 additions & 0 deletions src/cargo/ops/cargo_rustc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,12 +305,15 @@ fn rustc(cx: &mut Context, unit: &Unit, exec: Arc<Executor>) -> CargoResult<Work
// also need to be sure to add any -L paths for our plugins to the
// dynamic library load path as a plugin's dynamic library may be
// located somewhere in there.
// Finally, if custom environment variables have been produced by
// previous build scripts, we include them in the rustc invocation.
if let Some(build_deps) = build_deps {
let build_state = build_state.outputs.lock().unwrap();
add_native_deps(&mut rustc, &build_state, &build_deps,
pass_l_flag, &current_id)?;
add_plugin_deps(&mut rustc, &build_state, &build_deps,
&root_output)?;
add_custom_env(&mut rustc, &build_state, &build_deps, &current_id)?;
}

// FIXME(rust-lang/rust#18913): we probably shouldn't have to do
Expand Down Expand Up @@ -420,6 +423,21 @@ fn rustc(cx: &mut Context, unit: &Unit, exec: Arc<Executor>) -> CargoResult<Work
}
Ok(())
}

// Add all custom environment variables present in `state` (after they've
// been put there by one of the `build_scripts`) to the command provided.
fn add_custom_env(rustc: &mut ProcessBuilder,
build_state: &BuildMap,
_: &BuildScripts,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah feel free to just remvoe this parameter if it's not used.

current_id: &PackageId) -> CargoResult<()> {
let key = (current_id.clone(), Kind::Host);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes here you'll want to pass in a kind instead of just picking Kind::Host, which I believe will break cross-compiled builds. Typically most rules have unit.kind which can be ferried to this locaion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, so that's what host vs target means. Makes sense! Fixed.

if let Some(output) = build_state.get(&key) {
for &(ref name, ref value) in output.env.iter() {
rustc.env(name, value);
}
}
Ok(())
}
}

/// Link the compiled target (often of form foo-{metadata_hash}) to the
Expand Down
1 change: 1 addition & 0 deletions src/cargo/util/machine_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub struct BuildScript<'a> {
pub linked_libs: &'a [String],
pub linked_paths: &'a [String],
pub cfgs: &'a [String],
pub env: &'a [(String, String)],
}

impl<'a> Message for BuildScript<'a> {
Expand Down
84 changes: 82 additions & 2 deletions tests/build-script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1575,6 +1575,86 @@ fn cfg_override_doc() {
assert_that(&p.root().join("target/doc/bar/fn.bar.html"), existing_file());
}

#[test]
fn env_build() {
let build = project("builder")
.file("Cargo.toml", r#"
[package]
name = "builder"
version = "0.0.1"
authors = []
build = "build.rs"
"#)
.file("src/main.rs", r#"
const FOO: &'static str = env!("FOO");
fn main() {
println!("{}", FOO);
}
"#)
.file("build.rs", r#"
fn main() {
println!("cargo:rustc-env=FOO=foo");
}
"#);
assert_that(build.cargo_process("build").arg("-v"),
execs().with_status(0));
assert_that(build.cargo("run").arg("-v"),
execs().with_status(0).with_stdout("foo\n"));
}

#[test]
fn env_test() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that this test still fails at the build step (with FOO variable not being defined). There is apparently something more that needs to be done to carry the variables if the build scripts is invoked for a crate as dependency for the test crate.

let p = project("foo")
.file("Cargo.toml", r#"
[package]
name = "foo"
version = "0.0.1"
authors = []
build = "build.rs"
"#)
.file("build.rs", r#"
fn main() {
println!("cargo:rustc-env=FOO=foo");
}
"#)
.file("src/lib.rs", r#"
pub const FOO: &'static str = env!("FOO");
"#)
.file("tests/test.rs", r#"
extern crate foo;

#[test]
fn test_foo() {
assert_eq!("foo", foo::FOO);
}
"#);
assert_that(p.cargo_process("test").arg("-v"),
execs().with_stderr(format!("\
[COMPILING] foo v0.0.1 ({dir})
[RUNNING] [..] build.rs [..]
[RUNNING] `[..][/]build-script-build`
[RUNNING] [..] --cfg foo[..]
[RUNNING] [..] --cfg foo[..]
[RUNNING] [..] --cfg foo[..]
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
[RUNNING] `[..][/]foo-[..][EXE]`
[RUNNING] `[..][/]test-[..][EXE]`
[DOCTEST] foo
[RUNNING] [..] --cfg foo[..]", dir = p.url()))
.with_stdout("
running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured


running 1 test
test test_foo ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured

"));
}

#[test]
fn flags_go_into_tests() {
let p = project("foo")
Expand Down Expand Up @@ -1816,7 +1896,7 @@ fn fresh_builds_possible_with_link_libs() {
rustc-flags = \"-l z -L ./\"
", target))
.file("build.rs", "");

assert_that(p.cargo_process("build").arg("-v"),
execs().with_status(0).with_stderr("\
[COMPILING] foo v0.5.0 ([..]
Expand Down Expand Up @@ -1857,7 +1937,7 @@ fn fresh_builds_possible_with_multiple_metadata_overrides() {
e = \"\"
", target))
.file("build.rs", "");

assert_that(p.cargo_process("build").arg("-v"),
execs().with_status(0).with_stderr("\
[COMPILING] foo v0.5.0 ([..]
Expand Down