rustpkg: Simplify the PkgId struct

Get rid of special cases for names beginning with "rust-" or
containing hyphens, and just store a Path in a package ID. The Rust-identifier
for the crate is none of rustpkg's business.

mk/tests.mk

@@ -349,7 +349,8 @@ $(3)/stage$(1)/test/rustpkgtest-$(2)$$(X_$(2)):					\
 		$$(SREQ$(1)_T_$(2)_H_$(3)) \
 		$$(TLIB$(1)_T_$(2)_H_$(3))/$$(CFG_LIBSYNTAX_$(2)) \
 		$$(TLIB$(1)_T_$(2)_H_$(3))/$$(CFG_LIBRUSTC_$(2)) \
-		$$(TBIN$(1)_T_$(2)_H_$(3))/rustpkg$$(X_$(2))
+		$$(TBIN$(1)_T_$(2)_H_$(3))/rustpkg$$(X_$(2)) \
+		$$(TBIN$(1)_T_$(2)_H_$(3))/rustc$$(X_$(2))
 	@$$(call E, compile_and_link: $$@)
 	$$(STAGE$(1)_T_$(2)_H_$(3)) -o $$@ $$< --test
 

src/librustpkg/package_id.rs

@@ -8,33 +8,37 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-pub use package_path::{RemotePath, LocalPath, normalize, hash};
 use version::{try_getting_version, try_getting_local_version,
               Version, NoVersion, split_version};
