Auto merge of #13467 - ehuss:global-tracker-old-cargo, r=weihanglo

Add global_cache_tracker stability tests.

This adds some tests to ensure that the database used in the global cache tracker stays compatible across versions. These tests work by using rustup to run both the current cargo and the stable cargo, and verifying that when switching versions, everything works as expected.

These tests will be ignored on developer environments if they don't have rustup, or don't have the stable toolchain installed. It does assume that "stable" is at least 1.76. It is required for the tests to run in CI, but will be disabled in rust-lang/rust since it does not have rustup.

I'm not expecting too much trouble with these tests, but if they become too fiddly or broken, they can always be changed or removed.

The support code for running "cargo +stable" is very basic right now. If we expand to add similar tests in the future, then I think we could consider adding support functions (such as [`tc_process`](https://github.com/rust-lang/cargo/blob/64ccff290fe20e2aa7c04b9c71460a7fd962ea61/tests/testsuite/old_cargos.rs#L21-L36)) to make it easier or more reliable.

Author: bors

.github/workflows/main.yml

@@ -116,6 +116,8 @@ jobs:
       CARGO_PROFILE_TEST_DEBUG: 1
       CARGO_INCREMENTAL: 0
       CARGO_PUBLIC_NETWORK_TESTS: 1
+      # Workaround for https://github.com/rust-lang/rustup/issues/3036
+      RUSTUP_WINDOWS_PATH_ADD_BIN: 0
     strategy:
       matrix:
         include:
@@ -156,6 +158,10 @@ jobs:
     - uses: actions/checkout@v4
     - name: Dump Environment
       run: ci/dump-environment.sh
+    # Some tests require stable. Make sure it is set to the most recent stable
+    # so that we can predictably handle updates if necessary (and not randomly
+    # when GitHub updates its image).
+    - run: rustup update --no-self-update stable
     - run: rustup update --no-self-update ${{ matrix.rust }} && rustup default ${{ matrix.rust }}
     - run: rustup target add ${{ matrix.other }}
     - run: rustup component add rustc-dev llvm-tools-preview rust-docs
@@ -166,7 +172,6 @@ jobs:
     - name: Configure extra test environment
       run: echo CARGO_CONTAINER_TESTS=1 >> $GITHUB_ENV
       if: matrix.os == 'ubuntu-latest'
-
     - run: cargo test -p cargo
     - name: Clear intermediate test output
       run: ci/clean-test-output.sh

crates/cargo-test-macro/src/lib.rs

@@ -1,4 +1,5 @@
 use proc_macro::*;
+use std::path::Path;
 use std::process::Command;
 use std::sync::Once;
 
@@ -74,6 +75,12 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream {
                 requires_reason = true;
                 set_ignore!(is_not_nightly, "requires nightly");
             }
+            "requires_rustup_stable" => {
+                set_ignore!(
+                    !has_rustup_stable(),
+                    "rustup or stable toolchain not installed"
+                );
+            }
             s if s.starts_with("requires_") => {
                 let command = &s[9..];
                 set_ignore!(!has_command(command), "{command} not installed");
@@ -204,29 +211,28 @@ fn version() -> (u32, bool) {
     unsafe { VERSION }
 }
 
-fn has_command(command: &str) -> bool {
-    let output = match Command::new(command).arg("--version").output() {
+fn check_command(command_path: &Path, args: &[&str]) -> bool {
+    let mut command = Command::new(command_path);
+    let command_name = command.get_program().to_str().unwrap().to_owned();
+    command.args(args);
+    let output = match command.output() {
         Ok(output) => output,
         Err(e) => {
             // * hg is not installed on GitHub macOS or certain constrained
             //   environments like Docker. Consider installing it if Cargo
             //   gains more hg support, but otherwise it isn't critical.
             // * lldb is not pre-installed on Ubuntu and Windows, so skip.
-            if is_ci() && !["hg", "lldb"].contains(&command) {
-                panic!(
-                    "expected command `{}` to be somewhere in PATH: {}",
-                    command, e
-                );
+            if is_ci() && !matches!(command_name.as_str(), "hg" | "lldb") {
+                panic!("expected command `{command_name}` to be somewhere in PATH: {e}",);
             }
             return false;
         }
     };
     if !output.status.success() {
         panic!(
-            "expected command `{}` to be runnable, got error {}:\n\
+            "expected command `{command_name}` to be runnable, got error {}:\n\
             stderr:{}\n\
             stdout:{}\n",
-            command,
             output.status,
             String::from_utf8_lossy(&output.stderr),
             String::from_utf8_lossy(&output.stdout)
@@ -235,6 +241,34 @@ fn has_command(command: &str) -> bool {
     true
 }
 
+fn has_command(command: &str) -> bool {
+    check_command(Path::new(command), &["--version"])
+}
+
+fn has_rustup_stable() -> bool {
+    if option_env!("CARGO_TEST_DISABLE_NIGHTLY").is_some() {
+        // This cannot run on rust-lang/rust CI due to the lack of rustup.
+        return false;
+    }
+    if cfg!(windows) && !is_ci() && option_env!("RUSTUP_WINDOWS_PATH_ADD_BIN").is_none() {
+        // There is an issue with rustup that doesn't allow recursive cargo
+        // invocations. Disable this on developer machines if the environment
+        // variable is not enabled. This can be removed once
+        // https://github.com/rust-lang/rustup/issues/3036 is resolved.
+        return false;
+    }
+    // Cargo mucks with PATH on Windows, adding sysroot host libdir, which is
+    // "bin", which circumvents the rustup wrapper. Use the path directly from
+    // CARGO_HOME.
+    let home = match option_env!("CARGO_HOME") {
+        Some(home) => home,
+        None if is_ci() => panic!("expected to run under rustup"),
+        None => return false,
+    };
+    let cargo = Path::new(home).join("bin/cargo");
+    check_command(&cargo, &["+stable", "--version"])
+}
+
 /// Whether or not this running in a Continuous Integration environment.
 fn is_ci() -> bool {
     // Consider using `tracked_env` instead of option_env! when it is stabilized.

crates/cargo-test-support/src/lib.rs

@@ -764,6 +764,13 @@ impl Execs {
         self
     }
 
+    pub fn args<T: AsRef<OsStr>>(&mut self, args: &[T]) -> &mut Self {
+        if let Some(ref mut p) = self.process_builder {
+            p.args(args);
+        }
+        self
+    }
+
     pub fn cwd<T: AsRef<OsStr>>(&mut self, path: T) -> &mut Self {
         if let Some(ref mut p) = self.process_builder {
             if let Some(cwd) = p.get_cwd() {

src/cargo/core/global_cache_tracker.rs

@@ -499,7 +499,7 @@ impl GlobalCacheTracker {
         let mut stmt = self.conn.prepare_cached(
             "SELECT git_db.name, git_checkout.name, git_checkout.size, git_checkout.timestamp
              FROM git_db, git_checkout
-             WHERE git_checkout.registry_id = git_db.id",
+             WHERE git_checkout.git_id = git_db.id",
         )?;
         let rows = stmt
             .query_map([], |row| {

tests/testsuite/global_cache_tracker.rs

@@ -14,11 +14,12 @@ use cargo::GlobalContext;
 use cargo_test_support::paths::{self, CargoPathExt};
 use cargo_test_support::registry::{Package, RegistryBuilder};
 use cargo_test_support::{
-    basic_manifest, cargo_process, execs, git, project, retry, sleep_ms, thread_wait_timeout,
-    Project,
+    basic_manifest, cargo_process, execs, git, process, project, retry, sleep_ms,
+    thread_wait_timeout, Execs, Project,
 };
 use itertools::Itertools;
 use std::fmt::Write;
+use std::path::Path;
 use std::path::PathBuf;
 use std::process::Stdio;
 use std::time::{Duration, SystemTime};
@@ -153,6 +154,14 @@ fn populate_cache(
     (cache_dir, src_dir)
 }
 
+fn rustup_cargo() -> Execs {
+    // Get the path to the rustup cargo wrapper. This is necessary because
+    // cargo adds the "deps" directory into PATH on Windows, which points to
+    // the wrong cargo.
+    let rustup_cargo = Path::new(&std::env::var_os("CARGO_HOME").unwrap()).join("bin/cargo");
+    execs().with_process_builder(process(rustup_cargo))
+}
+
 #[cargo_test]
 fn auto_gc_gated() {
     // Requires -Zgc to both track last-use data and to run auto-gc.
@@ -1863,3 +1872,150 @@ fn clean_gc_quiet_is_quiet() {
         )
         .run();
 }
+
+#[cargo_test(requires_rustup_stable)]
+fn compatible_with_older_cargo() {
+    // Ensures that db stays backwards compatible across versions.
+
+    // T-4 months: Current version, build the database.
+    Package::new("old", "1.0.0").publish();
+    Package::new("middle", "1.0.0").publish();
+    Package::new("new", "1.0.0").publish();
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.1.0"
+
+                [dependencies]
+                old = "1.0"
+                middle = "1.0"
+                new = "1.0"
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+    // Populate the last-use data.
+    p.cargo("check -Zgc")
+        .masquerade_as_nightly_cargo(&["gc"])
+        .env("__CARGO_TEST_LAST_USE_NOW", months_ago_unix(4))
+        .run();
+    assert_eq!(
+        get_registry_names("src"),
+        ["middle-1.0.0", "new-1.0.0", "old-1.0.0"]
+    );
+    assert_eq!(
+        get_registry_names("cache"),
+        ["middle-1.0.0.crate", "new-1.0.0.crate", "old-1.0.0.crate"]
+    );
+
+    // T-2 months: Stable version, make sure it reads and deletes old src.
+    p.change_file(
+        "Cargo.toml",
+        r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+
+            [dependencies]
+            new = "1.0"
+            middle = "1.0"
+        "#,
+    );
+    rustup_cargo()
+        .args(&["+stable", "check", "-Zgc"])
+        .cwd(p.root())
+        .masquerade_as_nightly_cargo(&["gc"])
+        .env("__CARGO_TEST_LAST_USE_NOW", months_ago_unix(2))
+        .run();
+    assert_eq!(get_registry_names("src"), ["middle-1.0.0", "new-1.0.0"]);
+    assert_eq!(
+        get_registry_names("cache"),
+        ["middle-1.0.0.crate", "new-1.0.0.crate", "old-1.0.0.crate"]
+    );
+
+    // T-0 months: Current version, make sure it can read data from stable,
+    // deletes old crate and middle src.
+    p.change_file(
+        "Cargo.toml",
+        r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+
+            [dependencies]
+            new = "1.0"
+        "#,
+    );
+    p.cargo("check -Zgc")
+        .masquerade_as_nightly_cargo(&["gc"])
+        .run();
+    assert_eq!(get_registry_names("src"), ["new-1.0.0"]);
+    assert_eq!(
+        get_registry_names("cache"),
+        ["middle-1.0.0.crate", "new-1.0.0.crate"]
+    );
+}
+
+#[cargo_test(requires_rustup_stable)]
+fn forward_compatible() {
+    // Checks that db created in an older version can be read in a newer version.
+    Package::new("bar", "1.0.0").publish();
+    let git_project = git::new("from_git", |p| {
+        p.file("Cargo.toml", &basic_manifest("from_git", "1.0.0"))
+            .file("src/lib.rs", "")
+    });
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            &format!(
+                r#"
+                    [package]
+                    name = "foo"
+
+                    [dependencies]
+                    bar = "1.0.0"
+                    from_git = {{ git = '{}' }}
+                "#,
+                git_project.url()
+            ),
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    rustup_cargo()
+        .args(&["+stable", "check", "-Zgc"])
+        .cwd(p.root())
+        .masquerade_as_nightly_cargo(&["gc"])
+        .run();
+
+    let config = GlobalContextBuilder::new().unstable_flag("gc").build();
+    let lock = config
+        .acquire_package_cache_lock(CacheLockMode::MutateExclusive)
+        .unwrap();
+    let tracker = GlobalCacheTracker::new(&config).unwrap();
+    // Don't want to check the actual index name here, since although the
+    // names are semi-stable, they might change over long periods of time.
+    let indexes = tracker.registry_index_all().unwrap();
+    assert_eq!(indexes.len(), 1);
+    let crates = tracker.registry_crate_all().unwrap();
+    let names: Vec<_> = crates
+        .iter()
+        .map(|(krate, _timestamp)| krate.crate_filename)
+        .collect();
+    assert_eq!(names, &["bar-1.0.0.crate"]);
+    let srcs = tracker.registry_src_all().unwrap();
+    let names: Vec<_> = srcs
+        .iter()
+        .map(|(src, _timestamp)| src.package_dir)
+        .collect();
+    assert_eq!(names, &["bar-1.0.0"]);
+    let dbs: Vec<_> = tracker.git_db_all().unwrap();
+    assert_eq!(dbs.len(), 1);
+    let cos: Vec<_> = tracker.git_checkout_all().unwrap();
+    assert_eq!(cos.len(), 1);
+    drop(lock);
+}