Auto merge of #4817 - alexcrichton:incremental-by-default, r=matklad

Enable incremental by default

This commit enables incremental compilation by default in Cargo for all
dev-related profiles (aka anything without `--release` or `bench`. A
number of new configuration options were also added to tweak how
incremental compilation is exposed and/or used:

* A `profile.dev.incremental` field is added to `Cargo.toml` to disable
  it on a per-project basis (in case of bugs).
* A `build.incremental` field was added in `.cargo/config` to disable
  globally (or enable if we flip this default back off).

Otherwise `CARGO_INCREMENTAL` can still be used to configure one
particular compilation. The global `build.incremental` configuration
cannot currently be used to enable it for the release profile.

.travis.yml

@@ -28,8 +28,7 @@ matrix:
 
     - env: TARGET=x86_64-unknown-linux-gnu
            ALT=i686-unknown-linux-gnu
-      # FIXME(rust-lang/rust#46271) should use just `nightly`
-      rust: nightly-2017-11-20
+      rust: nightly
       install:
         - mdbook --help || cargo install mdbook --force
       script:

src/cargo/core/manifest.rs

@@ -177,6 +177,8 @@ pub struct Profile {
     pub check: bool,
     #[serde(skip_serializing)]
     pub panic: Option<String>,
+    #[serde(skip_serializing)]
+    pub incremental: bool,
 }
 
 #[derive(Default, Clone, Debug, PartialEq, Eq)]
@@ -631,6 +633,7 @@ impl Profile {
             debuginfo: Some(2),
             debug_assertions: true,
             overflow_checks: true,
+            incremental: true,
             ..Profile::default()
         }
     }
@@ -712,6 +715,7 @@ impl Default for Profile {
             run_custom_build: false,
             check: false,
             panic: None,
+            incremental: false,
         }
     }
 }

src/cargo/ops/cargo_rustc/context.rs

@@ -100,7 +100,7 @@ pub struct Context<'a, 'cfg: 'a> {
     target_info: TargetInfo,
     host_info: TargetInfo,
     profiles: &'a Profiles,
-    incremental_enabled: bool,
+    incremental_env: Option<bool>,
 
     /// For each Unit, a list all files produced as a triple of
     ///
@@ -154,24 +154,11 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
             None => None,
         };
 
