Skip to content

Commit 2891109

Browse files
pcloudsgitster
pclouds
authored and
gitster
committed
sparse checkout: do not eagerly decide the fate for whole directory
Sparse-setting code follows closely how files are excluded in read_directory(), every entry (including directories) are fed to excluded_from_list() to decide if the entry is suitable. Directories are treated no different than files. If a directory is matched (or not), the whole directory is considered matched (or not) and the process moves on. This generally works as long as there are no patterns to exclude parts of the directory. In case of sparse checkout code, the following patterns t !t/t0000-basic.sh will produce a worktree with full directory "t" even if t0000-basic.sh is requested to stay out. By the same reasoning, if a directory is to be excluded, any rules to re-include certain files within that directory will be ignored. Fix it by always checking files against patterns. If no pattern can be used to decide whether an entry is in our out (ie. excluded_from_list() returns -1), the entry will be included/excluded the same as their parent directory. Noticed-by: <skillzero@gmail.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent d61ebbe commit 2891109

2 files changed

Lines changed: 75 additions & 29 deletions

File tree

‎t/t1011-read-tree-sparse-checkout.sh‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,47 @@ test_expect_success 'match directories without trailing slash' '
106106
test -f sub/added
107107
'
108108

109+
test_expect_success 'match directories with negated patterns' '
110+
cat >expected.swt-negation <<\EOF &&
111+
S init.t
112+
S sub/added
113+
H sub/addedtoo
114+
S subsub/added
115+
EOF
116+
117+
cat >.git/info/sparse-checkout <<\EOF &&
118+
sub
119+
!sub/added
120+
EOF
121+
git read-tree -m -u HEAD &&
122+
git ls-files -t >result &&
123+
test_cmp expected.swt-negation result &&
124+
test ! -f init.t &&
125+
test ! -f sub/added &&
126+
test -f sub/addedtoo
127+
'
128+
129+
test_expect_success 'match directories with negated patterns (2)' '
130+
cat >expected.swt-negation2 <<\EOF &&
131+
H init.t
132+
H sub/added
133+
S sub/addedtoo
134+
H subsub/added
135+
EOF
136+
137+
cat >.git/info/sparse-checkout <<\EOF &&
138+
/*
139+
!sub
140+
sub/added
141+
EOF
142+
git read-tree -m -u HEAD &&
143+
git ls-files -t >result &&
144+
test_cmp expected.swt-negation2 result &&
145+
test -f init.t &&
146+
test -f sub/added &&
147+
test ! -f sub/addedtoo
148+
'
149+
109150
test_expect_success 'match directory pattern' '
110151
echo "s?b" >.git/info/sparse-checkout &&
111152
git read-tree -m -u HEAD &&

‎unpack-trees.c‎

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -814,43 +814,45 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str
814814
return mask;
815815
}
816816

817+
static int clear_ce_flags_1(struct cache_entry **cache, int nr,
818+
char *prefix, int prefix_len,
819+
int select_mask, int clear_mask,
820+
struct exclude_list *el, int defval);
821+
817822
/* Whole directory matching */
818823
static int clear_ce_flags_dir(struct cache_entry **cache, int nr,
819824
char *prefix, int prefix_len,
820825
char *basename,
821826
int select_mask, int clear_mask,
822-
struct exclude_list *el)
827+
struct exclude_list *el, int defval)
823828
{
824-
struct cache_entry **cache_end = cache + nr;
829+
struct cache_entry **cache_end;
825830
int dtype = DT_DIR;
826831
int ret = excluded_from_list(prefix, prefix_len, basename, &dtype, el);
827832

828833
prefix[prefix_len++] = '/';
829834

830-
/* included, no clearing for any entries under this directory */
831-
if (!ret) {
832-
for (; cache != cache_end; cache++) {
833-
struct cache_entry *ce = *cache;
834-
if (strncmp(ce->name, prefix, prefix_len))
835-
break;
836-
}
837-
return nr - (cache_end - cache);
838-
}
835+
/* If undecided, use matching result of parent dir in defval */
836+
if (ret < 0)
837+
ret = defval;
839838

840-
/* excluded, clear all selected entries under this directory. */
841-
if (ret == 1) {
842-
for (; cache != cache_end; cache++) {
843-
struct cache_entry *ce = *cache;
844-
if (select_mask && !(ce->ce_flags & select_mask))
845-
continue;
846-
if (strncmp(ce->name, prefix, prefix_len))
847-
break;
848-
ce->ce_flags &= ~clear_mask;
849-
}
850-
return nr - (cache_end - cache);
839+
for (cache_end = cache; cache_end != cache + nr; cache_end++) {
840+
struct cache_entry *ce = *cache_end;
841+
if (strncmp(ce->name, prefix, prefix_len))
842+
break;
851843
}
852844

853-
return 0;
845+
/*
846+
* TODO: check el, if there are no patterns that may conflict
847+
* with ret (iow, we know in advance the incl/excl
848+
* decision for the entire directory), clear flag here without
849+
* calling clear_ce_flags_1(). That function will call
850+
* the expensive excluded_from_list() on every entry.
851+
*/
852+
return clear_ce_flags_1(cache, cache_end - cache,
853+
prefix, prefix_len,
854+
select_mask, clear_mask,
855+
el, ret);
854856
}
855857

856858
/*
@@ -871,7 +873,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,
871873
static int clear_ce_flags_1(struct cache_entry **cache, int nr,
872874
char *prefix, int prefix_len,
873875
int select_mask, int clear_mask,
874-
struct exclude_list *el)
876+
struct exclude_list *el, int defval)
875877
{
876878
struct cache_entry **cache_end = cache + nr;
877879

@@ -882,7 +884,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,
882884
while(cache != cache_end) {
883885
struct cache_entry *ce = *cache;
884886
const char *name, *slash;
885-
int len, dtype;
887+
int len, dtype, ret;
886888

887889
if (select_mask && !(ce->ce_flags & select_mask)) {
888890
cache++;
@@ -911,7 +913,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,
911913
prefix, prefix_len + len,
912914
prefix + prefix_len,
913915
select_mask, clear_mask,
914-
el);
916+
el, defval);
915917

916918
/* clear_c_f_dir eats a whole dir already? */
917919
if (processed) {
@@ -922,13 +924,16 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,
922924
prefix[prefix_len + len++] = '/';
923925
cache += clear_ce_flags_1(cache, cache_end - cache,
924926
prefix, prefix_len + len,
925-
select_mask, clear_mask, el);
927+
select_mask, clear_mask, el, defval);
926928
continue;
927929
}
928930

929931
/* Non-directory */
930932
dtype = ce_to_dtype(ce);
931-
if (excluded_from_list(ce->name, ce_namelen(ce), name, &dtype, el) > 0)
933+
ret = excluded_from_list(ce->name, ce_namelen(ce), name, &dtype, el);
934+
if (ret < 0)
935+
ret = defval;
936+
if (ret > 0)
932937
ce->ce_flags &= ~clear_mask;
933938
cache++;
934939
}
@@ -943,7 +948,7 @@ static int clear_ce_flags(struct cache_entry **cache, int nr,
943948
return clear_ce_flags_1(cache, nr,
944949
prefix, 0,
945950
select_mask, clear_mask,
946-
el);
951+
el, 0);
947952
}
948953

949954
/*

0 commit comments

Comments
 (0)