Skip to content

Commit 607c8ca

Browse files
committed
Assure the extra-logic in dirwalks applies to index files as well
This is the case for the git2 implementation naturally, but as `gitoxide` doesn't yet have a dirwalk iterator, it's much less intuitive here.
1 parent a4f048b commit 607c8ca

File tree

1 file changed

+91
-72
lines changed

1 file changed

+91
-72
lines changed

src/cargo/sources/path.rs

Lines changed: 91 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -533,26 +533,30 @@ impl<'gctx> PathSource<'gctx> {
533533
&mut delegate,
534534
)?;
535535

536-
let mut files = delegate.into_result()?;
536+
let (mut files, mut subpackages_found) = delegate.into_result()?;
537537
// Append all normal files that might be tracked in `target/`.
538-
files.extend(
539-
index
540-
.prefixed_entries(target_prefix.as_ref())
541-
.unwrap_or_default()
542-
.iter()
543-
.filter(|entry| {
544-
entry.stage() == 0
545-
&& entry
546-
.mode
547-
.to_tree_entry_mode()
548-
.map(|mode| mode.kind())
549-
.map_or(false, |kind| {
550-
use gix::object::tree::EntryKind::*;
551-
matches!(kind, Blob | BlobExecutable | Link)
552-
})
553-
})
554-
.map(|entry| root.join(gix::path::from_bstr(entry.path(&index)))),
555-
);
538+
for entry in index
539+
.prefixed_entries(target_prefix.as_ref())
540+
.unwrap_or_default()
541+
.iter()
542+
// probably not needed as conflicts prevent this to run, but let's be explicit.
543+
.filter(|entry| entry.stage() == 0)
544+
{
545+
handle_path(
546+
entry.path(&index),
547+
// Do not trust what's recorded in the index, enforce checking the disk.
548+
// This traversal is not part of a `status()`, and tracking things in `target/`
549+
// is rare.
550+
None,
551+
root,
552+
pkg,
553+
pkg_path,
554+
self,
555+
&mut files,
556+
&mut subpackages_found,
557+
filter,
558+
)?
559+
}
556560
return Ok(files);
557561

558562
struct Delegate<'a, 'gctx> {
@@ -602,10 +606,10 @@ impl<'gctx> PathSource<'gctx> {
602606
})
603607
}
604608

