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.

Sign up for GitHub

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

Fix env/cfg set for cargo test and cargo run. #9122

Merged
merged 2 commits into from
Feb 3, 2021
Merged

Fix env/cfg set for cargo test and cargo run. #9122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d8673d9
Fix env/cfg set for `cargo test` and `cargo run`.
ehuss Feb 2, 2021
1607a68
Add note about `cargo:rustc-env` variables set at runtime.
ehuss Feb 3, 2021
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
94 changes: 64 additions & 30 deletions src/cargo/core/compiler/compilation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::{BTreeSet, HashMap, HashSet};
use std::collections::{BTreeSet, HashMap};
use std::env;
use std::ffi::{OsStr, OsString};
use std::path::PathBuf;
Expand All @@ -7,9 +7,8 @@ use cargo_platform::CfgExpr;
use semver::Version;

use super::BuildContext;
use crate::core::compiler::CompileKind;
use crate::core::compiler::Unit;
use crate::core::{Edition, Package, PackageId};
use crate::core::compiler::{CompileKind, Metadata, Unit};
use crate::core::{Edition, Package};
use crate::util::{self, config, join_paths, process, CargoResult, Config, ProcessBuilder};

/// Structure with enough information to run `rustdoc --test`.
Expand All @@ -22,20 +21,35 @@ pub struct Doctest {
pub unstable_opts: bool,
/// The -Clinker value to use.
pub linker: Option<PathBuf>,
/// The script metadata, if this unit's package has a build script.
///
/// This is used for indexing [`Compilation::extra_env`].
pub script_meta: Option<Metadata>,
}

/// Information about the output of a unit.
#[derive(Ord, PartialOrd, Eq, PartialEq)]
pub struct UnitOutput {
/// The unit that generated this output.
pub unit: Unit,
/// Path to the unit's primary output (an executable or cdylib).
pub path: PathBuf,
/// The script metadata, if this unit's package has a build script.
///
/// This is used for indexing [`Compilation::extra_env`].
pub script_meta: Option<Metadata>,
}

/// A structure returning the result of a compilation.
pub struct Compilation<'cfg> {
/// An array of all tests created during this compilation.
/// `(unit, path_to_test_exe)` where `unit` contains information such as the
/// package, compile target, etc.
pub tests: Vec<(Unit, PathBuf)>,
pub tests: Vec<UnitOutput>,

/// An array of all binaries created.
pub binaries: Vec<(Unit, PathBuf)>,
pub binaries: Vec<UnitOutput>,

/// An array of all cdylibs created.
pub cdylibs: Vec<(Unit, PathBuf)>,
pub cdylibs: Vec<UnitOutput>,

/// All directories for the output of native build commands.
///
Expand All @@ -60,17 +74,14 @@ pub struct Compilation<'cfg> {

/// Extra environment variables that were passed to compilations and should
/// be passed to future invocations of programs.
pub extra_env: HashMap<PackageId, Vec<(String, String)>>,
///
/// The key is the build script metadata for uniquely identifying the
/// `RunCustomBuild` unit that generated these env vars.
pub extra_env: HashMap<Metadata, Vec<(String, String)>>,

/// Libraries to test with rustdoc.
pub to_doc_test: Vec<Doctest>,

/// Features per package enabled during this compilation.
pub cfgs: HashMap<PackageId, HashSet<String>>,

/// Flags to pass to rustdoc when invoked from cargo test, per package.
pub rustdocflags: HashMap<PackageId, Vec<String>>,

/// The target host triple.
pub host: String,

Expand Down Expand Up @@ -127,8 +138,6 @@ impl<'cfg> Compilation<'cfg> {
cdylibs: Vec::new(),
extra_env: HashMap::new(),
to_doc_test: Vec::new(),
cfgs: HashMap::new(),
rustdocflags: HashMap::new(),
config: bcx.config,
host: bcx.host_triple().to_string(),
rustc_process: rustc,
Expand All @@ -144,7 +153,13 @@ impl<'cfg> Compilation<'cfg> {
})
}

/// See `process`.
/// Returns a [`ProcessBuilder`] for running `rustc`.
///
/// `is_primary` is true if this is a "primary package", which means it
/// was selected by the user on the command-line (such as with a `-p`
/// flag), see [`crate::core::compiler::Context::primary_packages`].
///
/// `is_workspace` is true if this is a workspace member.
pub fn rustc_process(
&self,
unit: &Unit,
Expand All @@ -160,14 +175,18 @@ impl<'cfg> Compilation<'cfg> {
};

let cmd = fill_rustc_tool_env(rustc, unit);
self.fill_env(cmd, &unit.pkg, unit.kind, true)
self.fill_env(cmd, &unit.pkg, None, unit.kind, true)
}

