core: Use a better termination condition in os::mkdir_recursive

Instead of checking whether the parent is "." or "/", check the
number of components.

Also, more tests.

src/libcore/os.rs

@@ -643,20 +643,22 @@ pub fn make_dir(p: &Path, mode: c_int) -> bool {
 /// Returns true iff creation
 /// succeeded. Also creates all intermediate subdirectories
 /// if they don't already exist, giving all of them the same mode.
+
+// tjc: if directory exists but with different permissions,
+// should we return false?
 pub fn mkdir_recursive(p: &Path, mode: c_int) -> bool {
     if path_is_dir(p) {
         return true;
     }
-    let parent = p.dir_path();
+    else if p.components.is_empty() {
-    debug!("mkdir_recursive: parent = %s",
+        return false;
-           parent.to_str());
+    }
-    if parent.to_str() == ~"."
+    else if p.components.len() == 1 {
-        || parent.to_str() == ~"/" { // !!!
         // No parent directories to create
-        path_is_dir(&parent) && make_dir(p, mode)
+        path_is_dir(p) || make_dir(p, mode)
     }
     else {
-        mkdir_recursive(&parent, mode) && make_dir(p, mode)
+        mkdir_recursive(&p.pop(), mode) && make_dir(p, mode)
     }
 }
 
@@ -1267,6 +1269,8 @@ mod tests {
     use run;
     use str;
     use vec;
+    use libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR};
+
 
     #[test]
     pub fn last_os_error() {
@@ -1490,16 +1494,16 @@ mod tests {
     }
 
     #[test]
-    fn recursive_mkdir_ok() {
+    fn recursive_mkdir_slash() {
-        use libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR};
+        let path = Path("/");
-
+        assert!(os::mkdir_recursive(&path,  (S_IRUSR | S_IWUSR | S_IXUSR) as i32));
-        let root = os::tmpdir();
-        let path = "xy/z/zy";
-        let nested = root.push(path);
-        assert!(os::mkdir_recursive(&nested,  (S_IRUSR | S_IWUSR | S_IXUSR) as i32));
-        assert!(os::path_is_dir(&root.push("xy")));
-        assert!(os::path_is_dir(&root.push("xy/z")));
-        assert!(os::path_is_dir(&nested));
     }
 
+    #[test]
+    fn recursive_mkdir_empty() {
+        let path = Path("");
+        assert!(!os::mkdir_recursive(&path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32));
+    }
+
+    // More recursive_mkdir tests are in std::tempfile
 }

src/libstd/tempfile.rs

@@ -23,9 +23,62 @@ pub fn mkdtemp(tmpdir: &Path, suffix: &str) -> Option<Path> {
     None
 }
 
-#[test]
+#[cfg(test)]
-fn test_mkdtemp() {
+mod tests {
-    let p = mkdtemp(&Path("."), "foobar").unwrap();
+    use tempfile::mkdtemp;
-    os::remove_dir(&p);
+    use tempfile;
-    assert!(str::ends_with(p.to_str(), "foobar"));
+
-}
+    #[test]
+    fn test_mkdtemp() {
+        let p = mkdtemp(&Path("."), "foobar").unwrap();
+        os::remove_dir(&p);
+        assert!(str::ends_with(p.to_str(), "foobar"));
+    }
+
+    // Ideally these would be in core::os but then core would need
+    // to depend on std
+    #[test]
+    fn recursive_mkdir_rel() {
+        use core::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR};
+        use core::os;
+
+        let root = mkdtemp(&os::tmpdir(), "temp").expect("recursive_mkdir_rel");
+        os::change_dir(&root);
+        let path = Path("frob");
+        assert!(os::mkdir_recursive(&path,  (S_IRUSR | S_IWUSR | S_IXUSR) as i32));
+        assert!(os::path_is_dir(&path));
+        assert!(os::mkdir_recursive(&path,  (S_IRUSR | S_IWUSR | S_IXUSR) as i32));
+        assert!(os::path_is_dir(&path));
+    }
+
+    #[test]
+    fn recursive_mkdir_dot() {
+        use core::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR};
+        use core::os;
+
+        let dot = Path(".");
+        assert!(os::mkdir_recursive(&dot,  (S_IRUSR | S_IWUSR | S_IXUSR) as i32));
+        let dotdot = Path("..");
+        assert!(os::mkdir_recursive(&dotdot,  (S_IRUSR | S_IWUSR | S_IXUSR) as i32));
+    }
+
+    #[test]
+    fn recursive_mkdir_rel_2() {
+        use core::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR};
+        use core::os;
+
+        let root = mkdtemp(&os::tmpdir(), "temp").expect("recursive_mkdir_rel_2");
+        os::change_dir(&root);
+        let path = Path("./frob/baz");
+        debug!("...Making: %s in cwd %s", path.to_str(), os::getcwd().to_str());
+        assert!(os::mkdir_recursive(&path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32));
+        assert!(os::path_is_dir(&path));
+        assert!(os::path_is_dir(&path.pop()));
+        let path2 = Path("quux/blat");
+        debug!("Making: %s in cwd %s", path2.to_str(), os::getcwd().to_str());
+        assert!(os::mkdir_recursive(&path2, (S_IRUSR | S_IWUSR | S_IXUSR) as i32));
+        assert!(os::path_is_dir(&path2));
+        assert!(os::path_is_dir(&path2.pop()));
+    }
+
+}