605-
fn into_result(self) -> CargoResult<Vec<PathBuf>> {
609+
fn into_result(self) -> CargoResult<(Vec<PathBuf>, Vec<PathBuf>)> {
606610
match self.err {
607611
None => {
608-
return Ok(self.paths);
612+
return Ok((self.paths, self.subpackages_found));
609613
}
610614
Some(e) => return Err(e),
611615
}
@@ -615,65 +619,80 @@ impl<'gctx> PathSource<'gctx> {
615619
if entry.status == Status::Untracked && entry.rela_path.as_ref() == "Cargo.lock" {
616620
return Ok(());
617621
}
618-
let file_path = self
619-
.root
620-
.join(gix::path::from_bstr(entry.rela_path.as_ref()));
621-
if file_path.file_name().and_then(|name| name.to_str()) == Some("Cargo.toml") {
622-
// Keep track of all sub-packages found and also strip out all
623-
// matches we've found so far. Note, though, that if we find
624-
// our own `Cargo.toml`, we keep going.
625-
let path = file_path.parent().unwrap();
626-
if path != self.pkg_path {
627-
warn!("subpackage found: {}", path.display());
628-
self.paths.retain(|p| !p.starts_with(path));
629-
self.subpackages_found.push(path.to_path_buf());
630-
return Ok(());
631-
}
632-
}
622+
handle_path(
623+
entry.rela_path.as_ref(),
624+
entry.disk_kind,
625+
self.root,
626+
self.pkg,
627+
self.pkg_path,
628+
self.parent,
629+
&mut self.paths,
630+
&mut self.subpackages_found,
631+
self.filter,
632+
)
633+
}
634+
}
633635

634-
// If this file is part of any other sub-package we've found so far,
635-
// skip it.
636-
if self
637-
.subpackages_found
638-
.iter()
639-
.any(|p| file_path.starts_with(p))
640-
{
636+
fn handle_path(
637+
rela_path: &gix::bstr::BStr,
638+
kind: Option<gix::dir::entry::Kind>,
639+
root: &Path,
640+
pkg: &Package,
641+
pkg_path: &Path,
642+
source: &PathSource<'_>,
643+
paths: &mut Vec<PathBuf>,
644+
subpackages_found: &mut Vec<PathBuf>,
645+
filter: &dyn Fn(&Path, bool) -> bool,
646+
) -> CargoResult<()> {
647+
let file_path = root.join(gix::path::from_bstr(rela_path));
648+
if file_path.file_name().and_then(|name| name.to_str()) == Some("Cargo.toml") {
649+
// Keep track of all sub-packages found and also strip out all
650+
// matches we've found so far. Note, though, that if we find
651+
// our own `Cargo.toml`, we keep going.
652+
let path = file_path.parent().unwrap();
653+
if path != pkg_path {
654+
warn!("subpackage found: {}", path.display());
655+
paths.retain(|p| !p.starts_with(path));
656+
subpackages_found.push(path.to_path_buf());
641657
return Ok(());
642658
}
659+
}
643660

644-
let is_dir = entry.disk_kind.map_or(false, |kind| {
645-
if kind == gix::dir::entry::Kind::Symlink {
646-
// Symlinks must be checked to see if they point to a directory
647-
// we should traverse.
648-
file_path.is_dir()
649-
} else {
650-
kind.is_dir()
661+
// If this file is part of any other sub-package we've found so far,
662+
// skip it.
663+
if subpackages_found.iter().any(|p| file_path.starts_with(p)) {
664+
return Ok(());
665+
}
666+
667+
let is_dir = kind.map_or(false, |kind| {
668+
if kind == gix::dir::entry::Kind::Symlink {
669+
// Symlinks must be checked to see if they point to a directory
670+
// we should traverse.
671+
file_path.is_dir()
672+
} else {
673+
kind.is_dir()
674+
}
675+
});
676+
if is_dir {
677+
// This could be a submodule, or a sub-repository. In any case, we prefer to walk
678+
// it with git-support to leverage ignored files and to avoid pulling in entire
679+
// .git repositories.
680+
match gix::open_opts(&file_path, gix::open::Options::isolated()) {
681+
Ok(sub_repo) => {
682+
let files = source.list_files_gix(pkg, &sub_repo, filter)?;
683+
paths.extend(files);
651684
}
652-
});
653-
if is_dir {
654-
// This could be a submodule, or a sub-repository. In any case, we prefer to walk
655-
// it with git-support to leverage ignored files and to avoid pulling in entire
656-
// .git repositories.
657-
match gix::open_opts(&file_path, gix::open::Options::isolated()) {
658-
Ok(sub_repo) => {
659-
let files =
660-
self.parent
661-
.list_files_gix(self.pkg, &sub_repo, self.filter)?;
662-
self.paths.extend(files);
663-
}
664-
_ => {
665-
self.parent
666-
.walk(&file_path, &mut self.paths, false, self.filter)?;
667-
}
685+
_ => {
686+
source.walk(&file_path, paths, false, filter)?;
668687
}
669-
} else if (self.filter)(&file_path, is_dir) {
670-
assert!(!is_dir);
671-
// We found a file!
672-
warn!(" found {}", file_path.display());
673-
self.paths.push(file_path);
674688
}
675-
Ok(())
689+
} else if (filter)(&file_path, is_dir) {
690+
assert!(!is_dir);
691+
// We found a file!
692+
warn!(" found {}", file_path.display());
693+
paths.push(file_path);
676694
}
695+
Ok(())
677696
}
678697
}
679698

0 commit comments

Comments
 (0)