Auto merge of #31360 - pitdicker:fs_tests_cleanup, r=alexcrichton

- use `symlink_file` and `symlink_dir` instead of the old `soft_link`
- create a junction instead of a directory symlink for testing recursive_rmdir (as it causes the
  same troubles, but can be created by users without `SeCreateSymbolicLinkPrivilege`)
- `remove_dir_all` was unable to remove directory symlinks and junctions
- only run tests that create symlinks if we have the right permissions.
- rename `Path2` to `Path`
- remove the global `#[allow(deprecated)]` and outdated comments
- After factoring out `create_junction()` from the test `directory_junctions_are_directories` and
  removing needlessly complex code, what I was left with was:
  ```
  #[test]
  #[cfg(windows)]
  fn directory_junctions_are_directories() {
      use sys::fs::create_junction;

      let tmpdir = tmpdir();

      let foo = tmpdir.join("foo");
      let bar = tmpdir.join("bar");

      fs::create_dir(&foo).unwrap();
      check!(create_junction(&foo, &bar));
      assert!(bar.metadata().unwrap().is_dir());
  }
  ```
  It test whether a junction is a directory instead of a reparse point. But it actually test the
  target of the junction (which is a directory if it exists) instead of the junction itself, which
  should always be a symlink. So this test is invalid, and I expect it only exists because the
  author was suprised by it. So I removed it.

Some things that do not yet work right:
- relative symlinks do not accept forward slashes
- the conversion of paths for `create_junction` is hacky
- `remove_dir_all` now messes with the internal data of `FileAttr` to be able to remove symlinks.
  We should add some method like `is_symlink_dir()` to it, so code outside the standard library
  can see the difference between file and directory symlinks too.
This commit is contained in:
bors 2016-02-04 18:48:41 +00:00
commit d0ef740266
3 changed files with 191 additions and 182 deletions

View File

