extra::tempfile: replace mkdtemp with an RAII wrapper (Closes #9763)

src/libextra/tempfile.rs

@@ -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,

src/libextra/test.rs

@@ -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() {
@@ -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);
@@ -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);
     }
 }

src/librustpkg/package_source.rs

@@ -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(),

src/librustpkg/source_control.rs

@@ -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;
 
@@ -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());
@@ -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)
     }
 }