From c8e4f8e483bb35996f7b963b796fa09c5c9c2c5b Mon Sep 17 00:00:00 2001
From: Eric Huss <eric@huss.org>
Date: Sat, 10 Jul 2021 18:47:52 -0700
Subject: [PATCH] Serialize `cargo fix`

---
 src/cargo/ops/fix.rs   | 10 +++++----
 tests/testsuite/fix.rs | 47 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs
index 8c7f4405019..57933e4a1df 100644
--- a/src/cargo/ops/fix.rs
+++ b/src/cargo/ops/fix.rs
@@ -420,10 +420,12 @@ fn rustfix_crate(
     // process at a time. If two invocations concurrently check a crate then
     // it's likely to corrupt it.
     //
-    // We currently do this by assigning the name on our lock to the manifest
-    // directory.
-    let dir = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR is missing?");
-    let _lock = LockServerClient::lock(&lock_addr.parse()?, dir)?;
+    // Historically this used per-source-file locking, then per-package
+    // locking. It now uses a single, global lock as some users do things like
+    // #[path] or include!() of shared files between packages. Serializing
+    // makes it slower, but is the only safe way to prevent concurrent
+    // modification.
+    let _lock = LockServerClient::lock(&lock_addr.parse()?, "global")?;
 
     // Next up, this is a bit suspicious, but we *iteratively* execute rustc and
     // collect suggestions to feed to rustfix. Once we hit our limit of times to
diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs
index 1d2877b4b16..3e66643a227 100644
--- a/tests/testsuite/fix.rs
+++ b/tests/testsuite/fix.rs
@@ -1,6 +1,7 @@
 //! Tests for the `cargo fix` command.
 
 use cargo::core::Edition;
+use cargo_test_support::compare::assert_match_exact;
 use cargo_test_support::git;
 use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::registry::{Dependency, Package};
@@ -1553,3 +1554,49 @@ fn fix_edition_2021() {
         .run();
     assert!(p.read_file("src/lib.rs").contains(r#"0..=100 => true,"#));
 }
+
+#[cargo_test]
+fn fix_shared_cross_workspace() {
+    // Fixing a file that is shared between multiple packages in the same workspace.
+    // Make sure two processes don't try to fix the same file at the same time.
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [workspace]
+                members = ["foo", "bar"]
+            "#,
+        )
+        .file("foo/Cargo.toml", &basic_manifest("foo", "0.1.0"))
+        .file("foo/src/lib.rs", "pub mod shared;")
+        // This will fix both unused and bare trait.
+        .file("foo/src/shared.rs", "pub fn fixme(x: Box<&Fn() -> ()>) {}")
+        .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.0"))
+        .file(
+            "bar/src/lib.rs",
+            r#"
+                #[path="../../foo/src/shared.rs"]
+                pub mod shared;
+            "#,
+        )
+        .build();
+
+    // The output here can be either of these two, depending on who runs first:
+    //     [FIXED] bar/src/../../foo/src/shared.rs (2 fixes)
+    //     [FIXED] foo/src/shared.rs (2 fixes)
+    p.cargo("fix --allow-no-vcs")
+        .with_stderr_unordered(
+            "\
+[CHECKING] foo v0.1.0 [..]
+[CHECKING] bar v0.1.0 [..]
+[FIXED] [..]foo/src/shared.rs (2 fixes)
+[FINISHED] [..]
+",
+        )
+        .run();
+
+    assert_match_exact(
+        "pub fn fixme(_x: Box<&dyn Fn() -> ()>) {}",
+        &p.read_file("foo/src/shared.rs"),
+    );
+}