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.

Author: catamorphism

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,43 +8,44 @@
 // 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
-    remote_path: RemotePath,
-    /// Local path: for example, /home/quux/github.com/mozilla/quux_whatever
-    /// Note that '-' normalizes to '_' when mapping a remote path
-    /// onto a local path
-    /// Also, this will change when we implement #6407, though we'll still
-    /// need to keep track of separate local and remote paths
-    local_path: LocalPath,
-    /// Short name. This is the local path's filestem, but we store it
+    /// This is a path, on the local filesystem, referring to where the
+    /// files for this package live. For example:
+    /// github.com/mozilla/quux-whatever (it's assumed that if we're
+    /// working with a package ID of this form, rustpkg has already cloned
+    /// the sources into a local directory in the RUST_PATH).
+    path: Path,
+    /// Short name. This is the 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))
     }
 }
 
 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);
-        if p.is_absolute {
-            return cond.raise((p, ~"absolute pkgid"));
+        let path = Path(s);
+        if path.is_absolute {
+            return cond.raise((path, ~"absolute pkgid"));
         }
-        if p.components.len() < 1 {
-            return cond.raise((p, ~"0-length pkgid"));
+        if path.components.len() < 1 {
+            return cond.raise((path, ~"0-length pkgid"));
         }
-        let remote_path = RemotePath(p);
-        let local_path = normalize(remote_path.clone());
-        let short_name = local_path.clone().filestem().expect(fmt!("Strange path! %s", s));
+        let short_name = 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,
-            remote_path: remote_path,
+            path: path,
             short_name: short_name,
             version: version
         }
     }
 
     pub fn hash(&self) -> ~str {
-        fmt!("%s-%s-%s", self.remote_path.to_str(),
-             hash(self.remote_path.to_str() + self.version.to_str()),
+        fmt!("%s-%s-%s", self.path.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

@@ -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(),
-               self.root.to_str());
+        debug!("Pushing onto root: %s | %s", self.id.path.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());
-            git_clone(&*self.id.local_path, &local, &self.id.version);
+                   self.id.path.to_str(), local.to_str());
+            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();
-        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
+        p.filestem().map_default(false, |p| { p == &self.id.short_name })
     }
 
     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/package_source.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,22 +158,20 @@ 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,
-                         Build, workspace, "build")
+    library_in_workspace(&pkgid.path, pkgid.short_name, 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))),
-                         short_name, Install, workspace, "lib")
+    library_in_workspace(&Path(short_name), short_name, Install, workspace, "lib")
 }
 
 
 /// This doesn't take a PkgId, so we can use it for `extern mod` inference, where we
 /// 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() },