Skip to content

Commit

Permalink
[fud2] Even more confidently delete temp dir (#2072)
Browse files Browse the repository at this point in the history
In #2058, I made fud2 clean up the `.fud2` temporary directory even when
Ninja exited with an erorr. Unfortunately, that doesn't cover other
possible errors, such as when launching the Ninja process fails
altogether:
#2058 (comment)

Rather than patching one error at a time, this uses `Drop` to be sure we
do this when any error occurs. It also means it will get deleted even
when we panic, which is *definitely* overkill, but I guess it's not a
big deal.
  • Loading branch information
sampsyo authored May 29, 2024
1 parent 513fda8 commit ad8a828
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 13 deletions.
2 changes: 1 addition & 1 deletion fud2/fud-core/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ pub fn cli(driver: &Driver) -> anyhow::Result<()> {
Mode::ShowPlan => run.show(),
Mode::ShowDot => run.show_dot(),
Mode::EmitNinja => run.emit_to_stdout()?,
Mode::Generate => run.emit_to_dir(&workdir)?,
Mode::Generate => run.emit_to_dir(&workdir)?.keep(),
Mode::Run => run.emit_and_run(&workdir)?,
}

Expand Down
56 changes: 44 additions & 12 deletions fud2/fud-core/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,19 +191,20 @@ impl<'a> Run<'a> {
}

/// Ensure that a directory exists and write `build.ninja` inside it.
pub fn emit_to_dir(&self, dir: &Utf8Path) -> EmitResult {
std::fs::create_dir_all(dir)?;
let ninja_path = dir.join("build.ninja");
pub fn emit_to_dir(&self, path: &Utf8Path) -> Result<TempDir, RunError> {
let dir = TempDir::new(path, self.global_config.keep_build_dir)?;

let ninja_path = path.join("build.ninja");
let ninja_file = std::fs::File::create(ninja_path)?;
self.emit(ninja_file)?;

self.emit(ninja_file)
Ok(dir)
}

/// Emit `build.ninja` to a temporary directory and then actually execute ninja.
pub fn emit_and_run(&self, dir: &Utf8Path) -> EmitResult {
// Emit the Ninja file.
let stale_dir = dir.exists();
self.emit_to_dir(dir)?;
let dir = self.emit_to_dir(dir)?;

// Capture stdin.
if self.plan.stdin {
Expand All @@ -218,7 +219,7 @@ impl<'a> Run<'a> {

// Run `ninja` in the working directory.
let mut cmd = Command::new(&self.global_config.ninja);
cmd.current_dir(dir);
cmd.current_dir(&dir.path);
cmd.stdout(std::io::stderr()); // Send Ninja's stdout to our stderr.
let status = cmd.status()?;

Expand All @@ -232,11 +233,6 @@ impl<'a> Run<'a> {
)?;
}

// Remove the temporary directory unless it already existed at the start *or* the user specified `--keep`.
if !self.global_config.keep_build_dir && !stale_dir {
std::fs::remove_dir_all(dir)?;
}

if status.success() {
Ok(())
} else {
Expand Down Expand Up @@ -418,3 +414,39 @@ impl<'a> Emitter<'a> {
self.build_cmd(&[filename], "get-rsrc", &[], &[])
}
}

/// A directory that can optionally delete itself when we're done with it.
pub struct TempDir {
path: Utf8PathBuf,
delete: bool,
}

impl TempDir {
/// Create a directory *or* use an existing directory.
///
/// If the directory already exists, we will not delete it (regardless of `keep`). Otherwise,
/// we will create a new one, and we will delete it when this object is dropped, unless
/// `keep` is true.
pub fn new(path: &Utf8Path, keep: bool) -> std::io::Result<Self> {
let delete = !path.exists() && !keep;
std::fs::create_dir_all(path)?;
Ok(Self {
path: path.into(),
delete,
})
}

/// If this directory would otherwise be deleted, don't.
pub fn keep(&mut self) {
self.delete = false;
}
}

impl Drop for TempDir {
fn drop(&mut self) {
if self.delete {
// We must ignore errors when attempting to delete.
let _ = std::fs::remove_dir_all(&self.path);
}
}
}

0 comments on commit ad8a828

Please sign in to comment.