Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 1 addition & 83 deletions builtin/reset.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,88 +174,6 @@ static void update_index_from_diff(struct diff_queue_struct *q,
}
}

static int pathspec_needs_expanded_index(const struct pathspec *pathspec)
{
unsigned int i, pos;
int res = 0;
char *skip_worktree_seen = NULL;

/*
* When using a magic pathspec, assume for the sake of simplicity that
* the index needs to be expanded to match all matchable files.
*/
if (pathspec->magic)
return 1;

for (i = 0; i < pathspec->nr; i++) {
struct pathspec_item item = pathspec->items[i];

/*
* If the pathspec item has a wildcard, the index should be expanded
* if the pathspec has the possibility of matching a subset of entries inside
* of a sparse directory (but not the entire directory).
*
* If the pathspec item is a literal path, the index only needs to be expanded
* if a) the pathspec isn't in the sparse checkout cone (to make sure we don't
* expand for in-cone files) and b) it doesn't match any sparse directories
* (since we can reset whole sparse directories without expanding them).
*/
if (item.nowildcard_len < item.len) {
/*
* Special case: if the pattern is a path inside the cone
* followed by only wildcards, the pattern cannot match
* partial sparse directories, so we know we don't need to
* expand the index.
*
* Examples:
* - in-cone/foo***: doesn't need expanded index
* - not-in-cone/bar*: may need expanded index
* - **.c: may need expanded index
*/
if (strspn(item.original + item.nowildcard_len, "*") == item.len - item.nowildcard_len &&
path_in_cone_mode_sparse_checkout(item.original, &the_index))
continue;

for (pos = 0; pos < active_nr; pos++) {
struct cache_entry *ce = active_cache[pos];

if (!S_ISSPARSEDIR(ce->ce_mode))
continue;

/*
* If the pre-wildcard length is longer than the sparse
* directory name and the sparse directory is the first
* component of the pathspec, need to expand the index.
*/
if (item.nowildcard_len > ce_namelen(ce) &&
!strncmp(item.original, ce->name, ce_namelen(ce))) {
res = 1;
break;
}

/*
* If the pre-wildcard length is shorter than the sparse
* directory and the pathspec does not match the whole
* directory, need to expand the index.
*/
if (!strncmp(item.original, ce->name, item.nowildcard_len) &&
wildmatch(item.original, ce->name, 0)) {
res = 1;
break;
}
}
} else if (!path_in_cone_mode_sparse_checkout(item.original, &the_index) &&
!matches_skip_worktree(pathspec, i, &skip_worktree_seen))
res = 1;

if (res > 0)
break;
}

free(skip_worktree_seen);
return res;
}

static int read_from_tree(const struct pathspec *pathspec,
struct object_id *tree_oid,
int intent_to_add)
Expand All @@ -273,7 +191,7 @@ static int read_from_tree(const struct pathspec *pathspec,
opt.change = diff_change;
opt.add_remove = diff_addremove;

if (pathspec->nr && the_index.sparse_index && pathspec_needs_expanded_index(pathspec))
if (pathspec->nr && the_index.sparse_index && pathspec_needs_expanded_index(&the_index, pathspec))
ensure_full_index(&the_index);

if (do_diff_cache(tree_oid, &opt))
Expand Down
9 changes: 7 additions & 2 deletions builtin/rm.c
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,10 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
char *seen;

git_config(git_default_config, NULL);
if (the_repository->gitdir) {
prepare_repo_settings(the_repository);
the_repository->settings.command_requires_full_index = 0;
}
Comment on lines +265 to +268
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can avoid the need for if (the_repository->gitdir) - which is when the index is needed before parse_options() is called (link) - by putting the prepare_repo_settings() immediately before the index is accessed. In this case, I think that's right before the hold_locked_index() call (line 294).

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly suspicious here.

You can avoid the need for if (the_repository->gitdir) - which is when the index is needed before parse_options() is called (link)

In this commit they actually use if (the_repository->gitdir) as the way to go:

However, it is unfortunately not that simple. In cmd_pack_objects(),
for example, the repo settings need to be fully populated so that the
command-line options --sparse/--no-sparse can override them, not the
other way round.

Therefore, we choose to imitate the strategy taken in cmd_diff(),
where we simply do not bother to prepare and initialize the repo
settings unless we have a gitdir.

Though I'm following your advice right now because it does make sense to me ;-)


argc = parse_options(argc, argv, prefix, builtin_rm_options,
builtin_rm_usage, 0);
Expand Down Expand Up @@ -296,8 +300,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)

