Remove all usage of change_dir_locked

src/libextra/tempfile.rs

@@ -28,97 +28,6 @@ pub fn mkdtemp(tmpdir: &Path, suffix: &str) -> Option<Path> {
     None
 }
 
-#[cfg(test)]
-mod tests {
-
-    use tempfile::mkdtemp;
-
-    use std::os;
-
-    #[test]
-    fn test_mkdtemp() {
-        let p = mkdtemp(&Path("."), "foobar").unwrap();
-        os::remove_dir(&p);
-        assert!(p.to_str().ends_with("foobar"));
-    }
-
-    // Ideally these would be in std::os but then core would need
-    // to depend on std
-    #[test]
-    fn recursive_mkdir_rel() {
-        use std::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR};
-        use std::os;
-        use std::unstable::change_dir_locked;
-
-        let root = mkdtemp(&os::tmpdir(), "recursive_mkdir_rel").
-            expect("recursive_mkdir_rel");
-        assert!(do change_dir_locked(&root) {
-            let path = Path("frob");
-            debug!("recursive_mkdir_rel: Making: %s in cwd %s [%?]", path.to_str(),
-                   os::getcwd().to_str(),
-                   os::path_exists(&path));
-            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 std::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR};
-        use std::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 std::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR};
-        use std::os;
-        use std::unstable::change_dir_locked;
-
-        let root = mkdtemp(&os::tmpdir(), "recursive_mkdir_rel_2").
-            expect("recursive_mkdir_rel_2");
-        assert!(do change_dir_locked(&root) {
-            let path = Path("./frob/baz");
-            debug!("recursive_mkdir_rel_2: Making: %s in cwd %s [%?]", path.to_str(),
-                   os::getcwd().to_str(), os::path_exists(&path));
-            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!("recursive_mkdir_rel_2: 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()));
-        });
-    }
-
-    // Ideally this would be in core, but needs mkdtemp
-    #[test]
-    pub fn test_rmdir_recursive_ok() {
-        use std::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR};
-        use std::os;
-
-        let rwx = (S_IRUSR | S_IWUSR | S_IXUSR) as i32;
-
-        let tmpdir = mkdtemp(&os::tmpdir(), "test").expect("test_rmdir_recursive_ok: \
-                                            couldn't create temp dir");
-        let root = tmpdir.push("foo");
-
-        debug!("making %s", root.to_str());
-        assert!(os::make_dir(&root, rwx));
-        assert!(os::make_dir(&root.push("foo"), rwx));
-        assert!(os::make_dir(&root.push("foo").push("bar"), rwx));
-        assert!(os::make_dir(&root.push("foo").push("bar").push("blat"), rwx));
-        assert!(os::remove_dir_recursive(&root));
-        assert!(!os::path_exists(&root));
-        assert!(!os::path_exists(&root.push("bar")));
-        assert!(!os::path_exists(&root.push("bar").push("blat")));
-    }
-}
+// the tests for this module need to change the path using change_dir,
+// and this doesn't play nicely with other tests so these unit tests are located
+// in src/test/run-pass/tempfile.rs

src/librustpkg/tests.rs

@@ -819,26 +819,25 @@ fn rust_path_test() {
 }
 
 #[test]
+#[ignore] // FIXME(#9184) tests can't change the cwd (other tests are sad then)
 fn rust_path_contents() {
-    use std::unstable::change_dir_locked;
-
     let dir = mkdtemp(&os::tmpdir(), "rust_path").expect("rust_path_contents failed");
     let abc = &dir.push("A").push("B").push("C");
     assert!(os::mkdir_recursive(&abc.push(".rust"), U_RWX));
     assert!(os::mkdir_recursive(&abc.pop().push(".rust"), U_RWX));
     assert!(os::mkdir_recursive(&abc.pop().pop().push(".rust"), U_RWX));
-    assert!(do change_dir_locked(&dir.push("A").push("B").push("C")) {
-        let p = rust_path();
-        let cwd = os::getcwd().push(".rust");
-        let parent = cwd.pop().pop().push(".rust");
-        let grandparent = cwd.pop().pop().pop().push(".rust");
-        assert!(p.contains(&cwd));
-        assert!(p.contains(&parent));
-        assert!(p.contains(&grandparent));
-        for a_path in p.iter() {
-            assert!(!a_path.components.is_empty());
-        }
-    });
+    assert!(os::change_dir(abc));
+
+    let p = rust_path();
+    let cwd = os::getcwd().push(".rust");
+    let parent = cwd.pop().pop().push(".rust");
+    let grandparent = cwd.pop().pop().pop().push(".rust");
+    assert!(p.contains(&cwd));
+    assert!(p.contains(&parent));
+    assert!(p.contains(&grandparent));
+    for a_path in p.iter() {
+        assert!(!a_path.components.is_empty());
+    }
 }
 
 #[test]

src/libstd/unstable/mod.rs

@@ -68,59 +68,6 @@ fn test_run_in_bare_thread_exchange() {
     }
 }
 