/// See `process`.
pub fn rustdoc_process(&self, unit: &Unit) -> CargoResult<ProcessBuilder> {
/// Returns a [`ProcessBuilder`] for running `rustdoc`.
pub fn rustdoc_process(
&self,
unit: &Unit,
script_meta: Option<Metadata>,
) -> CargoResult<ProcessBuilder> {
let rustdoc = process(&*self.config.rustdoc()?);
let cmd = fill_rustc_tool_env(rustdoc, unit);
let mut p = self.fill_env(cmd, &unit.pkg, unit.kind, true)?;
let mut p = self.fill_env(cmd, &unit.pkg, script_meta, unit.kind, true)?;
if unit.target.edition() != Edition::Edition2015 {
p.arg(format!("--edition={}", unit.target.edition()));
}
Expand All @@ -179,25 +198,37 @@ impl<'cfg> Compilation<'cfg> {
Ok(p)
}

/// See `process`.
/// Returns a [`ProcessBuilder`] appropriate for running a process for the
/// host platform.
///
/// This is currently only used for running build scripts. If you use this
/// for anything else, please be extra careful on how environment
/// variables are set!
pub fn host_process<T: AsRef<OsStr>>(
&self,
cmd: T,
pkg: &Package,
) -> CargoResult<ProcessBuilder> {
self.fill_env(process(cmd), pkg, CompileKind::Host, false)
self.fill_env(process(cmd), pkg, None, CompileKind::Host, false)
}

pub fn target_runner(&self, kind: CompileKind) -> Option<&(PathBuf, Vec<String>)> {
self.target_runners.get(&kind).and_then(|x| x.as_ref())
}

/// See `process`.
/// Returns a [`ProcessBuilder`] appropriate for running a process for the
/// target platform. This is typically used for `cargo run` and `cargo
/// test`.
///
/// `script_meta` is the metadata for the `RunCustomBuild` unit that this
/// unit used for its build script. Use `None` if the package did not have
/// a build script.
pub fn target_process<T: AsRef<OsStr>>(
&self,
cmd: T,
kind: CompileKind,
pkg: &Package,
script_meta: Option<Metadata>,
) -> CargoResult<ProcessBuilder> {
let builder = if let Some((runner, args)) = self.target_runner(kind) {
let mut builder = process(runner);
Expand All @@ -207,7 +238,7 @@ impl<'cfg> Compilation<'cfg> {
} else {
process(cmd)
};
self.fill_env(builder, pkg, kind, false)
self.fill_env(builder, pkg, script_meta, kind, false)
}

/// Prepares a new process with an appropriate environment to run against
Expand All @@ -219,6 +250,7 @@ impl<'cfg> Compilation<'cfg> {
&self,
mut cmd: ProcessBuilder,
pkg: &Package,
script_meta: Option<Metadata>,
kind: CompileKind,
is_rustc_tool: bool,
) -> CargoResult<ProcessBuilder> {
Expand Down Expand Up @@ -258,9 +290,11 @@ impl<'cfg> Compilation<'cfg> {
let search_path = join_paths(&search_path, util::dylib_path_envvar())?;

cmd.env(util::dylib_path_envvar(), &search_path);
if let Some(env) = self.extra_env.get(&pkg.package_id()) {
for &(ref k, ref v) in env {
cmd.env(k, v);
if let Some(meta) = script_meta {
if let Some(env) = self.extra_env.get(&meta) {
for (k, v) in env {
cmd.env(k, v);
}
}
}

Expand Down
83 changes: 41 additions & 42 deletions src/cargo/core/compiler/context/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::collections::{BTreeSet, HashMap, HashSet};
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex};

use filetime::FileTime;
use jobserver::Client;

use crate::core::compiler::{self, compilation, Unit};
use crate::core::compiler::compilation::{self, UnitOutput};
use crate::core::compiler::{self, Unit};
use crate::core::PackageId;
use crate::util::errors::{CargoResult, CargoResultExt};
use crate::util::profile;
Expand Down Expand Up @@ -174,16 +175,16 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
if unit.mode == CompileMode::Test {
self.compilation
.tests
.push((unit.clone(), output.path.clone()));
.push(self.unit_output(unit, &output.path));
} else if unit.target.is_executable() {
self.compilation
.binaries
.push((unit.clone(), bindst.clone()));
.push(self.unit_output(unit, bindst));
} else if unit.target.is_cdylib() {
if !self.compilation.cdylibs.iter().any(|(u, _)| u == unit) {
if !self.compilation.cdylibs.iter().any(|uo| uo.unit == *unit) {
self.compilation
.cdylibs
.push((unit.clone(), bindst.clone()));
.push(self.unit_output(unit, bindst));
}
}
}
Expand All @@ -198,9 +199,10 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
.build_script_out_dir(&dep.unit)
.display()
.to_string();
let script_meta = self.get_run_build_script_metadata(&dep.unit);
self.compilation
.extra_env
.entry(dep.unit.pkg.package_id())
.entry(script_meta)
.or_insert_with(Vec::new)
.push(("OUT_DIR".to_string(), out_dir));
}
Expand All @@ -212,50 +214,36 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
let mut unstable_opts = false;
let mut args = compiler::extern_args(&self, unit, &mut unstable_opts)?;
args.extend(compiler::lto_args(&self, unit));
for feature in &unit.features {
args.push("--cfg".into());
args.push(format!("feature=\"{}\"", feature).into());
}
let script_meta = self.find_build_script_metadata(unit);
if let Some(meta) = script_meta {
if let Some(output) = self.build_script_outputs.lock().unwrap().get(meta) {
for cfg in &output.cfgs {
args.push("--cfg".into());
args.push(cfg.into());
}
}
}
args.extend(self.bcx.rustdocflags_args(unit).iter().map(Into::into));
self.compilation.to_doc_test.push(compilation::Doctest {
unit: unit.clone(),
args,
unstable_opts,
linker: self.bcx.linker(unit.kind),
script_meta,
});
}

// Collect the enabled features.
let feats = &unit.features;
if !feats.is_empty() {
self.compilation
.cfgs
.entry(unit.pkg.package_id())
.or_insert_with(|| {
feats
.iter()
.map(|feat| format!("feature=\"{}\"", feat))
.collect()
});
}

// Collect rustdocflags.
let rustdocflags = self.bcx.rustdocflags_args(unit);
if !rustdocflags.is_empty() {
self.compilation
.rustdocflags
.entry(unit.pkg.package_id())
.or_insert_with(|| rustdocflags.to_vec());
}

super::output_depinfo(&mut self, unit)?;
}

for (pkg_id, output) in self.build_script_outputs.lock().unwrap().iter() {
self.compilation
.cfgs
.entry(pkg_id)
.or_insert_with(HashSet::new)
.extend(output.cfgs.iter().cloned());

for (script_meta, output) in self.build_script_outputs.lock().unwrap().iter() {
self.compilation
.extra_env
.entry(pkg_id)
.entry(*script_meta)
.or_insert_with(Vec::new)
.extend(output.env.iter().cloned());

Expand Down Expand Up @@ -352,11 +340,11 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
/// Returns the RunCustomBuild Unit associated with the given Unit.
///
/// If the package does not have a build script, this returns None.
pub fn find_build_script_unit(&self, unit: Unit) -> Option<Unit> {
pub fn find_build_script_unit(&self, unit: &Unit) -> Option<Unit> {
if unit.mode.is_run_custom_build() {
return Some(unit);
return Some(unit.clone());
}
self.bcx.unit_graph[&unit]
self.bcx.unit_graph[unit]
.iter()
.find(|unit_dep| {
unit_dep.unit.mode.is_run_custom_build()
Expand All @@ -369,7 +357,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
/// the given unit.
///
/// If the package does not have a build script, this returns None.
pub fn find_build_script_metadata(&self, unit: Unit) -> Option<Metadata> {
pub fn find_build_script_metadata(&self, unit: &Unit) -> Option<Metadata> {
let script_unit = self.find_build_script_unit(unit)?;
Some(self.get_run_build_script_metadata(&script_unit))
}
Expand Down Expand Up @@ -398,6 +386,17 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
Ok(inputs.into_iter().collect())
}

/// Returns a [`UnitOutput`] which represents some information about the
/// output of a unit.
pub fn unit_output(&self, unit: &Unit, path: &Path) -> UnitOutput {
let script_meta = self.find_build_script_metadata(unit);
UnitOutput {
unit: unit.clone(),
path: path.to_path_buf(),
script_meta,
}
}

fn check_collisions(&self) -> CargoResult<()> {
let mut output_collisions = HashMap::new();
let describe_collision = |unit: &Unit, other_unit: &Unit, path: &PathBuf| -> String {
Expand Down
Loading