Skip to content

Commit

Permalink
extra::tempfile: replace mkdtemp with an RAII wrapper
Browse files Browse the repository at this point in the history
this incidentally stops `make check` from leaving directories in `/tmp`
  • Loading branch information
ben0x539 committed Oct 11, 2013
1 parent 5bddcc1 commit 63e9e49
Show file tree
Hide file tree
Showing 10 changed files with 616 additions and 468 deletions.
64 changes: 55 additions & 9 deletions src/libextra/tempfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,63 @@ use std::os;
use std::rand::Rng;
use std::rand;

/// Attempts to make a temporary directory inside of `tmpdir` whose name will
/// have the suffix `suffix`. If no directory can be created, None is returned.
pub fn mkdtemp(tmpdir: &Path, suffix: &str) -> Option<Path> {
let mut r = rand::rng();
for _ in range(0u, 1000) {
let p = tmpdir.push(r.gen_ascii_str(16) + suffix);
if os::make_dir(&p, 0x1c0) { // 700
return Some(p);
/// A wrapper for a path to temporary directory implementing automatic
/// scope-pased deletion.
pub struct TempDir {
priv path: Option<Path>
}

impl TempDir {
/// Attempts to make a temporary directory inside of `tmpdir` whose name
/// will have the suffix `suffix`. The directory will be automatically
/// deleted once the returned wrapper is destroyed.
///
/// If no directory can be created, None is returned.
pub fn new_in(tmpdir: &Path, suffix: &str) -> Option<TempDir> {
if !tmpdir.is_absolute() {
let abs_tmpdir = os::make_absolute(tmpdir);
return TempDir::new_in(&abs_tmpdir, suffix);
}

let mut r = rand::rng();
for _ in range(0u, 1000) {
let p = tmpdir.push(r.gen_ascii_str(16) + suffix);
if os::make_dir(&p, 0x1c0) { // 700
return Some(TempDir { path: Some(p) });
}
}
None
}

/// Attempts to make a temporary directory inside of `os::tmpdir()` whose
/// name will have the suffix `suffix`. The directory will be automatically
/// deleted once the returned wrapper is destroyed.
///
/// If no directory can be created, None is returned.
pub fn new(suffix: &str) -> Option<TempDir> {
TempDir::new_in(&os::tmpdir(), suffix)
}

/// Unwrap the wrapped `std::path::Path` from the `TempDir` wrapper.
/// This discards the wrapper so that the automatic deletion of the
/// temporary directory is prevented.
pub fn unwrap(self) -> Path {
let mut tmpdir = self;
tmpdir.path.take_unwrap()
}

/// Access the wrapped `std::path::Path` to the temporary directory.
pub fn path<'a>(&'a self) -> &'a Path {
self.path.get_ref()
}
}

impl Drop for TempDir {
fn drop(&mut self) {
for path in self.path.iter() {
os::remove_dir_recursive(path);
}
}
None
}

// the tests for this module need to change the path using change_dir,
Expand Down
10 changes: 3 additions & 7 deletions src/libextra/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1148,8 +1148,7 @@ mod tests {
use test::{TestOpts, run_test};

use std::comm::{stream, SharedChan};
use tempfile;
use std::os;
use tempfile::TempDir;

#[test]
pub fn do_not_run_ignored_tests() {
Expand Down Expand Up @@ -1392,9 +1391,8 @@ mod tests {

pub fn ratchet_test() {

let dpth = tempfile::mkdtemp(&os::tmpdir(),
"test-ratchet").expect("missing test for ratchet");
let pth = dpth.push("ratchet.json");
let dpth = TempDir::new("test-ratchet").expect("missing test for ratchet");
let pth = dpth.path().push("ratchet.json");

let mut m1 = MetricMap::new();
m1.insert_metric("runtime", 1000.0, 2.0);
Expand Down Expand Up @@ -1432,7 +1430,5 @@ mod tests {
assert_eq!(m4.len(), 2);
assert_eq!(*(m4.find(&~"runtime").unwrap()), Metric { value: 1100.0, noise: 2.0 });
assert_eq!(*(m4.find(&~"throughput").unwrap()), Metric { value: 50.0, noise: 2.0 });

os::remove_dir_recursive(&dpth);
}
}
3 changes: 0 additions & 3 deletions src/librustpkg/package_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,6 @@ impl PkgSrc {
pub fn fetch_git(local: &Path, pkgid: &PkgId) -> Option<Path> {
use conditions::git_checkout_failed::cond;

// We use a temporary directory because if the git clone fails,
// it creates the target directory anyway and doesn't delete it

debug2!("Checking whether {} (path = {}) exists locally. Cwd = {}, does it? {:?}",
pkgid.to_str(), pkgid.path.to_str(),
os::getcwd().to_str(),
Expand Down
18 changes: 9 additions & 9 deletions src/librustpkg/source_control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

use std::{io, os, run, str};
use std::run::{ProcessOutput, ProcessOptions, Process};
use extra::tempfile;
use extra::tempfile::TempDir;
use version::*;
use path_util::chmod_read_only;

Expand All @@ -22,14 +22,6 @@ use path_util::chmod_read_only;
/// directory (that the callee may use, for example, to check out remote sources into).
/// Returns `CheckedOutSources` if the clone succeeded.
pub fn safe_git_clone(source: &Path, v: &Version, target: &Path) -> CloneResult {
use conditions::failed_to_create_temp_dir::cond;

let scratch_dir = tempfile::mkdtemp(&os::tmpdir(), "rustpkg");
let clone_target = match scratch_dir {
Some(d) => d.push("rustpkg_temp"),
None => cond.raise(~"Failed to create temporary directory for fetching git sources")
};

if os::path_exists(source) {
debug2!("{} exists locally! Cloning it into {}",
source.to_str(), target.to_str());
Expand Down Expand Up @@ -77,6 +69,14 @@ pub fn safe_git_clone(source: &Path, v: &Version, target: &Path) -> CloneResult
}
CheckedOutSources
} else {
use conditions::failed_to_create_temp_dir::cond;

let scratch_dir = TempDir::new("rustpkg");
let clone_target = match scratch_dir {
Some(d) => d.unwrap().push("rustpkg_temp"),
None => cond.raise(~"Failed to create temporary directory for fetching git sources")
};

DirToUse(clone_target)
}
}
Expand Down
Loading

5 comments on commit 63e9e49

@bors
Copy link
Contributor

@bors bors commented on 63e9e49 Oct 11, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from alexcrichton
at ben0x539@63e9e49

@bors
Copy link
Contributor

@bors bors commented on 63e9e49 Oct 11, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging ben0x539/rust/mkdtemp-raii = 63e9e49 into auto

@bors
Copy link
Contributor

@bors bors commented on 63e9e49 Oct 11, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ben0x539/rust/mkdtemp-raii = 63e9e49 merged ok, testing candidate = 93a08f4

@bors
Copy link
Contributor

@bors bors commented on 63e9e49 Oct 11, 2013

@bors
Copy link
Contributor

@bors bors commented on 63e9e49 Oct 11, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 93a08f4

Please sign in to comment.