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

0.7.0-alpha.1: error: failed to move query file when running cargo sqlx prepare #2395

Closed
aschey opened this issue Mar 11, 2023 · 2 comments · Fixed by #2398
Closed

0.7.0-alpha.1: error: failed to move query file when running cargo sqlx prepare #2395

aschey opened this issue Mar 11, 2023 · 2 comments · Fixed by #2398
Labels

Comments

@aschey
Copy link
Contributor

aschey commented Mar 11, 2023

This issue appears on the latest commit to main at the time of writing this.

Bug Description

The PR to implement one file per query: #2363 implemented logic to write queries to a temp file and then persist them to SQLX_OFFLINE_DIR here:

let mut tmp_file = tempfile::NamedTempFile::new()
.map_err(|err| format!("failed to create query file: {:?}", err))?;
serde_json::to_writer_pretty(tmp_file.as_file_mut(), self)
.map_err(|err| format!("failed to serialize query data to file: {:?}", err))?;
tmp_file
.persist(dir.as_ref().join(format!("query-{}.json", self.hash)))
.map_err(|err| format!("failed to move query file: {:?}", err))?;

However, the docs for NamedTempFile::persist mention the following:

Note: Temporary files cannot be persisted across filesystems.

On Linux, the /tmp directory is commonly mounted as tmpfs which means the call to tmp_file.persist will result in the following error: failed to move query file: PersistError(Os { code: 18, kind: CrossesDevices, message: "Invalid cross-device link" }).

Perhaps we could use tempfile::NamedTempFile::new_in(dir.as_ref()) to create the temporary files directly in SQLX_OFFLINE_DIR to ensure we're not crossing any filesystem boundaries?

Minimal Reproduction

Run cargo sqlx migrate on a Linux system with /tmp mounted separately from the main OS.

Info

  • SQLx version: 0.7.0-alpha.1
  • SQLx features enabled: "sqlite", "any", "macros", "runtime-tokio"
  • Database server and version: SQLite
  • Operating system: Arch Linux
  • rustc --version: rustc 1.68.0 (2c8cc3432 2023-03-06)
@aschey aschey added the bug label Mar 11, 2023
@cycraig
Copy link
Contributor

cycraig commented Mar 11, 2023

Darn. Temporary query files were previously stored under a subdirectory of target; we could also switch back to that to prevent potentially polluting a git project?

To give some context here, I used tempfile in #2363 for two reasons:

  1. The target path was sometimes relative instead of absolute, causing errors. This was partially solved by overriding CARGO_TARGET_DIR when invoking cargo manually in sqlx prepare. Some developers specify the target manually, so to avoid conflicts I replaced that behaviour with tempfile.
  2. The previous temporary query-{}.json filenames used a hash of the input span. This was meant to be unique but conflicted when compiling multiple targets as once, e.g. test or --all-targets. tempfile generates random names anyway so I chose not to reinvent the wheel.

Point being, using tempfile::NamedTempFile::new_in sounds good to me.

For a workaround in the interim, setting the TMPDIR environment variable to somewhere other than /tmp should control where tempfile outputs.

@aschey
Copy link
Contributor Author

aschey commented Mar 11, 2023

@cycraig thanks for the background info. I took a stab at a fix here: #2398. Please let me know if this is what you had in mind or if there's a better way, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants