rustpkg: Do not guess version if not given

klutzy · klutzy · commit fa84593fc309 · 2014-01-23T03:02:39.000+09:00
rustpkg accessed git repo to read tags and guess package version,
but it's not quite useful: version can be given explicitly by user,
and implicit guess may cause confusions.
diff --git a/src/librustpkg/crate_id.rs b/src/librustpkg/crate_id.rs
@@ -8,8 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use version::{try_getting_version, try_getting_local_version,
-              Version, NoVersion, ExactRevision};
+use version::{Version, NoVersion, ExactRevision};
 use std::hash::Streaming;
 use std::hash;
 use syntax::crateid;
@@ -53,17 +52,9 @@ impl CrateId {
         let raw_crateid = raw_crateid.unwrap();
         let crateid::CrateId { path, name, version } = raw_crateid;
         let path = Path::new(path);
-        let given_version = version.map(|v| ExactRevision(v));
-
-        let version = match given_version {
-            Some(v) => v,
-            None => match try_getting_local_version(&path) {
-                Some(v) => v,
-                None => match try_getting_version(&path) {
-                    Some(v) => v,
-                    None => NoVersion
-                }
-            }
+        let version = match version {
+            Some(v) => ExactRevision(v),
+            None => NoVersion,
         };
 
         CrateId {
diff --git a/src/librustpkg/tests.rs b/src/librustpkg/tests.rs
@@ -752,46 +752,6 @@ fn test_crate_ids_must_be_relative_path_like() {
     })
 }
 
-#[test]
-fn test_package_version() {
-    let local_path = "mockgithub.com/catamorphism/test_pkg_version";
-    let repo = init_git_repo(&Path::new(local_path));
-    let repo = repo.path();
-    let repo_subdir = repo.join_many(["mockgithub.com", "catamorphism", "test_pkg_version"]);
-    debug!("Writing files in: {}", repo_subdir.display());
-    fs::mkdir_recursive(&repo_subdir, io::UserRWX);
-    writeFile(&repo_subdir.join("main.rs"),
-              "fn main() { let _x = (); }");
-    writeFile(&repo_subdir.join("lib.rs"),
-              "pub fn f() { let _x = (); }");
-    writeFile(&repo_subdir.join("test.rs"),
-              "#[test] pub fn f() { (); }");
-    writeFile(&repo_subdir.join("bench.rs"),
-              "#[bench] pub fn f() { (); }");
-    add_git_tag(&repo_subdir, ~"0.4");
-
-    // It won't pick up the 0.4 version because the dir isn't in the RUST_PATH, but...
-    let temp_pkg_id = CrateId::new("mockgithub.com/catamorphism/test_pkg_version");
-    // This should look at the prefix, clone into a workspace, then build.
-    command_line_test([~"install", ~"mockgithub.com/catamorphism/test_pkg_version"],
-                      repo);
-    let ws = repo.join(".rust");
-    // we can still match on the filename to make sure it contains the 0.4 version
-    assert!(match built_library_in_workspace(&temp_pkg_id,
-                                             &ws) {
-        Some(p) => {
-            let suffix = format!("0.4{}", os::consts::DLL_SUFFIX);
-            p.as_vec().ends_with(suffix.as_bytes())
-        }
-        None    => false
-    });
-    assert!(built_executable_in_workspace(&temp_pkg_id, &ws)
-            == Some(target_build_dir(&ws).join_many(["mockgithub.com",
-                                                     "catamorphism",
-                                                     "test_pkg_version",
-                                                     "test_pkg_version"])));
-}
-
 #[test]
 fn test_package_request_version() {
     let local_path = "mockgithub.com/catamorphism/test_pkg_version";
@@ -2183,9 +2143,9 @@ fn test_installed_read_only() {
               "fn main() { let _x = (); }");
     writeFile(&repo_subdir.join("lib.rs"),
               "pub fn f() { let _x = (); }");
-    add_git_tag(&repo_subdir, ~"0.1"); // this has the effect of committing the files
+    add_git_tag(&repo_subdir, ~"0.0"); // this has the effect of committing the files
     // update crateid to what will be auto-detected
-    temp_pkg_id.version = ExactRevision(~"0.1");
+    temp_pkg_id.version = ExactRevision(~"0.0");
 
     // FIXME (#9639): This needs to handle non-utf8 paths
     command_line_test([~"install", temp_pkg_id.path.as_str().unwrap().to_owned()], repo);
diff --git a/src/librustpkg/version.rs b/src/librustpkg/version.rs
@@ -14,9 +14,7 @@
 extern mod std;
 
 use extra::semver;
-use std::{char, result, run, str};
-use extra::tempfile::TempDir;
-use path_util::rust_path;
+use std::{char, result};
 
 #[deriving(Clone)]
 pub enum Version {
@@ -93,91 +91,6 @@ pub fn parse_vers(vers: ~str) -> result::Result<semver::Version, ~str> {
     }
 }
 
-/// If `local_path` is a git repo in the RUST_PATH, and the most recent tag
-/// in that repo denotes a version, return it; otherwise, `None`
-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(local_path);
-        let git_dir = local_path.join(".git");
-        if !git_dir.is_dir() {
-            continue;
-        }
-        // FIXME (#9639): This needs to handle non-utf8 paths
-        let opt_outp = run::process_output("git",
-                                   ["--git-dir=" + git_dir.as_str().unwrap(), ~"tag", ~"-l"]);
-        let outp = opt_outp.expect("Failed to exec `git`");
-
-        debug!("git --git-dir={} tag -l ~~~> {:?}", git_dir.display(), outp.status);
-
-        if !outp.status.success() {
-            continue;
-        }
-
-        let mut output = None;
-        let output_text = str::from_utf8(outp.output).unwrap();
-        for l in output_text.lines() {
-            if !l.is_whitespace() {
-                output = Some(l);
-            }
-            match output.and_then(try_parsing_version) {
-                Some(v) => return Some(v),
-                None    => ()
-            }
-        }
-    }
-    None
-}
-
-/// 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: &Path) -> Option<Version> {
-    if is_url_like(remote_path) {
-        let tmp_dir = TempDir::new("test");
-        let tmp_dir = tmp_dir.expect("try_getting_version: couldn't create temp dir");
-        let tmp_dir = tmp_dir.path();
-        debug!("(to get version) executing \\{git clone https://{} {}\\}",
-               remote_path.display(),
-               tmp_dir.display());
-        // FIXME (#9639): This needs to handle non-utf8 paths
-        let opt_outp = run::process_output("git", [~"clone", format!("https://{}",
-                                                                     remote_path.as_str().unwrap()),
-                                                   tmp_dir.as_str().unwrap().to_owned()]);
-        let outp = opt_outp.expect("Failed to exec `git`");
-        if outp.status.success() {
-            debug!("Cloned it... ( {}, {} )",
-                   str::from_utf8(outp.output).unwrap(),
-                   str::from_utf8(outp.error).unwrap());
-            let mut output = None;
-            let git_dir = tmp_dir.join(".git");
-            debug!("(getting version, now getting tags) executing \\{git --git-dir={} tag -l\\}",
-                   git_dir.display());
-            // FIXME (#9639): This needs to handle non-utf8 paths
-            let opt_outp = run::process_output("git",
-                                               ["--git-dir=" + git_dir.as_str().unwrap(),
-                                                ~"tag", ~"-l"]);
-            let outp = opt_outp.expect("Failed to exec `git`");
-            let output_text = str::from_utf8(outp.output).unwrap();
-            debug!("Full output: ( {} ) [{:?}]", output_text, outp.status);
-            for l in output_text.lines() {
-                debug!("A line of output: {}", l);
-                if !l.is_whitespace() {
-                    output = Some(l);
-                }
-            }
-
-            output.and_then(try_parsing_version)
-        }
-        else {
-            None
-        }
-    }
-    else {
-        None
-    }
-}
-
 // Being lazy since we don't have a regexp library now
 #[deriving(Eq)]
 enum ParseState {
@@ -207,12 +120,6 @@ pub fn try_parsing_version(s: &str) -> Option<Version> {
     }
 }
 
-/// Just an approximation
-fn is_url_like(p: &Path) -> bool {
-    // check if there are more than 2 /-separated components
-    p.as_vec().split(|b| *b == '/' as u8).nth(2).is_some()
-}
-
 /// If s is of the form foo#bar, where bar is a valid version
 /// number, return the prefix before the # and the version.
 /// Otherwise, return None.
