Auto merge of #8073 - ehuss:hash-channel, r=alexcrichton

Use the same filename hash for pre-release channels.

This changes it so that filenames do not hash the entire verbose version from rustc if they are a pre-release version. The intent is to avoid leaving stale artifacts in the target directory whenever someone updates a nightly or beta release. This should help reduce disk space usage for someone who updates these toolchains frequently.

I tested with the rustc repo, and it seems to be OK. It keeps everything in separate target directories, so I think it should be generally safe. This should only affect someone switching between different nightlies and wanting to avoid recompiling when switching back. I suspect that is a rare use case, though if there are complaints this can be easily reverted (or made a config option). cargo-bisect-rustc should also be safe since it uses a different target directory for each toolchain.

One concern here for me was incremental support. It looks like ([src](https://github.com/rust-lang/rust/blob/6387b09153939b2a104cd63148598a5f458de2c2/src/librustc_incremental/persist/file_format.rs#L88-L100)) the incremental cache includes the detailed rustc version, so I think that is safe. It also looks like it is [smart enough](https://github.com/rust-lang/rust/blob/6387b09153939b2a104cd63148598a5f458de2c2/src/librustc_incremental/persist/load.rs#L40-L49) to delete stale files.

We will need to be more careful in the future when changing the target directory structure or the format of files. We previously relied on the fact that each new nightly will use different filenames. If we change the structure in a backwards-incompatible way, we will need to be careful to update the version (`1.hash` in `compute_metadata`).

Author: bors

src/cargo/core/compiler/context/compilation_files.rs

@@ -614,7 +614,7 @@ fn compute_metadata<'a, 'cfg>(
     unit.target.name().hash(&mut hasher);
     unit.target.kind().hash(&mut hasher);
 
-    bcx.rustc().verbose_version.hash(&mut hasher);
+    hash_rustc_version(bcx, &mut hasher);
 
     if cx.bcx.ws.is_member(unit.pkg) {
         // This is primarily here for clippy. This ensures that the clippy
@@ -641,3 +641,35 @@ fn compute_metadata<'a, 'cfg>(
 
     Some(Metadata(hasher.finish()))
 }
+
+fn hash_rustc_version(bcx: &BuildContext<'_, '_>, hasher: &mut SipHasher) {
+    let vers = &bcx.rustc().version;
+    if vers.pre.is_empty() || bcx.config.cli_unstable().separate_nightlies {
+        // For stable, keep the artifacts separate. This helps if someone is
+        // testing multiple versions, to avoid recompiles.
+        bcx.rustc().verbose_version.hash(hasher);
+        return;
+    }
+    // On "nightly"/"beta"/"dev"/etc, keep each "channel" separate. Don't hash
+    // the date/git information, so that whenever someone updates "nightly",
+    // they won't have a bunch of stale artifacts in the target directory.
+    //
+    // This assumes that the first segment is the important bit ("nightly",
+    // "beta", "dev", etc.). Skip other parts like the `.3` in `-beta.3`.
+    vers.pre[0].hash(hasher);
+    // Keep "host" since some people switch hosts to implicitly change
+    // targets, (like gnu vs musl or gnu vs msvc). In the future, we may want
+    // to consider hashing `unit.kind.short_name()` instead.
+    bcx.rustc().host.hash(hasher);
+    // None of the other lines are important. Currently they are:
+    // binary: rustc  <-- or "rustdoc"
+    // commit-hash: 38114ff16e7856f98b2b4be7ab4cd29b38bed59a
+    // commit-date: 2020-03-21
+    // host: x86_64-apple-darwin
+    // release: 1.44.0-nightly
+    // LLVM version: 9.0
+    //
+    // The backend version ("LLVM version") might become more relevant in
+    // the future when cranelift sees more use, and people want to switch
+    // between different backends without recompiling.
+}

src/cargo/core/features.rs

@@ -343,6 +343,7 @@ pub struct CliUnstable {
     pub jobserver_per_rustc: bool,
     pub features: Option<Vec<String>>,
     pub crate_versions: bool,
+    pub separate_nightlies: bool,
 }
 
 impl CliUnstable {
@@ -420,6 +421,7 @@ impl CliUnstable {
             "jobserver-per-rustc" => self.jobserver_per_rustc = parse_empty(k, v)?,
             "features" => self.features = Some(parse_features(v)),
             "crate-versions" => self.crate_versions = parse_empty(k, v)?,
+            "separate-nightlies" => self.separate_nightlies = parse_empty(k, v)?,
             _ => bail!("unknown `-Z` flag specified: {}", k),
         }
 

src/cargo/util/rustc.rs

@@ -25,6 +25,8 @@ pub struct Rustc {
     pub workspace_wrapper: Option<PathBuf>,
     /// Verbose version information (the output of `rustc -vV`)
     pub verbose_version: String,
+    /// The rustc version (`1.23.4-beta.2`), this comes from verbose_version.
+    pub version: semver::Version,
     /// The host triple (arch-platform-OS), this comes from verbose_version.
     pub host: InternedString,
     cache: Mutex<Cache>,
@@ -51,25 +53,34 @@ impl Rustc {
         cmd.arg("-vV");
         let verbose_version = cache.cached_output(&cmd)?.0;
 
-        let host = {
-            let triple = verbose_version
+        let extract = |field: &str| -> CargoResult<&str> {
+            verbose_version
                 .lines()
-                .find(|l| l.starts_with("host: "))
-                .map(|l| &l[6..])
+                .find(|l| l.starts_with(field))
+                .map(|l| &l[field.len()..])
                 .ok_or_else(|| {
                     anyhow::format_err!(
-                        "`rustc -vV` didn't have a line for `host:`, got:\n{}",
+                        "`rustc -vV` didn't have a line for `{}`, got:\n{}",
+                        field.trim(),
                         verbose_version
                     )
-                })?;
-            InternedString::new(triple)
+                })
         };
 
+        let host = InternedString::new(extract("host: ")?);
+        let version = semver::Version::parse(extract("release: ")?).chain_err(|| {
+            format!(
+                "rustc version does not appear to be a valid semver version, from:\n{}",
+                verbose_version
+            )
+        })?;
+
         Ok(Rustc {
             path,
             wrapper,
             workspace_wrapper,
             verbose_version,
+            version,
             host,
             cache: Mutex::new(cache),
         })

tests/testsuite/freshness.rs

@@ -11,8 +11,7 @@ use std::time::SystemTime;
 
 use cargo_test_support::paths::{self, CargoPathExt};
 use cargo_test_support::registry::Package;
-use cargo_test_support::sleep_ms;
-use cargo_test_support::{basic_manifest, is_coarse_mtime, project};
+use cargo_test_support::{basic_manifest, is_coarse_mtime, project, rustc_host, sleep_ms};
 
 #[cargo_test]
 fn modifying_and_moving() {
@@ -2150,3 +2149,177 @@ fn rerun_if_changes() {
         .with_stderr("[FINISHED] [..]")
         .run();
 }
+
+#[cargo_test]
+fn channel_shares_filenames() {
+    // Test that different "nightly" releases use the same output filename.
+
+    // Create separate rustc binaries to emulate running different toolchains.
+    let nightly1 = format!(
+        "\
+rustc 1.44.0-nightly (38114ff16 2020-03-21)
+binary: rustc
+commit-hash: 38114ff16e7856f98b2b4be7ab4cd29b38bed59a
+commit-date: 2020-03-21
+host: {}
+release: 1.44.0-nightly
+LLVM version: 9.0
+",
+        rustc_host()
+    );
+
+    let nightly2 = format!(
+        "\
+rustc 1.44.0-nightly (a5b09d354 2020-03-31)
+binary: rustc
+commit-hash: a5b09d35473615e7142f5570f5c5fad0caf68bd2
+commit-date: 2020-03-31
+host: {}
+release: 1.44.0-nightly
+LLVM version: 9.0
+",
+        rustc_host()
+    );
+
+    let beta1 = format!(
+        "\
+rustc 1.43.0-beta.3 (4c587bbda 2020-03-25)
+binary: rustc
+commit-hash: 4c587bbda04ab55aaf56feab11dfdfe387a85d7a
+commit-date: 2020-03-25
+host: {}
+release: 1.43.0-beta.3
+LLVM version: 9.0
+",
+        rustc_host()
+    );
+
+    let beta2 = format!(
+        "\
+rustc 1.42.0-beta.5 (4e1c5f0e9 2020-02-28)
+binary: rustc
+commit-hash: 4e1c5f0e9769a588b91c977e3d81e140209ef3a2
+commit-date: 2020-02-28
+host: {}
+release: 1.42.0-beta.5
+LLVM version: 9.0
+",
+        rustc_host()
+    );
+
+    let stable1 = format!(
+        "\
+rustc 1.42.0 (b8cedc004 2020-03-09)
+binary: rustc
+commit-hash: b8cedc00407a4c56a3bda1ed605c6fc166655447
+commit-date: 2020-03-09
+host: {}
+release: 1.42.0
+LLVM version: 9.0
+",
+        rustc_host()
+    );
+
+    let stable2 = format!(
+        "\
+rustc 1.41.1 (f3e1a954d 2020-02-24)
+binary: rustc
+commit-hash: f3e1a954d2ead4e2fc197c7da7d71e6c61bad196
+commit-date: 2020-02-24
+host: {}
+release: 1.41.1
+LLVM version: 9.0
+",
+        rustc_host()
+    );
+
+    let compiler = project()
+        .at("compiler")
+        .file("Cargo.toml", &basic_manifest("compiler", "0.1.0"))
+        .file(
+            "src/main.rs",
+            r#"
+            fn main() {
+                if std::env::args_os().any(|a| a == "-vV") {
+                    print!("{}", env!("FUNKY_VERSION_TEST"));
+                    return;
+                }
+                let mut cmd = std::process::Command::new("rustc");
+                cmd.args(std::env::args_os().skip(1));
+                assert!(cmd.status().unwrap().success());
+            }
+            "#,
+        )
+        .build();
+
+    let makeit = |version, vv| {
+        // Force a rebuild.
+        compiler.target_debug_dir().join("deps").rm_rf();
+        compiler.cargo("build").env("FUNKY_VERSION_TEST", vv).run();
+        fs::rename(compiler.bin("compiler"), compiler.bin(version)).unwrap();
+    };
+    makeit("nightly1", nightly1);
+    makeit("nightly2", nightly2);
+    makeit("beta1", beta1);
+    makeit("beta2", beta2);
+    makeit("stable1", stable1);
+    makeit("stable2", stable2);
+
+    // Run `cargo check` with different rustc versions to observe its behavior.
+    let p = project().file("src/lib.rs", "").build();
+
+    // Runs `cargo check` and returns the rmeta filename created.
+    // Checks that the freshness matches the given value.
+    let check = |version, fresh| -> String {
+        let output = p
+            .cargo("check --message-format=json")
+            .env("RUSTC", compiler.bin(version))
+            .exec_with_output()
+            .unwrap();
+        // Collect the filenames generated.
+        let mut artifacts: Vec<_> = std::str::from_utf8(&output.stdout)
+            .unwrap()
+            .lines()
+            .filter_map(|line| {
+                let value: serde_json::Value = serde_json::from_str(line).unwrap();
+                if value["reason"].as_str().unwrap() == "compiler-artifact" {
+                    assert_eq!(value["fresh"].as_bool().unwrap(), fresh);
+                    let filenames = value["filenames"].as_array().unwrap();
+                    assert_eq!(filenames.len(), 1);
+                    Some(filenames[0].to_string())
+                } else {
+                    None
+                }
+            })
+            .collect();
+        // Should only generate one rmeta file.
+        assert_eq!(artifacts.len(), 1);
+        artifacts.pop().unwrap()
+    };
+
+    let nightly1_name = check("nightly1", false);
+    assert_eq!(check("nightly1", true), nightly1_name);
+    assert_eq!(check("nightly2", false), nightly1_name); // same as before
+    assert_eq!(check("nightly2", true), nightly1_name);
+    // Should rebuild going back to nightly1.
+    assert_eq!(check("nightly1", false), nightly1_name);
+
+    let beta1_name = check("beta1", false);
+    assert_ne!(beta1_name, nightly1_name);
+    assert_eq!(check("beta1", true), beta1_name);
+    assert_eq!(check("beta2", false), beta1_name); // same as before
+    assert_eq!(check("beta2", true), beta1_name);
+    // Should rebuild going back to beta1.
+    assert_eq!(check("beta1", false), beta1_name);
+
+    let stable1_name = check("stable1", false);
+    assert_ne!(stable1_name, nightly1_name);
+    assert_ne!(stable1_name, beta1_name);
+    let stable2_name = check("stable2", false);
+    assert_ne!(stable1_name, stable2_name);
+    // Check everything is fresh.
+    assert_eq!(check("stable1", true), stable1_name);
+    assert_eq!(check("stable2", true), stable2_name);
+    assert_eq!(check("beta1", true), beta1_name);
+    assert_eq!(check("nightly1", true), nightly1_name);
+}