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

Inform build scripts of rustc compiler context #9601

Merged
merged 19 commits into from
Jul 20, 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
12 changes: 11 additions & 1 deletion src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ fn output_err_info(cmd: &ProcessBuilder, stdout: &str, stderr: &str) -> String {
///
/// The locations are:
///
/// - the `CARGO_ENCODED_RUSTFLAGS` environment variable
/// - the `RUSTFLAGS` environment variable
///
/// then if this was not found
Expand Down Expand Up @@ -595,7 +596,16 @@ fn env_args(
return Ok(Vec::new());
}

// First try RUSTFLAGS from the environment
// First try CARGO_ENCODED_RUSTFLAGS from the environment.
// Prefer this over RUSTFLAGS since it's less prone to encoding errors.
if let Ok(a) = env::var(format!("CARGO_ENCODED_{}", name)) {
if a.is_empty() {
return Ok(Vec::new());
}
Comment on lines +602 to +604
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem necessary?

Suggested change
if a.is_empty() {
return Ok(Vec::new());
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, it is, because .split() on an empty string returns an iterator with one item (""), which we'd pass as an empty argument to rustc, which can make it confused. See also #9601 (comment).

return Ok(a.split('\x1f').map(str::to_string).collect());
}

// Then try RUSTFLAGS from the environment
if let Ok(a) = env::var(name) {
let args = a
.split(' ')
Expand Down
18 changes: 18 additions & 0 deletions src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,24 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
}
}

// Also inform the build script of the rustc compiler context.
if let Some(wrapper) = bcx.rustc().wrapper.as_ref() {
cmd.env("RUSTC_WRAPPER", wrapper);
} else {
cmd.env_remove("RUSTC_WRAPPER");
}
cmd.env_remove("RUSTC_WORKSPACE_WRAPPER");
if cx.bcx.ws.is_member(&unit.pkg) {
if let Some(wrapper) = bcx.rustc().workspace_wrapper.as_ref() {
cmd.env("RUSTC_WORKSPACE_WRAPPER", wrapper);
}
}
jonhoo marked this conversation as resolved.
Show resolved Hide resolved
cmd.env(
"CARGO_ENCODED_RUSTFLAGS",
bcx.rustflags_args(unit).join("\x1f"),
);
cmd.env_remove("RUSTFLAGS");

// Gather the set of native dependencies that this package has along with
// some other variables to close over.
//
Expand Down
19 changes: 17 additions & 2 deletions src/doc/src/reference/environment-variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,17 @@ system:
* `RUSTDOCFLAGS` — A space-separated list of custom flags to pass to all `rustdoc`
invocations that Cargo performs. In contrast with [`cargo rustdoc`], this is
useful for passing a flag to *all* `rustdoc` instances. See
[`build.rustdocflags`] for some more ways to set flags.
[`build.rustdocflags`] for some more ways to set flags. This string is
split by whitespace; for a more robust encoding of multiple arguments,
set `CARGO_ENCODED_RUSTDOCFLAGS` instead with arguments separated by
`0x1f` (ASCII Unit Separator).
Copy link
Member

Choose a reason for hiding this comment

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

Maybe use 0xff instead? That is not valid UTF-8 and as such is not allowed in arguments to rustc. 0x1f is and as such could be used eg in -Clink-arg or --extern.

Copy link
Member

Choose a reason for hiding this comment

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

Do all platforms support arbitrary bytes in environment variables? Using invalid UTF-8 would also mean you'd have to consume it with env::var_os as an OsString, which is more annoying to process without methods like split.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I also consider the delimiter being valid UTF-8 a benefit. It's true that someone could used 0x1f in an argument, but I don't see how anyone would in practice?

* `RUSTFLAGS` — A space-separated list of custom flags to pass to all compiler
invocations that Cargo performs. In contrast with [`cargo rustc`], this is
useful for passing a flag to *all* compiler instances. See
[`build.rustflags`] for some more ways to set flags.
[`build.rustflags`] for some more ways to set flags. This string is
split by whitespace; for a more robust encoding of multiple arguments,
set `CARGO_ENCODED_RUSTFLAGS` instead with arguments separated by
`0x1f` (ASCII Unit Separator).
* `CARGO_INCREMENTAL` — If this is set to 1 then Cargo will force [incremental
compilation] to be enabled for the current compilation, and when set to 0 it
will force disabling it. If this env var isn't present then cargo's defaults
Expand Down Expand Up @@ -334,11 +340,20 @@ let out_dir = env::var("OUT_DIR").unwrap();
* `RUSTC`, `RUSTDOC` — the compiler and documentation generator that Cargo has
resolved to use, passed to the build script so it might
use it as well.
* `RUSTC_WRAPPER` — the `rustc` wrapper, if any, that Cargo is using.
See [`build.rustc-wrapper`].
* `RUSTC_WORKSPACE_WRAPPER` — the `rustc` wrapper, if any, that Cargo is
using for workspace members. See
[`build.rustc-workspace-wrapper`].
* `RUSTC_LINKER` — The path to the linker binary that Cargo has resolved to use
for the current target, if specified. The linker can be
changed by editing `.cargo/config.toml`; see the documentation
about [cargo configuration][cargo-config] for more
information.
* `CARGO_ENCODED_RUSTFLAGS` — extra flags that Cargo invokes `rustc`
with, separated by a `0x1f` character
(ASCII Unit Separator). See
[`build.rustflags`].
* `CARGO_PKG_<var>` - The package information variables, with the same names and values as are [provided during crate building][variables set for crates].

[unix-like platforms]: ../../reference/conditional-compilation.html#unix-and-windows
Expand Down
180 changes: 178 additions & 2 deletions tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use cargo_test_support::compare::assert_match_exact;
use cargo_test_support::paths::CargoPathExt;
use cargo_test_support::registry::Package;
use cargo_test_support::tools;
use cargo_test_support::{basic_manifest, cross_compile, is_coarse_mtime, project};
use cargo_test_support::{rustc_host, sleep_ms, slow_cpu_multiplier, symlink_supported};
use cargo_util::paths::remove_dir_all;
Expand Down Expand Up @@ -82,7 +83,6 @@ fn custom_build_env_vars() {
let file_content = format!(
r#"
use std::env;
use std::io::prelude::*;
use std::path::Path;

fn main() {{
Expand Down Expand Up @@ -115,21 +115,197 @@ fn custom_build_env_vars() {
let rustdoc = env::var("RUSTDOC").unwrap();
assert_eq!(rustdoc, "rustdoc");

assert!(env::var("RUSTC_WRAPPER").is_err());
assert!(env::var("RUSTC_WORKSPACE_WRAPPER").is_err());

assert!(env::var("RUSTC_LINKER").is_err());

assert!(env::var("RUSTFLAGS").is_err());
let rustflags = env::var("CARGO_ENCODED_RUSTFLAGS").unwrap();
assert_eq!(rustflags, "");
}}
"#,
p.root()
.join("target")
.join("debug")
.join("build")
.display()
.display(),
);

let p = p.file("bar/build.rs", &file_content).build();

p.cargo("build --features bar_feat").run();
}

#[cargo_test]
fn custom_build_env_var_rustflags() {
let rustflags = "--cfg=special";
let rustflags_alt = "--cfg=notspecial";
let p = project()
.file(
".cargo/config",
&format!(
r#"
[build]
rustflags = ["{}"]
"#,
rustflags
),
)
.file(
"build.rs",
&format!(
r#"
use std::env;

fn main() {{
// Static assertion that exactly one of the cfg paths is always taken.
assert!(env::var("RUSTFLAGS").is_err());
let x;
#[cfg(special)]
{{ assert_eq!(env::var("CARGO_ENCODED_RUSTFLAGS").unwrap(), "{}"); x = String::new(); }}
#[cfg(notspecial)]
{{ assert_eq!(env::var("CARGO_ENCODED_RUSTFLAGS").unwrap(), "{}"); x = String::new(); }}
let _ = x;
}}
"#,
rustflags, rustflags_alt,
),
)
.file("src/lib.rs", "")
.build();

p.cargo("check").run();

// RUSTFLAGS overrides build.rustflags, so --cfg=special shouldn't be passed
p.cargo("check").env("RUSTFLAGS", rustflags_alt).run();
}

#[cargo_test]
fn custom_build_env_var_encoded_rustflags() {
// NOTE: We use "-Clink-arg=-B nope" here rather than, say, "-A missing_docs", since for the
// latter it won't matter if the whitespace accidentally gets split, as rustc will do the right
// thing either way.
let p = project()
.file(
".cargo/config",
r#"
[build]
rustflags = ["-Clink-arg=-B nope", "--cfg=foo"]
"#,
)
.file(
"build.rs",
r#"
use std::env;

fn main() {{
assert_eq!(env::var("CARGO_ENCODED_RUSTFLAGS").unwrap(), "-Clink-arg=-B nope\x1f--cfg=foo");
}}
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("check").run();
}

#[cargo_test]
fn custom_build_env_var_rustc_wrapper() {
let wrapper = tools::echo_wrapper();
let p = project()
.file(
"build.rs",
r#"
use std::env;

fn main() {{
assert_eq!(
env::var("RUSTC_WRAPPER").unwrap(),
env::var("CARGO_RUSTC_WRAPPER_CHECK").unwrap()
);
}}
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("check")
.env("CARGO_BUILD_RUSTC_WRAPPER", &wrapper)
.env("CARGO_RUSTC_WRAPPER_CHECK", &wrapper)
.run();
}

#[cargo_test]
fn custom_build_env_var_rustc_workspace_wrapper() {
let wrapper = tools::echo_wrapper();

// Workspace wrapper should be set for any crate we're operating directly on.
let p = project()
.file(
"build.rs",
r#"
use std::env;

fn main() {{
assert_eq!(
env::var("RUSTC_WORKSPACE_WRAPPER").unwrap(),
env::var("CARGO_RUSTC_WORKSPACE_WRAPPER_CHECK").unwrap()
);
}}
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("check")
.env("CARGO_BUILD_RUSTC_WORKSPACE_WRAPPER", &wrapper)
.env("CARGO_RUSTC_WORKSPACE_WRAPPER_CHECK", &wrapper)
.run();

// But should not be set for a crate from the registry, as then it's not in a workspace.
Package::new("bar", "0.1.0")
.file(
"Cargo.toml",
r#"
[package]
name = "bar"
version = "0.1.0"
links = "a"
"#,
)
.file(
"build.rs",
r#"
use std::env;

fn main() {{
assert!(env::var("RUSTC_WORKSPACE_WRAPPER").is_err());
}}
"#,
)
.file("src/lib.rs", "")
.publish();
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"

[dependencies]
bar = "0.1"
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("check")
.env("CARGO_BUILD_RUSTC_WORKSPACE_WRAPPER", &wrapper)
.run();
}

#[cargo_test]
fn custom_build_env_var_rustc_linker() {
if cross_compile::disabled() {
Expand Down