summary refs log tree commit diff
path: root/src
diff options
context:
space:
mode:
authorAlex Crichton <alex@alexcrichton.com>2013-09-13 21:41:28 -0700
committerAlex Crichton <alex@alexcrichton.com>2013-09-13 21:58:00 -0700
commit0af2bd829e6aaab4faf2cc135bd8b01db728417b (patch)
tree3005f9d734e173f60ba589e0808b130a04e93bee /src
parenta241deb97931b7c993e88c600d2b35912730a7e8 (diff)
downloadrust-0af2bd829e6aaab4faf2cc135bd8b01db728417b.tar.gz
rust-0af2bd829e6aaab4faf2cc135bd8b01db728417b.zip
Remove all usage of change_dir_locked
While usage of change_dir_locked is synchronized against itself, it's not
synchronized against other relative path usage, so I'm of the opinion that it
just really doesn't help in running tests. In order to prevent the problems that
have been cropping up, this completely removes the function.

All existing tests (except one) using it have been moved to run-pass tests where
they get their own process and don't need to be synchronized with anyone else.

There is one now-ignored rustpkg test because when I moved it to a run-pass test
apparently run-pass isn't set up to have 'extern mod rustc' (it ends up having
linkage failures).
Diffstat (limited to 'src')
-rw-r--r--src/libextra/tempfile.rs97
-rw-r--r--src/librustpkg/tests.rs27
-rw-r--r--src/libstd/unstable/mod.rs53
-rw-r--r--src/rt/rust_builtin.cpp12
-rw-r--r--src/rt/rustrt.def.in2
-rw-r--r--src/test/run-pass/glob-std.rs6
-rw-r--r--src/test/run-pass/tempfile.rs100
7 files changed, 119 insertions, 178 deletions
diff --git a/src/libextra/tempfile.rs b/src/libextra/tempfile.rs
index cb1bde14056..9044c23ff7a 100644
--- a/src/libextra/tempfile.rs
+++ b/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
diff --git a/src/librustpkg/tests.rs b/src/librustpkg/tests.rs
index e5ca38daeb5..30a5e438f34 100644
--- a/src/librustpkg/tests.rs
+++ b/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]
diff --git a/src/libstd/unstable/mod.rs b/src/libstd/unstable/mod.rs
index 51de3caf2ae..e16e6384a4f 100644
--- a/src/libstd/unstable/mod.rs
+++ b/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
diff --git a/src/rt/rust_builtin.cpp b/src/rt/rust_builtin.cpp
index 1871e7f36b3..4b718303f2c 100644
--- a/src/rt/rust_builtin.cpp
+++ b/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() {
diff --git a/src/rt/rustrt.def.in b/src/rt/rustrt.def.in
index 55f0195c192..4cbee0dcbd0 100644
--- a/src/rt/rustrt.def.in
+++ b/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
diff --git a/src/test/run-pass/glob-std.rs b/src/test/run-pass/glob-std.rs
index 73b3b16fcae..e0c0fdd396b 100644
--- a/src/test/run-pass/glob-std.rs
+++ b/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);
         }
     }
diff --git a/src/test/run-pass/tempfile.rs b/src/test/run-pass/tempfile.rs
new file mode 100644
index 00000000000..cde6cc102af
--- /dev/null
+++ b/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);
+}