Skip to content

Commit

Permalink
Fix env/cfg set for cargo test and cargo run.
Browse files Browse the repository at this point in the history
  • Loading branch information
@ehuss
ehuss committed Feb 2, 2021
1 parent e099df2 commit d8673d9
Show file tree
Hide file tree
Showing 12 changed files with 259 additions and 148 deletions.
94 changes: 64 additions & 30 deletions src/cargo/core/compiler/compilation.rs
View file
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
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

0 comments on commit d8673d9

Please sign in to comment.