Skip to content

Commit

Permalink
Incremental profile cleanup.
Browse files Browse the repository at this point in the history
  • Loading branch information
ehuss committed Feb 20, 2019
1 parent dc83ead commit 2b6fd6f
Show file tree
Hide file tree
Showing 12 changed files with 89 additions and 104 deletions.
7 changes: 0 additions & 7 deletions src/cargo/core/compiler/build_context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ pub struct BuildContext<'a, 'cfg: 'a> {
pub target_config: TargetConfig,
pub target_info: TargetInfo,
pub host_info: TargetInfo,
pub incremental_env: Option<bool>,
}

impl<'a, 'cfg> BuildContext<'a, 'cfg> {
Expand All @@ -51,11 +50,6 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
profiles: &'a Profiles,
extra_compiler_args: HashMap<Unit<'a>, Vec<String>>,
) -> CargoResult<BuildContext<'a, 'cfg>> {
let incremental_env = match env::var("CARGO_INCREMENTAL") {
Ok(v) => Some(v == "1"),
Err(_) => None,
};

let rustc = config.rustc(Some(ws))?;
let host_config = TargetConfig::new(config, &rustc.host)?;
let target_config = match build_config.requested_target.as_ref() {
Expand Down Expand Up @@ -84,7 +78,6 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
host_info,
build_config,
profiles,
incremental_env,
extra_compiler_args,
})
}
Expand Down
55 changes: 0 additions & 55 deletions src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,61 +386,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
deps
}

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
// have:
//
// * `CARGO_INCREMENTAL` - this is blanket used unconditionally to turn
// on/off incremental compilation for any cargo subcommand. We'll
// respect this if set.
// * `build.incremental` - in `.cargo/config` this blanket key can
// globally for a system configure whether incremental compilation is
// enabled. Note that setting this to `true` will not actually affect
// all builds though. For example a `true` value doesn't enable
// release incremental builds, only dev incremental builds. This can
// be useful to globally disable incremental compilation like
// `CARGO_INCREMENTAL`.
// * `profile.dev.incremental` - in `Cargo.toml` specific profiles can
// be configured to enable/disable incremental compilation. This can
// be primarily used to disable incremental when buggy for a package.
// * Finally, each profile has a default for whether it will enable
// incremental compilation or not. Primarily development profiles
// have it enabled by default while release profiles have it disabled
// by default.
let global_cfg = self
.bcx
.config
.get_bool("build.incremental")?
.map(|c| c.val);
let incremental = match (
self.bcx.incremental_env,
global_cfg,
unit.profile.incremental,
) {
(Some(v), _, _) => v,
(None, Some(false), _) => false,
(None, _, other) => other,
};

if !incremental {
return Ok(Vec::new());
}

// Only enable incremental compilation for sources the user can
// modify (aka path sources). For things that change infrequently,
// non-incremental builds yield better performance in the compiler
// itself (aka crates.io / git dependencies)
//
// (see also https://github.com/rust-lang/cargo/issues/3972)
if !unit.pkg.package_id().source_id().is_path() {
return Ok(Vec::new());
}

let dir = self.files().layout(unit.kind).incremental().display();
Ok(vec!["-C".to_string(), format!("incremental={}", dir)])
}

