Auto merge of #1131 - alexcrichton:issue-880, r=brson

This fixes a number of bugs along the way:

* Submodules are now recursed into explicitly for packaging, fixing #943
* A whitelist has been implemented, fixing #880
* Git repos are now always used if there is a package that resides at the root,
  not just if the current package resides at the root.

src/cargo/core/manifest.rs

@@ -20,6 +20,7 @@ pub struct Manifest {
     links: Option<String>,
     warnings: Vec<String>,
     exclude: Vec<String>,
+    include: Vec<String>,
     metadata: ManifestMetadata,
 }
 
@@ -412,7 +413,10 @@ impl Show for Target {
 impl Manifest {
     pub fn new(summary: Summary, targets: Vec<Target>,
                target_dir: Path, doc_dir: Path,
-               build: Vec<String>, exclude: Vec<String>, links: Option<String>,
+               build: Vec<String>,
+               exclude: Vec<String>,
+               include: Vec<String>,
+               links: Option<String>,
                metadata: ManifestMetadata) -> Manifest {
         Manifest {
             summary: summary,
@@ -422,6 +426,7 @@ impl Manifest {
             build: build,     // TODO: deprecated, remove
             warnings: Vec::new(),
             exclude: exclude,
+            include: include,
             links: links,
             metadata: metadata,
         }
@@ -479,6 +484,10 @@ impl Manifest {
         self.exclude.as_slice()
     }
 
+    pub fn get_include(&self) -> &[String] {
+        self.include.as_slice()
+    }
+
     pub fn get_metadata(&self) -> &ManifestMetadata { &self.metadata }
 
     pub fn set_summary(&mut self, summary: Summary) {

src/cargo/ops/cargo_new.rs

@@ -9,7 +9,7 @@ use git2::Config;
 use util::{GitRepo, HgRepo, CargoResult, human, ChainError, config, internal};
 use core::shell::MultiShell;
 
-#[deriving(Copy, Show, PartialEq)]
+#[derive(Copy, Show, PartialEq)]
 pub enum VersionControl { Git, Hg, NoVcs }
 
 pub struct NewOptions<'a> {

src/cargo/sources/path.rs

@@ -6,7 +6,7 @@ use git2;
 
 use core::{Package, PackageId, Summary, SourceId, Source, Dependency, Registry};
 use ops;
-use util::{CargoResult, internal, internal_error};
+use util::{CargoResult, internal, internal_error, human, ChainError};
 
 pub struct PathSource {
     id: SourceId,
@@ -71,34 +71,46 @@ impl PathSource {
     pub fn list_files(&self, pkg: &Package) -> CargoResult<Vec<Path>> {
         let root = pkg.get_manifest_path().dir_path();
 
-        // Check whether the package itself is a git repository.
-        let candidates = match git2::Repository::open(&root) {
-            Ok(repo) => try!(self.list_files_git(pkg, repo)),
+        let exclude = pkg.get_manifest().get_exclude().iter().map(|p| {
+            Pattern::new(p.as_slice())
+        }).collect::<Vec<Pattern>>();
+        let include = pkg.get_manifest().get_include().iter().map(|p| {
+            Pattern::new(p.as_slice())
+        }).collect::<Vec<Pattern>>();
 
-            // If not, check whether the package is in a sub-directory of the main repository.
-            Err(..) if self.path.is_ancestor_of(&root) => {
-                match git2::Repository::open(&self.path) {
-                    Ok(repo) => try!(self.list_files_git(pkg, repo)),
-                    _ => try!(self.list_files_walk(pkg))
-                }
+        let mut filter = |&mut: p: &Path| {
+            let relative_path = p.path_relative_from(&root).unwrap();
+            include.iter().any(|p| p.matches_path(&relative_path)) || {
+                include.len() == 0 &&
+                 !exclude.iter().any(|p| p.matches_path(&relative_path))
             }
-            // If neither is true, fall back to walking the filesystem.
-            _ => try!(self.list_files_walk(pkg))
         };
 
-        let pats = pkg.get_manifest().get_exclude().iter().map(|p| {
-            Pattern::new(p.as_slice())
-        }).collect::<Vec<Pattern>>();
-
-        Ok(candidates.into_iter().filter(|candidate| {
-            let relative_path = candidate.path_relative_from(&root).unwrap();
-            !pats.iter().any(|p| p.matches_path(&relative_path)) &&
-                candidate.is_file()
-        }).collect())
+        // If this package is a git repository, then we really do want to query
+        // the git repository as it takes into account items such as .gitignore.
+        // We're not quite sure where the git repository is, however, so we do a
+        // bit of a probe.
+        //
+        // We check all packages in this source that are ancestors of the
+        // specified package (including the same package) to see if they're at
+        // the root of the git repository. This isn't always true, but it'll get
+        // us there most of the time!.
+        let repo = self.packages.iter()
+                       .map(|pkg| pkg.get_root())
+                       .filter(|path| path.is_ancestor_of(&root))
+                       .filter_map(|path| git2::Repository::open(&path).ok())
+                       .next();
+        match repo {
+            Some(repo) => self.list_files_git(pkg, repo, &mut filter),
+            None => self.list_files_walk(pkg, filter),
+        }
     }
 
-    fn list_files_git(&self, pkg: &Package, repo: git2::Repository)
-                      -> CargoResult<Vec<Path>> {
+    fn list_files_git<F>(&self, pkg: &Package, repo: git2::Repository,
+                         filter: &mut F)
+                         -> CargoResult<Vec<Path>>
+        where F: FnMut(&Path) -> bool
+    {
         warn!("list_files_git {}", pkg.get_package_id());
         let index = try!(repo.index());
         let root = match repo.workdir() {
@@ -108,8 +120,7 @@ impl PathSource {
         let pkg_path = pkg.get_manifest_path().dir_path();
 
         let mut ret = Vec::new();
-        'outer: for i in range(0, index.len()) {
-            let entry = match index.get(i) { Some(e) => e, None => continue };
+        'outer: for entry in index.iter() {
             let fname = entry.path.as_bytes_no_nul();
             let file_path = root.join(fname);
 
@@ -123,30 +134,52 @@ impl PathSource {
             // Filter out sub-packages of this package
             for other_pkg in self.packages.iter().filter(|p| *p != pkg) {
                 let other_path = other_pkg.get_manifest_path().dir_path();
-                if pkg_path.is_ancestor_of(&other_path) && other_path.is_ancestor_of(&file_path) {
+                if pkg_path.is_ancestor_of(&other_path) &&
+                   other_path.is_ancestor_of(&file_path) {
                     continue 'outer;
                 }
             }
 
-            // We found a file!
-            warn!("  found {}", file_path.display());
-            ret.push(file_path);
+            // TODO: the `entry` has a mode we should be able to look at instead
+            //       of just calling stat() again
+            if file_path.is_dir() {
+                warn!("  found submodule {}", file_path.display());
+                let rel = file_path.path_relative_from(&root).unwrap();
+                let rel = try!(rel.as_str().chain_error(|| {
+                    human(format!("invalid utf-8 filename: {}", rel.display()))
+                }));
+                let submodule = try!(repo.find_submodule(rel));
+                let repo = try!(submodule.open());
+                let files = try!(self.list_files_git(pkg, repo, filter));
+                ret.extend(files.into_iter());
+            } else if (*filter)(&file_path) {
+                // We found a file!
+                warn!("  found {}", file_path.display());
+                ret.push(file_path);
+            }
         }
         Ok(ret)
     }
 
-    fn list_files_walk(&self, pkg: &Package) -> CargoResult<Vec<Path>> {
+    fn list_files_walk<F>(&self, pkg: &Package, mut filter: F)
+                          -> CargoResult<Vec<Path>>
+        where F: FnMut(&Path) -> bool
+    {
         let mut ret = Vec::new();
         for pkg in self.packages.iter().filter(|p| *p == pkg) {
             let loc = pkg.get_manifest_path().dir_path();
-            try!(walk(&loc, &mut ret, true));
+            try!(walk(&loc, &mut ret, true, &mut filter));
         }
         return Ok(ret);
 
-        fn walk(path: &Path, ret: &mut Vec<Path>,
-                is_root: bool) -> CargoResult<()> {
+        fn walk<F>(path: &Path, ret: &mut Vec<Path>,
+                   is_root: bool, filter: &mut F) -> CargoResult<()>
+            where F: FnMut(&Path) -> bool
+        {
             if !path.is_dir() {
-                ret.push(path.clone());
+                if (*filter)(path) {
+                    ret.push(path.clone());
+                }
                 return Ok(())
             }
             // Don't recurse into any sub-packages that we have
@@ -158,7 +191,7 @@ impl PathSource {
                     (true, Some("Cargo.lock")) => continue,
                     _ => {}
                 }
-                try!(walk(dir, ret, false));
+                try!(walk(dir, ret, false, filter));
             }
             return Ok(())
         }

src/cargo/util/toml.rs

@@ -261,6 +261,7 @@ pub struct TomlProject {
     build: Option<BuildCommand>,       // TODO: `String` instead
     links: Option<String>,
     exclude: Option<Vec<String>>,
+    include: Option<Vec<String>>,
 
     // package metadata
     description: Option<String>,
@@ -508,6 +509,7 @@ impl TomlManifest {
         }
 
         let exclude = project.exclude.clone().unwrap_or(Vec::new());
+        let include = project.include.clone().unwrap_or(Vec::new());
 
         let has_old_build = old_build.len() >= 1;
 
@@ -531,6 +533,7 @@ impl TomlManifest {
                                          layout.root.join("doc"),
                                          old_build,
                                          exclude,
+                                         include,
                                          project.links.clone(),
                                          metadata);
         if used_deprecated_lib {

src/doc/crates-io.md

@@ -144,7 +144,18 @@ exclude = [
 ```
 
 The syntax of each element in this array is what
-[rust-lang/glob](https://github.com/rust-lang/glob) accepts.
+[rust-lang/glob](https://github.com/rust-lang/glob) accepts. If you'd rather
+roll with a whitelist instead of a blacklist, Cargo also supports an `include`
+key:
+
+```toml
+[package]
+# ...
+include = [
+    "**/*.rs",
+    "Cargo.toml",
+]
+```
 
 ## Uploading the crate
 

tests/test_cargo_package.rs

@@ -194,3 +194,51 @@ test!(package_verification {
         compiling = COMPILING,
         dir = p.url()).as_slice()));
 });
+
+test!(exclude {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            exclude = ["*.txt"]
+        "#)
+        .file("src/main.rs", r#"
+            fn main() { println!("hello"); }
+        "#)
+        .file("bar.txt", "")
+        .file("src/bar.txt", "");
+
+    assert_that(p.cargo_process("package").arg("--no-verify").arg("-v"),
+                execs().with_status(0).with_stdout(format!("\
+{packaging} foo v0.0.1 ([..])
+{archiving} [..]
+{archiving} [..]
+", packaging = PACKAGING, archiving = ARCHIVING).as_slice()));
+});
+
+test!(include {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            exclude = ["*.txt"]
+            include = ["foo.txt", "**/*.rs", "Cargo.toml"]
+        "#)
+        .file("foo.txt", "")
+        .file("src/main.rs", r#"
+            fn main() { println!("hello"); }
+        "#)
+        .file("src/bar.txt", ""); // should be ignored when packaging
+
+    assert_that(p.cargo_process("package").arg("--no-verify").arg("-v"),
+                execs().with_status(0).with_stdout(format!("\
+{packaging} foo v0.0.1 ([..])
+{archiving} [..]
+{archiving} [..]
+{archiving} [..]
+", packaging = PACKAGING, archiving = ARCHIVING).as_slice()));
+});

tests/test_cargo_registry.rs

@@ -497,7 +497,7 @@ test!(bad_license_file {
         .file("src/main.rs", r#"
             fn main() {}
         "#);
-    assert_that(p.cargo_process("publish"),
+    assert_that(p.cargo_process("publish").arg("-v"),
                 execs().with_status(101)
                        .with_stderr("\
 the license file `foo` does not exist"));