From 41519fbf10af4876474b154afc8323f1d05b1278 Mon Sep 17 00:00:00 2001
From: Eric Huss <eric@huss.org>
Date: Thu, 13 Dec 2018 14:29:26 -0800
Subject: [PATCH] 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.
---
 src/cargo/ops/fix.rs         |  8 ++++----
 src/cargo/util/lockserver.rs |  5 ++---
 tests/testsuite/fix.rs       | 14 ++++++++++++++
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs
index f9e5ce1e88f..b020d6bc27a 100644
--- a/src/cargo/ops/fix.rs
+++ b/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
diff --git a/src/cargo/util/lockserver.rs b/src/cargo/util/lockserver.rs
index 9a0ce39b1bc..9c4878dfc14 100644
--- a/src/cargo/util/lockserver.rs
+++ b/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];
diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs
index d31693a6c6f..679d4786155 100644
--- a/tests/testsuite/fix.rs
+++ b/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() {}");
+}