-        // Enable incremental builds if the user opts in. For now,
-        // this is an environment variable until things stabilize a
-        // bit more.
-        let incremental_enabled = match env::var("CARGO_INCREMENTAL") {
-            Ok(v) => v == "1",
-            Err(_) => false,
+        let incremental_env = match env::var("CARGO_INCREMENTAL") {
+            Ok(v) => Some(v == "1"),
+            Err(_) => None,
         };
 
-        // -Z can only be used on nightly builds; other builds complain loudly.
-        // Since incremental builds only work on nightly anyway, we silently
-        // ignore CARGO_INCREMENTAL on anything but nightly. This allows users
-        // to always have CARGO_INCREMENTAL set without getting unexpected
-        // errors on stable/beta builds.
-        let is_nightly =
-            config.rustc()?.verbose_version.contains("-nightly") ||
-            config.rustc()?.verbose_version.contains("-dev");
-        let incremental_enabled = incremental_enabled && is_nightly;
-
         // Load up the jobserver that we'll use to manage our parallelism. This
         // is the same as the GNU make implementation of a jobserver, and
         // intentionally so! It's hoped that we can interact with GNU make and
@@ -206,7 +193,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
             build_explicit_deps: HashMap::new(),
             links: Links::new(),
             used_in_plugin: HashSet::new(),
-            incremental_enabled: incremental_enabled,
+            incremental_env,
             jobserver: jobserver,
             build_script_overridden: HashSet::new(),
 
@@ -1082,24 +1069,53 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
     }
 
     pub fn incremental_args(&self, unit: &Unit) -> CargoResult<Vec<String>> {
-        if self.incremental_enabled {
-            if unit.pkg.package_id().source_id().is_path() {
-                // Only enable incremental compilation for sources the user can modify.
-                // For things that change infrequently, non-incremental builds yield
-                // better performance.
-                // (see also /~https://github.com/rust-lang/cargo/issues/3972)
-                return Ok(vec![format!("-Zincremental={}",
-                                       self.layout(unit.kind).incremental().display())]);
-            } else if unit.profile.codegen_units.is_none() {
-                // For non-incremental builds we set a higher number of
-                // codegen units so we get faster compiles. It's OK to do
-                // so because the user has already opted into slower
-                // runtime code by setting CARGO_INCREMENTAL.
-                return Ok(vec![format!("-Ccodegen-units={}", ::num_cpus::get())]);
-            }
+        // 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 project.
+        // * 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.config.get_bool("build.incremental")?.map(|c| c.val);
+        let incremental = match (self.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())
         }
 
-        Ok(vec![])
+        let dir = self.layout(unit.kind).incremental().display();
+        Ok(vec![
+            "-C".to_string(),
+            format!("incremental={}", dir),
+        ])
     }
 
     pub fn rustflags_args(&self, unit: &Unit) -> CargoResult<Vec<String>> {

src/cargo/ops/cargo_rustc/fingerprint.rs

@@ -408,7 +408,7 @@ fn calculate<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
     let fingerprint = Arc::new(Fingerprint {
         rustc: util::hash_u64(&cx.config.rustc()?.verbose_version),
         target: util::hash_u64(&unit.target),
-        profile: util::hash_u64(&unit.profile),
+        profile: util::hash_u64(&(&unit.profile, cx.incremental_args(unit)?)),
         // Note that .0 is hashed here, not .1 which is the cwd. That doesn't
         // actually affect the output artifact so there's no need to hash it.
         path: util::hash_u64(&super::path_args(cx, unit).0),

src/cargo/ops/cargo_rustc/mod.rs

@@ -348,7 +348,6 @@ fn rustc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
     }.with_extension("d");
     let dep_info_loc = fingerprint::dep_info_loc(cx, unit);
 
-    rustc.args(&cx.incremental_args(unit)?);
     rustc.args(&cx.rustflags_args(unit)?);
     let json_messages = cx.build_config.json_messages;
     let package_id = unit.pkg.package_id().clone();
@@ -651,7 +650,7 @@ fn prepare_rustc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
                            unit: &Unit<'a>) -> CargoResult<ProcessBuilder> {
     let mut base = cx.compilation.rustc_process(unit.pkg)?;
     base.inherit_jobserver(&cx.jobserver);
-    build_base_args(cx, &mut base, unit, crate_types);
+    build_base_args(cx, &mut base, unit, crate_types)?;
     build_deps_args(&mut base, cx, unit)?;
     Ok(base)
 }
@@ -743,11 +742,11 @@ fn add_path_args(cx: &Context, unit: &Unit, cmd: &mut ProcessBuilder) {
 fn build_base_args<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
                              cmd: &mut ProcessBuilder,
                              unit: &Unit<'a>,
-                             crate_types: &[&str]) {
+                             crate_types: &[&str]) -> CargoResult<()> {
     let Profile {
         ref opt_level, lto, codegen_units, ref rustc_args, debuginfo,
         debug_assertions, overflow_checks, rpath, test, doc: _doc,
-        run_custom_build, ref panic, rustdoc_args: _, check,
+        run_custom_build, ref panic, rustdoc_args: _, check, incremental: _,
     } = *unit.profile;
     assert!(!run_custom_build);
 
@@ -888,6 +887,9 @@ fn build_base_args<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
 
     opt(cmd, "-C", "ar=", cx.ar(unit.kind).map(|s| s.as_ref()));
     opt(cmd, "-C", "linker=", cx.linker(unit.kind).map(|s| s.as_ref()));
+    cmd.args(&cx.incremental_args(unit)?);
+
+    Ok(())
 }
 
 

src/cargo/util/toml/mod.rs

@@ -338,6 +338,7 @@ pub struct TomlProfile {
     panic: Option<String>,
     #[serde(rename = "overflow-checks")]
     overflow_checks: Option<bool>,
+    incremental: Option<bool>,
 }
 
 #[derive(Clone, Debug, Serialize)]
@@ -1123,7 +1124,7 @@ fn build_profiles(profiles: &Option<TomlProfiles>) -> Profiles {
     fn merge(profile: Profile, toml: Option<&TomlProfile>) -> Profile {
         let &TomlProfile {
             ref opt_level, lto, codegen_units, ref debug, debug_assertions, rpath,
-            ref panic, ref overflow_checks,
+            ref panic, ref overflow_checks, ref incremental,
         } = match toml {
             Some(toml) => toml,
             None => return profile,
@@ -1149,6 +1150,7 @@ fn build_profiles(profiles: &Option<TomlProfiles>) -> Profiles {
             run_custom_build: profile.run_custom_build,
             check: profile.check,
             panic: panic.clone().or(profile.panic),
+            incremental: incremental.unwrap_or(profile.incremental),
         }
     }
 }

src/doc/config.md

@@ -102,6 +102,7 @@ rustdoc = "rustdoc"       # the doc generator tool
 target = "triple"         # build for the target triple
 target-dir = "target"     # path of where to place all generated artifacts
 rustflags = ["..", ".."]  # custom flags to pass to all compiler invocations
+incremental = true        # whether or not to enable incremental compilation
 
 [term]
 verbose = false        # whether cargo provides verbose output

src/doc/environment-variables.md

@@ -28,6 +28,10 @@ system:
 * `RUSTFLAGS` - A space-separated list of custom flags to pass to all compiler
   invocations that Cargo performs. In contrast with `cargo rustc`, this is
   useful for passing a flag to *all* compiler instances.
+* `CARGO_INCREMENTAL` - If this is set to 1 then Cargo will force incremental
+  compilation to be enabled for the current compilation, and when set to 0 it
+  will force disabling it. If this env var isn't present then Cargo's defaults
+  will otherwise be used.
 
 Note that Cargo will also read environment variables for `.cargo/config`
 configuration values, as described in [that documentation][config-env]

src/doc/manifest.md

@@ -192,7 +192,7 @@ license-file = "..."
 
 # Appveyor: `repository` is required. `branch` is optional; default is `master`
 # `service` is optional; valid values are `github` (default), `bitbucket`, and
-# `gitlab`; `id` is optional; you can specify the appveyor project id if you 
+# `gitlab`; `id` is optional; you can specify the appveyor project id if you
 # want to use that instead. `project_name` is optional; use when the repository
 # name differs from the appveyor project name.
 appveyor = { repository = "...", branch = "master", service = "github" }
@@ -289,6 +289,7 @@ codegen-units = 1  # if > 1 enables parallel code generation which improves
                    # compile times, but prevents some optimizations.
                    # Passes `-C codegen-units`. Ignored when `lto = true`.
 panic = 'unwind'   # panic strategy (`-C panic=...`), can also be 'abort'
+incremental = true # whether or not incremental compilation is enabled
 
 # The release profile, used for `cargo build --release`.
 [profile.release]
@@ -299,6 +300,7 @@ lto = false
 debug-assertions = false
 codegen-units = 1
 panic = 'unwind'
+incremental = false
 
 # The testing profile, used for `cargo test`.
 [profile.test]
@@ -309,6 +311,7 @@ lto = false
 debug-assertions = true
 codegen-units = 1
 panic = 'unwind'
+incremental = true
 
 # The benchmarking profile, used for `cargo bench` and `cargo test --release`.
 [profile.bench]
@@ -319,6 +322,7 @@ lto = false
 debug-assertions = false
 codegen-units = 1
 panic = 'unwind'
+incremental = false
 
 # The documentation profile, used for `cargo doc`.
 [profile.doc]
@@ -329,6 +333,7 @@ lto = false
 debug-assertions = true
 codegen-units = 1
 panic = 'unwind'
+incremental = true
 ```
 
 # The `[features]` section

tests/build.rs

@@ -58,16 +58,90 @@ fn cargo_compile_incremental() {
     assert_that(
         p.cargo("build").arg("-v").env("CARGO_INCREMENTAL", "1"),
         execs().with_stderr_contains(
-            "[RUNNING] `rustc [..] -Zincremental=[..][/]target[/]debug[/]incremental`\n")
+            "[RUNNING] `rustc [..] -C incremental=[..][/]target[/]debug[/]incremental[..]`\n")
             .with_status(0));
 
     assert_that(
         p.cargo("test").arg("-v").env("CARGO_INCREMENTAL", "1"),
         execs().with_stderr_contains(
-            "[RUNNING] `rustc [..] -Zincremental=[..][/]target[/]debug[/]incremental`\n")
+            "[RUNNING] `rustc [..] -C incremental=[..][/]target[/]debug[/]incremental[..]`\n")
                .with_status(0));
 }
 
+#[test]
+fn incremental_profile() {
+    if !is_nightly() {
+        return
+    }
+
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+
+            [profile.dev]
+            incremental = false
+
+            [profile.release]
+            incremental = true
+        "#)
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    assert_that(
+        p.cargo("build").arg("-v").env_remove("CARGO_INCREMENTAL"),
+        execs().with_stderr_does_not_contain("[..]C incremental=[..]")
+            .with_status(0));
+
+    assert_that(
+        p.cargo("build").arg("-v").env("CARGO_INCREMENTAL", "1"),
+        execs().with_stderr_contains("[..]C incremental=[..]")
+            .with_status(0));
+
+    assert_that(
+        p.cargo("build").arg("--release").arg("-v").env_remove("CARGO_INCREMENTAL"),
+        execs().with_stderr_contains("[..]C incremental=[..]")
+            .with_status(0));
+
+    assert_that(
+        p.cargo("build").arg("--release").arg("-v").env("CARGO_INCREMENTAL", "0"),
+        execs().with_stderr_does_not_contain("[..]C incremental=[..]")
+            .with_status(0));
+}
+
+#[test]
+fn incremental_config() {
+    if !is_nightly() {
+        return
+    }
+
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("src/main.rs", "fn main() {}")
+        .file(".cargo/config", r#"
+            [build]
+            incremental = false
+        "#)
+        .build();
+
+    assert_that(
+        p.cargo("build").arg("-v").env_remove("CARGO_INCREMENTAL"),
+        execs().with_stderr_does_not_contain("[..]C incremental=[..]")
+            .with_status(0));
+
+    assert_that(
+        p.cargo("build").arg("-v").env("CARGO_INCREMENTAL", "1"),
+        execs().with_stderr_contains("[..]C incremental=[..]")
+            .with_status(0));
+}
+
 #[test]
 fn cargo_compile_manifest_path() {
     let p = project("foo")