Auto merge of #6434 - ehuss:fix-fix-concurrent, r=dwijnand

cargo fix: fix targets with shared sources.

If `cargo fix` attempts to fix multiple targets concurrently that have shared
source files, it would apply fixes multiple times causing corruption of the
source code. Fix this by locking on the package path instead of the target
filename, essentially serializing all targets within a package.

Fixes #6415.

Author: bors

src/cargo/ops/fix.rs

@@ -22,7 +22,6 @@ const FIX_ENV: &str = "__CARGO_FIX_PLZ";
 const BROKEN_CODE_ENV: &str = "__CARGO_FIX_BROKEN_CODE";
 const PREPARE_FOR_ENV: &str = "__CARGO_FIX_PREPARE_FOR";
 const EDITION_ENV: &str = "__CARGO_FIX_EDITION";
-
 const IDIOMS_ENV: &str = "__CARGO_FIX_IDIOMS";
 
 pub struct FixOptions<'a> {
@@ -258,9 +257,10 @@ fn rustfix_crate(
     // process at a time. If two invocations concurrently check a crate then
     // it's likely to corrupt it.
     //
-    // Currently we do this by assigning the name on our lock to the first
-    // argument that looks like a Rust file.
-    let _lock = LockServerClient::lock(&lock_addr.parse()?, filename)?;
+    // Currently we 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)?;
 
     // 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

src/cargo/util/lockserver.rs

@@ -14,7 +14,6 @@
 use std::collections::HashMap;
 use std::io::{BufRead, BufReader, Read, Write};
 use std::net::{SocketAddr, TcpListener, TcpStream};
-use std::path::Path;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Mutex};
 use std::thread::{self, JoinHandle};
@@ -156,11 +155,11 @@ impl Drop for LockServerStarted {
 }
 
 impl LockServerClient {
-    pub fn lock(addr: &SocketAddr, name: &Path) -> Result<LockServerClient, Error> {
+    pub fn lock(addr: &SocketAddr, name: impl AsRef<[u8]>) -> Result<LockServerClient, Error> {
         let mut client = TcpStream::connect(&addr)
             .with_context(|_| "failed to connect to parent lock server")?;
         client
-            .write_all(name.display().to_string().as_bytes())
+            .write_all(name.as_ref())
             .and_then(|_| client.write_all(b"\n"))
             .with_context(|_| "failed to write to lock server")?;
         let mut buf = [0];

tests/testsuite/fix.rs

@@ -1235,3 +1235,17 @@ fn fix_to_broken_code() {
         "pub fn foo() { let x = 3; drop(x); }"
     );
 }
+
+#[test]
+fn fix_with_common() {
+    let p = project()
+        .file("src/lib.rs", "")
+        .file("tests/t1.rs", "mod common; #[test] fn t1() { common::try(); }")
+        .file("tests/t2.rs", "mod common; #[test] fn t2() { common::try(); }")
+        .file("tests/common/mod.rs", "pub fn try() {}")
+        .build();
+
+    p.cargo("fix --edition --allow-no-vcs").run();
+
+    assert_eq!(p.read_file("tests/common/mod.rs"), "pub fn r#try() {}");
+}