Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialize cargo fix #9677

Merged
merged 1 commit into from
Jul 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"),
);
}