Skip to content

Commit

Permalink
fix(files): Writing new files works across devices
Browse files Browse the repository at this point in the history
`persist` of `tempfile` does not work across
device boundaries, so if the device backing the
temporary directory is e.g. `tmpfs`, then
`persist` fails. It actually had no useful
benefit, so we can just remove it and write to the
destination directly.

Reproduced bug and confirmed fix on a Debian
machine with:

```console
$ sudo mkdir -p /path/to/temp_dir
$ sudo mount -t tmpfs -o size=100m tmpfs /path/to/temp_dir
$ sudo mount | rg 'temp'
tmpfs on /path/to/temp_dir type tmpfs (rw,relatime,size=102400k,inode64)
$ export TMPDIR=/path/to/temp_dir
$ cargo run -- --python class a whatever
```

`TMPDIR` env var is respected by `tempfile`. The
above `cargo run` fails without this patch. (There
is an end-to-end test for file writing, but that
has never failed for some reason... seems like
none of my machines (including Ubuntu 24) or CI
exhibited the issue).

Closes #145
  • Loading branch information
alexpovel committed Oct 19, 2024
1 parent 3b8abfa commit 1b27825
Showing 1 changed file with 5 additions and 17 deletions.
22 changes: 5 additions & 17 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
//! deals with CLI argument handling, I/O, threading, and more.
use std::error::Error;
use std::fs::File;
use std::fs::{self, File};
use std::io::{self, stdout, Read, Write};
use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex};
Expand Down Expand Up @@ -535,17 +535,11 @@ fn process_path(
}

if changed {
writeln!(stdout, "{}", path.display())?;

debug!("Got new file contents, writing to file: {:?}", path);
let mut file = tempfile::Builder::new()
.prefix(env!("CARGO_PKG_NAME"))
.tempfile()?;
trace!("Writing to temporary file: {:?}", file.path());
file.write_all(new_contents.as_bytes())?;

// Atomically replace so SIGINT etc. do not leave dangling crap.
file.persist(&path)?;
fs::write(&path, new_contents.as_bytes())?;

// Confirm after successful write.
writeln!(stdout, "{}", path.display())?;
} else {
debug!(
"Skipping writing file anew (nothing changed): {}",
Expand Down Expand Up @@ -759,12 +753,6 @@ impl From<ApplicationError> for PathProcessingError {
}
}

impl From<tempfile::PersistError> for PathProcessingError {
fn from(err: tempfile::PersistError) -> Self {
Self::IoError(err.error, Some(err.file.path().to_owned()))
}
}

impl Error for PathProcessingError {}

#[derive(Debug)]
Expand Down

0 comments on commit 1b27825

Please sign in to comment.