pub fn is_primary_package(&self, unit: &Unit<'a>) -> bool {
self.primary_packages.contains(&unit.pkg.package_id())
}
Expand Down
7 changes: 1 addition & 6 deletions src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,12 +492,7 @@ fn calculate<'a, 'cfg>(
} else {
bcx.rustflags_args(unit)?
};
let profile_hash = util::hash_u64(&(
&unit.profile,
unit.mode,
bcx.extra_args_for(unit),
cx.incremental_args(unit)?,
));
let profile_hash = util::hash_u64(&(&unit.profile, unit.mode, bcx.extra_args_for(unit)));
let fingerprint = Arc::new(Fingerprint {
rustc: util::hash_u64(&bcx.rustc.verbose_version),
target: util::hash_u64(&unit.target),
Expand Down
7 changes: 5 additions & 2 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ fn build_base_args<'a, 'cfg>(
overflow_checks,
rpath,
ref panic,
incremental,
..
} = unit.profile;
let test = unit.mode.is_any_test();
Expand Down Expand Up @@ -902,8 +903,10 @@ fn build_base_args<'a, 'cfg>(
"linker=",
bcx.linker(unit.kind).map(|s| s.as_ref()),
);
cmd.args(&cx.incremental_args(unit)?);

if incremental {
let dir = cx.files().layout(unit.kind).incremental().as_os_str();
opt(cmd, "-C", "incremental=", Some(dir));
}
Ok(())
}

Expand Down
7 changes: 7 additions & 0 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ pub struct VirtualManifest {
workspace: WorkspaceConfig,
profiles: Profiles,
warnings: Warnings,
features: Features,
}

/// General metadata about a package which is just blindly uploaded to the
Expand Down Expand Up @@ -541,13 +542,15 @@ impl VirtualManifest {
patch: HashMap<Url, Vec<Dependency>>,
workspace: WorkspaceConfig,
profiles: Profiles,
features: Features,
) -> VirtualManifest {
VirtualManifest {
replace,
patch,
workspace,
profiles,
warnings: Warnings::new(),
features,
}
}

Expand All @@ -574,6 +577,10 @@ impl VirtualManifest {
pub fn warnings(&self) -> &Warnings {
&self.warnings
}

pub fn features(&self) -> &Features {
&self.features
}
}

impl Target {
Expand Down
30 changes: 27 additions & 3 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::collections::HashSet;
use std::{cmp, fmt, hash};
use std::{cmp, env, fmt, hash};

use serde::Deserialize;

Expand All @@ -19,6 +19,10 @@ pub struct Profiles {
test: ProfileMaker,
bench: ProfileMaker,
doc: ProfileMaker,
/// Incremental compilation can be overridden globally via:
/// - `CARGO_INCREMENTAL` environment variable.
/// - `build.incremental` config value.
incremental: Option<bool>,
}

impl Profiles {
Expand All @@ -33,7 +37,11 @@ impl Profiles {
}

let config_profiles = config.profiles()?;
config_profiles.validate(features, warnings)?;

let incremental = match env::var_os("CARGO_INCREMENTAL") {
Some(v) => Some(v == "1"),
None => config.get::<Option<bool>>("build.incremental")?,
};

Ok(Profiles {
dev: ProfileMaker {
Expand Down Expand Up @@ -61,6 +69,7 @@ impl Profiles {
toml: profiles.and_then(|p| p.doc.clone()),
config: None,
},
incremental,
})
}

Expand Down Expand Up @@ -100,10 +109,25 @@ impl Profiles {
CompileMode::Doc { .. } => &self.doc,
};
let mut profile = maker.get_profile(Some(pkg_id), is_member, unit_for);
// `panic` should not be set for tests/benches, or any of their dependencies.
// `panic` should not be set for tests/benches, or any of their
// dependencies.
if !unit_for.is_panic_ok() || mode.is_any_test() {
profile.panic = None;
}

// Incremental can be globally overridden.
if let Some(v) = self.incremental {
profile.incremental = v;
}
// Only enable incremental compilation for sources the user can
// modify (aka path sources). For things that change infrequently,
// non-incremental builds yield better performance in the compiler
// itself (aka crates.io / git dependencies)
//
// (see also https://github.com/rust-lang/cargo/issues/3972)
if !pkg_id.source_id().is_path() {
profile.incremental = false;
}
profile
}

Expand Down
51 changes: 30 additions & 21 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,13 +237,9 @@ impl<'cfg> Workspace<'cfg> {
}

pub fn profiles(&self) -> &Profiles {
let root = self
.root_manifest
.as_ref()
.unwrap_or(&self.current_manifest);
match *self.packages.get(root) {
MaybePackage::Package(ref p) => p.manifest().profiles(),
MaybePackage::Virtual(ref vm) => vm.profiles(),
match self.root_maybe() {
MaybePackage::Package(p) => p.manifest().profiles(),
MaybePackage::Virtual(vm) => vm.profiles(),
}
}

Expand All @@ -260,6 +256,15 @@ impl<'cfg> Workspace<'cfg> {
.unwrap()
}

/// Returns the root Package or VirtualManifest.
fn root_maybe(&self) -> &MaybePackage {
let root = self
.root_manifest
.as_ref()
.unwrap_or(&self.current_manifest);
self.packages.get(root)
}

pub fn target_dir(&self) -> Filesystem {
self.target_dir
.clone()
Expand All @@ -270,27 +275,19 @@ impl<'cfg> Workspace<'cfg> {
///
/// This may be from a virtual crate or an actual crate.
pub fn root_replace(&self) -> &[(PackageIdSpec, Dependency)] {
let path = match self.root_manifest {
Some(ref p) => p,
None => &self.current_manifest,
};
match *self.packages.get(path) {
MaybePackage::Package(ref p) => p.manifest().replace(),
MaybePackage::Virtual(ref vm) => vm.replace(),
match self.root_maybe() {
MaybePackage::Package(p) => p.manifest().replace(),
MaybePackage::Virtual(vm) => vm.replace(),
}
}

/// Returns the root [patch] section of this workspace.
///
/// This may be from a virtual crate or an actual crate.
pub fn root_patch(&self) -> &HashMap<Url, Vec<Dependency>> {
let path = match self.root_manifest {
Some(ref p) => p,
None => &self.current_manifest,
};
match *self.packages.get(path) {
MaybePackage::Package(ref p) => p.manifest().patch(),
MaybePackage::Virtual(ref vm) => vm.patch(),
match self.root_maybe() {
MaybePackage::Package(p) => p.manifest().patch(),
MaybePackage::Virtual(vm) => vm.patch(),
}
}

Expand Down Expand Up @@ -524,6 +521,18 @@ impl<'cfg> Workspace<'cfg> {
/// 2. All workspace members agree on this one root as the root.
/// 3. The current crate is a member of this workspace.
fn validate(&mut self) -> CargoResult<()> {
// Validate config profiles only once per workspace.
let features = match self.root_maybe() {
MaybePackage::Package(p) => p.manifest().features(),
MaybePackage::Virtual(vm) => vm.features(),
};
let mut warnings = Vec::new();
self.config.profiles()?.validate(features, &mut warnings)?;
for warning in warnings {
self.config.shell().warn(&warning)?;
}

// The rest of the checks require a VirtualManifest or multiple members.
if self.root_manifest.is_none() {
return Ok(());
}
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1137,7 +1137,7 @@ impl TomlManifest {
}
};
Ok((
VirtualManifest::new(replace, patch, workspace_config, profiles),
VirtualManifest::new(replace, patch, workspace_config, profiles, features),
nested_paths,
))
}
Expand Down
2 changes: 2 additions & 0 deletions src/doc/src/reference/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ target-dir = "target" # path of where to place all generated artifacts
rustflags = ["..", ".."] # custom flags to pass to all compiler invocations
rustdocflags = ["..", ".."] # custom flags to pass to rustdoc
incremental = true # whether or not to enable incremental compilation
# If `incremental` is not set, then the value from
# the profile is used.
dep-info-basedir = ".." # full path for the base directory for targets in depfiles

[term]
Expand Down
3 changes: 3 additions & 0 deletions src/doc/src/reference/manifest.md
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,9 @@ codegen-units = 16 # if > 1 enables parallel code generation which improves
# Passes `-C codegen-units`.
panic = 'unwind' # panic strategy (`-C panic=...`), can also be 'abort'
incremental = true # whether or not incremental compilation is enabled
# This can be overridden globally with the CARGO_INCREMENTAL
# environment variable or `build.incremental` config
# variable. Incremental is only used for path sources.
overflow-checks = true # use overflow checks for integer arithmetic.
# Passes the `-C overflow-checks=...` flag to the compiler.

Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn cargo_fail_with_no_stderr() {
}

/// Checks that the `CARGO_INCREMENTAL` environment variable results in
/// `rustc` getting `-Zincremental` passed to it.
/// `rustc` getting `-C incremental` passed to it.
#[test]
fn cargo_compile_incremental() {
let p = project()
Expand Down
Loading

0 comments on commit 2b6fd6f

Please sign in to comment.