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

Ignore incremental when lto is set. #6643

Closed
wants to merge 1 commit into from
Closed
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
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 @@ -763,6 +763,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 @@ -905,8 +906,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 @@ -539,13 +540,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 @@ -572,6 +575,10 @@ impl VirtualManifest {
pub fn warnings(&self) -> &Warnings {
&self.warnings
}

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

impl Target {
Expand Down
32 changes: 30 additions & 2 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,11 +109,30 @@ 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.
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;
}
// Incremental is incompatible with LTO.
if profile.lto != Lto::Bool(false) {
profile.incremental = false;
}
// 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()
}

/// Return 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
12 changes: 11 additions & 1 deletion src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,16 @@ impl TomlProfile {
}
_ => {}
}

if self.incremental == Some(true)
&& self.lto != None
&& self.lto != Some(StringOrBool::Bool(false))
{
warnings.push(format!(
"`incremental` setting is ignored for the `{}` profile when `lto` is enabled",
name
));
}
Ok(())
}

Expand Down Expand Up @@ -1144,7 +1154,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 @@ -123,6 +123,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
4 changes: 4 additions & 0 deletions src/doc/src/reference/manifest.md
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,10 @@ 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 CARGO_INCREMENTAL
# environment variable or `build.incremental` config
# variable. Incremental cannot be used with `lto`.
# 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
Loading