Skip to content

Commit 9185445

Browse files
committed
Fix some packaging logic in path sources
Currently the packaging logic depends on the old recursive nature of path sources for a few points: * Discovery of a git repository of a package. * Filtering out of sibling packages for only including the right set of files. For a non-recursive path source (now essentially the default) we can no longer assume that we have a listing of all packages. Subsequently this logic was tweaked to allow: * Instead of looking for packages at the root of a repo, we instead look for a Cargo.toml at the root of a git repository. * We keep track of all Cargo.toml files found in a repository and prune out all files which appear to be ancestors of that package.
1 parent f28c787 commit 9185445

File tree

1 file changed

+67
-39
lines changed

1 file changed

+67
-39
lines changed

src/cargo/sources/path.rs

Lines changed: 67 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -108,24 +108,39 @@ impl<'cfg> PathSource<'cfg> {
108108
}
109109
};
110110

111-
// If this package is a git repository, then we really do want to query
112-
// the git repository as it takes into account items such as .gitignore.
113-
// We're not quite sure where the git repository is, however, so we do a
114-
// bit of a probe.
111+
// If this package is in a git repository, then we really do want to
112+
// query the git repository as it takes into account items such as
113+
// .gitignore. We're not quite sure where the git repository is,
114+
// however, so we do a bit of a probe.
115115
//
116-
// We check all packages in this source that are ancestors of the
117-
// specified package (including the same package) to see if they're at
118-
// the root of the git repository. This isn't always true, but it'll get
119-
// us there most of the time!
120-
let repo = self.packages.iter()
121-
.map(|pkg| pkg.root())
122-
.filter(|path| root.starts_with(path))
123-
.filter_map(|path| git2::Repository::open(&path).ok())
124-
.next();
125-
match repo {
126-
Some(repo) => self.list_files_git(pkg, repo, &mut filter),
127-
None => self.list_files_walk(pkg, &mut filter),
116+
// We walk this package's path upwards and look for a sibling
117+
// Cargo.toml and .git folder. If we find one then we assume that we're
118+
// part of that repository.
119+
let mut cur = root;
120+
loop {
121+
if cur.join("Cargo.toml").is_file() {
122+
// If we find a git repository next to this Cargo.toml, we still
123+
// check to see if we are indeed part of the index. If not, then
124+
// this is likely an unrelated git repo, so keep going.
125+
if let Ok(repo) = git2::Repository::open(cur) {
126+
let index = try!(repo.index());
127+
let path = util::without_prefix(root, cur)
128+
.unwrap().join("Cargo.toml");
129+
if index.get_path(&path, 0).is_some() {
130+
return self.list_files_git(pkg, repo, &mut filter);
131+
}
132+
}
133+
}
134+
// don't cross submodule boundaries
135+
if cur.join(".git").is_dir() {
136+
break
137+
}
138+
match cur.parent() {
139+
Some(parent) => cur = parent,
140+
None => break,
141+
}
128142
}
143+
self.list_files_walk(pkg, &mut filter)
129144
}
130145

131146
fn list_files_git(&self, pkg: &Package, repo: git2::Repository,
@@ -138,7 +153,7 @@ impl<'cfg> PathSource<'cfg> {
138153
}));
139154
let pkg_path = pkg.root();
140155

141-
let mut ret = Vec::new();
156+
let mut ret = Vec::<PathBuf>::new();
142157

143158
// We use information from the git repository to guide us in traversing
144159
// its tree. The primary purpose of this is to take advantage of the
@@ -165,32 +180,48 @@ impl<'cfg> PathSource<'cfg> {
165180
}
166181
});
167182

183+
let mut subpackages_found = Vec::new();
184+
168185
'outer: for (file_path, is_dir) in index_files.chain(untracked) {
169186
let file_path = try!(file_path);
170187

171-
// Filter out files outside this package.
172-
if !file_path.starts_with(pkg_path) { continue }
173-
174-
// Filter out Cargo.lock and target always
175-
{
176-
let fname = file_path.file_name().and_then(|s| s.to_str());
177-
if fname == Some("Cargo.lock") { continue }
178-
if fname == Some("target") { continue }
188+
// Filter out files blatantly outside this package. This is helped a
189+
// bit obove via the `pathspec` function call, but we need to filter
190+
// the entries in the index as well.
191+
if !file_path.starts_with(pkg_path) {
192+
continue
179193
}
180194

181-
// Filter out sub-packages of this package
182-
for other_pkg in self.packages.iter().filter(|p| *p != pkg) {
183-
let other_path = other_pkg.root();
184-
if other_path.starts_with(pkg_path) &&
185-
file_path.starts_with(other_path) {
186-
continue 'outer;
195+
match file_path.file_name().and_then(|s| s.to_str()) {
196+
// Filter out Cargo.lock and target always, we don't want to
197+
// package a lock file no one will ever read and we also avoid
198+
// build artifacts
199+
Some("Cargo.lock") |
200+
Some("target") => continue,
201+
202+
// Keep track of all sub-packages found and also strip out all
203+
// matches we've found so far. Note, though, that if we find
204+
// our own `Cargo.toml` we keep going.
205+
Some("Cargo.toml") => {
206+
let path = file_path.parent().unwrap();
207+
if path != pkg_path {
208+
warn!("subpackage found: {}", path.display());
209+
ret.retain(|p| !p.starts_with(path));
210+
subpackages_found.push(path.to_path_buf());
211+
continue
212+
}
187213
}
214+
215+
_ => {}
188216
}
189217

190-
let is_dir = is_dir.or_else(|| {
191-
fs::metadata(&file_path).ok().map(|m| m.is_dir())
192-
}).unwrap_or(false);
193-
if is_dir {
218+
// If this file is part of any other sub-package we've found so far,
219+
// skip it.
220+
if subpackages_found.iter().any(|p| file_path.starts_with(p)) {
221+
continue
222+
}
223+
224+
if is_dir.unwrap_or_else(|| file_path.is_dir()) {
194225
warn!(" found submodule {}", file_path.display());
195226
let rel = util::without_prefix(&file_path, &root).unwrap();
196227
let rel = try!(rel.to_str().chain_error(|| {
@@ -237,10 +268,7 @@ impl<'cfg> PathSource<'cfg> {
237268
fn list_files_walk(&self, pkg: &Package, filter: &mut FnMut(&Path) -> bool)
238269
-> CargoResult<Vec<PathBuf>> {
239270
let mut ret = Vec::new();
240-
for pkg in self.packages.iter().filter(|p| *p == pkg) {
241-
let loc = pkg.root();
242-
try!(PathSource::walk(loc, &mut ret, true, filter));
243-
}
271+
try!(PathSource::walk(pkg.root(), &mut ret, true, filter));
244272
Ok(ret)
245273
}
246274

0 commit comments

Comments
 (0)