History

make sure that `*foo*` prefixes don't end up matching any directory.

Even though wildcard pathspecs shouldn't prevent recursion into direcotries, we should not end up declaring them as matches even though nothing inside matched.

Sebastian Thiel committed 2 years ago

Commit 482d6f31 parent a663e9f3 files · +305 −18Browse files

Unified · Split

3 files
  1. gix-dir/src/walk/readdir.rs +15 −17
  2. gix-dir/tests/fixtures/many.sh +21 −0
  3. gix-dir/tests/walk/mod.rs +269 −1
gix-dir/src/walk/readdir.rs+15−17
@@ -66,7 +66,7 @@ pub(super) fn recursive( · 65 unmodified lines
6666 if can_recurse(current_bstr.as_bstr(), info, opts.for_deletion, delegate) {
6767 let (action, subdir_prevent_collapse) =
6868 recursive(false, current, current_bstr, info, ctx, opts, delegate, out, state)?;
69− prevent_collapse = subdir_prevent_collapse;
69+ prevent_collapse |= subdir_prevent_collapse;
7070 if action != Action::Continue {
7171 break;
7272 }
@@ -158,6 +158,7 @@ impl Mark { · 85 unmodified lines
158158 } else {
159159 dir_info.disk_kind
160160 },
161+ pathspec_match: filter_dir_pathspec(dir_info.pathspec_match),
161162 ..dir_info
162163 };
163164 if opts.should_hold(empty_info.status) {
@@ -174,18 +175,10 @@ impl Mark { · 10 unmodified lines
174175 }
175176 } else if *prevent_collapse {
176177 self.emit_all_held(state, opts, out, delegate)
177− } else if let Some(action) = self.try_collapse(
178− dir_rela_path,
179− dir_info,
180− state,
181− prevent_collapse,
182− out,
183− opts,
184− ctx,
185− delegate,
186− ) {
178+ } else if let Some(action) = self.try_collapse(dir_rela_path, dir_info, state, out, opts, ctx, delegate) {
187179 action
188180 } else {
181+ *prevent_collapse = true;
189182 self.emit_all_held(state, opts, out, delegate)
190183 }
191184 }
@@ -213,7 +206,6 @@ impl Mark { · 21 unmodified lines
213206 dir_rela_path: &BStr,
214207 dir_info: classify::Outcome,
215208 state: &mut State,
216− prevent_collapse: &mut bool,
217209 out: &mut walk::Outcome,
218210 opts: Options,
219211 ctx: &mut Context<'_>,
@@ -229,7 +221,6 @@ impl Mark { · 9 unmodified lines
229221 {
230222 entries += 1;
231223 if kind == Some(entry::Kind::Repository) {
232− *prevent_collapse = true;
233224 return None;
234225 }
235226 if pathspec_match.map_or(false, |m| {
@@ -287,12 +278,10 @@ impl Mark { · 51 unmodified lines
287278 .filter_map(|e| e.pathspec_match)
288279 .max()
289280 .or_else(|| {
290− // Only take directory matches for value if they are above the 'guessed' ones.
281+ // Only take directory matches as value if they are above the 'guessed' ones.
291282 // Otherwise we end up with seemingly matched entries in the parent directory which
292283 // affects proper folding.
293− dir_info
294− .pathspec_match
295− .filter(|m| matches!(m, PathspecMatch::WildcardMatch | PathspecMatch::Verbatim))
284+ filter_dir_pathspec(dir_info.pathspec_match)
296285 });
297286 let mut removed_without_emitting = 0;
298287 let mut action = Action::Continue;
@@ -317,6 +306,15 @@ impl Mark { · 18 unmodified lines
317306 }
318307 }
319308
309+fn filter_dir_pathspec(current: Option<PathspecMatch>) -> Option<PathspecMatch> {
310+ current.filter(|m| {
311+ matches!(
312+ m,
313+ PathspecMatch::Always | PathspecMatch::WildcardMatch | PathspecMatch::Verbatim
314+ )
315+ })
316+}
317+
320318 impl Options {
321319 fn should_hold(&self, status: entry::Status) -> bool {
322320 if status.is_pruned() {
gix-dir/tests/fixtures/many.sh+21−0
@@ -57,6 +57,16 @@ cp -R ignored-dir ignored-dir-with-nested-bare-repository · 56 unmodified lines
5757 git init --bare bare
5858 )
5959
60+cp -R ignored-dir-with-nested-bare-repository ignored-dir-nested-minimal
61+(cd ignored-dir-nested-minimal
62+ (cd bare
63+ rm -Rf hooks config description
64+ )
65+ (cd dir/subdir/nested-bare
66+ rm -Rf refs hooks config description
67+ )
68+)
69+
6070 mkdir untracked-hidden-bare
6171 (cd untracked-hidden-bare
6272 mkdir subdir
@@ -245,6 +255,17 @@ git init expendable-and-precious · 182 unmodified lines
245255 git commit -m "init"
246256 )
247257
258+git init expendable-and-precious-nested-in-ignored-dir
259+(cd expendable-and-precious-nested-in-ignored-dir
260+ echo 'ignored/' > .gitignore
261+ git add .gitignore && git commit -m "init"
262+ mkdir -p ignored/other
263+ cp -Rv ../expendable-and-precious ignored/d
264+ rm -Rf ignored/d/*-by-filematch ignored/d/some-*
265+ mkdir -p other/ignored && >other/ignored/a
266+)
267+
268+
248269 mkdir empty-and-untracked-dir
249270 (cd empty-and-untracked-dir
250271 mkdir empty untracked
gix-dir/tests/walk/mod.rs+269−1
@@ -578,7 +578,7 @@ fn untracked_and_ignored_pathspec_guidance() -> crate::Result { · 577 unmodified lines
578578 }
579579
580580 #[test]
581−fn untracked_and_ignored_for_deletion_negative_spec() -> crate::Result {
581+fn untracked_and_ignored_for_deletion_negative_wildcard_spec() -> crate::Result {
582582 let root = fixture("subdir-untracked-and-ignored");
583583 let (out, entries) = collect_filtered(
584584 &root,
@@ -632,6 +632,274 @@ fn untracked_and_ignored_for_deletion_negative_spec() -> crate::Result { · 47 unmodified lines
632632 Ok(())
633633 }
634634
635+#[test]
636+fn untracked_and_ignored_for_deletion_positive_wildcard_spec() -> crate::Result {
637+ let root = fixture("subdir-untracked-and-ignored");
638+ let (out, entries) = collect_filtered(
639+ &root,
640+ |keep, ctx| {
641+ walk(
642+ &root,
643+ &root,
644+ ctx,
645+ walk::Options {
646+ emit_ignored: Some(CollapseDirectory),
647+ emit_untracked: CollapseDirectory,
648+ emit_pruned: true,
649+ for_deletion: Some(Default::default()),
650+ ..options()
651+ },
652+ keep,
653+ )
654+ },
655+ Some("*generated*"),
656+ );
657+ assert_eq!(
658+ out,
659+ walk::Outcome {
660+ read_dir_calls: 8,
661+ returned_entries: entries.len(),
662+ seen_entries: 27,
663+ },
664+ );
665+ assert_eq!(
666+ &entries,
667+ &[
668+ entry_nokind(".git", DotGit),
669+ entry_nomatch(".gitignore", Pruned, File),
670+ entry_nomatch("a.o", Ignored(Expendable), File),
671+ entry_nomatch("b.o", Ignored(Expendable), File),
672+ entry_nomatch("c.o", Ignored(Expendable), File),
673+ entry_nomatch("d/a.o", Ignored(Expendable), File),
674+ entry_nomatch("d/b.o", Ignored(Expendable), File),
675+ entry_nomatch("d/d/a", Pruned, File),
676+ entry_nomatch("d/d/a.o", Ignored(Expendable), File),
677+ entry_nomatch("d/d/b.o", Ignored(Expendable), File),
678+ entryps("d/d/generated", Ignored(Expendable), Directory, WildcardMatch),
679+ entryps("d/generated", Ignored(Expendable), Directory, WildcardMatch),
680+ entryps("generated", Ignored(Expendable), Directory, WildcardMatch),
681+ entry_nomatch("objs", Ignored(Expendable), Directory),
682+ ],
683+ "'generated' folders are included, and collapsing is done where possible"
684+ );
685+ Ok(())
686+}
687+
688+#[test]
689+fn untracked_and_ignored_for_deletion_nonmatching_wildcard_spec() -> crate::Result {
690+ let root = fixture("subdir-untracked-and-ignored");
691+ let (out, entries) = collect_filtered(
692+ &root,
693+ |keep, ctx| {
694+ walk(
695+ &root,
696+ &root,
697+ ctx,
698+ walk::Options {
699+ emit_ignored: Some(CollapseDirectory),
700+ emit_untracked: CollapseDirectory,
701+ emit_pruned: true,
702+ for_deletion: Some(Default::default()),
703+ ..options()
704+ },
705+ keep,
706+ )
707+ },
708+ Some("*foo*"),
709+ );
710+ assert_eq!(
711+ out,
712+ walk::Outcome {
713+ read_dir_calls: 8,
714+ returned_entries: entries.len(),
715+ seen_entries: 28,
716+ },
717+ );
718+ assert_eq!(
719+ &entries,
720+ &[
721+ entry_nokind(".git", DotGit),
722+ entry_nomatch(".gitignore", Pruned, File),
723+ entry_nomatch("a.o", Ignored(Expendable), File),
724+ entry_nomatch("b.o", Ignored(Expendable), File),
725+ entry_nomatch("c.o", Ignored(Expendable), File),
726+ entry_nomatch("d/a.o", Ignored(Expendable), File),
727+ entry_nomatch("d/b.o", Ignored(Expendable), File),
728+ entry_nomatch("d/d", Ignored(Expendable), Directory),
729+ entry_nomatch("d/d/a", Pruned, File),
730+ entry_nomatch("d/generated", Ignored(Expendable), Directory),
731+ entry_nomatch("generated", Ignored(Expendable), Directory),
732+ entry_nomatch("objs", Ignored(Expendable), Directory),
733+ ],
734+ "'generated' folders are included, and collapsing is done where possible"
735+ );
736+ Ok(())
737+}
738+
739+#[test]
740+fn nested_ignored_dirs_for_deletion_nonmatching_wildcard_spec() -> crate::Result {
741+ let root = fixture("ignored-dir-nested-minimal");
742+ let (_out, entries) = collect_filtered(
743+ &root,
744+ |keep, ctx| {
745+ walk(
746+ &root,
747+ &root,
748+ ctx,
749+ walk::Options {
750+ emit_ignored: Some(CollapseDirectory),
751+ emit_untracked: CollapseDirectory,
752+ emit_pruned: false,
753+ for_deletion: Some(Default::default()),
754+ ..options()
755+ },
756+ keep,
757+ )
758+ },
759+ Some("*foo*"),
760+ );
761+ // NOTE: do not use `_out` as `.git` directory contents can change, it's controlled by Git, causing flakiness.
762+
763+ assert_eq!(
764+ &entries,
765+ &[],
766+ "it figures out that nothing actually matches, even though it has to check everything"
767+ );
768+
769+ let (_out, entries) = collect_filtered(
770+ &root,
771+ |keep, ctx| {
772+ walk(
773+ &root,
774+ &root,
775+ ctx,
776+ walk::Options {
777+ emit_ignored: Some(CollapseDirectory),
778+ emit_untracked: CollapseDirectory,
779+ emit_pruned: true,
780+ for_deletion: Some(Default::default()),
781+ ..options()
782+ },
783+ keep,
784+ )
785+ },
786+ Some("*foo*"),
787+ );
788+ // NOTE: do not use `_out` as `.git` directory contents can change, it's controlled by Git, causing flakiness.
789+
790+ assert_eq!(
791+ &entries,
792+ &[
793+ entry_nokind(".git", DotGit),
794+ entry_nomatch(".gitignore", Pruned, File),
795+ entry_nomatch("bare/HEAD", Pruned, File),
796+ entry_nomatch("bare/info/exclude", Pruned, File),
797+ entry_nomatch("bare/objects", Untracked, Directory),
798+ entry_nomatch("bare/refs", Untracked, Directory),
799+ entry_nomatch("dir", Ignored(Expendable), Directory),
800+ ],
801+ "it's possible to observe pruned entries like before"
802+ );
803+ Ok(())
804+}
805+
806+#[test]
807+fn expendable_and_precious_in_ignored_dir_with_pathspec() -> crate::Result {
808+ let root = fixture("expendable-and-precious-nested-in-ignored-dir");
809+ let (out, entries) = collect(&root, |keep, ctx| {
810+ walk(
811+ &root,
812+ &root,
813+ ctx,
814+ walk::Options {
815+ emit_ignored: Some(CollapseDirectory),
816+ emit_untracked: CollapseDirectory,
817+ emit_pruned: true,
818+ emit_tracked: true,
819+ for_deletion: Some(Default::default()),
820+ ..options()
821+ },
822+ keep,
823+ )
824+ });
825+ assert_eq!(
826+ out,
827+ walk::Outcome {
828+ read_dir_calls: 2,
829+ returned_entries: entries.len(),
830+ seen_entries: 5,
831+ },
832+ );
833+
834+ assert_eq!(
835+ &entries,
836+ &[
837+ entry_nokind(".git", DotGit),
838+ entry(".gitignore", Tracked, File).with_index_kind(File),
839+ entry("ignored", Ignored(Expendable), Directory),
840+ entry("other", Ignored(Expendable), Directory),
841+ ],
842+ "without pathspec, it collapses completely. \
843+ It's interesting that 'other' claims to be ignored - due to the collapse of `other/ignored` it inherits the sub-directory status.\
844+ However, it's what we want, compared to the alternative of leaving it empty, and then detecting it as empty\
845+ the next time we run."
846+ );
847+
848+ let (out, entries) = collect_filtered(
849+ &root,
850+ |keep, ctx| {
851+ walk(
852+ &root,
853+ &root,
854+ ctx,
855+ walk::Options {
856+ emit_ignored: Some(CollapseDirectory),
857+ emit_untracked: CollapseDirectory,
858+ emit_pruned: true,
859+ for_deletion: Some(Default::default()),
860+ ..options()
861+ },
862+ keep,
863+ )
864+ },
865+ Some("*ignored*"),
866+ );
867+ assert_eq!(
868+ out,
869+ walk::Outcome {
870+ read_dir_calls: 9,
871+ returned_entries: entries.len(),
872+ seen_entries: 19,
873+ },
874+ );
875+
876+ assert_eq!(
877+ &entries,
878+ &[
879+ entry_nokind(".git", DotGit),
880+ entry_nokind("ignored/d/.git", DotGit),
881+ entryps("ignored/d/.gitignore", Ignored(Expendable), File, WildcardMatch),
882+ entryps("ignored/d/a.o", Ignored(Expendable), File, WildcardMatch),
883+ entryps(
884+ "ignored/d/all-expendable",
885+ Ignored(Expendable),
886+ Directory,
887+ WildcardMatch
888+ ),
889+ entryps("ignored/d/all-precious", Ignored(Precious), Directory, WildcardMatch),
890+ entryps("ignored/d/mixed", Ignored(Expendable), Directory, WildcardMatch),
891+ entryps("ignored/d/precious", Ignored(Expendable), File, WildcardMatch),
892+ entryps("other", Ignored(Expendable), Directory, WildcardMatch),
893+ ],
894+ "with pathspec, we match what's inside and expect to have all the lowest-level paths that have 'ignored' in them.\
895+ It seems strange that 'precious' isn't precious, while 'all-precious' is. However, the ignore-search is special
896+ as it goes backward through directories (using directory-declarations), and aborts if it matched. Thus it finds
897+ that '$/all-precious/' matched, but in the other cases it maches 'ignored/'.
898+ 'other' gets folded and inherits, just like before."
899+ );
900+ Ok(())
901+}
902+
635903 #[test]
636904 fn untracked_and_ignored() -> crate::Result {
637905 let root = fixture("subdir-untracked-and-ignored");

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t