Skip to content
Open
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
56 changes: 55 additions & 1 deletion sqlx-cli/src/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ async fn prepare(ctx: &PrepareCtx<'_>) -> anyhow::Result<()> {
}

let prepare_dir = ctx.prepare_dir()?;
run_prepare_step(ctx, &prepare_dir)?;
let cache_dir = ctx.metadata.target_directory().join("sqlx-prepare");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should probably be a subdirectory of .sqlx instead, to avoid any cross-filesystem copying in case target is a symlink to a different fs. Maybe .sqlx/staging? You could put a .gitignore in .sqlx automatically to not have it tracked by git (though maybe this would be annoying for people on other version control systems?).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, do we already have this kind of temporary folder somewhere here in sqlx?
Using .sqlx looks weird for me, since it is a folder to be versioned, and may impact people not using git indeed, or already having an own custom .gitignore inside .sqlx/ folder.

About using the target directory: why that would be an issue with cross-filesystem copying? It should work, right?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cross-fs copies are slower and more error-prone because they're not atomic. I could see things breaking if in parallel with sqlx prepare, rust-analyzer executes one of the macros or something.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! Which is the best option? I can change the code, no problem.
I still prefer to save on target/ since it is a temporary folder on any cargo project, but the two options have pros and cons.

run_prepare_step(ctx, &cache_dir)?;
sync_query_data(&cache_dir, &prepare_dir)?;

// Warn if no queries were generated. Glob since the directory may contain unrelated files.
if glob_query_files(prepare_dir)?.is_empty() {
Expand Down Expand Up @@ -204,6 +206,58 @@ fn run_prepare_step(ctx: &PrepareCtx, cache_dir: &Path) -> anyhow::Result<()> {
Ok(())
}

fn sync_query_data(cache_dir: &Path, prepare_dir: &Path) -> anyhow::Result<()> {
fs::create_dir_all(prepare_dir).context(format!(
"Failed to create query cache directory: {:?}",
prepare_dir
))?;

let prepare_filenames: HashSet<String> = glob_query_files(prepare_dir)?
.into_iter()
.filter_map(|path| path.file_name().map(|f| f.to_string_lossy().into_owned()))
.collect();
let cache_filenames: HashSet<String> = glob_query_files(cache_dir)?
.into_iter()
.filter_map(|path| path.file_name().map(|f| f.to_string_lossy().into_owned()))
.collect();

for stale_filename in prepare_filenames.difference(&cache_filenames) {
let stale_file = prepare_dir.join(stale_filename);
fs::remove_file(&stale_file)
.with_context(|| format!("Failed to delete query file: {}", stale_file.display()))?;
}

for filename in cache_filenames {
let cache_file = cache_dir.join(&filename);
let prepare_file = prepare_dir.join(&filename);

let should_copy = match fs::read(&prepare_file) {
Ok(prepare_bytes) => {
let cache_json = load_json_file(&cache_file)?;
let prepare_json: serde_json::Value = serde_json::from_slice(&prepare_bytes)?;
prepare_json != cache_json
}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => true,
Err(err) => {
return Err(err)
.with_context(|| format!("failed to load file: {}", prepare_file.display()))
}
};

if should_copy {
fs::copy(&cache_file, &prepare_file).with_context(|| {
format!(
"Failed to copy query file from {} to {}",
cache_file.display(),
prepare_file.display()
)
})?;
}
}

Ok(())
}

#[derive(Debug, PartialEq)]
struct ProjectRecompileAction {
// The names of the packages
Expand Down
Loading