Skip to content

Commit

Permalink
Serialize cargo fix
Browse files Browse the repository at this point in the history
  • Loading branch information
ehuss committed Jul 11, 2021
1 parent 18f2598 commit c8e4f8e
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 4 deletions.
10 changes: 6 additions & 4 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
47 changes: 47 additions & 0 deletions tests/testsuite/fix.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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"),
);
}

0 comments on commit c8e4f8e

Please sign in to comment.