Auto merge of #13657 - weihanglo:package-cache-lock, r=epage

fix(generate-lockfile): hold lock before querying index

Author: bors

src/cargo/ops/cargo_generate_lockfile.rs

@@ -166,12 +166,17 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
     Ok(())
 }
 
+/// Prints lockfile change statuses.
+///
+/// This would acquire the package-cache lock, as it may update the index to
+/// show users latest available versions.
 pub fn print_lockfile_changes(
     gctx: &GlobalContext,
     previous_resolve: Option<&Resolve>,
     resolve: &Resolve,
     registry: &mut PackageRegistry<'_>,
 ) -> CargoResult<()> {
+    let _lock = gctx.acquire_package_cache_lock(CacheLockMode::DownloadExclusive)?;
     if let Some(previous_resolve) = previous_resolve {
         print_lockfile_sync(gctx, previous_resolve, resolve, registry)
     } else {
@@ -331,7 +336,7 @@ fn print_lockfile_sync(
     Ok(())
 }
 
-pub fn print_lockfile_updates(
+fn print_lockfile_updates(
     gctx: &GlobalContext,
     previous_resolve: &Resolve,
     resolve: &Resolve,

src/cargo/ops/mod.rs

@@ -9,7 +9,6 @@ pub use self::cargo_doc::{doc, DocOptions, OutputFormat};
 pub use self::cargo_fetch::{fetch, FetchOptions};
 pub use self::cargo_generate_lockfile::generate_lockfile;
 pub use self::cargo_generate_lockfile::print_lockfile_changes;
-pub use self::cargo_generate_lockfile::print_lockfile_updates;
 pub use self::cargo_generate_lockfile::update_lockfile;
 pub use self::cargo_generate_lockfile::UpdateOptions;
 pub use self::cargo_install::{install, install_list};

src/cargo/ops/resolve.rs

@@ -256,11 +256,6 @@ fn resolve_with_registry<'gctx>(
         false
     };
     if print {
-        // We only want one Cargo at a time resolving a crate graph since this can
-        // involve a lot of frobbing of the global caches.
-        let _lock = ws
-            .gctx()
-            .acquire_package_cache_lock(CacheLockMode::DownloadExclusive)?;
         ops::print_lockfile_changes(ws.gctx(), prev.as_ref(), &resolve, registry)?;
     }
     Ok(resolve)

tests/testsuite/generate_lockfile.rs

@@ -237,3 +237,39 @@ fn duplicate_entries_in_lockfile() {
         )
         .run();
 }
+
+#[cargo_test]
+fn generate_lockfile_holds_lock_and_offline() {
+    Package::new("syn", "1.0.0").publish();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+
+                [dependencies]
+                syn = "1.0"
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    p.cargo("generate-lockfile")
+        .with_stderr(
+            "\
+[UPDATING] `[..]` index
+[LOCKING] 2 packages
+",
+        )
+        .run();
+
+    p.cargo("generate-lockfile --offline")
+        .with_stderr_contains(
+            "\
+[LOCKING] 2 packages
+",
+        )
+        .run();
+}