refactor: replace hand-rolled temp dirs with tempfile::TempDir
commit
e4721c4refactor: replace hand-rolled temp dirs with tempfile::TempDir
Scratch (CLI) and TempDir (verify) were identical Drop-cleanup temp dirs. Both now use tempfile, promoted from a dev- to a normal dependency.
refactor: use tempfile::tempdir in fingerprint() refactor: use tempfile::tempdir in verify_certificate() Assisted-by: Claude:claude-opus-4-8
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
Cargo.lock
@@ -1025,6 +1025,7 @@
"clap",
"facet",
"git-store",
+ "tempfile",
"thiserror",
]
crates/git-ents-server/Cargo.toml
@@ -20,11 +20,11 @@
gix-hash = { workspace = true }
gix-object = { workspace = true }
maud = { workspace = true }
+tempfile = { workspace = true }
tokio = { workspace = true }
[dev-dependencies]
rstest = { workspace = true }
-tempfile = { workspace = true }
[lints]
workspace = true
crates/git-ents/Cargo.toml
@@ -9,6 +9,7 @@
clap = { workspace = true }
facet = { workspace = true }
git-store = { workspace = true }
+tempfile = { workspace = true }
thiserror = { workspace = true }
[lints]
crates/git-ents-server/src/verify.rs
@@ -8,9 +8,8 @@
//! whose signature verifies against one of those keys.
use std::io::Write;
-use std::path::{Path, PathBuf};
+use std::path::Path;
use std::process::{Command, Stdio};
-use std::sync::atomic::{AtomicUsize, Ordering};
use git_ents::signers::{self, Signer};
@@ -50,7 +49,7 @@
let (payload, signature) = certificate.split_at(split);
let principal = signer_principal(certificate);
- let workdir = TempDir::new()?;
+ let workdir = tempfile::tempdir().map_err(|e| format!("could not create temp dir: {e}"))?;
let allowed_path = workdir.path().join("allowed_signers");
let signature_path = workdir.path().join("cert.sig");
write_file(
@@ -119,29 +118,3 @@
fn write_file(path: &Path, bytes: &[u8]) -> Result<(), String> {
std::fs::write(path, bytes).map_err(|e| format!("could not write {}: {e}", path.display()))
}
-
-/// A uniquely named temporary directory removed when dropped, holding the short
-/// files `ssh-keygen` needs to read from disk.
-struct TempDir(PathBuf);
-
-impl TempDir {
- fn new() -> Result<Self, String> {
- static COUNTER: AtomicUsize = AtomicUsize::new(0);
- let n = COUNTER.fetch_add(1, Ordering::SeqCst);
- let dir = std::env::temp_dir().join(format!("git-ents-verify-{}-{n}", std::process::id()));
- std::fs::create_dir_all(&dir).map_err(|e| format!("could not create temp dir: {e}"))?;
- Ok(Self(dir))
- }
-
- fn path(&self) -> &Path {
- &self.0
- }
-}
-
-impl Drop for TempDir {
- fn drop(&mut self) {
- match std::fs::remove_dir_all(&self.0) {
- Ok(()) | Err(_) => {}
- }
- }
-}
crates/git-ents/src/main.rs
@@ -9,7 +9,6 @@
use std::path::{Path, PathBuf};
use std::process::{Command, ExitCode, Stdio};
-use std::sync::atomic::{AtomicUsize, Ordering};
use clap::{Parser, Subcommand};
use git_ents::checks::{self, CHECKS_REF, Check};
@@ -561,7 +560,8 @@
/// are filesystem-safe, unlike the slashes in a base64 SHA256 fingerprint that
/// would split the `signers/<name>` entry into a subtree.
fn fingerprint(public_key: &str) -> Result<String, String> {
- let scratch = Scratch::new()?;
+ let scratch =
+ tempfile::tempdir().map_err(|error| format!("could not create temp dir: {error}"))?;
let path = scratch.path().join("key.pub");
std::fs::write(&path, public_key).map_err(|error| format!("could not stage key: {error}"))?;
let output = Command::new("ssh-keygen")
@@ -658,29 +658,3 @@
}
String::from_utf8(output.stdout).map_err(|_invalid| "git produced non-UTF-8 output".to_owned())
}
-
-/// A uniquely named temporary directory removed when dropped.
-struct Scratch(PathBuf);
-
-impl Scratch {
- fn new() -> Result<Self, String> {
- static COUNTER: AtomicUsize = AtomicUsize::new(0);
- let n = COUNTER.fetch_add(1, Ordering::SeqCst);
- let dir = std::env::temp_dir().join(format!("git-ents-cli-{}-{n}", std::process::id()));
- std::fs::create_dir_all(&dir)
- .map_err(|error| format!("could not create temp dir: {error}"))?;
- Ok(Self(dir))
- }
-
- fn path(&self) -> &Path {
- &self.0
- }
-}
-
-impl Drop for Scratch {
- fn drop(&mut self) {
- match std::fs::remove_dir_all(&self.0) {
- Ok(()) | Err(_) => {}
- }
- }
-}