From d7c4a10fba7fca3d02ea7fb91ed7b00dea7b96f5 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Wed, 24 Jul 2013 18:39:25 -0700 Subject: [PATCH 1/4] rustpkg: Clean up usage messages for install --- src/librustpkg/usage.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/librustpkg/usage.rs b/src/librustpkg/usage.rs index 87ab3882df196..f5ac82b5684a6 100644 --- a/src/librustpkg/usage.rs +++ b/src/librustpkg/usage.rs @@ -63,20 +63,17 @@ List all installed packages."); } pub fn install() { - io::println("rustpkg [options..] install [url] [target] + io::println("rustpkg [options..] install [package-ID] -Install a package from a URL by Git or cURL (FTP, HTTP, etc.). -If target is provided, Git will checkout the branch or tag before -continuing. If the URL is a TAR file (with or without compression), -extract it before installing. If a URL isn't provided, the package will -be built and installed from the current directory (which is -functionally the same as `rustpkg build` and installing the result). +Install the given package ID if specified. With no package ID +argument, install the package in the current directory. +In that case, the current directory must be a direct child of a +`src` directory in a workspace. Examples: rustpkg install - rustpkg install git://github.com/mozilla/servo.git - rustpkg install git://github.com/mozilla/servo.git v0.1.2 - rustpkg install http://rust-lang.org/servo-0.1.2.tar.gz + rustpkg install github.com/mozilla/servo + rustpkg install github.com/mozilla/servo#0.1.2 Options: -c, --cfg Pass a cfg flag to the package script"); From bfac584a03f1e52808dc94aaa95703dd8847ab94 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Wed, 24 Jul 2013 18:42:14 -0700 Subject: [PATCH 2/4] rustpkg: Handle non-numeric versions; some cleanup Package IDs can now be of the form a/b/c#FOO, where (if a/b/c is a git repository) FOO is any tag in the repository. Non-numeric tags only match against package IDs with the same tag, and aren't compared linearly like numeric versions. While I was at it, refactored the code that calls `git clone`, and segregated build output properly for different packages. --- src/librustpkg/package_source.rs | 24 ++----- src/librustpkg/path_util.rs | 8 ++- src/librustpkg/source_control.rs | 44 +++++++++---- src/librustpkg/tests.rs | 106 ++++++++++++++++++++----------- src/librustpkg/version.rs | 4 +- 5 files changed, 117 insertions(+), 69 deletions(-) diff --git a/src/librustpkg/package_source.rs b/src/librustpkg/package_source.rs index f7eed969a69b8..3b3f08baa7e8a 100644 --- a/src/librustpkg/package_source.rs +++ b/src/librustpkg/package_source.rs @@ -11,14 +11,13 @@ use target::*; use package_id::PkgId; use std::path::Path; -use std::{os, run, str}; +use std::{os, str}; use context::*; use crate::Crate; use messages::*; -use source_control::git_clone; +use source_control::{git_clone, git_clone_general}; use path_util::pkgid_src_in_workspace; use util::compile_crate; -use version::{ExactRevision, SemanticVersion, NoVersion}; // An enumeration of the unpacked source of a package workspace. // This contains a list of files found in the source workspace. @@ -102,22 +101,13 @@ impl PkgSrc { } let url = fmt!("https://%s", self.id.remote_path.to_str()); - let branch_args = match self.id.version { - NoVersion => ~[], - ExactRevision(ref s) => ~[~"--branch", (*s).clone()], - SemanticVersion(ref s) => ~[~"--branch", s.to_str()] - }; - - - note(fmt!("Fetching package: git clone %s %s %?", url, local.to_str(), branch_args)); - - if run::process_output("git", - ~[~"clone", url.clone(), local.to_str()] + branch_args).status != 0 { - note(fmt!("fetching %s failed: can't clone repository", url)); - None + 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) { + Some(local) } else { - Some(local) + None } } diff --git a/src/librustpkg/path_util.rs b/src/librustpkg/path_util.rs index 700dbea8182d2..696e4b30d1b58 100644 --- a/src/librustpkg/path_util.rs +++ b/src/librustpkg/path_util.rs @@ -52,6 +52,7 @@ pub fn rust_path() -> ~[Path] { } None => ~[] }; + debug!("RUST_PATH entries from environment: %?", env_rust_path); let cwd = os::getcwd(); // now add in default entries env_rust_path.push(cwd.clone()); @@ -345,7 +346,12 @@ fn target_file_in_workspace(pkgid: &PkgId, workspace: &Path, let subdir = match what { Lib => "lib", Main | Test | Bench => "bin" }; - let result = workspace.push(subdir); + // 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), + _ => workspace.push(subdir) + }; if !os::path_exists(&result) && !mkdir_recursive(&result, U_RWX) { cond.raise((result.clone(), fmt!("target_file_in_workspace couldn't \ create the %s dir (pkgid=%s, workspace=%s, what=%?, where=%?", diff --git a/src/librustpkg/source_control.rs b/src/librustpkg/source_control.rs index 8d125300a3344..03b2b593b7005 100644 --- a/src/librustpkg/source_control.rs +++ b/src/librustpkg/source_control.rs @@ -18,22 +18,14 @@ pub fn git_clone(source: &Path, target: &Path, v: &Version) { assert!(os::path_is_dir(source)); assert!(is_git_dir(source)); if !os::path_exists(target) { - let version_args = match v { - &ExactRevision(ref s) => ~[~"--branch", s.to_owned()], - _ => ~[] - }; - debug!("Running: git clone %s %s %s", version_args.to_str(), source.to_str(), + debug!("Running: git clone %s %s", source.to_str(), target.to_str()); - let outp = run::process_output("git", ~[~"clone"] + version_args + - ~[source.to_str(), target.to_str()]); - if outp.status != 0 { - io::println(str::from_bytes_owned(outp.output.clone())); - io::println(str::from_bytes_owned(outp.error)); - fail!("Couldn't `git clone` %s", source.to_str()); - } + assert!(git_clone_general(source.to_str(), target, v)); } else { // Pull changes + // Note that this ignores tags, which is probably wrong. There are no tests for + // it, though. debug!("Running: git --work-tree=%s --git-dir=%s pull --no-edit %s", target.to_str(), target.push(".git").to_str(), source.to_str()); let outp = run::process_output("git", [fmt!("--work-tree=%s", target.to_str()), @@ -43,6 +35,34 @@ pub fn git_clone(source: &Path, target: &Path, v: &Version) { } } +/// Source can be either a URL or a local file path. +/// true if successful +pub fn git_clone_general(source: &str, target: &Path, v: &Version) -> bool { + let outp = run::process_output("git", [~"clone", source.to_str(), target.to_str()]); + if outp.status != 0 { + debug!(str::from_bytes_owned(outp.output.clone())); + debug!(str::from_bytes_owned(outp.error)); + false + } + else { + match v { + &ExactRevision(ref s) | &Tagged(ref s) => { + let outp = run::process_output_in_cwd("git", [~"checkout", fmt!("tags/%s", *s)], + target); + if outp.status != 0 { + debug!(str::from_bytes_owned(outp.output.clone())); + debug!(str::from_bytes_owned(outp.error)); + false + } + else { + true + } + } + _ => true + } + } +} + pub fn is_git_dir(p: &Path) -> bool { os::path_is_dir(&p.push(".git")) } diff --git a/src/librustpkg/tests.rs b/src/librustpkg/tests.rs index 286b1f84802c3..4e91fd514c8df 100644 --- a/src/librustpkg/tests.rs +++ b/src/librustpkg/tests.rs @@ -18,7 +18,7 @@ use std::run::ProcessOutput; use installed_packages::list_installed_packages; use package_path::*; use package_id::{PkgId}; -use version::{ExactRevision, NoVersion, Version}; +use version::{ExactRevision, NoVersion, Version, Tagged}; use path_util::{target_executable_in_workspace, target_library_in_workspace, target_test_in_workspace, target_bench_in_workspace, make_dir_rwx, U_RWX, library_in_workspace, @@ -61,6 +61,16 @@ fn git_repo_pkg() -> PkgId { } } +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()), + remote_path: remote, + short_name: ~"test_pkg", + version: Tagged(a_tag) + } +} + fn writeFile(file_path: &Path, contents: &str) { let out: @io::Writer = result::unwrap(io::file_writer(file_path, @@ -150,9 +160,13 @@ fn init_git_repo(p: &Path) -> Path { } } -fn add_git_tag(repo: &Path, tag: ~str) { - assert!(repo.is_absolute()); - let mut prog = run::Process::new("git", [~"add", ~"-A"], +fn add_all_and_commit(repo: &Path) { + git_add_all(repo); + git_commit(repo, ~"floop"); +} + +fn git_commit(repo: &Path, msg: ~str) { + let mut prog = run::Process::new("git", [~"commit", ~"-m", msg], run::ProcessOptions { env: None, dir: Some(repo), in_fd: None, @@ -161,9 +175,14 @@ fn add_git_tag(repo: &Path, tag: ~str) { }); let output = prog.finish_with_output(); if output.status != 0 { - fail!("Couldn't add all files in %s", repo.to_str()) + fail!("Couldn't commit in %s: output was %s", repo.to_str(), + str::from_bytes(output.output + output.error)) } - prog = run::Process::new("git", [~"commit", ~"-m", ~"whatever"], + +} + +fn git_add_all(repo: &Path) { + let mut prog = run::Process::new("git", [~"add", ~"-A"], run::ProcessOptions { env: None, dir: Some(repo), in_fd: None, @@ -172,10 +191,16 @@ fn add_git_tag(repo: &Path, tag: ~str) { }); let output = prog.finish_with_output(); if output.status != 0 { - fail!("Couldn't commit in %s", repo.to_str()) + fail!("Couldn't add all files in %s: output was %s", + repo.to_str(), str::from_bytes(output.output + output.error)) } +} - prog = run::Process::new("git", [~"tag", tag.clone()], +fn add_git_tag(repo: &Path, tag: ~str) { + assert!(repo.is_absolute()); + git_add_all(repo); + git_commit(repo, ~"whatever"); + let mut prog = run::Process::new("git", [~"tag", tag.clone()], run::ProcessOptions { env: None, dir: Some(repo), in_fd: None, @@ -624,31 +649,6 @@ fn test_package_request_version() { writeFile(&repo_subdir.push("version-0.4-file.txt"), "hello"); add_git_tag(&repo_subdir, ~"0.4"); -/* - - let pkg_src = PkgSrc::new(&repo, &repo, &temp_pkg_id); - match temp_pkg_id.version { - ExactRevision(~"0.3") => { - debug!("Version matches, calling fetch_git"); - match pkg_src.fetch_git() { - Some(p) => { - debug!("does version-0.3-file exist?"); - assert!(os::path_exists(&p.push("version-0.3-file.txt"))); - debug!("does version-0.4-file exist?"); - assert!(!os::path_exists(&p.push("version-0.4-file.txt"))); - - } - None => fail!("test_package_request_version: fetch_git failed") - } - } - ExactRevision(n) => { - fail!("n is %? and %? %s %?", n, n, if n == ~"0.3" { "==" } else { "!=" }, "0.3"); - } - _ => fail!(fmt!("test_package_version: package version was %?, expected ExactRevision(0.3)", - temp_pkg_id.version)) - } -*/ - command_line_test([~"install", fmt!("%s#0.3", local_path)], &repo); assert!(match installed_library_in_workspace("test_pkg_version", &repo.push(".rust")) { @@ -681,6 +681,7 @@ fn rustpkg_install_url_2() { } // FIXME: #7956: temporarily disabled +#[test] fn rustpkg_library_target() { let foo_repo = init_git_repo(&Path("foo")); let package_dir = foo_repo.push("foo"); @@ -707,8 +708,10 @@ fn rustpkg_local_pkg() { assert_executable_exists(&dir, "foo"); } +// FIXME: #7956: temporarily disabled +// Failing on dist-linux bot #[test] -#[ignore] // XXX Failing on dist-linux bot +#[ignore] fn package_script_with_default_build() { let dir = create_local_package(&PkgId::new("fancy-lib", &os::getcwd())); debug!("dir = %s", dir.to_str()); @@ -767,7 +770,7 @@ fn rustpkg_clean_no_arg() { } #[test] -#[ignore (reason = "Un-ignore when #7071 is fixed")] +#[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); @@ -776,9 +779,13 @@ fn rust_path_test() { let cwd = os::getcwd(); debug!("cwd = %s", cwd.to_str()); + debug!("Running command: cd %s; RUST_LOG=rustpkg RUST_PATH=%s rustpkg install foo", + cwd.to_str(), dir_for_path.to_str()); let mut prog = run::Process::new("rustpkg", [~"install", ~"foo"], - run::ProcessOptions { env: Some(&[(~"RUST_PATH", + run::ProcessOptions { env: Some(&[(~"RUST_LOG", + ~"rustpkg"), + (~"RUST_PATH", dir_for_path.to_str())]), dir: Some(&cwd), in_fd: None, @@ -956,7 +963,6 @@ fn do_rebuild_dep_only_contents_change() { } #[test] -#[ignore(reason = "list not yet implemented")] fn test_versions() { let workspace = create_local_package(&PkgId::new("foo#0.1", &os::getcwd())); create_local_package(&PkgId::new("foo#0.2", &os::getcwd())); @@ -994,7 +1000,6 @@ fn test_rustpkg_test() { } #[test] -#[ignore(reason = "uninstall not yet implemented")] fn test_uninstall() { let workspace = create_local_package(&PkgId::new("foo", &os::getcwd())); let _output = command_line_test([~"info", ~"foo"], &workspace); @@ -1002,3 +1007,28 @@ fn test_uninstall() { let output = command_line_test([~"list"], &workspace); assert!(!str::from_bytes(output.output).contains("foo")); } + +#[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_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 = (); }"); + add_git_tag(&repo_subdir, ~"testbranch"); + writeFile(&repo_subdir.push("testbranch_only"), "hello"); + add_git_tag(&repo_subdir, ~"another_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.remote_path.to_str())], + &repo); + let file1 = repo.push_many(["mockgithub.com", "catamorphism", + "test_pkg", "testbranch_only"]); + let file2 = repo.push_many(["mockgithub.com", "catamorphism", "test_pkg", + "master_only"]); + assert!(os::path_exists(&file1));' + assert!(!os::path_exists(&file2)); +} diff --git a/src/librustpkg/version.rs b/src/librustpkg/version.rs index 9e68ba54b6dfb..4fa72b713ace2 100644 --- a/src/librustpkg/version.rs +++ b/src/librustpkg/version.rs @@ -22,6 +22,8 @@ use extra::tempfile::mkdtemp; pub enum Version { ExactRevision(~str), // Should look like a m.n.(...).x SemanticVersion(semver::Version), + Tagged(~str), // String that can't be parsed as a version. + // Requirements get interpreted exactly NoVersion // user didn't specify a version -- prints as 0.1 } @@ -76,7 +78,7 @@ impl Ord for Version { impl ToStr for Version { fn to_str(&self) -> ~str { match *self { - ExactRevision(ref n) => fmt!("%s", n.to_str()), + ExactRevision(ref n) | Tagged(ref n) => fmt!("%s", n.to_str()), SemanticVersion(ref v) => fmt!("%s", v.to_str()), NoVersion => ~"0.1" } From c8780511b924705fbb567398e5201d9b492a2f32 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Fri, 26 Jul 2013 12:31:53 -0700 Subject: [PATCH 3/4] rustpkg: Don't assume a non-numeric refspec is a tag Just pass it directly to git, without prefixing it with tags/ --- src/librustpkg/source_control.rs | 11 +++++++++-- src/librustpkg/tests.rs | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/librustpkg/source_control.rs b/src/librustpkg/source_control.rs index 03b2b593b7005..e3b796a03bb25 100644 --- a/src/librustpkg/source_control.rs +++ b/src/librustpkg/source_control.rs @@ -10,7 +10,8 @@ // Utils for working with version control repositories. Just git right now. -use std::{io, os, run, str}; +use std::{os, run, str}; +use std::run::{ProcessOutput, ProcessOptions, Process}; use version::*; /// For a local git repo @@ -47,7 +48,7 @@ pub fn git_clone_general(source: &str, target: &Path, v: &Version) -> bool { else { match v { &ExactRevision(ref s) | &Tagged(ref s) => { - let outp = run::process_output_in_cwd("git", [~"checkout", fmt!("tags/%s", *s)], + let outp = process_output_in_cwd("git", [~"checkout", fmt!("%s", *s)], target); if outp.status != 0 { debug!(str::from_bytes_owned(outp.output.clone())); @@ -63,6 +64,12 @@ pub fn git_clone_general(source: &str, target: &Path, v: &Version) -> bool { } } +fn process_output_in_cwd(prog: &str, args: &[~str], cwd: &Path) -> ProcessOutput { + let mut prog = Process::new(prog, args, ProcessOptions{ dir: Some(cwd) + ,..ProcessOptions::new()}); + prog.finish_with_output() +} + pub fn is_git_dir(p: &Path) -> bool { os::path_is_dir(&p.push(".git")) } diff --git a/src/librustpkg/tests.rs b/src/librustpkg/tests.rs index 4e91fd514c8df..594298331b784 100644 --- a/src/librustpkg/tests.rs +++ b/src/librustpkg/tests.rs @@ -1029,6 +1029,6 @@ fn test_non_numeric_tag() { "test_pkg", "testbranch_only"]); let file2 = repo.push_many(["mockgithub.com", "catamorphism", "test_pkg", "master_only"]); - assert!(os::path_exists(&file1));' + assert!(os::path_exists(&file1)); assert!(!os::path_exists(&file2)); } From 7079ec6e2ed0d9de58d5a87b8ac7f6c31a65a4cd Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Fri, 26 Jul 2013 20:06:30 -0700 Subject: [PATCH 4/4] docs: Talk about tags that aren't versions in the "Package identifiers" section --- doc/rustpkg.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/rustpkg.md b/doc/rustpkg.md index 506fc2ad15a8c..46ab7afab43c2 100644 --- a/doc/rustpkg.md +++ b/doc/rustpkg.md @@ -76,6 +76,14 @@ A package ID can also specify a version, like: `github.com/mozilla/rust#0.3`. In this case, `rustpkg` will check that the repository `github.com/mozilla/rust` has a tag named `0.3`, and report an error otherwise. +A package ID can also specify a particular revision of a repository, like: +`github.com/mozilla/rust#release-0.7`. +When the refspec (portion of the package ID after the `#`) can't be parsed as a decimal number, +rustpkg passes the refspec along to the version control system without interpreting it. +rustpkg also interprets any dependencies on such a package ID literally +(as opposed to versions, where a newer version satisfies a dependency on an older version). +Thus, `github.com/mozilla/rust#5c4cd30f80` is also a valid package ID, +since git can deduce that 5c4cd30f80 refers to a revision of the desired repository. ## Source files