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

Profile Overrides (RFC #2282 Part 1) #5384

Merged
merged 23 commits into from
Apr 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
575d6e8
Profile Overrides (RFC #2282 Part 1)
ehuss Apr 18, 2018
62d6f0d
Minor cleanup.
ehuss Apr 18, 2018
7366074
Move `Profile` back into `Unit` as a copy.
ehuss Apr 18, 2018
138eb33
Fix a variety of profile bugs.
ehuss Apr 19, 2018
7051b63
Update for some review comments.
ehuss Apr 19, 2018
768f573
Add extra rustc/rustdoc args to the fingerprint and metadata.
ehuss Apr 19, 2018
0d8ebe9
Avoid running `build.rs` too often.
ehuss Apr 20, 2018
7dc9e07
Minor tweaks on how examples are handled with tests, and some panic p…
ehuss Apr 21, 2018
a0a880c
Add warnings for unknown profile overrides.
ehuss Apr 21, 2018
36b8769
Add warning if `panic` is set in test or bench profile.
ehuss Apr 21, 2018
ba537d7
Add test for profile override on non-dev/release.
ehuss Apr 21, 2018
c667fc2
Add more profile override validation tests.
ehuss Apr 21, 2018
ec7be84
Some test cleanup for profiles.
ehuss Apr 21, 2018
accc9b2
Remove last TODO comments.
ehuss Apr 21, 2018
a4947c2
rustfmt
ehuss Apr 21, 2018
9ca36de
Move profile override tests to a dedicated file.
ehuss Apr 21, 2018
10a6da6
Add thorough tests for target/profile selection.
ehuss Apr 21, 2018
b9181ef
Add some more tests.
ehuss Apr 22, 2018
025e23b
Update tests now that `cargo check` does not re-check bins.
ehuss Apr 22, 2018
195520b
Avoid building libs and bins twice in `cargo build --all-targets --re…
ehuss Apr 22, 2018
902cb98
Fix Unit/Context parameter order.
ehuss Apr 24, 2018
040f984
Comment why `deps_of` doesn't need to keep track of `profile_for` in …
ehuss Apr 24, 2018
e82e9c1
Minor style update.
ehuss Apr 27, 2018
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
23 changes: 15 additions & 8 deletions src/cargo/core/compiler/context/compilation_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use std::sync::Arc;

use lazycell::LazyCell;

use core::{TargetKind, Workspace};
use super::{Context, FileFlavor, Kind, Layout, Unit};
use core::{TargetKind, Workspace};
use util::{self, CargoResult};

#[derive(Clone, Hash, Eq, PartialEq, Ord, PartialOrd)]
Expand Down Expand Up @@ -97,7 +97,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
/// Returns the appropriate output directory for the specified package and
/// target.
pub fn out_dir(&self, unit: &Unit<'a>) -> PathBuf {
if unit.profile.doc {
if unit.mode.is_doc() {
self.layout(unit.kind).root().parent().unwrap().join("doc")
} else if unit.target.is_custom_build() {
self.build_script_dir(unit)
Expand Down Expand Up @@ -148,15 +148,15 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
/// Returns the appropriate directory layout for either a plugin or not.
pub fn build_script_dir(&self, unit: &Unit<'a>) -> PathBuf {
assert!(unit.target.is_custom_build());
assert!(!unit.profile.run_custom_build);
assert!(!unit.mode.is_run_custom_build());
let dir = self.pkg_dir(unit);
self.layout(Kind::Host).build().join(dir)
}

/// Returns the appropriate directory layout for either a plugin or not.
pub fn build_script_out_dir(&self, unit: &Unit<'a>) -> PathBuf {
assert!(unit.target.is_custom_build());
assert!(unit.profile.run_custom_build);
assert!(unit.mode.is_run_custom_build());
let dir = self.pkg_dir(unit);
self.layout(unit.kind).build().join(dir).join("out")
}
Expand Down Expand Up @@ -211,7 +211,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
} else {
Some((
out_dir.parent().unwrap().to_owned(),
if unit.profile.test {
if unit.mode.is_any_test() {
file_stem
} else {
bin_stem
Expand Down Expand Up @@ -244,7 +244,10 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
let mut ret = Vec::new();
let mut unsupported = Vec::new();
{
if unit.profile.check {
if unit.mode.is_check() {
// This is not quite correct for non-lib targets. rustc
// currently does not emit rmeta files, so there is nothing to
// check for! See #3624.
let path = out_dir.join(format!("lib{}.rmeta", file_stem));
let hardlink = link_stem
.clone()
Expand Down Expand Up @@ -296,7 +299,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
| TargetKind::Test => {
add("bin", FileFlavor::Normal)?;
}
TargetKind::Lib(..) | TargetKind::ExampleLib(..) if unit.profile.test => {
TargetKind::Lib(..) | TargetKind::ExampleLib(..) if unit.mode.is_any_test() => {
add("bin", FileFlavor::Normal)?;
}
TargetKind::ExampleLib(ref kinds) | TargetKind::Lib(ref kinds) => {
Expand Down Expand Up @@ -378,7 +381,7 @@ fn compute_metadata<'a, 'cfg>(
// just here for rustbuild. We need a more principled method
// doing this eventually.
let __cargo_default_lib_metadata = env::var("__CARGO_DEFAULT_LIB_METADATA");
if !(unit.profile.test || unit.profile.check)
if !(unit.mode.is_any_test() || unit.mode.is_check())
&& (unit.target.is_dylib() || unit.target.is_cdylib()
|| (unit.target.is_bin() && cx.build_config.target_triple().starts_with("wasm32-")))
&& unit.pkg.package_id().source_id().is_path()
Expand Down Expand Up @@ -423,6 +426,10 @@ fn compute_metadata<'a, 'cfg>(
// panic=abort and panic=unwind artifacts, additionally with various
// settings like debuginfo and whatnot.
unit.profile.hash(&mut hasher);
unit.mode.hash(&mut hasher);
if let Some(ref args) = cx.extra_args_for(unit) {
args.hash(&mut hasher);
}

// Artifacts compiled for the host should have a different metadata
// piece than those compiled for the target, so make sure we throw in
Expand Down
84 changes: 40 additions & 44 deletions src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#![allow(deprecated)]

use std::collections::{HashMap, HashSet};
use std::env;
use std::path::{Path, PathBuf};
Expand All @@ -8,25 +7,27 @@ use std::sync::Arc;

use jobserver::Client;

use core::{Package, PackageId, PackageSet, Profile, Resolve, Target};
use core::{Dependency, Profiles, Workspace};
use util::{internal, profile, Cfg, CfgExpr, Config};
use core::profiles::{Profile, Profiles};
use core::{Dependency, Workspace};
use core::{Package, PackageId, PackageSet, Resolve, Target};
use ops::CompileMode;
use util::errors::{CargoResult, CargoResultExt};
use util::{internal, profile, Cfg, CfgExpr, Config};

use super::TargetConfig;
use super::custom_build::{self, BuildDeps, BuildScripts, BuildState};
use super::fingerprint::Fingerprint;
use super::job_queue::JobQueue;
use super::layout::Layout;
use super::links::Links;
use super::TargetConfig;
use super::{BuildConfig, Compilation, Executor, Kind};

mod unit_dependencies;
use self::unit_dependencies::build_unit_dependencies;

mod compilation_files;
use self::compilation_files::{CompilationFiles, OutputFile};
pub use self::compilation_files::Metadata;
use self::compilation_files::{CompilationFiles, OutputFile};

mod target_info;
pub use self::target_info::{FileFlavor, TargetInfo};
Expand All @@ -45,7 +46,7 @@ pub use self::target_info::{FileFlavor, TargetInfo};
/// example, it needs to know the target architecture (OS, chip arch etc.) and it needs to know
/// whether you want a debug or release build. There is enough information in this struct to figure
/// all that out.
#[derive(Clone, Copy, Eq, PartialEq, Hash)]
#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug)]
pub struct Unit<'a> {
/// Information about available targets, which files to include/exclude, etc. Basically stuff in
/// `Cargo.toml`.
Expand All @@ -55,15 +56,18 @@ pub struct Unit<'a> {
/// build.
pub target: &'a Target,
/// The profile contains information about *how* the build should be run, including debug
/// level, extra args to pass to rustc, etc.
pub profile: &'a Profile,
/// level, etc.
pub profile: Profile,
/// Whether this compilation unit is for the host or target architecture.
///
/// For example, when
/// cross compiling and using a custom build script, the build script needs to be compiled for
/// the host architecture so the host rustc can use it (when compiling to the target
/// architecture).
pub kind: Kind,
/// The "mode" this unit is being compiled for. See `CompileMode` for
/// more details.
pub mode: CompileMode,
}

/// The build context, containing all information about a build task
Expand All @@ -87,10 +91,16 @@ pub struct Context<'a, 'cfg: 'a> {
pub links: Links<'a>,
pub used_in_plugin: HashSet<Unit<'a>>,
pub jobserver: Client,
pub profiles: &'a Profiles,
/// This is a workaround to carry the extra compiler args for either
/// `rustc` or `rustdoc` given on the command-line for the commands `cargo
/// rustc` and `cargo rustdoc`. These commands only support one target,
/// but we don't want the args passed to any dependencies, so we include
/// the `Unit` corresponding to the top-level target.
extra_compiler_args: Option<(Unit<'a>, Vec<String>)>,

target_info: TargetInfo,
host_info: TargetInfo,
profiles: &'a Profiles,
incremental_env: Option<bool>,

unit_dependencies: HashMap<Unit<'a>, Vec<Unit<'a>>>,
Expand All @@ -105,6 +115,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
config: &'cfg Config,
build_config: BuildConfig,
profiles: &'a Profiles,
extra_compiler_args: Option<(Unit<'a>, Vec<String>)>,
) -> CargoResult<Context<'a, 'cfg>> {
let incremental_env = match env::var("CARGO_INCREMENTAL") {
Ok(v) => Some(v == "1"),
Expand Down Expand Up @@ -156,6 +167,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {

unit_dependencies: HashMap::new(),
files: None,
extra_compiler_args,
};

cx.compilation.host_dylib_path = cx.host_info.sysroot_libdir.clone();
Expand All @@ -174,8 +186,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
let mut queue = JobQueue::new(&self);
self.prepare_units(export_dir, units)?;
self.prepare()?;
self.build_used_in_plugin_map(&units)?;
custom_build::build_map(&mut self, &units)?;
self.build_used_in_plugin_map(units)?;
custom_build::build_map(&mut self, units)?;

for unit in units.iter() {
// Build up a list of pending jobs, each of which represent
Expand All @@ -200,7 +212,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
None => &output.path,
};

if unit.profile.test {
if unit.mode.is_any_test() && !unit.mode.is_check() {
self.compilation.tests.push((
unit.pkg.clone(),
unit.target.kind().clone(),
Expand All @@ -224,7 +236,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
continue;
}

if dep.profile.run_custom_build {
if dep.mode.is_run_custom_build() {
let out_dir = self.files().build_script_out_dir(dep).display().to_string();
self.compilation
.extra_env
Expand All @@ -236,7 +248,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
if !dep.target.is_lib() {
continue;
}
if dep.profile.doc {
if dep.mode.is_doc() {
continue;
}

Expand Down Expand Up @@ -313,16 +325,10 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
None => None,
};

let deps = build_unit_dependencies(units, &self)?;
let deps = build_unit_dependencies(units, self)?;
self.unit_dependencies = deps;
let files = CompilationFiles::new(
units,
host_layout,
target_layout,
export_dir,
self.ws,
&self,
);
let files =
CompilationFiles::new(units, host_layout, target_layout, export_dir, self.ws, self);
self.files = Some(files);
Ok(())
}
Expand Down Expand Up @@ -414,7 +420,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
// dependencies. However, that code itself calls this method and
// gets a full pre-filtered set of dependencies. This is not super
// obvious, and clear, but it does work at the moment.
if unit.profile.run_custom_build {
if unit.target.is_custom_build() {
let key = (unit.pkg.package_id().clone(), unit.kind);
if self.build_script_overridden.contains(&key) {
return Vec::new();
Expand Down Expand Up @@ -476,25 +482,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
self.build_config.jobs
}

pub fn lib_profile(&self) -> &'a Profile {
let (normal, test) = if self.build_config.release {
(&self.profiles.release, &self.profiles.bench_deps)
} else {
(&self.profiles.dev, &self.profiles.test_deps)
};
if self.build_config.test {
test
} else {
normal
}
}

pub fn build_script_profile(&self, _pkg: &PackageId) -> &'a Profile {
// TODO: should build scripts always be built with the same library
// profile? How is this controlled at the CLI layer?
self.lib_profile()
}

pub fn incremental_args(&self, unit: &Unit) -> CargoResult<Vec<String>> {
// There's a number of ways to configure incremental compilation right
// now. In order of descending priority (first is highest priority) we
Expand Down Expand Up @@ -572,6 +559,15 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
Kind::Target => &self.target_info,
}
}

pub fn extra_args_for(&self, unit: &Unit<'a>) -> Option<&Vec<String>> {
if let Some((ref args_unit, ref args)) = self.extra_compiler_args {
if args_unit == unit {
return Some(args);
}
}
None
}
}

/// Acquire extra flags to pass to the compiler from various locations.
Expand Down
Loading