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

Fix prepare race condition in workspaces #2069

Merged
merged 6 commits into from
Aug 27, 2022
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
16 changes: 15 additions & 1 deletion sqlx-cli/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ pub struct Metadata {
///
/// Typically `target` at the workspace root, but can be overridden
target_directory: PathBuf,
/// Package metadata for the crate in the current working directory, None if run from
/// a workspace with the `merged` flag.
current_package: Option<Package>,
}

impl Metadata {
Expand All @@ -75,6 +78,10 @@ impl Metadata {
&self.target_directory
}

pub fn current_package(&self) -> Option<&Package> {
self.current_package.as_ref()
}

/// Gets all dependents (direct and transitive) of `id`
pub fn all_dependents_of(&self, id: &MetadataId) -> BTreeSet<&MetadataId> {
let mut dependents = BTreeSet::new();
Expand All @@ -101,13 +108,19 @@ impl FromStr for Metadata {
type Err = anyhow::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let cargo_metadata: CargoMetadata = serde_json::from_str(s)?;

// Extract the package for the current working directory, will be empty if running
// from a workspace root.
let current_package: Option<Package> = cargo_metadata.root_package().map(Package::from);

let CargoMetadata {
packages: metadata_packages,
workspace_members,
resolve,
target_directory,
..
} = serde_json::from_str(s)?;
} = cargo_metadata;

let mut packages = BTreeMap::new();
for metadata_package in metadata_packages {
Expand Down Expand Up @@ -136,6 +149,7 @@ impl FromStr for Metadata {
workspace_members,
reverse_deps,
target_directory,
current_package,
})
}
}
18 changes: 17 additions & 1 deletion sqlx-cli/src/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,23 @@ hint: This command only works in the manifest directory of a Cargo package."#
bail!("`cargo check` failed with status: {}", check_status);
}

let pattern = metadata.target_directory().join("sqlx/query-*.json");
let package_dir = if merge {
// Merge queries from all workspace crates.
"**"
} else {
// Use a separate sub-directory for each crate in a workspace. This avoids a race condition
// where `prepare` can pull in queries from multiple crates if they happen to be generated
// simultaneously (e.g. Rust Analyzer building in the background).
metadata
.current_package()
.map(|pkg| pkg.name())
.context("Resolving the crate package for the current working directory failed")?
};
let pattern = metadata
.target_directory()
.join("sqlx")
.join(package_dir)
.join("query-*.json");

let mut data = BTreeMap::new();

Expand Down
2 changes: 1 addition & 1 deletion sqlx-cli/tests/assets/sample_metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -30506,7 +30506,7 @@
"features": []
}
],
"root": null
"root": "b_in_workspace_lib 0.1.0 (path+file:///home/user/problematic/workspace/b_in_workspace_lib)"
},
"target_directory": "/home/user/problematic/workspace/target",
"version": 1,
Expand Down
17 changes: 16 additions & 1 deletion sqlx-macros/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ struct Metadata {
offline: bool,
database_url: Option<String>,
#[cfg(feature = "offline")]
package_name: String,
#[cfg(feature = "offline")]
target_dir: PathBuf,
#[cfg(feature = "offline")]
workspace_root: Arc<Mutex<Option<PathBuf>>>,
Expand Down Expand Up @@ -74,6 +76,11 @@ static METADATA: Lazy<Metadata> = Lazy::new(|| {
.expect("`CARGO_MANIFEST_DIR` must be set")
.into();

#[cfg(feature = "offline")]
let package_name: String = env("CARGO_PKG_NAME")
.expect("`CARGO_PKG_NAME` must be set")
.into();

#[cfg(feature = "offline")]
let target_dir = env("CARGO_TARGET_DIR").map_or_else(|_| "target".into(), |dir| dir.into());

Expand Down Expand Up @@ -110,6 +117,8 @@ static METADATA: Lazy<Metadata> = Lazy::new(|| {
offline,
database_url,
#[cfg(feature = "offline")]
package_name,
#[cfg(feature = "offline")]
target_dir,
#[cfg(feature = "offline")]
workspace_root: Arc::new(Mutex::new(None)),
Expand Down Expand Up @@ -402,7 +411,13 @@ where
// If the build is offline, the cache is our input so it's pointless to also write data for it.
#[cfg(feature = "offline")]
if !offline {
let save_dir = METADATA.target_dir.join("sqlx");
// Use a separate sub-directory for each crate in a workspace. This avoids a race condition
// where `prepare` can pull in queries from multiple crates if they happen to be generated
// simultaneously (e.g. Rust Analyzer building in the background).
let save_dir = METADATA
.target_dir
.join("sqlx")
.join(&METADATA.package_name);
std::fs::create_dir_all(&save_dir)?;
data.save_in(save_dir, input.src_span)?;
}
Expand Down