From 081e130132b835d60bd14a1304d0cdf830f247dc Mon Sep 17 00:00:00 2001 From: Garrett Berg Date: Thu, 22 Mar 2018 12:27:50 -0600 Subject: [PATCH 1/9] WIP #6: add absolute() method on PathArc, still need tests --- src/arc.rs | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/src/arc.rs b/src/arc.rs index c62de4e..6bb623e 100644 --- a/src/arc.rs +++ b/src/arc.rs @@ -9,7 +9,10 @@ use std::fmt; use std::fs; +use std::io; +use std::env; use std::ffi::OsStr; +use std::path::{Component, Prefix, PrefixComponent}; use std_prelude::*; use super::{Error, Result}; @@ -146,6 +149,105 @@ impl PathArc { pub fn as_path(&self) -> &Path { self.as_ref() } + + /// Convert the path to an absolute one, this is different from + /// [`canonicalize`] in that it _preserves_ symlinks. The destination + /// must exist. + /// + /// This function will strip any `.` components (`/a/./c` -> `/a/c`) + /// as well as resolve leading `..` using [`env::current_dir()`]. However, + /// any `..` in the middle of the path will cause `io::ErrorKind::InvalidInput` + /// to be returned. + /// + /// > On windows, this will always call `canonicalize()` on the first component + /// > to guarantee it is the canonicalized prefix. + /// + /// [`canonicalize`]: struct.PathAbs.html#method.ca + /// [`env::current_dir()`]: https://doc.rust-lang.org/std/env/fn.current_dir.html + pub fn absolute(&self) -> Result { + let mut got_nonparent = false; + let mut components = self.components(); + let mut stack: Vec = Vec::new(); + + fn to_os(c: Component) -> OsString { + c.as_os_str().to_os_string() + } + + macro_rules! pop_stack { [] => {{ + if let None = stack.pop() { + return Err(Error::new( + io::Error::new(io::ErrorKind::NotFound, ".. consumed root"), + "resolving absolute", + self.clone(), + )); + } + }}} + + // Get a canonicalized path from "root" + { + loop { + let component = match components.next() { + None => break, + Some(c) => c, + }; + + match component { + Component::CurDir => { + // ignore + continue; + } + Component::Prefix(_) | Component::RootDir => { + if cfg!(windows) { + // In windows we have to make sure the prefix is added to the root + // path. + let c = PathArc::new(component.as_os_str()).canonicalize()?; + stack.extend(c.components().map(to_os)); + } else { + stack.push(to_os(component)); + } + } + Component::ParentDir | Component::Normal(_) => { + // First item is either a ParentDir or Normal, in either + // case we need to get current_dir + let cwd = env::current_dir().map_err(|e| { + Error::new( + e, + "getting current_dir while resolving absolute", + self.clone(), + ) + })?; + let cwd = PathArc::new(cwd).canonicalize()?; + stack.extend(cwd.components().map(to_os)); + match component { + Component::ParentDir => pop_stack!(), + Component::Normal(_) => stack.push(to_os(component)), + _ => unreachable!(), + } + } + } + break; + } + } + + for component in components { + match component { + Component::CurDir => { /* ignore, probably impossible */ } + Component::Prefix(_) | Component::RootDir => unreachable!(), + Component::ParentDir => pop_stack!(), + Component::Normal(_) => stack.push(to_os(component)), + } + } + + if stack.is_empty() { + return Err(Error::new( + io::Error::new(io::ErrorKind::NotFound, "resolving resulted in empty path"), + "resolving absolute", + self.clone(), + )); + } + + Ok(PathAbs(PathArc(Arc::new(PathBuf::from_iter(stack))))) + } } impl fmt::Debug for PathArc { From 6abb1cf12b45db7c9c810bb1d71ecf74d7560e90 Mon Sep 17 00:00:00 2001 From: Garrett Berg Date: Thu, 22 Mar 2018 13:18:24 -0600 Subject: [PATCH 2/9] convert to absolute vs canonical for PathAbs --- src/abs.rs | 2 +- src/arc.rs | 12 ++++++---- src/dir.rs | 27 +++++++++++++++++----- src/file.rs | 25 ++++++++++++++++---- src/lib.rs | 1 - tests/test_absolute.rs | 52 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 102 insertions(+), 17 deletions(-) create mode 100644 tests/test_absolute.rs diff --git a/src/abs.rs b/src/abs.rs index bddbda5..e3d3e87 100644 --- a/src/abs.rs +++ b/src/abs.rs @@ -31,7 +31,7 @@ impl PathAbs { /// ``` pub fn new>(path: P) -> Result { let arc = PathArc::new(path); - arc.canonicalize() + arc.absolute() } /// Resolve the `PathAbs` as a `PathFile`. Return an error if it is not a file. diff --git a/src/arc.rs b/src/arc.rs index 6bb623e..0088a21 100644 --- a/src/arc.rs +++ b/src/arc.rs @@ -104,8 +104,6 @@ impl PathArc { /// Returns the canonical form of the path with all intermediate components normalized and /// symbolic links resolved. /// - /// > This is identical to `PathAbs::new(path)`. - /// /// This function is identical to [std::path::Path::canonicalize][0] except: /// - It returns a `PathAbs` object /// - It has error messages which include the action and the path @@ -216,8 +214,14 @@ impl PathArc { self.clone(), ) })?; - let cwd = PathArc::new(cwd).canonicalize()?; - stack.extend(cwd.components().map(to_os)); + // Only canonicalie the first entry in current_dir + let mut cwd_components = cwd.components(); + let root = PathArc::new( + cwd_components.next().expect("current_dir was empty"), + ).canonicalize()?; + stack.extend(root.components().map(to_os)); + stack.extend(cwd_components.map(to_os)); + match component { Component::ParentDir => pop_stack!(), Component::Normal(_) => stack.push(to_os(component)), diff --git a/src/dir.rs b/src/dir.rs index 56242c8..e18d175 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -287,21 +287,26 @@ impl PathDir { /// let dir = PathDir::create(example)?; /// let file = PathFile::create(dir.join("example.txt"))?; /// - /// dir.symlink(example_sym)?; - /// let dir_sym = PathDir::new(example_sym)?; + /// let dir_sym = dir.symlink(example_sym)?; + /// + /// // They have a different "absolute path" + /// assert_ne!(dir, dir_sym); + /// + /// // But they can be canonicalized to the same file. + /// let dir_can = dir_sym.canonicalize()?; + /// assert_eq!(dir, dir_can); /// - /// // They are canonicalized to the same file. - /// assert_eq!(dir, dir_sym); /// # Ok(()) } fn main() { try_main().unwrap() } /// ``` - pub fn symlink>(&self, dst: P) -> Result<()> { + pub fn symlink>(&self, dst: P) -> Result { symlink_dir(&self, &dst).map_err(|err| { Error::new( err, &format!("linking to {} from", dst.as_ref().display()), self.clone().into(), ) - }) + })?; + PathDir::new(dst) } /// Return a reference to a basic `std::path::Path` @@ -309,6 +314,16 @@ impl PathDir { self.as_ref() } + /// Returns the canonical form of the path with all intermediate components normalized and + /// symbolic links resolved. + /// + /// See [`PathAbs::canonicalize`] + /// + /// [`PathAbs::canonicalize`]: struct.PathAbs.html#method.canonicalize + pub fn canonicalize(&self) -> Result { + Ok(PathDir(self.0.canonicalize()?)) + } + /// Create a mock dir type. *For use in tests only*. /// /// See the docs for [`PathAbs::mock`](struct.PathAbs.html#method.mock) diff --git a/src/file.rs b/src/file.rs index dc274f4..5d8fa8a 100644 --- a/src/file.rs +++ b/src/file.rs @@ -375,21 +375,26 @@ impl PathFile { /// /// let contents = "This is some contents"; /// file.write_str(contents); - /// file.symlink(example_sym)?; - /// let file_sym = PathFile::new(example_sym)?; + /// let file_sym = file.symlink(example_sym)?; /// /// // They are canonicalized to the same file. - /// assert_eq!(file, file_sym); + /// assert_ne!(file, file_sym); + /// + /// + /// // But they can be canonicalized to the same file. + /// let file_can = file_sym.canonicalize()?; + /// assert_eq!(file, file_can); /// # Ok(()) } fn main() { try_main().unwrap() } /// ``` - pub fn symlink>(&self, dst: P) -> Result<()> { + pub fn symlink>(&self, dst: P) -> Result { symlink_file(&self, &dst).map_err(|err| { Error::new( err, &format!("linking to {} from", dst.as_ref().display()), self.clone().into(), ) - }) + })?; + PathFile::new(dst) } /// Remove (delete) the file from the filesystem, consuming self. @@ -424,6 +429,16 @@ impl PathFile { self.as_ref() } + /// Returns the canonical form of the path with all intermediate components normalized and + /// symbolic links resolved. + /// + /// See [`PathAbs::canonicalize`] + /// + /// [`PathAbs::canonicalize`]: struct.PathAbs.html#method.canonicalize + pub fn canonicalize(&self) -> Result { + Ok(PathFile(self.0.canonicalize()?)) + } + /// Create a mock file type. *For use in tests only*. /// /// See the docs for [`PathAbs::mock`](struct.PathAbs.html#method.mock) diff --git a/src/lib.rs b/src/lib.rs index 92a0263..66edee3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -372,7 +372,6 @@ mod tests { } #[test] - /// Tests to make sure the error messages look like we expect. fn sanity_errors() { let tmp_dir = TempDir::new("example").expect("create temp dir"); diff --git a/tests/test_absolute.rs b/tests/test_absolute.rs new file mode 100644 index 0000000..b1206ed --- /dev/null +++ b/tests/test_absolute.rs @@ -0,0 +1,52 @@ +//! The absolute paths have some gotchas that need to be tested. +//! +//! - Using the current working directory +//! - `..` paths that consume the "root" + +extern crate path_abs; +extern crate tempdir; +use path_abs::*; +use std::path::{Path, PathBuf}; +use std::env; + +#[test] +fn test_absolute() { + let tmp = tempdir::TempDir::new("ex").unwrap(); + let tmp_abs = PathArc::new(&tmp).canonicalize().unwrap(); + env::set_current_dir(&tmp_abs).unwrap(); + + // Create directory like: + // a/ + // + e/ -> b/c/d + // + b/ + // + c/ + // + d/ + + let a = PathDir::create(&tmp.path().join("a")).unwrap(); + let b = PathDir::create(&a.join("b")).unwrap(); + let c = PathDir::create(&b.join("c")).unwrap(); + let d = PathDir::create(&b.join("d")).unwrap(); + + let a_cwd = Path::new("a"); + let b_cwd = a.join("b"); + let c_cwd = b.join("c"); + let d_cwd = c.join("d"); + let e_cwd = a.join("e"); + + // create symbolic link from a/e -> a/b/c/d + let e_sym = d.symlink(&a.join("e")).unwrap(); + + assert_ne!(d, e_sym); + assert_eq!(d, e_sym.canonicalize().unwrap()); + + assert_eq!(b, PathDir::new(c.join("..")).unwrap()); + assert_eq!(a, PathDir::new(c.join("..").join("..")).unwrap()); + + let mut root_dots = tmp_abs.to_path_buf(); + for _ in 0..(tmp_abs.components().count() - 1) { + root_dots.push(".."); + } + let root = PathDir::new(root_dots).unwrap(); + assert_eq!(PathDir::new("/").unwrap(), root); + assert!(PathDir::new(root.join("..")).is_err()); +} From a0cab69cae04ec00d78fb9d6dea5a385addfb481 Mon Sep 17 00:00:00 2001 From: Garrett Berg Date: Thu, 22 Mar 2018 13:45:58 -0600 Subject: [PATCH 3/9] fix as_ref bug --- src/arc.rs | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/arc.rs b/src/arc.rs index 0088a21..d32c653 100644 --- a/src/arc.rs +++ b/src/arc.rs @@ -152,13 +152,17 @@ impl PathArc { /// [`canonicalize`] in that it _preserves_ symlinks. The destination /// must exist. /// - /// This function will strip any `.` components (`/a/./c` -> `/a/c`) - /// as well as resolve leading `..` using [`env::current_dir()`]. However, - /// any `..` in the middle of the path will cause `io::ErrorKind::InvalidInput` - /// to be returned. + /// This function will: + /// - Use [`env::current_dir()`] to resolve relative paths. + /// - Strip any `.` components (`/a/./c` -> `/a/c`) + /// - Resolve `..` _semantically_ (not using the file system). So, `a/b/c/../d => a/b/d` will + /// _always_ be true regardless of symlinks. If you want symlinks correctly resolved, use + /// `canonicalize()` instead. /// - /// > On windows, this will always call `canonicalize()` on the first component - /// > to guarantee it is the canonicalized prefix. + /// > On windows, this will always call `canonicalize()` on the first component to guarantee + /// > it is the correct canonicalized prefix. + /// + /// > In linux, the only syscall this can make is to get the current_dir for relative paths. /// /// [`canonicalize`]: struct.PathAbs.html#method.ca /// [`env::current_dir()`]: https://doc.rust-lang.org/std/env/fn.current_dir.html @@ -196,11 +200,16 @@ impl PathArc { } Component::Prefix(_) | Component::RootDir => { if cfg!(windows) { - // In windows we have to make sure the prefix is added to the root - // path. + // In windows we have to make sure the prefix is the correct form. let c = PathArc::new(component.as_os_str()).canonicalize()?; stack.extend(c.components().map(to_os)); } else { + // In linux, `Root` is the only prefix + if cfg!(unix) { + if let Component::Prefix(_) = component { + panic!("Component::Prefix in unix"); + } + } stack.push(to_os(component)); } } @@ -214,12 +223,14 @@ impl PathArc { self.clone(), ) })?; - // Only canonicalie the first entry in current_dir let mut cwd_components = cwd.components(); - let root = PathArc::new( - cwd_components.next().expect("current_dir was empty"), - ).canonicalize()?; - stack.extend(root.components().map(to_os)); + if cfg!(windows) { + // For windows, canonicalize the first entry in current_dir + let root = PathArc::new( + cwd_components.next().expect("current_dir was empty").as_os_str(), + ).canonicalize()?; + stack.extend(root.components().map(to_os)); + } stack.extend(cwd_components.map(to_os)); match component { From 8273c6f66f8117bc4afbf8399b6bec959ad4d445 Mon Sep 17 00:00:00 2001 From: Garrett Berg Date: Thu, 22 Mar 2018 14:13:45 -0600 Subject: [PATCH 4/9] fix windows root/prefix bug --- src/arc.rs | 10 +++++++++- src/ty.rs | 6 +----- tests/test_absolute.rs | 8 ++++++++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/arc.rs b/src/arc.rs index d32c653..095b51a 100644 --- a/src/arc.rs +++ b/src/arc.rs @@ -247,7 +247,15 @@ impl PathArc { for component in components { match component { Component::CurDir => { /* ignore, probably impossible */ } - Component::Prefix(_) | Component::RootDir => unreachable!(), + Component::Prefix(_) => unreachable!(), + Component::RootDir => { + if cfg!(unix) { + unreachable!("root is already handled on unix"); + } + // This is actually possible on windows because root is distinct + // from prefix (?) + stack.push(to_os(component)); + } Component::ParentDir => pop_stack!(), Component::Normal(_) => stack.push(to_os(component)), } diff --git a/src/ty.rs b/src/ty.rs index 4de79e5..b8ce77e 100644 --- a/src/ty.rs +++ b/src/ty.rs @@ -54,11 +54,7 @@ impl PathType { } else if ty.is_dir() { Ok(PathType::Dir(PathDir(abs))) } else { - Err(Error::new( - io::Error::new(io::ErrorKind::InvalidInput, "path is not a dir or a file"), - "resolving", - abs.into(), - )) + unreachable!("rust docs: The fs::metadata function follows symbolic links") } } diff --git a/tests/test_absolute.rs b/tests/test_absolute.rs index b1206ed..a5f59b6 100644 --- a/tests/test_absolute.rs +++ b/tests/test_absolute.rs @@ -35,6 +35,8 @@ fn test_absolute() { // create symbolic link from a/e -> a/b/c/d let e_sym = d.symlink(&a.join("e")).unwrap(); + let ty = e_sym.symlink_metadata().unwrap().file_type(); + assert!(ty.is_symlink(), "{}", e_sym.display()); assert_ne!(d, e_sym); assert_eq!(d, e_sym.canonicalize().unwrap()); @@ -49,4 +51,10 @@ fn test_absolute() { let root = PathDir::new(root_dots).unwrap(); assert_eq!(PathDir::new("/").unwrap(), root); assert!(PathDir::new(root.join("..")).is_err()); + + + // just create a ty + let ty = PathType::new(&e_sym).unwrap(); + + let t = tmp.into_path(); } From 1b118299f6bc9566147bf3db76c6156fcc240fa3 Mon Sep 17 00:00:00 2001 From: Garrett Berg Date: Thu, 22 Mar 2018 16:11:55 -0600 Subject: [PATCH 5/9] set RUST_BACKTRACE in appveyor builds --- appveyor.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/appveyor.yml b/appveyor.yml index 37e6831..a1c9c74 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -42,6 +42,7 @@ branches: # channels to enable unstable features when building for nightly. Or you could add additional # matrix entries to test different combinations of features. environment: + RUST_BACKTRACE: 1 matrix: ### MSVC Toolchains ### From 50fc4bdad2a08d32e6f9c2680e457217cc225c76 Mon Sep 17 00:00:00 2001 From: Garrett Berg Date: Thu, 22 Mar 2018 16:26:05 -0600 Subject: [PATCH 6/9] fix up docs --- src/abs.rs | 2 +- src/arc.rs | 5 ++--- src/dir.rs | 11 +---------- src/edit.rs | 2 +- src/file.rs | 10 ++-------- src/lib.rs | 12 ++++++------ src/open.rs | 2 +- src/read.rs | 2 +- src/ty.rs | 3 +-- src/write.rs | 2 +- 10 files changed, 17 insertions(+), 34 deletions(-) diff --git a/src/abs.rs b/src/abs.rs index e3d3e87..4cc9670 100644 --- a/src/abs.rs +++ b/src/abs.rs @@ -13,7 +13,7 @@ use std_prelude::*; use super::{PathArc, PathDir, PathFile, Result}; #[derive(Clone, Eq, Hash, PartialEq, PartialOrd, Ord)] -/// An absolute ([canonicalized][1]) path that is guaranteed (when created) to exist. +/// An absolute (not _necessarily_ [canonicalized][1]) path that may or may not exist. /// /// [1]: https://doc.rust-lang.org/std/path/struct.Path.html?search=#method.canonicalize pub struct PathAbs(pub(crate) PathArc); diff --git a/src/arc.rs b/src/arc.rs index 095b51a..91b3c79 100644 --- a/src/arc.rs +++ b/src/arc.rs @@ -148,9 +148,8 @@ impl PathArc { self.as_ref() } - /// Convert the path to an absolute one, this is different from - /// [`canonicalize`] in that it _preserves_ symlinks. The destination - /// must exist. + /// Convert the path to an absolute one, this is different from [`canonicalize`] in that it + /// _preserves_ symlinks. The destination may or may not exist. /// /// This function will: /// - Use [`env::current_dir()`] to resolve relative paths. diff --git a/src/dir.rs b/src/dir.rs index e18d175..4eb3d01 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -61,11 +61,6 @@ impl PathDir { /// /// If the path is actually a file returns `io::ErrorKind::InvalidInput`. /// - /// > This does not call [`Path::cannonicalize()`][1], instead trusting that the input is - /// > already a fully qualified path. - /// - /// [1]: https://doc.rust-lang.org/std/path/struct.Path.html?search=#method.canonicalize - /// /// # Examples /// ```rust /// # extern crate path_abs; @@ -172,10 +167,6 @@ impl PathDir { /// List the contents of the directory, returning an iterator of `PathType`s. /// - /// > **Warning**: because `PathAbs` is the canonicalized path, symlinks are always resolved. - /// > This means that if the directory contains a symlink you may get a path from a completely - /// > _different directory_. - /// /// # Examples /// ```rust /// # extern crate path_abs; @@ -302,7 +293,7 @@ impl PathDir { symlink_dir(&self, &dst).map_err(|err| { Error::new( err, - &format!("linking to {} from", dst.as_ref().display()), + &format!("linking from {} to", dst.as_ref().display()), self.clone().into(), ) })?; diff --git a/src/edit.rs b/src/edit.rs index 9d58d22..2e00d5d 100644 --- a/src/edit.rs +++ b/src/edit.rs @@ -52,7 +52,7 @@ impl FileEdit { Ok(FileEdit(FileOpen::open(path, options)?)) } - /// Shortcut to open the file if the path is already canonicalized. + /// Shortcut to open the file if the path is already absolute. pub(crate) fn open_path(path: PathFile, mut options: fs::OpenOptions) -> Result { options.write(true); options.read(true); diff --git a/src/file.rs b/src/file.rs index 5d8fa8a..2abdd9d 100644 --- a/src/file.rs +++ b/src/file.rs @@ -41,11 +41,6 @@ impl PathFile { /// /// If the path is actually a dir returns `io::ErrorKind::InvalidInput`. /// - /// > This does not call [`Path::cannonicalize()`][1], instead trusting that the input is - /// > already a fully qualified path. - /// - /// [1]: https://doc.rust-lang.org/std/path/struct.Path.html?search=#method.canonicalize - /// /// # Examples /// ```rust /// # extern crate path_abs; @@ -377,10 +372,9 @@ impl PathFile { /// file.write_str(contents); /// let file_sym = file.symlink(example_sym)?; /// - /// // They are canonicalized to the same file. + /// // They have a different "absolute path" /// assert_ne!(file, file_sym); /// - /// /// // But they can be canonicalized to the same file. /// let file_can = file_sym.canonicalize()?; /// assert_eq!(file, file_can); @@ -390,7 +384,7 @@ impl PathFile { symlink_file(&self, &dst).map_err(|err| { Error::new( err, - &format!("linking to {} from", dst.as_ref().display()), + &format!("linking from {} to", dst.as_ref().display()), self.clone().into(), ) })?; diff --git a/src/lib.rs b/src/lib.rs index 66edee3..0e7f1cc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -57,12 +57,12 @@ //! - [`PathArc`](struct.PathArc.html): a reference counted `PathBuf` with methods reimplemented //! with better error messages. Use this for a generic serializable path that may or may //! not exist. -//! - [`PathAbs`](struct.PathAbs.html): a reference counted absolute (canonicalized) path that is -//! guaranteed (on initialization) to exist. -//! - [`PathFile`](struct.PathFile.html): a `PathAbs` that is guaranteed to be a file, with -//! associated methods. -//! - [`PathDir`](struct.PathDir.html): a `PathAbs` that is guaranteed to be a directory, with -//! associated methods. +//! - [`PathAbs`](struct.PathAbs.html): a reference counted absolute (_not necessarily_ +//! canonicalized) path that is not necessarily guaranteed to exist. +//! - [`PathFile`](struct.PathFile.html): a `PathAbs` that is guaranteed (at instantiation) to +//! exist and be a file, with associated methods. +//! - [`PathDir`](struct.PathDir.html): a `PathAbs` that is guaranteed (at instantiation) to exist +//! and be a directory, with associated methods. //! - [`PathType`](struct.PathType.html): an enum containing either a PathFile or a PathDir. //! Returned by [`PathDir::list`][dir_list] //! diff --git a/src/open.rs b/src/open.rs index 7706284..ce94510 100644 --- a/src/open.rs +++ b/src/open.rs @@ -36,7 +36,7 @@ impl FileOpen { }) } - /// Shortcut to open the file if the path is already canonicalized. + /// Shortcut to open the file if the path is already absolute. /// /// Typically you should use `PathFile::open` instead (i.e. `file.open(options)` or /// `file.read()`). diff --git a/src/read.rs b/src/read.rs index 3f40e9e..51f1d46 100644 --- a/src/read.rs +++ b/src/read.rs @@ -51,7 +51,7 @@ impl FileRead { Ok(FileRead(FileOpen::open(path, options)?)) } - /// Shortcut to open the file if the path is already canonicalized. + /// Shortcut to open the file if the path is already absolute. pub(crate) fn read_path(path: PathFile) -> Result { let mut options = fs::OpenOptions::new(); options.read(true); diff --git a/src/ty.rs b/src/ty.rs index b8ce77e..ca7dc67 100644 --- a/src/ty.rs +++ b/src/ty.rs @@ -20,8 +20,7 @@ use super::{PathAbs, PathArc, PathDir, PathFile}; /// - The items returned from `PathDir::list` /// - Serializing paths of different types. /// -/// > Note: symlinks are not supported because they are -/// > *impossible* for canonicalized paths. +/// Note that for symlinks, this returns the underlying file type. pub enum PathType { File(PathFile), Dir(PathDir), diff --git a/src/write.rs b/src/write.rs index 39509f7..0ef5e92 100644 --- a/src/write.rs +++ b/src/write.rs @@ -49,7 +49,7 @@ impl FileWrite { Ok(FileWrite(FileOpen::open(path, options)?)) } - /// Shortcut to open the file if the path is already canonicalized. + /// Shortcut to open the file if the path is already absolute. pub(crate) fn open_path( path_file: PathFile, mut options: fs::OpenOptions, From eb4928c1dfcfd8125afde9024c5e7b520ccd5eec Mon Sep 17 00:00:00 2001 From: Garrett Berg Date: Thu, 22 Mar 2018 16:54:42 -0600 Subject: [PATCH 7/9] add a basic windows test --- src/arc.rs | 45 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/src/arc.rs b/src/arc.rs index 91b3c79..92d7c6e 100644 --- a/src/arc.rs +++ b/src/arc.rs @@ -197,20 +197,31 @@ impl PathArc { // ignore continue; } - Component::Prefix(_) | Component::RootDir => { + Component::Prefix(prefix) => { + assert!(!cfg!(unix), "Component::Prefix in unix"); + match prefix.kind() { + Prefix::Disk(_) | Prefix::UNC(_, _) => { + // Make the prefix a more "standard" form + let c = PathArc::new(component.as_os_str()).canonicalize()?; + stack.extend(c.components().map(to_os)); + } + _ => { + // Already in the "most standardized" form + // TODO: some more testing to make sure that canoninicalize() + // cannot be called on these forms would be good + stack.push(to_os(component)); + } + } + } + Component::RootDir => { if cfg!(windows) { // In windows we have to make sure the prefix is the correct form. let c = PathArc::new(component.as_os_str()).canonicalize()?; stack.extend(c.components().map(to_os)); } else { - // In linux, `Root` is the only prefix - if cfg!(unix) { - if let Component::Prefix(_) = component { - panic!("Component::Prefix in unix"); - } - } stack.push(to_os(component)); } + } Component::ParentDir | Component::Normal(_) => { // First item is either a ParentDir or Normal, in either @@ -354,3 +365,23 @@ impl Into for PathArc { } } } + +#[test] +fn test_prefix_windows() { + fn f>(p: P) -> Result { + PathArc::new(p).absolute() + } + assert!(f(r"\\?\C:\blah\blah").is_ok()); + assert!(f(r"\blah\blah").is_ok()); + + // TODO: this is how to get the hostname, but getting the "share name" + // seems to be more difficult. + // let hostname = ::std::process::Command::new("hostname") + // .output() + // .expect("could not get hostname") + // .stdout; + // let hostname = ::std::str::from_utf8(&hostname).unwrap().trim(); + + // assert!(f(format!(r"\\{}\share", hostname)).is_ok()); + // assert!(f(format!(r"\\?\UNC\{}\share", hostname)).is_ok()); +} From ba7dedcc1d3bcca9a1034a5c72a6f4abca0c28b4 Mon Sep 17 00:00:00 2001 From: Garrett Berg Date: Thu, 22 Mar 2018 17:07:38 -0600 Subject: [PATCH 8/9] hopefully fix windows --- tests/test_absolute.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/tests/test_absolute.rs b/tests/test_absolute.rs index a5f59b6..8a76dfe 100644 --- a/tests/test_absolute.rs +++ b/tests/test_absolute.rs @@ -6,7 +6,7 @@ extern crate path_abs; extern crate tempdir; use path_abs::*; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::env; #[test] @@ -27,11 +27,11 @@ fn test_absolute() { let c = PathDir::create(&b.join("c")).unwrap(); let d = PathDir::create(&b.join("d")).unwrap(); - let a_cwd = Path::new("a"); - let b_cwd = a.join("b"); - let c_cwd = b.join("c"); - let d_cwd = c.join("d"); - let e_cwd = a.join("e"); + let _a_cwd = Path::new("a"); + let _b_cwd = a.join("b"); + let _c_cwd = b.join("c"); + let _d_cwd = c.join("d"); + let _e_cwd = a.join("e"); // create symbolic link from a/e -> a/b/c/d let e_sym = d.symlink(&a.join("e")).unwrap(); @@ -45,7 +45,12 @@ fn test_absolute() { assert_eq!(a, PathDir::new(c.join("..").join("..")).unwrap()); let mut root_dots = tmp_abs.to_path_buf(); - for _ in 0..(tmp_abs.components().count() - 1) { + let mut dots = tmp_abs.components().count() - 1; + if cfg!(windows) { + // windows has _two_ "roots", prefix _and_ "root". + dots -= 1; + } + for _ in 0..dots { root_dots.push(".."); } let root = PathDir::new(root_dots).unwrap(); @@ -54,7 +59,5 @@ fn test_absolute() { // just create a ty - let ty = PathType::new(&e_sym).unwrap(); - - let t = tmp.into_path(); + let _ = PathType::new(&e_sym).unwrap(); } From 365fda4efeb68711e90a02dcfa8e8aa4d8fa1a40 Mon Sep 17 00:00:00 2001 From: Garrett Berg Date: Thu, 22 Mar 2018 17:18:36 -0600 Subject: [PATCH 9/9] windows is fuuun --- src/arc.rs | 13 +++++++++++-- src/ty.rs | 3 +-- tests/test_absolute.rs | 10 +++++++--- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/arc.rs b/src/arc.rs index 92d7c6e..165a160 100644 --- a/src/arc.rs +++ b/src/arc.rs @@ -12,7 +12,7 @@ use std::fs; use std::io; use std::env; use std::ffi::OsStr; -use std::path::{Component, Prefix, PrefixComponent}; +use std::path::{Component, Prefix}; use std_prelude::*; use super::{Error, Result}; @@ -166,7 +166,6 @@ impl PathArc { /// [`canonicalize`]: struct.PathAbs.html#method.ca /// [`env::current_dir()`]: https://doc.rust-lang.org/std/env/fn.current_dir.html pub fn absolute(&self) -> Result { - let mut got_nonparent = false; let mut components = self.components(); let mut stack: Vec = Vec::new(); @@ -209,6 +208,11 @@ impl PathArc { // Already in the "most standardized" form // TODO: some more testing to make sure that canoninicalize() // cannot be called on these forms would be good + // + // Maybe it can be called when _both_ prefix and root are included? + // i.e. `\\?\C:` is not enough, need + // `\\?\C:\` (??) + // stack.push(to_os(component)); } } @@ -216,6 +220,10 @@ impl PathArc { Component::RootDir => { if cfg!(windows) { // In windows we have to make sure the prefix is the correct form. + // + // DAMNIT WINDOWS: + // The filename, directory name, or volume label syntax is + // incorrect. (os error 123) when canonicalizing \ let c = PathArc::new(component.as_os_str()).canonicalize()?; stack.extend(c.components().map(to_os)); } else { @@ -373,6 +381,7 @@ fn test_prefix_windows() { } assert!(f(r"\\?\C:\blah\blah").is_ok()); assert!(f(r"\blah\blah").is_ok()); + assert!(f(r"C:\blah\blah").is_ok()); // TODO: this is how to get the hostname, but getting the "share name" // seems to be more difficult. diff --git a/src/ty.rs b/src/ty.rs index ca7dc67..90e6c4c 100644 --- a/src/ty.rs +++ b/src/ty.rs @@ -5,10 +5,9 @@ * http://opensource.org/licenses/MIT>, at your option. This file may not be * copied, modified, or distributed except according to those terms. */ -use std::io; use std_prelude::*; -use super::{Error, Result}; +use super::Result; use super::{PathAbs, PathArc, PathDir, PathFile}; #[cfg_attr(feature = "serialize", derive(Serialize, Deserialize))] diff --git a/tests/test_absolute.rs b/tests/test_absolute.rs index 8a76dfe..22414c4 100644 --- a/tests/test_absolute.rs +++ b/tests/test_absolute.rs @@ -43,6 +43,8 @@ fn test_absolute() { assert_eq!(b, PathDir::new(c.join("..")).unwrap()); assert_eq!(a, PathDir::new(c.join("..").join("..")).unwrap()); + // just create a PathType + let _ = PathType::new(&e_sym).unwrap(); let mut root_dots = tmp_abs.to_path_buf(); let mut dots = tmp_abs.components().count() - 1; @@ -54,10 +56,12 @@ fn test_absolute() { root_dots.push(".."); } let root = PathDir::new(root_dots).unwrap(); - assert_eq!(PathDir::new("/").unwrap(), root); + if cfg!(windows) { + assert_eq!(PathDir::new("\\").unwrap(), root); + } else { + assert_eq!(PathDir::new("/").unwrap(), root); + } assert!(PathDir::new(root.join("..")).is_err()); - // just create a ty - let _ = PathType::new(&e_sym).unwrap(); }