seen = xcalloc(pathspec.nr, 1);

/* TODO: audit for interaction with sparse-index. */
ensure_full_index(&the_index);
if (pathspec_needs_expanded_index(&the_index, &pathspec))
ensure_full_index(&the_index);

for (i = 0; i < active_nr; i++) {
const struct cache_entry *ce = active_cache[i];

Expand Down
89 changes: 89 additions & 0 deletions pathspec.c
Original file line number Diff line number Diff line change
Expand Up @@ -759,3 +759,92 @@ int match_pathspec_attrs(struct index_state *istate,

return 1;
}

int pathspec_needs_expanded_index(struct index_state *istate,
const struct pathspec *pathspec)
{
unsigned int i, pos;
int res = 0;
char *skip_worktree_seen = NULL;

/*
* If index is not sparse, no index expansion is needed.
*/
if (!istate->sparse_index)
return 0;

/*
* When using a magic pathspec, assume for the sake of simplicity that
* the index needs to be expanded to match all matchable files.
*/
if (pathspec->magic)
return 1;

for (i = 0; i < pathspec->nr; i++) {
struct pathspec_item item = pathspec->items[i];

/*
* If the pathspec item has a wildcard, the index should be expanded
* if the pathspec has the possibility of matching a subset of entries inside
* of a sparse directory (but not the entire directory).
*
* If the pathspec item is a literal path, the index only needs to be expanded
* if a) the pathspec isn't in the sparse checkout cone (to make sure we don't
* expand for in-cone files) and b) it doesn't match any sparse directories
* (since we can reset whole sparse directories without expanding them).
*/
if (item.nowildcard_len < item.len) {
/*
* Special case: if the pattern is a path inside the cone
* followed by only wildcards, the pattern cannot match
* partial sparse directories, so we know we don't need to
* expand the index.
*
* Examples:
* - in-cone/foo***: doesn't need expanded index
* - not-in-cone/bar*: may need expanded index
* - **.c: may need expanded index
*/
if (strspn(item.original + item.nowildcard_len, "*") == item.len - item.nowildcard_len &&
path_in_cone_mode_sparse_checkout(item.original, istate))
continue;

for (pos = 0; pos < istate->cache_nr; pos++) {
struct cache_entry *ce = istate->cache[pos];

if (!S_ISSPARSEDIR(ce->ce_mode))
continue;

/*
* If the pre-wildcard length is longer than the sparse
* directory name and the sparse directory is the first
* component of the pathspec, need to expand the index.
*/
if (item.nowildcard_len > ce_namelen(ce) &&
!strncmp(item.original, ce->name, ce_namelen(ce))) {
res = 1;
break;
}

/*
* If the pre-wildcard length is shorter than the sparse
* directory and the pathspec does not match the whole
* directory, need to expand the index.
*/
if (!strncmp(item.original, ce->name, item.nowildcard_len) &&
wildmatch(item.original, ce->name, 0)) {
res = 1;
break;
}
}
} else if (!path_in_cone_mode_sparse_checkout(item.original, istate) &&
!matches_skip_worktree(pathspec, i, &skip_worktree_seen))
res = 1;

if (res > 0)
break;
}

free(skip_worktree_seen);
return res;
}
12 changes: 12 additions & 0 deletions pathspec.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,4 +171,16 @@ int match_pathspec_attrs(struct index_state *istate,
const char *name, int namelen,
const struct pathspec_item *item);

/*
* Determine whether a pathspec will match only entire index entries (non-sparse
* files and/or entire sparse directories). If the pathspec has the potential to
* match partial contents of a sparse directory, return 1 to indicate the index
* should be expanded to match the appropriate index entries.
*
* For the sake of simplicity, always return 1 if using a more complex "magic"
* pathspec.
*/
int pathspec_needs_expanded_index(struct index_state *istate,
const struct pathspec *pathspec);

#endif /* PATHSPEC_H */
1 change: 1 addition & 0 deletions t/perf/p2000-sparse-operations.sh
Original file line number Diff line number Diff line change
Expand Up @@ -123,5 +123,6 @@ test_perf_on_all git blame $SPARSE_CONE/f3/a
test_perf_on_all git read-tree -mu HEAD
test_perf_on_all git checkout-index -f --all
test_perf_on_all git update-index --add --remove $SPARSE_CONE/a
test_perf_on_all git rm -f $SPARSE_CONE/a

test_done
73 changes: 72 additions & 1 deletion t/t1092-sparse-checkout-compatibility.sh
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ test_expect_success 'read-tree --prefix' '
test_all_match git read-tree --prefix=deep/deeper1/deepest -u deepest &&
test_all_match git status --porcelain=v2 &&

test_all_match git rm -rf --sparse folder1/ &&
run_on_all git rm -rf --sparse folder1/ &&
test_all_match git read-tree --prefix=folder1/ -u update-folder1 &&
test_all_match git status --porcelain=v2 &&

Expand Down Expand Up @@ -1853,4 +1853,75 @@ test_expect_success 'mv directory from out-of-cone to in-cone' '
grep -e "H deep/0/1" actual
'

test_expect_success 'rm pathspec inside sparse definition' '
init_repos &&

test_all_match git rm deep/a &&
test_all_match git status --porcelain=v2 &&

# test wildcard
run_on_all git reset --hard &&
test_all_match git rm deep/* &&
test_all_match git status --porcelain=v2 &&

# test recursive rm
run_on_all git reset --hard &&
test_all_match git rm -r deep &&
test_all_match git status --porcelain=v2
'

test_expect_success 'rm pathspec outside sparse definition' '
init_repos &&

for file in folder1/a folder1/0/1
do
test_sparse_match test_must_fail git rm $file &&
test_sparse_match test_must_fail git rm --cached $file &&
test_sparse_match git rm --sparse $file &&
test_sparse_match git status --porcelain=v2
done &&

cat >folder1-full <<-EOF &&
rm ${SQ}folder1/0/0/0${SQ}
rm ${SQ}folder1/0/1${SQ}
rm ${SQ}folder1/a${SQ}
EOF

cat >folder1-sparse <<-EOF &&
rm ${SQ}folder1/${SQ}
EOF

# test wildcard
run_on_sparse git reset --hard &&
run_on_sparse git sparse-checkout reapply &&
test_sparse_match test_must_fail git rm folder1/* &&
run_on_sparse git rm --sparse folder1/* &&
test_cmp folder1-full sparse-checkout-out &&
test_cmp folder1-sparse sparse-index-out &&
test_sparse_match git status --porcelain=v2 &&

# test recursive rm
run_on_sparse git reset --hard &&
run_on_sparse git sparse-checkout reapply &&
test_sparse_match test_must_fail git rm --sparse folder1 &&
run_on_sparse git rm --sparse -r folder1 &&
test_cmp folder1-full sparse-checkout-out &&
test_cmp folder1-sparse sparse-index-out &&
test_sparse_match git status --porcelain=v2
'

test_expect_success 'sparse index is not expanded: rm' '
init_repos &&

ensure_not_expanded rm deep/a &&

# test in-cone wildcard
git -C sparse-index reset --hard &&
ensure_not_expanded rm deep/* &&

# test recursive rm
git -C sparse-index reset --hard &&
ensure_not_expanded rm -r deep
'

test_done