+use std::rt::io::Writer;
+use std::hash::Streaming;
+use std::hash;
 
 /// Path-fragment identifier of a package such as
 /// 'github.com/graydon/test'; path must be a relative
 /// path with >=1 component.
 #[deriving(Clone)]
 pub struct PkgId {
-    /// Remote path: for example, github.com/mozilla/quux-whatever
+    /// This is a path, on the local filesystem, referring to where the
-    remote_path: RemotePath,
+    /// files for this package live. For example:
-    /// Local path: for example, /home/quux/github.com/mozilla/quux_whatever
+    /// github.com/mozilla/quux-whatever (it's assumed that if we're
-    /// Note that '-' normalizes to '_' when mapping a remote path
+    /// working with a package ID of this form, rustpkg has already cloned
-    /// onto a local path
+    /// the sources into a local directory in the RUST_PATH).
-    /// Also, this will change when we implement #6407, though we'll still
+    path: Path,
-    /// need to keep track of separate local and remote paths
+    /// Short name. This is the path's filestem, but we store it
-    local_path: LocalPath,
-    /// Short name. This is the local path's filestem, but we store it
     /// redundantly so as to not call get() everywhere (filestem() returns an
     /// option)
+    /// The short name does not need to be a valid Rust identifier.
+    /// Users can write: `extern mod foo = "...";` to get around the issue
+    /// of package IDs whose short names aren't valid Rust identifiers.
     short_name: ~str,
+    /// The requested package version.
     version: Version
 }
 
 impl Eq for PkgId {
     fn eq(&self, p: &PkgId) -> bool {
-        *p.local_path == *self.local_path && p.version == self.version
+        p.path == self.path && p.version == self.version
     }
     fn ne(&self, p: &PkgId) -> bool {
         !(self.eq(p))
@@ -42,9 +46,6 @@ impl Eq for PkgId {
 }
 
 impl PkgId {
-    // The PkgId constructor takes a Path argument so as
-    // to be able to infer the version if the path refers
-    // to a local git repository
     pub fn new(s: &str) -> PkgId {
         use conditions::bad_pkg_id::cond;
 
@@ -63,40 +64,37 @@ impl PkgId {
             }
         };
 
-        let p = Path(s);
+        let path = Path(s);
-        if p.is_absolute {
+        if path.is_absolute {
-            return cond.raise((p, ~"absolute pkgid"));
+            return cond.raise((path, ~"absolute pkgid"));
         }
-        if p.components.len() < 1 {
+        if path.components.len() < 1 {
-            return cond.raise((p, ~"0-length pkgid"));
+            return cond.raise((path, ~"0-length pkgid"));
         }
-        let remote_path = RemotePath(p);
+        let short_name = path.clone().filestem().expect(fmt!("Strange path! %s", s));
-        let local_path = normalize(remote_path.clone());
-        let short_name = local_path.clone().filestem().expect(fmt!("Strange path! %s", s));
 
         let version = match given_version {
             Some(v) => v,
-            None => match try_getting_local_version(&*local_path) {
+            None => match try_getting_local_version(&path) {
                 Some(v) => v,
-                None => match try_getting_version(&remote_path) {
+                None => match try_getting_version(&path) {
                     Some(v) => v,
                     None => NoVersion
                 }
             }
         };
 
-        debug!("local_path = %s, remote_path = %s", local_path.to_str(), remote_path.to_str());
+        debug!("path = %s", path.to_str());
         PkgId {
-            local_path: local_path,
+            path: path,
-            remote_path: remote_path,
             short_name: short_name,
             version: version
         }
     }
 
     pub fn hash(&self) -> ~str {
-        fmt!("%s-%s-%s", self.remote_path.to_str(),
+        fmt!("%s-%s-%s", self.path.to_str(),
-             hash(self.remote_path.to_str() + self.version.to_str()),
+             hash(self.path.to_str() + self.version.to_str()),
              self.version.to_str())
     }
 
@@ -106,13 +104,24 @@ impl PkgId {
 
     /// True if the ID has multiple components
     pub fn is_complex(&self) -> bool {
-        self.short_name != self.local_path.to_str()
+        self.short_name != self.path.to_str()
      }
 }
 
 impl ToStr for PkgId {
     fn to_str(&self) -> ~str {
         // should probably use the filestem and not the whole path
-        fmt!("%s-%s", self.local_path.to_str(), self.version.to_str())
+        fmt!("%s-%s", self.path.to_str(), self.version.to_str())
     }
 }
+
+
+pub fn write<W: Writer>(writer: &mut W, string: &str) {
+    writer.write(string.as_bytes());
+}
+
+pub fn hash(data: ~str) -> ~str {
+    let hasher = &mut hash::default_state();
+    write(hasher, data);
+    hasher.result_str()
+}

src/librustpkg/package_path.rs

@@ -1,68 +0,0 @@
-// 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.
-
-// rustpkg utilities having to do with local and remote paths
-
-use std::clone::Clone;
-use std::hash::Streaming;
-use std::hash;
-use std::option::Some;
-use std::path::Path;
-use std::rt::io::Writer;
-
-/// Wrappers to prevent local and remote paths from getting confused
-/// (These will go away after #6407)
-pub struct RemotePath (Path);
-
-impl Clone for RemotePath {
-    fn clone(&self) -> RemotePath {
-        RemotePath((**self).clone())
-    }
-}
-
-pub struct LocalPath (Path);
-
-impl Clone for LocalPath {
-    fn clone(&self) -> LocalPath {
-        LocalPath((**self).clone())
-    }
-}
-
-
-// normalize should be the only way to construct a LocalPath
-// (though this isn't enforced)
-/// Replace all occurrences of '-' in the stem part of path with '_'
-/// This is because we treat rust-foo-bar-quux and rust_foo_bar_quux
-/// as the same name
-pub fn normalize(p_: RemotePath) -> LocalPath {
-    let RemotePath(p) = p_;
-    match p.filestem() {
-        None => LocalPath(p),
-        Some(st) => {
-            let replaced = st.replace("-", "_");
-            if replaced != st {
-                LocalPath(p.with_filestem(replaced))
-            }
-            else {
-                LocalPath(p)
-            }
-        }
-    }
-}
-
-pub fn write<W: Writer>(writer: &mut W, string: &str) {
-    writer.write(string.as_bytes());
-}
-
-pub fn hash(data: ~str) -> ~str {
-    let hasher = &mut hash::default_state();
-    write(hasher, data);
-    hasher.result_str()
-}

src/librustpkg/package_source.rs

@@ -11,7 +11,7 @@
 use target::*;
 use package_id::PkgId;
 use std::path::Path;
-use std::{os, str};
+use std::os;
 use context::*;
 use crate::Crate;
 use messages::*;
@@ -51,8 +51,7 @@ impl PkgSrc {
     fn check_dir(&self) -> Path {
         use conditions::nonexistent_package::cond;
 
-        debug!("Pushing onto root: %s | %s", self.id.remote_path.to_str(),
+        debug!("Pushing onto root: %s | %s", self.id.path.to_str(), self.root.to_str());
-               self.root.to_str());
         let dir;
         let dirs = pkgid_src_in_workspace(&self.id, &self.root);
         debug!("Checking dirs: %?", dirs);
@@ -86,18 +85,18 @@ impl PkgSrc {
         os::remove_dir_recursive(&local);
 
         debug!("Checking whether %s exists locally. Cwd = %s, does it? %?",
-               self.id.local_path.to_str(),
+               self.id.path.to_str(),
                os::getcwd().to_str(),
-               os::path_exists(&*self.id.local_path));
+               os::path_exists(&self.id.path));
 
-        if os::path_exists(&*self.id.local_path) {
+        if os::path_exists(&self.id.path) {
             debug!("%s exists locally! Cloning it into %s",
-                   self.id.local_path.to_str(), local.to_str());
+                   self.id.path.to_str(), local.to_str());
-            git_clone(&*self.id.local_path, &local, &self.id.version);
+            git_clone(&self.id.path, &local, &self.id.version);
             return Some(local);
         }
 
-        let url = fmt!("https://%s", self.id.remote_path.to_str());
+        let url = fmt!("https://%s", self.id.path.to_str());
         note(fmt!("Fetching package: git clone %s %s [version=%s]",
                   url, local.to_str(), self.id.version.to_str()));
         if git_clone_general(url, &local, &self.id.version) {
@@ -122,25 +121,8 @@ impl PkgSrc {
     }
 
     /// True if the given path's stem is self's pkg ID's stem
-    /// or if the pkg ID's stem is <rust-foo> and the given path's
-    /// stem is foo
-    /// Requires that dashes in p have already been normalized to
-    /// underscores
     fn stem_matches(&self, p: &Path) -> bool {
-        let self_id = self.id.local_path.filestem();
+        p.filestem().map_default(false, |p| { p == &self.id.short_name })
-        if self_id == p.filestem() {
-            return true;
-        }
-        else {
-            for pth in self_id.iter() {
-                if pth.starts_with("rust_") // because p is already normalized
-                    && match p.filestem() {
-                           Some(s) => str::eq_slice(s, pth.slice(5, pth.len())),
-                           None => false
-                       } { return true; }
-            }
-        }
-        false
     }
 
     fn push_crate(cs: &mut ~[Crate], prefix: uint, p: &Path) {
@@ -161,7 +143,7 @@ impl PkgSrc {
         let dir = self.check_dir();
         debug!("Called check_dir, I'm in %s", dir.to_str());
         let prefix = dir.components.len();
-        debug!("Matching against %?", self.id.local_path.filestem());
+        debug!("Matching against %?", self.id.short_name);
         do os::walk_dir(&dir) |pth| {
             match pth.filename() {
                 Some(~"lib.rs") => PkgSrc::push_crate(&mut self.libs,

src/librustpkg/path_util.rs

@@ -10,7 +10,6 @@
 
 // rustpkg utilities having to do with paths and directories
 
-pub use package_path::{RemotePath, LocalPath, normalize};
 pub use package_id::PkgId;
 pub use target::{OutputType, Main, Lib, Test, Bench, Target, Build, Install};
 pub use version::{Version, NoVersion, split_version_general};
@@ -94,9 +93,9 @@ pub fn workspace_contains_package_id(pkgid: &PkgId, workspace: &Path) -> bool {
 pub fn pkgid_src_in_workspace(pkgid: &PkgId, workspace: &Path) -> ~[Path] {
     let mut results = ~[];
     let result = workspace.push("src").push(fmt!("%s-%s",
-                     pkgid.local_path.to_str(), pkgid.version.to_str()));
+                     pkgid.path.to_str(), pkgid.version.to_str()));
     results.push(result);
-    results.push(workspace.push("src").push_rel(&*pkgid.remote_path));
+    results.push(workspace.push("src").push_rel(&pkgid.path));
     results
 }
 
@@ -159,14 +158,12 @@ fn output_in_workspace(pkgid: &PkgId, workspace: &Path, what: OutputType) -> Opt
 /// Figure out what the library name for <pkgid> in <workspace>'s build
 /// directory is, and if the file exists, return it.
 pub fn built_library_in_workspace(pkgid: &PkgId, workspace: &Path) -> Option<Path> {
-    library_in_workspace(&pkgid.local_path, pkgid.short_name,
+    library_in_workspace(&pkgid.path, pkgid.short_name, Build, workspace, "build")
-                         Build, workspace, "build")
 }
 
 /// Does the actual searching stuff
 pub fn installed_library_in_workspace(short_name: &str, workspace: &Path) -> Option<Path> {
-    library_in_workspace(&normalize(RemotePath(Path(short_name))),
+    library_in_workspace(&Path(short_name), short_name, Install, workspace, "lib")
-                         short_name, Install, workspace, "lib")
 }
 
 
@@ -174,7 +171,7 @@ pub fn installed_library_in_workspace(short_name: &str, workspace: &Path) -> Opt
 /// don't know the entire package ID.
 /// `workspace` is used to figure out the directory to search.
 /// `short_name` is taken as the link name of the library.
-pub fn library_in_workspace(path: &LocalPath, short_name: &str, where: Target,
+pub fn library_in_workspace(path: &Path, short_name: &str, where: Target,
                         workspace: &Path, prefix: &str) -> Option<Path> {
     debug!("library_in_workspace: checking whether a library named %s exists",
            short_name);
@@ -186,7 +183,7 @@ pub fn library_in_workspace(path: &LocalPath, short_name: &str, where: Target,
             prefix = %s", short_name, where, workspace.to_str(), prefix);
 
     let dir_to_search = match where {
-        Build => workspace.push(prefix).push_rel(&**path),
+        Build => workspace.push(prefix).push_rel(path),
         Install => workspace.push(prefix)
     };
     debug!("Listing directory %s", dir_to_search.to_str());
@@ -302,7 +299,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 {
-                Build => workspace.push(subdir).push_rel(&*pkgid.local_path),
+                Build => workspace.push(subdir).push_rel(&pkgid.path),
                 _     => workspace.push(subdir)
     };
     if !os::path_exists(&result) && !mkdir_recursive(&result, U_RWX) {
@@ -321,7 +318,7 @@ pub fn build_pkg_id_in_workspace(pkgid: &PkgId, workspace: &Path) -> Path {
     let mut result = workspace.push("build");
     // n.b. Should actually use a target-specific
     // subdirectory of build/
-    result = result.push_rel(&*pkgid.local_path);
+    result = result.push_rel(&pkgid.path);
     if os::path_exists(&result) || os::mkdir_recursive(&result, U_RWX) {
         result
     }
@@ -342,7 +339,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.push_rel(&*pkg_id.local_path)
+        Build => workspace.push_rel(&pkg_id.path)
     };
     debug!("[%?:%?] mk_output_path: short_name = %s, path = %s", what, where,
            if what == Lib { short_name_with_version.clone() } else { pkg_id.short_name.clone() },

src/librustpkg/rustpkg.rs

@@ -55,7 +55,6 @@ mod crate;
 mod installed_packages;
 mod messages;
 mod package_id;
-mod package_path;
 mod package_source;
 mod path_util;
 mod search;
@@ -268,7 +267,7 @@ impl CtxMethods for Ctx {
             "list" => {
                 io::println("Installed packages:");
                 do installed_packages::list_installed_packages |pkg_id| {
-                    println(pkg_id.local_path.to_str());
+                    println(pkg_id.path.to_str());
                     true
                 };
             }
@@ -322,18 +321,18 @@ impl CtxMethods for Ctx {
     fn build(&self, workspace: &Path, pkgid: &PkgId) {
         debug!("build: workspace = %s (in Rust path? %? is git dir? %? \
                 pkgid = %s", workspace.to_str(),
-               in_rust_path(workspace), is_git_dir(&workspace.push_rel(&*pkgid.local_path)),
+               in_rust_path(workspace), is_git_dir(&workspace.push_rel(&pkgid.path)),
                pkgid.to_str());
         let src_dir   = first_pkgid_src_in_workspace(pkgid, workspace);
 
         // 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
         debug!("%? %? %s", in_rust_path(workspace),
-               is_git_dir(&workspace.push_rel(&*pkgid.local_path)),
+               is_git_dir(&workspace.push_rel(&pkgid.path)),
                workspace.to_str());
-        if !in_rust_path(workspace) && is_git_dir(&workspace.push_rel(&*pkgid.local_path)) {
+        if !in_rust_path(workspace) && is_git_dir(&workspace.push_rel(&pkgid.path)) {
-            let out_dir = default_workspace().push("src").push_rel(&*pkgid.local_path);
+            let out_dir = default_workspace().push("src").push_rel(&pkgid.path);
-            source_control::git_clone(&workspace.push_rel(&*pkgid.local_path),
+            source_control::git_clone(&workspace.push_rel(&pkgid.path),
                                       &out_dir, &pkgid.version);
             let default_ws = default_workspace();
             debug!("Calling build recursively with %? and %?", default_ws.to_str(),

src/librustpkg/tests.rs

@@ -12,11 +12,10 @@
 
 use context::Ctx;
 use std::hashmap::HashMap;
-use std::{io, libc, os, result, run, str};
+use std::{io, libc, os, run, str};
 use extra::tempfile::mkdtemp;
 use std::run::ProcessOutput;
 use installed_packages::list_installed_packages;
-use package_path::*;
 use package_id::{PkgId};
 use version::{ExactRevision, NoVersion, Version, Tagged};
 use path_util::{target_executable_in_workspace, target_library_in_workspace,
@@ -44,31 +43,25 @@ fn fake_ctxt(sysroot_opt: Option<@Path>) -> Ctx {
 
 fn fake_pkg() -> PkgId {
     let sn = ~"bogus";
-    let remote = RemotePath(Path(sn));
     PkgId {
-        local_path: normalize(remote.clone()),
+        path: Path(sn),
-        remote_path: remote,
         short_name: sn,
         version: NoVersion
     }
 }
 
 fn git_repo_pkg() -> PkgId {
-    let remote = RemotePath(Path("mockgithub.com/catamorphism/test-pkg"));
     PkgId {
-        local_path: normalize(remote.clone()),
+        path: Path("mockgithub.com/catamorphism/test-pkg"),
-        remote_path: remote,
+        short_name: ~"test-pkg",
-        short_name: ~"test_pkg",
         version: NoVersion
     }
 }
 
 fn git_repo_pkg_with_tag(a_tag: ~str) -> PkgId {
-    let remote = RemotePath(Path("mockgithub.com/catamorphism/test-pkg"));
     PkgId {
-        local_path: normalize(remote.clone()),
+        path: Path("mockgithub.com/catamorphism/test-pkg"),
-        remote_path: remote,
+        short_name: ~"test-pkg",
-        short_name: ~"test_pkg",
         version: Tagged(a_tag)
     }
 }
@@ -78,13 +71,13 @@ fn writeFile(file_path: &Path, contents: &str) {
     out.write_line(contents);
 }
 
-fn mk_empty_workspace(short_name: &LocalPath, version: &Version) -> Path {
+fn mk_empty_workspace(short_name: &Path, version: &Version) -> Path {
     let workspace_dir = mkdtemp(&os::tmpdir(), "test").expect("couldn't create temp dir");
     mk_workspace(&workspace_dir, short_name, version);
     workspace_dir
 }
 
-fn mk_workspace(workspace: &Path, short_name: &LocalPath, version: &Version) -> Path {
+fn mk_workspace(workspace: &Path, short_name: &Path, version: &Version) -> Path {
     // include version number in directory name
     let package_dir = workspace.push("src").push(fmt!("%s-%s",
                                                       short_name.to_str(), version.to_str()));
@@ -92,7 +85,7 @@ fn mk_workspace(workspace: &Path, short_name: &LocalPath, version: &Version) ->
     package_dir
 }
 
-fn mk_temp_workspace(short_name: &LocalPath, version: &Version) -> Path {
+fn mk_temp_workspace(short_name: &Path, version: &Version) -> Path {
     let package_dir = mk_empty_workspace(short_name,
                                          version).push("src").push(fmt!("%s-%s",
                                                             short_name.to_str(),
@@ -255,7 +248,7 @@ to make sure the command succeeded
 }
 
 fn create_local_package(pkgid: &PkgId) -> Path {
-    let parent_dir = mk_temp_workspace(&pkgid.local_path, &pkgid.version);
+    let parent_dir = mk_temp_workspace(&pkgid.path, &pkgid.version);
     debug!("Created empty package dir for %s, returning %s", pkgid.to_str(), parent_dir.to_str());
     parent_dir.pop().pop()
 }
@@ -312,7 +305,7 @@ fn create_local_package_with_custom_build_hook(pkgid: &PkgId,
 
 }
 
-fn assert_lib_exists(repo: &Path, short_name: &str, v: Version) {
+fn assert_lib_exists(repo: &Path, short_name: &str, _v: Version) { // ??? version?
     debug!("assert_lib_exists: repo = %s, short_name = %s", repo.to_str(), short_name);
     let lib = installed_library_in_workspace(short_name, repo);
     debug!("assert_lib_exists: checking whether %? exists", lib);
@@ -357,11 +350,11 @@ fn command_line_test_output_with_env(args: &[~str], env: ~[(~str, ~str)]) -> ~[~
     result
 }
 
-// assumes short_name and local_path are one and the same -- I should fix
+// assumes short_name and path are one and the same -- I should fix
 fn lib_output_file_name(workspace: &Path, parent: &str, short_name: &str) -> Path {
     debug!("lib_output_file_name: given %s and parent %s and short name %s",
            workspace.to_str(), parent, short_name);
-    library_in_workspace(&normalize(RemotePath(Path(short_name))),
+    library_in_workspace(&Path(short_name),
                          short_name,
                          Build,
                          workspace,
@@ -436,7 +429,7 @@ fn test_install_valid() {
     debug!("sysroot = %s", sysroot.to_str());
     let ctxt = fake_ctxt(Some(@sysroot));
     let temp_pkg_id = fake_pkg();
-    let temp_workspace = mk_temp_workspace(&temp_pkg_id.local_path, &NoVersion).pop().pop();
+    let temp_workspace = mk_temp_workspace(&temp_pkg_id.path, &NoVersion).pop().pop();
     debug!("temp_workspace = %s", temp_workspace.to_str());
     // should have test, bench, lib, and main
     ctxt.install(&temp_workspace, &temp_pkg_id);
@@ -489,7 +482,7 @@ fn test_install_git() {
     let sysroot = test_sysroot();
     debug!("sysroot = %s", sysroot.to_str());
     let temp_pkg_id = git_repo_pkg();
-    let repo = init_git_repo(&Path(temp_pkg_id.local_path.to_str()));
+    let repo = init_git_repo(&temp_pkg_id.path);
     let repo_subdir = repo.push("mockgithub.com").push("catamorphism").push("test_pkg");
     writeFile(&repo_subdir.push("main.rs"),
               "fn main() { let _x = (); }");
@@ -502,9 +495,9 @@ fn test_install_git() {
     add_git_tag(&repo_subdir, ~"0.1"); // this has the effect of committing the files
 
     debug!("test_install_git: calling rustpkg install %s in %s",
-           temp_pkg_id.local_path.to_str(), repo.to_str());
+           temp_pkg_id.path.to_str(), repo.to_str());
     // should have test, bench, lib, and main
-    command_line_test([~"install", temp_pkg_id.local_path.to_str()], &repo);
+    command_line_test([~"install", temp_pkg_id.path.to_str()], &repo);
     // Check that all files exist
     debug!("Checking for files in %s", repo.to_str());
     let exec = target_executable_in_workspace(&temp_pkg_id, &repo);
@@ -551,7 +544,7 @@ fn test_package_ids_must_be_relative_path_like() {
     let whatever = PkgId::new("foo");
 
     assert_eq!(~"foo-0.1", whatever.to_str());
-    assert!("github.com/catamorphism/test_pkg-0.1" ==
+    assert!("github.com/catamorphism/test-pkg-0.1" ==
             PkgId::new("github.com/catamorphism/test-pkg").to_str());
 
     do cond.trap(|(p, e)| {
@@ -755,7 +748,7 @@ fn rustpkg_clean_no_arg() {
 #[ignore (reason = "Specifying env doesn't work -- see #8028")]
 fn rust_path_test() {
     let dir_for_path = mkdtemp(&os::tmpdir(), "more_rust").expect("rust_path_test failed");
-    let dir = mk_workspace(&dir_for_path, &normalize(RemotePath(Path("foo"))), &NoVersion);
+    let dir = mk_workspace(&dir_for_path, &Path("foo"), &NoVersion);
     debug!("dir = %s", dir.to_str());
     writeFile(&dir.push("main.rs"), "fn main() { let _x = (); }");
 
@@ -877,7 +870,7 @@ fn install_check_duplicates() {
     let mut contents = ~[];
     let check_dups = |p: &PkgId| {
         if contents.contains(p) {
-            fail!("package %s appears in `list` output more than once", p.local_path.to_str());
+            fail!("package %s appears in `list` output more than once", p.path.to_str());
         }
         else {
             contents.push((*p).clone());
@@ -992,8 +985,8 @@ fn test_uninstall() {
 #[test]
 fn test_non_numeric_tag() {
     let temp_pkg_id = git_repo_pkg();
-    let repo = init_git_repo(&Path(temp_pkg_id.local_path.to_str()));
+    let repo = init_git_repo(&temp_pkg_id.path);
-    let repo_subdir = repo.push("mockgithub.com").push("catamorphism").push("test_pkg");
+    let repo_subdir = repo.push("mockgithub.com").push("catamorphism").push("test-pkg");
     writeFile(&repo_subdir.push("foo"), "foo");
     writeFile(&repo_subdir.push("lib.rs"),
               "pub fn f() { let _x = (); }");
@@ -1003,12 +996,10 @@ fn test_non_numeric_tag() {
     writeFile(&repo_subdir.push("not_on_testbranch_only"), "bye bye");
     add_all_and_commit(&repo_subdir);
 
-
+    command_line_test([~"install", fmt!("%s#testbranch", temp_pkg_id.path.to_str())], &repo);
-    command_line_test([~"install", fmt!("%s#testbranch", temp_pkg_id.remote_path.to_str())],
-                      &repo);
     let file1 = repo.push_many(["mockgithub.com", "catamorphism",
-                                "test_pkg", "testbranch_only"]);
+                                "test-pkg", "testbranch_only"]);
-    let file2 = repo.push_many(["mockgithub.com", "catamorphism", "test_pkg",
+    let file2 = repo.push_many(["mockgithub.com", "catamorphism", "test-pkg",
                                 "master_only"]);
     assert!(os::path_exists(&file1));
     assert!(!os::path_exists(&file2));

src/librustpkg/util.rs

@@ -166,7 +166,7 @@ pub fn compile_input(ctxt: &Ctx,
     // tjc: by default, use the package ID name as the link name
     // not sure if we should support anything else
 
-    let out_dir = workspace.push("build").push_rel(&*pkg_id.local_path);
+    let out_dir = workspace.push("build").push_rel(&pkg_id.path);
 
     let binary = os::args()[0].to_managed();
 
@@ -246,8 +246,8 @@ pub fn compile_input(ctxt: &Ctx,
     // Inject the link attributes so we get the right package name and version
     if attr::find_linkage_metas(crate.attrs).is_empty() {
         let name_to_use = match what {
-            Test  => fmt!("%stest", pkg_id.local_path.to_str()).to_managed(),
+            Test  => fmt!("%stest", pkg_id.short_name).to_managed(),
-            Bench => fmt!("%sbench", pkg_id.local_path.to_str()).to_managed(),
+            Bench => fmt!("%sbench", pkg_id.short_name).to_managed(),
             _     => pkg_id.short_name.to_managed()
         };
         debug!("Injecting link name: %s", name_to_use);
@@ -256,7 +256,7 @@ pub fn compile_input(ctxt: &Ctx,
               attr::mk_name_value_item_str(@"vers", pkg_id.version.to_str().to_managed())] +
                         if pkg_id.is_complex() {
                         ~[attr::mk_name_value_item_str(@"package_id",
-                                                       pkg_id.local_path.to_str().to_managed())]
+                                                       pkg_id.path.to_str().to_managed())]
                 } else { ~[] };
 
         debug!("link options: %?", link_options);

src/librustpkg/version.rs

@@ -15,7 +15,6 @@ extern mod std;
 
 use extra::semver;
 use std::{char, os, result, run, str};
-use package_path::RemotePath;
 use extra::tempfile::mkdtemp;
 use path_util::rust_path;
 
@@ -128,7 +127,7 @@ pub fn try_getting_local_version(local_path: &Path) -> Option<Version> {
 /// If `remote_path` refers to a git repo that can be downloaded,
 /// and the most recent tag in that repo denotes a version, return it;
 /// otherwise, `None`
-pub fn try_getting_version(remote_path: &RemotePath) -> Option<Version> {
+pub fn try_getting_version(remote_path: &Path) -> Option<Version> {
     debug!("try_getting_version: %s", remote_path.to_str());
     if is_url_like(remote_path) {
         debug!("Trying to fetch its sources..");
@@ -199,7 +198,7 @@ fn try_parsing_version(s: &str) -> Option<Version> {
 }
 
 /// Just an approximation
-fn is_url_like(p: &RemotePath) -> bool {
+fn is_url_like(p: &Path) -> bool {
     let str = p.to_str();
     str.split_iter('/').len_() > 2
 }

src/librustpkg/workspace.rs

@@ -25,7 +25,7 @@ pub fn each_pkg_parent_workspace(pkgid: &PkgId, action: &fn(&Path) -> bool) -> b
         // tjc: make this a condition
         fail!("Package %s not found in any of \
                     the following workspaces: %s",
-                   pkgid.remote_path.to_str(),
+                   pkgid.path.to_str(),
                    rust_path().to_str());
     }
     for ws in workspaces.iter() {