@ -1265,20 +1265,7 @@ pub fn remove_dir<P: AsRef<Path>>(path: P) -> io::Result<()> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn remove_dir_all<P: AsRef<Path>>(path: P) -> io::Result<()> {
_remove_dir_all(path.as_ref())
}
fn _remove_dir_all(path: &Path) -> io::Result<()> {
for child in try!(read_dir(path)) {
let child = try!(child).path();
let stat = try!(symlink_metadata(&*child));
if stat.is_dir() {
try!(remove_dir_all(&*child));
} else {
try!(remove_file(&*child));
}
}
remove_dir(path)
fs_imp::remove_dir_all(path.as_ref())
}
/// Returns an iterator over the entries within a directory.
@ -1489,19 +1476,27 @@ impl AsInnerMut<fs_imp::DirBuilder> for DirBuilder {
#[cfg(test)]
mod tests {
#![allow(deprecated)] //rand
use prelude::v1::*;
use io::prelude::*;
use env;
use fs::{self, File, OpenOptions};
use io::{ErrorKind, SeekFrom};
use path::PathBuf;
use path::Path as Path2;
use path::{Path, PathBuf};
use rand::{self, StdRng, Rng};
use str;
#[cfg(windows)]
use os::windows::fs::{symlink_dir, symlink_file};
#[cfg(windows)]
use sys::fs::symlink_junction;
#[cfg(unix)]
use os::unix::fs::symlink as symlink_dir;
#[cfg(unix)]
use os::unix::fs::symlink as symlink_file;
#[cfg(unix)]
use os::unix::fs::symlink as symlink_junction;
macro_rules! check { ($e:expr) => (
match $e {
Ok(t) => t,
@ -1525,7 +1520,7 @@ mod tests {
p.join(path)
}
fn path<'a>(&'a self) -> &'a Path2 {
fn path<'a>(&'a self) -> &'a Path {
let TempDir(ref p) = *self;
p
}
@ -1548,6 +1543,27 @@ mod tests {
TempDir(ret)
}
// Several test fail on windows if the user does not have permission to
// create symlinks (the `SeCreateSymbolicLinkPrivilege`). Instead of
// disabling these test on Windows, use this function to test whether we
// have permission, and return otherwise. This way, we still don't run these
// tests most of the time, but at least we do if the user has the right
// permissions.
pub fn got_symlink_permission(tmpdir: &TempDir) -> bool {
if cfg!(unix) { return true }
let link = tmpdir.join("some_hopefully_unique_link_name");
match symlink_file(r"nonexisting_target", link) {
Ok(_) => true,
Err(ref err) =>
if err.to_string().contains("A required privilege is not held by the client.") {
false
} else {
true
}
}
}
#[test]
fn file_test_io_smoke_test() {
let message = "it's alright. have a good time";
@ -1578,8 +1594,9 @@ mod tests {
if cfg!(unix) {
error!(result, "o such file or directory");
}
// error!(result, "couldn't open path as file");
// error!(result, format!("path={}; mode=open; access=read", filename.display()));
if cfg!(windows) {
error!(result, "The system cannot find the file specified");
}
}
#[test]
@ -1592,8 +1609,9 @@ mod tests {
if cfg!(unix) {
error!(result, "o such file or directory");
}
// error!(result, "couldn't unlink path");
// error!(result, format!("path={}", filename.display()));
if cfg!(windows) {
error!(result, "The system cannot find the file specified");
}
}
#[test]
@ -1799,6 +1817,7 @@ mod tests {
}
#[test]
#[allow(deprecated)]
fn file_test_walk_dir() {
let tmpdir = tmpdir();
let dir = &tmpdir.join("walk_dir");
@ -1855,19 +1874,13 @@ mod tests {
let result = fs::create_dir_all(&file);
assert!(result.is_err());
// error!(result, "couldn't recursively mkdir");
// error!(result, "couldn't create directory");
// error!(result, "mode=0700");
// error!(result, format!("path={}", file.display()));
}
#[test]
fn recursive_mkdir_slash() {
check!(fs::create_dir_all(&Path2::new("/")));
check!(fs::create_dir_all(&Path::new("/")));
}
// FIXME(#12795) depends on lstat to work on windows
#[cfg(not(windows))]
#[test]
fn recursive_rmdir() {
let tmpdir = tmpdir();
@ -1879,7 +1892,7 @@ mod tests {
check!(fs::create_dir_all(&dtt));
check!(fs::create_dir_all(&d2));
check!(check!(File::create(&canary)).write(b"foo"));
check!(fs::soft_link(&d2, &dt.join("d2")));
check!(symlink_junction(&d2, &dt.join("d2")));
check!(fs::remove_dir_all(&d1));
assert!(!d1.is_dir());
@ -1888,8 +1901,8 @@ mod tests {
#[test]
fn unicode_path_is_dir() {
assert!(Path2::new(".").is_dir());
assert!(!Path2::new("test/stdtest/fs.rs").is_dir());
assert!(Path::new(".").is_dir());
assert!(!Path::new("test/stdtest/fs.rs").is_dir());
let tmpdir = tmpdir();
@ -1907,21 +1920,21 @@ mod tests {
#[test]
fn unicode_path_exists() {
assert!(Path2::new(".").exists());
assert!(!Path2::new("test/nonexistent-bogus-path").exists());
assert!(Path::new(".").exists());
assert!(!Path::new("test/nonexistent-bogus-path").exists());
let tmpdir = tmpdir();
let unicode = tmpdir.path();
let unicode = unicode.join(&format!("test-각丁ー再见"));
check!(fs::create_dir(&unicode));
assert!(unicode.exists());
assert!(!Path2::new("test/unicode-bogus-path-각丁ー再见").exists());
assert!(!Path::new("test/unicode-bogus-path-각丁ー再见").exists());
}
#[test]
fn copy_file_does_not_exist() {
let from = Path2::new("test/nonexistent-bogus-path");
let to = Path2::new("test/other-bogus-path");
let from = Path::new("test/nonexistent-bogus-path");
let to = Path::new("test/other-bogus-path");
match fs::copy(&from, &to) {
Ok(..) => panic!(),
@ -1935,7 +1948,7 @@ mod tests {
#[test]
fn copy_src_does_not_exist() {
let tmpdir = tmpdir();
let from = Path2::new("test/nonexistent-bogus-path");
let from = Path::new("test/nonexistent-bogus-path");
let to = tmpdir.join("out.txt");
check!(check!(File::create(&to)).write(b"hello"));
assert!(fs::copy(&from, &to).is_err());
@ -2026,19 +2039,17 @@ mod tests {
assert_eq!(v, b"carrot".to_vec());
}
#[cfg(not(windows))] // FIXME(#10264) operation not permitted?
#[test]
fn symlinks_work() {
let tmpdir = tmpdir();
if !got_symlink_permission(&tmpdir) { return };
let input = tmpdir.join("in.txt");
let out = tmpdir.join("out.txt");
check!(check!(File::create(&input)).write("foobar".as_bytes()));
check!(fs::soft_link(&input, &out));
// if cfg!(not(windows)) {
// assert_eq!(check!(lstat(&out)).kind, FileType::Symlink);
// assert_eq!(check!(out.lstat()).kind, FileType::Symlink);
// }
check!(symlink_file(&input, &out));
assert!(check!(out.symlink_metadata()).file_type().is_symlink());
assert_eq!(check!(fs::metadata(&out)).len(),
check!(fs::metadata(&input)).len());
let mut v = Vec::new();
@ -2046,14 +2057,17 @@ mod tests {
assert_eq!(v, b"foobar".to_vec());
}
#[cfg(not(windows))] // apparently windows doesn't like symlinks
#[test]
fn symlink_noexist() {
// Symlinks can point to things that don't exist
let tmpdir = tmpdir();
// symlinks can point to things that don't exist
check!(fs::soft_link(&tmpdir.join("foo"), &tmpdir.join("bar")));
assert_eq!(check!(fs::read_link(&tmpdir.join("bar"))),
tmpdir.join("foo"));
if !got_symlink_permission(&tmpdir) { return };
// Use a relative path for testing. Symlinks get normalized by Windows,
// so we may not get the same path back for absolute paths
check!(symlink_file(&"foo", &tmpdir.join("bar")));
assert_eq!(check!(fs::read_link(&tmpdir.join("bar"))).to_str().unwrap(),
"foo");
}
#[test]
@ -2346,9 +2360,10 @@ mod tests {
}
#[test]
#[cfg(not(windows))]
fn realpath_works() {
let tmpdir = tmpdir();
if !got_symlink_permission(&tmpdir) { return };
let tmpdir = fs::canonicalize(tmpdir.path()).unwrap();
let file = tmpdir.join("test");
let dir = tmpdir.join("test2");
@ -2357,8 +2372,8 @@ mod tests {
File::create(&file).unwrap();
fs::create_dir(&dir).unwrap();
fs::soft_link(&file, &link).unwrap();
fs::soft_link(&dir, &linkdir).unwrap();
symlink_file(&file, &link).unwrap();
symlink_dir(&dir, &linkdir).unwrap();
assert!(link.symlink_metadata().unwrap().file_type().is_symlink());
@ -2370,11 +2385,11 @@ mod tests {
}
#[test]
#[cfg(not(windows))]
fn realpath_works_tricky() {
let tmpdir = tmpdir();
let tmpdir = fs::canonicalize(tmpdir.path()).unwrap();
if !got_symlink_permission(&tmpdir) { return };
let tmpdir = fs::canonicalize(tmpdir.path()).unwrap();
let a = tmpdir.join("a");
let b = a.join("b");
let c = b.join("c");
@ -2385,8 +2400,14 @@ mod tests {
fs::create_dir_all(&b).unwrap();
fs::create_dir_all(&d).unwrap();
File::create(&f).unwrap();
fs::soft_link("../d/e", &c).unwrap();
fs::soft_link("../f", &e).unwrap();
if cfg!(not(windows)) {
symlink_dir("../d/e", &c).unwrap();
symlink_file("../f", &e).unwrap();
}
if cfg!(windows) {
symlink_dir(r"..\d\e", &c).unwrap();
symlink_file(r"..\f", &e).unwrap();
}
assert_eq!(fs::canonicalize(&c).unwrap(), f);
assert_eq!(fs::canonicalize(&e).unwrap(), f);
@ -2420,4 +2441,31 @@ mod tests {
let res = fs::read_dir("/path/that/does/not/exist");
assert_eq!(res.err().unwrap().kind(), ErrorKind::NotFound);
}
#[test]
fn create_dir_all_with_junctions() {
let tmpdir = tmpdir();
let target = tmpdir.join("target");
let junction = tmpdir.join("junction");
let b = junction.join("a/b");
let link = tmpdir.join("link");
let d = link.join("c/d");
fs::create_dir(&target).unwrap();
check!(symlink_junction(&target, &junction));
check!(fs::create_dir_all(&b));
// the junction itself is not a directory, but `is_dir()` on a Path
// follows links
assert!(junction.is_dir());
assert!(b.exists());
if !got_symlink_permission(&tmpdir) { return };
check!(symlink_dir(&target, &link));
check!(fs::create_dir_all(&d));
assert!(link.is_dir());
assert!(d.exists());
}
}

View File

@ -554,6 +554,18 @@ pub fn rmdir(p: &Path) -> io::Result<()> {
Ok(())
}
pub fn remove_dir_all(path: &Path) -> io::Result<()> {
for child in try!(readdir(path)) {
let child = try!(child);
if try!(child.file_type()).is_dir() {
try!(remove_dir_all(&child.path()));
} else {
try!(unlink(&child.path()));
}
}
rmdir(path)
}
pub fn readlink(p: &Path) -> io::Result<PathBuf> {
let c_path = try!(cstr(p));
let p = c_path.as_ptr();

View File

@ -35,7 +35,7 @@ pub struct FileAttr {
#[derive(Copy, Clone, PartialEq, Eq, Hash)]
pub enum FileType {
Dir, File, Symlink, ReparsePoint, MountPoint,
Dir, File, SymlinkFile, SymlinkDir, ReparsePoint, MountPoint,
}
pub struct ReadDir {
@ -450,23 +450,30 @@ impl FilePermissions {
impl FileType {
fn new(attrs: c::DWORD, reparse_tag: c::DWORD) -> FileType {
if attrs & c::FILE_ATTRIBUTE_REPARSE_POINT != 0 {
match reparse_tag {
c::IO_REPARSE_TAG_SYMLINK => FileType::Symlink,
c::IO_REPARSE_TAG_MOUNT_POINT => FileType::MountPoint,
_ => FileType::ReparsePoint,
}
} else if attrs & c::FILE_ATTRIBUTE_DIRECTORY != 0 {
FileType::Dir
} else {
FileType::File
match (attrs & c::FILE_ATTRIBUTE_DIRECTORY != 0,
attrs & c::FILE_ATTRIBUTE_REPARSE_POINT != 0,
reparse_tag) {
(false, false, _) => FileType::File,
(true, false, _) => FileType::Dir,
(false, true, c::IO_REPARSE_TAG_SYMLINK) => FileType::SymlinkFile,
(true, true, c::IO_REPARSE_TAG_SYMLINK) => FileType::SymlinkDir,
(true, true, c::IO_REPARSE_TAG_MOUNT_POINT) => FileType::MountPoint,
(_, true, _) => FileType::ReparsePoint,
// Note: if a _file_ has a reparse tag of the type IO_REPARSE_TAG_MOUNT_POINT it is
// invalid, as junctions always have to be dirs. We set the filetype to ReparsePoint
// to indicate it is something symlink-like, but not something you can follow.
}
}
pub fn is_dir(&self) -> bool { *self == FileType::Dir }
pub fn is_file(&self) -> bool { *self == FileType::File }
pub fn is_symlink(&self) -> bool {
*self == FileType::Symlink || *self == FileType::MountPoint
*self == FileType::SymlinkFile ||
*self == FileType::SymlinkDir ||
*self == FileType::MountPoint
}
pub fn is_symlink_dir(&self) -> bool {
*self == FileType::SymlinkDir || *self == FileType::MountPoint
}
}
@ -523,6 +530,21 @@ pub fn rmdir(p: &Path) -> io::Result<()> {
Ok(())
}
pub fn remove_dir_all(path: &Path) -> io::Result<()> {
for child in try!(readdir(path)) {
let child = try!(child);
let child_type = try!(child.file_type());
if child_type.is_dir() {
try!(remove_dir_all(&child.path()));
} else if child_type.is_symlink_dir() {
try!(rmdir(&child.path()));
} else {
try!(unlink(&child.path()));
}
}
rmdir(path)
}
pub fn readlink(p: &Path) -> io::Result<PathBuf> {
let file = try!(File::open_reparse_point(p, false));
file.readlink()
@ -641,124 +663,51 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
Ok(size as u64)
}
#[test]
fn directory_junctions_are_directories() {
use ffi::OsStr;
use env;
use rand::{self, Rng};
use vec::Vec;
macro_rules! t {
($e:expr) => (match $e {
Ok(e) => e,
Err(e) => panic!("{} failed with: {}", stringify!($e), e),
})
}
#[allow(dead_code)]
pub fn symlink_junction<P: AsRef<Path>, Q: AsRef<Path>>(src: P, dst: Q) -> io::Result<()> {
symlink_junction_inner(src.as_ref(), dst.as_ref())
}
// Creating a directory junction on windows involves dealing with reparse
// points and the DeviceIoControl function, and this code is a skeleton of
// what can be found here:
//
// http://www.flexhex.com/docs/articles/hard-links.phtml
#[allow(dead_code)]
fn symlink_junction_inner(target: &Path, junction: &Path) -> io::Result<()> {
let d = DirBuilder::new();
let p = env::temp_dir();
let mut r = rand::thread_rng();
let ret = p.join(&format!("rust-{}", r.next_u32()));
let foo = ret.join("foo");
let bar = ret.join("bar");
t!(d.mkdir(&ret));
t!(d.mkdir(&foo));
t!(d.mkdir(&bar));
try!(d.mkdir(&junction));
let f = try!(File::open_reparse_point(junction, true));
let h = f.handle().raw();
t!(create_junction(&bar, &foo));
let metadata = stat(&bar);
t!(delete_junction(&bar));
t!(rmdir(&foo));
t!(rmdir(&bar));
t!(rmdir(&ret));
let metadata = t!(metadata);
assert!(metadata.file_type().is_dir());
// Creating a directory junction on windows involves dealing with reparse
// points and the DeviceIoControl function, and this code is a skeleton of
// what can be found here:
//
// http://www.flexhex.com/docs/articles/hard-links.phtml
fn create_junction(src: &Path, dst: &Path) -> io::Result<()> {
let f = try!(opendir(src, true));
let h = f.handle().raw();
unsafe {
let mut data = [0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
let mut db = data.as_mut_ptr()
as *mut c::REPARSE_MOUNTPOINT_DATA_BUFFER;
let buf = &mut (*db).ReparseTarget as *mut _;
let mut i = 0;
let v = br"\??\";
let v = v.iter().map(|x| *x as u16);
for c in v.chain(dst.as_os_str().encode_wide()) {
*buf.offset(i) = c;
i += 1;
}
*buf.offset(i) = 0;
unsafe {
let mut data = [0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
let mut db = data.as_mut_ptr()
as *mut c::REPARSE_MOUNTPOINT_DATA_BUFFER;
let buf = &mut (*db).ReparseTarget as *mut _;
let mut i = 0;
// FIXME: this conversion is very hacky
let v = br"\??\";
let v = v.iter().map(|x| *x as u16);
for c in v.chain(target.as_os_str().encode_wide()) {
*buf.offset(i) = c;
i += 1;
(*db).ReparseTag = c::IO_REPARSE_TAG_MOUNT_POINT;
(*db).ReparseTargetMaximumLength = (i * 2) as c::WORD;
(*db).ReparseTargetLength = ((i - 1) * 2) as c::WORD;
(*db).ReparseDataLength =
(*db).ReparseTargetLength as c::DWORD + 12;
let mut ret = 0;
cvt(c::DeviceIoControl(h as *mut _,
c::FSCTL_SET_REPARSE_POINT,
data.as_ptr() as *mut _,
(*db).ReparseDataLength + 8,
ptr::null_mut(), 0,
&mut ret,
ptr::null_mut())).map(|_| ())
}
}
*buf.offset(i) = 0;
i += 1;
(*db).ReparseTag = c::IO_REPARSE_TAG_MOUNT_POINT;
(*db).ReparseTargetMaximumLength = (i * 2) as c::WORD;
(*db).ReparseTargetLength = ((i - 1) * 2) as c::WORD;
(*db).ReparseDataLength =
(*db).ReparseTargetLength as c::DWORD + 12;
fn opendir(p: &Path, write: bool) -> io::Result<File> {
unsafe {
let mut token = ptr::null_mut();
let mut tp: c::TOKEN_PRIVILEGES = mem::zeroed();
try!(cvt(c::OpenProcessToken(c::GetCurrentProcess(),
c::TOKEN_ADJUST_PRIVILEGES,
&mut token)));
let name: &OsStr = if write {
"SeRestorePrivilege".as_ref()
} else {
"SeBackupPrivilege".as_ref()
};
let name = name.encode_wide().chain(Some(0)).collect::<Vec<_>>();
try!(cvt(c::LookupPrivilegeValueW(ptr::null(),
name.as_ptr(),
&mut tp.Privileges[0].Luid)));
tp.PrivilegeCount = 1;
tp.Privileges[0].Attributes = c::SE_PRIVILEGE_ENABLED;
let size = mem::size_of::<c::TOKEN_PRIVILEGES>() as c::DWORD;
try!(cvt(c::AdjustTokenPrivileges(token, c::FALSE, &mut tp, size,
ptr::null_mut(), ptr::null_mut())));
try!(cvt(c::CloseHandle(token)));
File::open_reparse_point(p, write)
}
}
fn delete_junction(p: &Path) -> io::Result<()> {
unsafe {
let f = try!(opendir(p, true));
let h = f.handle().raw();
let mut data = [0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
let mut db = data.as_mut_ptr()
as *mut c::REPARSE_MOUNTPOINT_DATA_BUFFER;
(*db).ReparseTag = c::IO_REPARSE_TAG_MOUNT_POINT;
let mut bytes = 0;
cvt(c::DeviceIoControl(h as *mut _,
c::FSCTL_DELETE_REPARSE_POINT,
data.as_ptr() as *mut _,
(*db).ReparseDataLength + 8,
ptr::null_mut(), 0,
&mut bytes,
ptr::null_mut())).map(|_| ())
}
let mut ret = 0;
cvt(c::DeviceIoControl(h as *mut _,
c::FSCTL_SET_REPARSE_POINT,
data.as_ptr() as *mut _,
(*db).ReparseDataLength + 8,
ptr::null_mut(), 0,
&mut ret,
ptr::null_mut())).map(|_| ())
}
}