-
-/// Changes the current working directory to the specified
-/// path while acquiring a global lock, then calls `action`.
-/// If the change is successful, releases the lock and restores the
-/// CWD to what it was before, returning true.
-/// Returns false if the directory doesn't exist or if the directory change
-/// is otherwise unsuccessful.
-///
-/// This is used by test cases to avoid cwd races.
-///
-/// # Safety Note
-///
-/// This uses a pthread mutex so descheduling in the action callback
-/// can lead to deadlock. Calling change_dir_locked recursively will
-/// also deadlock.
-pub fn change_dir_locked(p: &Path, action: &fn()) -> bool {
-    #[fixed_stack_segment]; #[inline(never)];
-
-    use os;
-    use os::change_dir;
-    use unstable::sync::atomically;
-    use unstable::finally::Finally;
-
-    unsafe {
-        // This is really sketchy. Using a pthread mutex so descheduling
-        // in the `action` callback can cause deadlock. Doing it in
-        // `task::atomically` to try to avoid that, but ... I don't know
-        // this is all bogus.
-        return do atomically {
-            rust_take_change_dir_lock();
-
-            do (||{
-                let old_dir = os::getcwd();
-                if change_dir(p) {
-                    action();
-                    change_dir(&old_dir)
-                }
-                else {
-                    false
-                }
-            }).finally {
-                rust_drop_change_dir_lock();
-            }
-        }
-    }
-
-    extern {
-        fn rust_take_change_dir_lock();
-        fn rust_drop_change_dir_lock();
-    }
-}
-
-
 /// Dynamically inquire about whether we're running under V.
 /// You should usually not use this unless your test definitely
 /// can't run correctly un-altered. Valgrind is there to help

src/rt/rust_builtin.cpp

@@ -603,18 +603,6 @@ rust_get_global_args_ptr() {
     return &global_args_ptr;
 }
 
-static lock_and_signal change_dir_lock;
-
-extern "C" CDECL void
-rust_take_change_dir_lock() {
-    change_dir_lock.lock();
-}
-
-extern "C" CDECL void
-rust_drop_change_dir_lock() {
-    change_dir_lock.unlock();
-}
-
 // Used by i386 __morestack
 extern "C" CDECL uintptr_t
 rust_get_task() {

src/rt/rustrt.def.in

@@ -186,8 +186,6 @@ rust_get_num_cpus
 rust_get_global_args_ptr
 rust_take_global_args_lock
 rust_drop_global_args_lock
-rust_take_change_dir_lock
-rust_drop_change_dir_lock
 rust_take_linenoise_lock
 rust_drop_linenoise_lock
 rust_get_test_int

src/test/run-pass/glob-std.rs

@@ -19,9 +19,9 @@ use std::{io, os, unstable};
 
 pub fn main() {
     fn change_then_remove(p: &Path, f: &fn()) {
-        do (|| {
-            unstable::change_dir_locked(p, || f());
-        }).finally {
+        assert!(os::change_dir(p));
+
+        do f.finally {
             os::remove_dir_recursive(p);
         }
     }

src/test/run-pass/tempfile.rs

@@ -0,0 +1,100 @@
+// Copyright 2013 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// xfail-fast windows doesn't like 'extern mod extra'
+
+// These tests are here to exercise the functionality of the `tempfile` module.
+// One might expect these tests to be located in that module, but sadly they
+// cannot. The tests need to invoke `os::change_dir` which cannot be done in the
+// normal test infrastructure. If the tests change the current working
+// directory, then *all* tests which require relative paths suddenly break b/c
+// they're in a different location than before. Hence, these tests are all run
+// serially here.
+
+extern mod extra;
+
+use extra::tempfile::mkdtemp;
+use std::os;
+use std::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR};
+
+fn test_mkdtemp() {
+    let p = mkdtemp(&Path("."), "foobar").unwrap();
+    os::remove_dir(&p);
+    assert!(p.to_str().ends_with("foobar"));
+}
+
+// Ideally these would be in std::os but then core would need
+// to depend on std
+fn recursive_mkdir_rel() {
+    let path = Path("frob");
+    debug!("recursive_mkdir_rel: Making: %s in cwd %s [%?]", path.to_str(),
+           os::getcwd().to_str(),
+           os::path_exists(&path));
+    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));
+}
+
+fn recursive_mkdir_dot() {
+    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));
+}
+
+fn recursive_mkdir_rel_2() {
+    let path = Path("./frob/baz");
+    debug!("recursive_mkdir_rel_2: Making: %s in cwd %s [%?]", path.to_str(),
+           os::getcwd().to_str(), os::path_exists(&path));
+    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!("recursive_mkdir_rel_2: 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()));
+}
+
+// Ideally this would be in core, but needs mkdtemp
+pub fn test_rmdir_recursive_ok() {
+    let rwx = (S_IRUSR | S_IWUSR | S_IXUSR) as i32;
+
+    let tmpdir = mkdtemp(&os::tmpdir(), "test").expect("test_rmdir_recursive_ok: \
+                                        couldn't create temp dir");
+    let root = tmpdir.push("foo");
+
+    debug!("making %s", root.to_str());
+    assert!(os::make_dir(&root, rwx));
+    assert!(os::make_dir(&root.push("foo"), rwx));
+    assert!(os::make_dir(&root.push("foo").push("bar"), rwx));
+    assert!(os::make_dir(&root.push("foo").push("bar").push("blat"), rwx));
+    assert!(os::remove_dir_recursive(&root));
+    assert!(!os::path_exists(&root));
+    assert!(!os::path_exists(&root.push("bar")));
+    assert!(!os::path_exists(&root.push("bar").push("blat")));
+}
+
+fn in_tmpdir(f: &fn()) {
+    let tmpdir = mkdtemp(&os::tmpdir(), "test").expect("can't make tmpdir");
+    assert!(os::change_dir(&tmpdir));
+
+    f();
+}
+
+fn main() {
+    in_tmpdir(test_mkdtemp);
+    in_tmpdir(recursive_mkdir_rel);
+    in_tmpdir(recursive_mkdir_dot);
+    in_tmpdir(recursive_mkdir_rel_2);
+    in_tmpdir(test_rmdir_recursive_ok);
+}