path2: Update based on more review feedback

Standardize the is_sep() functions to be the same in both posix and
windows, and re-export from path. Update extra::glob to use this.

Remove the usage of either, as it's going away.

Move the WindowsPath-specific methods out of WindowsPath and make them
top-level functions of path::windows instead. This way you cannot
accidentally write code that will fail to compile on non-windows
architectures without typing ::windows anywhere.

Remove GenericPath::from_c_str() and just impl BytesContainer for
CString instead.

Remove .join_path() and .push_path() and just implement BytesContainer
for Path instead.

Remove FilenameDisplay and add a boolean flag to Display instead.

Remove .each_parent(). It only had one caller, so just inline its
definition there.

Author: lilyball

src/compiletest/runtest.rs

@@ -189,7 +189,7 @@ fn run_pretty_test(config: &config, props: &TestProps, testfile: &Path) {
 
     let mut expected = match props.pp_exact {
         Some(ref file) => {
-            let filepath = testfile.dir_path().join_path(file);
+            let filepath = testfile.dir_path().join(file);
             io::read_whole_file_str(&filepath).unwrap()
           }
           None => { srcs[srcs.len() - 2u].clone() }
@@ -657,7 +657,7 @@ fn make_lib_name(config: &config, auxfile: &Path, testfile: &Path) -> Path {
     // what we return here is not particularly important, as it
     // happens; rustc ignores everything except for the directory.
     let auxname = output_testname(auxfile);
-    aux_output_dir_name(config, testfile).join_path(&auxname)
+    aux_output_dir_name(config, testfile).join(&auxname)
 }
 
 fn make_exe_name(config: &config, testfile: &Path) -> Path {
@@ -757,7 +757,7 @@ fn output_testname(testfile: &Path) -> Path {
 
 fn output_base_name(config: &config, testfile: &Path) -> Path {
     config.build_base
-        .join_path(&output_testname(testfile))
+        .join(&output_testname(testfile))
         .with_extension(config.stage_id.as_slice())
 }
 

src/libextra/glob.rs

@@ -24,6 +24,7 @@
  */
 
 use std::{os, path};
+use std::path::is_sep;
 
 use sort;
 
@@ -81,11 +82,7 @@ pub fn glob(pattern: &str) -> GlobIterator {
  */
 pub fn glob_with(pattern: &str, options: MatchOptions) -> GlobIterator {
     #[cfg(windows)]
-    use is_sep = std::path::windows::is_sep2;
-    #[cfg(not(windows))]
-    fn is_sep(c: char) -> bool { c <= '\x7F' && ::std::path::posix::is_sep(&(c as u8)) }
-    #[cfg(windows)]
-    fn check_windows_verbatim(p: &Path) -> bool { p.is_verbatim() }
+    fn check_windows_verbatim(p: &Path) -> bool { path::windows::is_verbatim(p) }
     #[cfg(not(windows))]
     fn check_windows_verbatim(_: &Path) -> bool { false }
 
@@ -98,7 +95,7 @@ pub fn glob_with(pattern: &str, options: MatchOptions) -> GlobIterator {
             // since we can't very well find all UNC shares with a 1-letter server name.
             return GlobIterator { root: root, dir_patterns: ~[], options: options, todo: ~[] };
         }
-        root.push_path(pat_root.get_ref());
+        root.push(pat_root.get_ref());
     }
 
     let root_len = pat_root.map_move_default(0u, |p| p.as_vec().len());
@@ -462,7 +459,7 @@ fn in_char_specifiers(specifiers: &[CharSpecifier], c: char, options: MatchOptio
 
 /// A helper function to determine if two chars are (possibly case-insensitively) equal.
 fn chars_eq(a: char, b: char, case_sensitive: bool) -> bool {
-    if cfg!(windows) && path::windows::is_sep2(a) && path::windows::is_sep2(b) {
+    if cfg!(windows) && path::windows::is_sep(a) && path::windows::is_sep(b) {
         true
     } else if !case_sensitive && a.is_ascii() && b.is_ascii() {
         // FIXME: work with non-ascii chars properly (issue #1347)
@@ -472,16 +469,6 @@ fn chars_eq(a: char, b: char, case_sensitive: bool) -> bool {
     }
 }
 
-/// A helper function to determine if a char is a path separator on the current platform.
-fn is_sep(c: char) -> bool {
-    if cfg!(windows) {
-        path::windows::is_sep2(c)
-    } else {
-        c <= '\x7F' && path::posix::is_sep(&(c as u8))
-    }
-}
-
-
 /**
  * Configuration options to modify the behaviour of `Pattern::matches_with(..)`
  */

src/librustc/back/rpath.rs

@@ -45,7 +45,7 @@ pub fn get_rpath_flags(sess: session::Session, out_filename: &Path)
 
 fn get_sysroot_absolute_rt_lib(sess: session::Session) -> Path {
     let r = filesearch::relative_target_lib_path(sess.opts.target_triple);
-    let mut p = sess.filesearch.sysroot().join_path(&r);
+    let mut p = sess.filesearch.sysroot().join(&r);
     p.push(os::dll_filename("rustrt"));
     p
 }
@@ -148,7 +148,7 @@ pub fn get_install_prefix_rpath(target_triple: &str) -> ~str {
 
     let tlib = filesearch::relative_target_lib_path(target_triple);
     let mut path = Path::new(install_prefix);
-    path.push_path(&tlib);
+    path.push(&tlib);
     let path = os::make_absolute(&path);
     // FIXME (#9639): This needs to handle non-utf8 paths
     path.as_str().expect("non-utf8 component in rpath").to_owned()

src/librustc/metadata/filesearch.rs

@@ -100,7 +100,7 @@ pub fn mk_filesearch(maybe_sysroot: &Option<@Path>,
         }
         fn get_target_lib_file_path(&self, file: &Path) -> Path {
             let mut p = self.get_target_lib_path();
-            p.push_path(file);
+            p.push(file);
             p
         }
     }
@@ -148,7 +148,7 @@ pub fn relative_target_lib_path(target_triple: &str) -> Path {
 
 fn make_target_lib_path(sysroot: &Path,
                         target_triple: &str) -> Path {
-    sysroot.join_path(&relative_target_lib_path(target_triple))
+    sysroot.join(&relative_target_lib_path(target_triple))
 }
 
 fn make_rustpkg_target_lib_path(dir: &Path,
@@ -196,7 +196,7 @@ pub fn rust_path() -> ~[Path] {
         }
         None => ~[]
     };
-    let cwd = os::getcwd();
+    let mut cwd = os::getcwd();
     // now add in default entries
     let cwd_dot_rust = cwd.join(".rust");
     if !env_rust_path.contains(&cwd_dot_rust) {
@@ -205,30 +205,27 @@ pub fn rust_path() -> ~[Path] {
     if !env_rust_path.contains(&cwd) {
         env_rust_path.push(cwd.clone());
     }
-    do cwd.each_parent() |p| {
-        if !env_rust_path.contains(&p.join(".rust")) {
-            push_if_exists(&mut env_rust_path, p);
+    loop {
+        let f = cwd.pop();
+        if f.is_none() || bytes!("..") == f.unwrap() {
+            break;
         }
-        true
-    };
+        cwd.push(".rust");
+        if !env_rust_path.contains(&cwd) && os::path_exists(&cwd) {
+            env_rust_path.push(cwd.clone());
+        }
+        cwd.pop();
+    }
     let h = os::homedir();
     for h in h.iter() {
-        if !env_rust_path.contains(&h.join(".rust")) {
-            push_if_exists(&mut env_rust_path, h);
+        let p = h.join(".rust");
+        if !env_rust_path.contains(&p) && os::path_exists(&p) {
+            env_rust_path.push(p);
         }
     }
     env_rust_path
 }
 
-
-/// Adds p/.rust into vec, only if it exists
-fn push_if_exists(vec: &mut ~[Path], p: &Path) {
-    let maybe_dir = p.join(".rust");
-    if os::path_exists(&maybe_dir) {
-        vec.push(maybe_dir);
-    }
-}
-
 // The name of the directory rustc expects libraries to be located.
 // On Unix should be "lib", on windows "bin"
 pub fn libdir() -> ~str {

src/librustpkg/package_source.rs

@@ -95,20 +95,20 @@ impl PkgSrc {
             // We search for sources under both src/ and build/ , because build/ is where
             // automatically-checked-out sources go.
             let mut result = source_workspace.join("src");
-            result.push_path(&id.path.dir_path());
+            result.push(&id.path.dir_path());
             result.push(format!("{}-{}", id.short_name, id.version.to_str()));
             to_try.push(result);
             let mut result = source_workspace.join("src");
-            result.push_path(&id.path);
+            result.push(&id.path);
             to_try.push(result);
 
             let mut result = build_dir.join("src");
-            result.push_path(&id.path.dir_path());
+            result.push(&id.path.dir_path());
             result.push_str(format!("{}-{}", id.short_name, id.version.to_str()));
             to_try.push(result.clone());
             output_names.push(result);
             let mut other_result = build_dir.join("src");
-            other_result.push_path(&id.path);
+            other_result.push(&id.path);
             to_try.push(other_result.clone());
             output_names.push(other_result);
 
@@ -146,7 +146,7 @@ impl PkgSrc {
                                     source_workspace: source.clone(),
                                     build_in_destination: build_in_destination,
                                     destination_workspace: destination,
-                                    start_dir: start.join_path(&suffix),
+                                    start_dir: start.join(&suffix),
                                     id: id,
                                     libs: ~[],
                                     mains: ~[],
@@ -371,7 +371,7 @@ impl PkgSrc {
                     cfgs: &[~str],
                     what: OutputType) {
         for crate in crates.iter() {
-            let path = self.start_dir.join_path(&crate.file);
+            let path = self.start_dir.join(&crate.file);
             debug2!("build_crates: compiling {}", path.display());
             let cfgs = crate.cfgs + cfgs;
 
@@ -416,7 +416,7 @@ impl PkgSrc {
         debug2!("In declare inputs, self = {}", self.to_str());
         for cs in to_do.iter() {
             for c in cs.iter() {
-                let path = self.start_dir.join_path(&c.file);
+                let path = self.start_dir.join(&c.file);
                 debug2!("Declaring input: {}", path.display());
                 // FIXME (#9639): This needs to handle non-utf8 paths
                 prep.declare_input("file", path.as_str().unwrap(),

src/librustpkg/path_util.rs

@@ -68,7 +68,7 @@ pub fn workspace_contains_package_id_(pkgid: &PkgId, workspace: &Path,
     let mut found = None;
     do os::walk_dir(&src_dir) |p| {
         if os::path_is_dir(p) {
-            if *p == src_dir.join_path(&pkgid.path) || {
+            if *p == src_dir.join(&pkgid.path) || {
                 let pf = p.filename_str();
                 do pf.iter().any |&g| {
                     match split_version_general(g, '-') {
@@ -196,7 +196,7 @@ pub fn library_in_workspace(path: &Path, short_name: &str, where: Target,
             prefix = {}", short_name, where, workspace.display(), prefix);
 
     let dir_to_search = match where {
-        Build => target_build_dir(workspace).join_path(path),
+        Build => target_build_dir(workspace).join(path),
         Install => target_lib_dir(workspace)
     };
 
@@ -273,7 +273,7 @@ fn library_in(short_name: &str, version: &Version, dir_to_search: &Path) -> Opti
     // Return the filename that matches, which we now know exists
     // (if result_filename != None)
     let abs_path = do result_filename.map |result_filename| {
-        let absolute_path = dir_to_search.join_path(&result_filename);
+        let absolute_path = dir_to_search.join(&result_filename);
         debug2!("result_filename = {}", absolute_path.display());
         absolute_path
     };
@@ -329,7 +329,7 @@ fn target_file_in_workspace(pkgid: &PkgId, workspace: &Path,
     // Artifacts in the build directory live in a package-ID-specific subdirectory,
     // but installed ones don't.
     let result = match (where, what) {
-                (Build, _)      => target_build_dir(workspace).join_path(&pkgid.path),
+                (Build, _)      => target_build_dir(workspace).join(&pkgid.path),
                 (Install, Lib)  => target_lib_dir(workspace),
                 (Install, _)    => target_bin_dir(workspace)
     };
@@ -347,7 +347,7 @@ pub fn build_pkg_id_in_workspace(pkgid: &PkgId, workspace: &Path) -> Path {
     use conditions::bad_path::cond;
 
     let mut result = target_build_dir(workspace);
-    result.push_path(&pkgid.path);
+    result.push(&pkgid.path);
     debug2!("Creating build dir {} for package id {}", result.display(),
            pkgid.to_str());
     if os::path_exists(&result) || os::mkdir_recursive(&result, U_RWX) {
@@ -370,7 +370,7 @@ pub fn mk_output_path(what: OutputType, where: Target,
         // If we're installing, it just goes under <workspace>...
         Install => workspace,
         // and if we're just building, it goes in a package-specific subdir
-        Build => workspace.join_path(&pkg_id.path)
+        Build => workspace.join(&pkg_id.path)
     };
     debug2!("[{:?}:{:?}] mk_output_path: short_name = {}, path = {}", what, where,
            if what == Lib { short_name_with_version.clone() } else { pkg_id.short_name.clone() },
@@ -388,7 +388,7 @@ pub fn mk_output_path(what: OutputType, where: Target,
                            os::EXE_SUFFIX))
     };
     if !output_path.is_absolute() {
-        output_path = os::getcwd().join_path(&output_path);
+        output_path = os::getcwd().join(&output_path);
     }
     debug2!("mk_output_path: returning {}", output_path.display());
     output_path

src/librustpkg/rustpkg.rs

@@ -416,15 +416,15 @@ impl CtxMethods for BuildContext {
 
         debug2!("build: workspace = {} (in Rust path? {:?} is git dir? {:?} \
                 pkgid = {} pkgsrc start_dir = {}", workspace.display(),
-               in_rust_path(&workspace), is_git_dir(&workspace.join_path(&pkgid.path)),
+               in_rust_path(&workspace), is_git_dir(&workspace.join(&pkgid.path)),
                pkgid.to_str(), pkg_src.start_dir.display());
 
         // If workspace isn't in the RUST_PATH, and it's a git repo,
         // then clone it into the first entry in RUST_PATH, and repeat
-        if !in_rust_path(&workspace) && is_git_dir(&workspace.join_path(&pkgid.path)) {
+        if !in_rust_path(&workspace) && is_git_dir(&workspace.join(&pkgid.path)) {
             let mut out_dir = default_workspace().join("src");
-            out_dir.push_path(&pkgid.path);
-            let git_result = source_control::safe_git_clone(&workspace.join_path(&pkgid.path),
+            out_dir.push(&pkgid.path);
+            let git_result = source_control::safe_git_clone(&workspace.join(&pkgid.path),
                                                             &pkgid.version,
                                                             &out_dir);
             match git_result {
@@ -494,7 +494,7 @@ impl CtxMethods for BuildContext {
                     // We expect that p is relative to the package source's start directory,
                     // so check that assumption
                     debug2!("JustOne: p = {}", p.display());
-                    assert!(os::path_exists(&pkg_src.start_dir.join_path(p)));
+                    assert!(os::path_exists(&pkg_src.start_dir.join(p)));
                     if is_lib(p) {
                         PkgSrc::push_crate(&mut pkg_src.libs, 0, p);
                     } else if is_main(p) {
@@ -553,7 +553,7 @@ impl CtxMethods for BuildContext {
         debug2!("In declare inputs for {}", id.to_str());
         for cs in to_do.iter() {
             for c in cs.iter() {
-                let path = pkg_src.start_dir.join_path(&c.file);
+                let path = pkg_src.start_dir.join(&c.file);
                 debug2!("Recording input: {}", path.display());
                 // FIXME (#9639): This needs to handle non-utf8 paths
                 inputs.push((~"file", path.as_str().unwrap().to_owned()));

src/librustpkg/tests.rs

@@ -156,7 +156,7 @@ fn run_git(args: &[~str], env: Option<~[(~str, ~str)]>, cwd: &Path, err_msg: &st
 fn init_git_repo(p: &Path) -> TempDir {
     assert!(p.is_relative());
     let tmp = TempDir::new("git_local").expect("couldn't create temp dir");
-    let work_dir = tmp.path().join_path(p);
+    let work_dir = tmp.path().join(p);
     let work_dir_for_opts = work_dir.clone();
     assert!(os::mkdir_recursive(&work_dir, U_RWX));
     debug2!("Running: git init in {}", work_dir.display());
@@ -793,7 +793,7 @@ fn test_package_request_version() {
             == repo.join_many([".rust", "bin", "test_pkg_version"]));
 
     let mut dir = target_build_dir(&repo.join(".rust"));
-    dir.push_path(&Path::new("src/mockgithub.com/catamorphism/test_pkg_version-0.3"));
+    dir.push(&Path::new("src/mockgithub.com/catamorphism/test_pkg_version-0.3"));
     debug2!("dir = {}", dir.display());
     assert!(os::path_is_dir(&dir));
     assert!(os::path_exists(&dir.join("version-0.3-file.txt")));

src/librustpkg/util.rs

@@ -177,7 +177,7 @@ pub fn compile_input(context: &BuildContext,
     // not sure if we should support anything else
 
     let mut out_dir = target_build_dir(workspace);
-    out_dir.push_path(&pkg_id.path);
+    out_dir.push(&pkg_id.path);
     // Make the output directory if it doesn't exist already
     assert!(os::mkdir_recursive(&out_dir, U_RWX));
 

src/librustpkg/version.rs

@@ -98,7 +98,7 @@ pub fn parse_vers(vers: ~str) -> result::Result<semver::Version, ~str> {
 pub fn try_getting_local_version(local_path: &Path) -> Option<Version> {
     let rustpath = rust_path();
     for rp in rustpath.iter() {
-        let local_path = rp.join_path(local_path);
+        let local_path = rp.join(local_path);
         let git_dir = local_path.join(".git");
         if !os::path_is_dir(&git_dir) {
             continue;