Skip to content

Commit

Permalink
Don't use separate temp dir for query jsons (2) (#2803)
Browse files Browse the repository at this point in the history
* Don't use temp dir for query jsons

Fixes #2663

* Return early when exclusive create fails

* Use atomic-file-write for writing query data
  • Loading branch information
mattfbacon authored Oct 17, 2023
1 parent f93bbe8 commit b16fbeb
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 49 deletions.
13 changes: 0 additions & 13 deletions FAQ.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,19 +201,6 @@ as an ergonomic choice it does _not_ block committing if `cargo sqlx prepare` fa
We're working on a way for the macros to save their data to the filesystem automatically which should be part of SQLx 0.7,
so your pre-commit hook would then just need to stage the changed files. This can be enabled by creating a directory
and setting the `SQLX_OFFLINE_DIR` environment variable to it before compiling.
Additionally, if you're not using Cargo or have a nonstandard setup, you may want to set the `SQLX_TMP`
variable in order to store temporary query files somewhere that isn't picked up by git.
These files should get cleaned up automatically, but they may not if there's a failure. For example:

```shell
$ mkdir .sqlx
$ export SQLX_OFFLINE_DIR="./.sqlx"`
$ # Optional and only useful if using a nonstandard setup, ensures temp files won't get picked up by git on failure
$ mkdir ./my-custom-target/sqlx
$ export SQLX_TMP="./my-custom-target/sqlx-tmp"
$ cargo check
```

However, this behaviour is not considered stable and it is still recommended to use `cargo sqlx prepare`.

----
Expand Down
1 change: 0 additions & 1 deletion sqlx-cli/src/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ fn run_prepare_step(ctx: &PrepareCtx, cache_dir: &Path) -> anyhow::Result<()> {
check_command
.arg("check")
.args(&ctx.cargo_args)
.env("SQLX_TMP", tmp_dir)
.env("DATABASE_URL", &ctx.connect_opts.database_url)
.env("SQLX_OFFLINE", "false")
.env("SQLX_OFFLINE_DIR", cache_dir);
Expand Down
1 change: 1 addition & 0 deletions sqlx-macros-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ tokio = { workspace = true, optional = true }

dotenvy = { workspace = true }

atomic-write-file = { version = "0.1" }
hex = { version = "0.4.3" }
heck = { version = "0.4", features = ["unicode"] }
either = "1.6.1"
Expand Down
33 changes: 12 additions & 21 deletions sqlx-macros-core/src/query/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,31 +150,22 @@ where
}
}

pub(super) fn save_in(
&self,
dir: impl AsRef<Path>,
tmp_dir: impl AsRef<Path>,
) -> crate::Result<()> {
// Output to a temporary file first, then move it atomically to avoid clobbering
// other invocations trying to write to the same path.

// Use a temp directory inside the workspace to avoid potential issues
// with persisting the file across filesystems.
let mut tmp_file = tempfile::NamedTempFile::new_in(tmp_dir)
.map_err(|err| format!("failed to create query file: {err:?}"))?;

serde_json::to_writer_pretty(tmp_file.as_file_mut(), self)
pub(super) fn save_in(&self, dir: impl AsRef<Path>) -> crate::Result<()> {
let path = dir.as_ref().join(format!("query-{}.json", self.hash));
let mut file = atomic_write_file::AtomicWriteFile::open(&path)
.map_err(|err| format!("failed to open the temporary file: {err:?}"))?;

serde_json::to_writer_pretty(file.as_file_mut(), self)
.map_err(|err| format!("failed to serialize query data to file: {err:?}"))?;
// Ensure there is a newline at the end of the JSON file to avoid accidental modification by IDE
// and make github diff tool happier
tmp_file
.as_file_mut()

// Ensure there is a newline at the end of the JSON file to avoid
// accidental modification by IDE and make github diff tool happier.
file.as_file_mut()
.write_all(b"\n")
.map_err(|err| format!("failed to append a newline 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:?}"))?;
file.commit()
.map_err(|err| format!("failed to commit the query data to {path:?}: {err:?}"))?;

Ok(())
}
Expand Down
15 changes: 1 addition & 14 deletions sqlx-macros-core/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,19 +356,6 @@ where
if let Ok(dir) = env("SQLX_OFFLINE_DIR") {
let path = PathBuf::from(&dir);

// Prefer SQLX_TMP if set explicitly.
// Otherwise fallback to CARGO_TARGET_DIR and then the standard target directory.
let tmp_dir = if let Ok(tmp_dir) = env("SQLX_TMP") {
PathBuf::from(tmp_dir)
} else if let Ok(target_dir) = env("CARGO_TARGET_DIR") {
PathBuf::from(target_dir)
} else {
let tmp_target = PathBuf::from("./target/sqlx");
fs::create_dir_all(&tmp_target)
.map_err(|e| format!("Error creating cache directory: {e:?}"))?;
tmp_target
};

match fs::metadata(&path) {
Err(e) => {
if e.kind() != io::ErrorKind::NotFound {
Expand All @@ -387,7 +374,7 @@ where
}

// .sqlx exists and is a directory, store data.
data.save_in(path, tmp_dir)?;
data.save_in(path)?;
}
}
}
Expand Down

0 comments on commit b16fbeb

Please sign in to comment.