Matheus Tavares[PATCH v3 0/7] add/rm: honor sparse checkout and warn on sparse paths
Make `rm` honor sparse checkouts, and make both `rm` and `add` warn when they are asked to update sparse entries.
Main changes since v2:
Patch 2: - Removed unnecessary `rm` in t3705's auxiliary function and added comment about why `update-index --force-remove` is used there.
Patch 4: - Replaced the boolean function parameter with a more descriptive enum. This way, readers don't have to go to the function definition to understand what the caller wants.
Patches 5 and 6: - Reworded both commit messages and the advice.updateSparsePath docs to avoid saying "pathspecs that *only* match sparse entries", as the situations to which they refer might also include matches among ignored paths. - Rephrased the warning message about sparse paths so that it doesn't contradict the message on ignored paths when they are displayed together.
Patch 7: - Adjusted the tests to check for the new sparse warning message.
Matheus Tavares (7): add: include magic part of pathspec on --refresh error t3705: add tests for `git add` in sparse checkouts add: make --chmod and --renormalize honor sparse checkouts pathspec: allow to ignore SKIP_WORKTREE entries on index matching refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag add: warn when asked to update SKIP_WORKTREE entries rm: honor sparse checkout patterns
Documentation/config/advice.txt | 4 + Documentation/git-rm.txt | 4 +- advice.c | 20 ++++ advice.h | 4 + builtin/add.c | 77 ++++++++++++--- builtin/check-ignore.c | 3 +- builtin/rm.c | 35 ++++--- cache.h | 15 +-- pathspec.c | 25 ++++- pathspec.h | 18 +++- read-cache.c | 5 +- t/t3602-rm-sparse-checkout.sh | 78 ++++++++++++++++ t/t3700-add.sh | 6 ++ t/t3705-add-sparse-checkout.sh | 155 +++++++++++++++++++++++++++++++ t/t7011-skip-worktree-reading.sh | 5 - t/t7012-skip-worktree-writing.sh | 19 ---- 16 files changed, 409 insertions(+), 64 deletions(-) create mode 100755 t/t3602-rm-sparse-checkout.sh create mode 100755 t/t3705-add-sparse-checkout.sh
Range-diff against v2:
1: 46147a8637 = 1: 05b66a8c2b add: include magic part of pathspec on --refresh error
2: 2de55420fa ! 2: d93c3f5146 t3705: add tests for `git add` in sparse checkouts
@@ t/t3705-add-sparse-checkout.sh (new)
+
+# Optionally take a printf format string to write to the sparse_entry file
+setup_sparse_entry () {
-+ rm -f sparse_entry &&
++ # 'sparse_entry' might already be in the index with the skip-worktree
++ # bit set. Remove it so that the subsequent git add can update it.
+ git update-index --force-remove sparse_entry &&
-+
+ if test $# -eq 1
+ then
+ printf "$1" >sparse_entry
3: 604480ade9 = 3: c90d5e3c23 add: make --chmod and --renormalize honor sparse checkouts
4: 1494e65316 ! 4: 69d51579b3 pathspec: allow to ignore SKIP_WORKTREE entries on index matching
@@ Metadata
## Commit message ##
pathspec: allow to ignore SKIP_WORKTREE entries on index matching
- Add the 'ignore_skip_worktree' boolean parameter to both
- add_pathspec_matches_against_index() and
- find_pathspecs_matching_against_index(). When true, these functions will
- not try to match the given pathspec with SKIP_WORKTREE entries. This
- will be used in a future patch to make `git add` display a hint
- when the pathspec matches only sparse paths.
+ Add a new enum parameter to `add_pathspec_matches_against_index()` and
+ `find_pathspecs_matching_against_index()`, allowing callers to specify
+ whether these function should attempt to match SKIP_WORKTREE entries or
+ not. This will be used in a future patch to make `git add` display a
+ hint when it is asked to update SKIP_WORKTREE entries.
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
@@ builtin/add.c: static char *prune_directory(struct dir_struct *dir, struct paths
}
dir->nr = dst - dir->entries;
- add_pathspec_matches_against_index(pathspec, &the_index, seen);
-+ add_pathspec_matches_against_index(pathspec, &the_index, seen, 0);
++ add_pathspec_matches_against_index(pathspec, &the_index, seen,
++ PS_HEED_SKIP_WORKTREE);
return seen;
}
@@ builtin/add.c: int cmd_add(int argc, const char **argv, const char *prefix)
if (!seen)
- seen = find_pathspecs_matching_against_index(&pathspec, &the_index);
-+ seen = find_pathspecs_matching_against_index(&pathspec, &the_index, 0);
++ seen = find_pathspecs_matching_against_index(&pathspec,
++ &the_index, PS_HEED_SKIP_WORKTREE);
/*
* file_exists() assumes exact match
@@ builtin/check-ignore.c: static int check_ignore(struct dir_struct *dir,
* 'git status', 'git add' etc.
*/
- seen = find_pathspecs_matching_against_index(&pathspec, &the_index);
-+ seen = find_pathspecs_matching_against_index(&pathspec, &the_index, 0);
++ seen = find_pathspecs_matching_against_index(&pathspec, &the_index,
++ PS_HEED_SKIP_WORKTREE);
for (i = 0; i < pathspec.nr; i++) {
full_path = pathspec.items[i].match;
pattern = NULL;
@@ pathspec.c
void add_pathspec_matches_against_index(const struct pathspec *pathspec,
const struct index_state *istate,
- char *seen)
-+ char *seen, int ignore_skip_worktree)
++ char *seen,
++ enum ps_skip_worktree_action sw_action)
{
int num_unmatched = 0, i;
@@ pathspec.c: void add_pathspec_matches_against_index(const struct pathspec *paths
return;
for (i = 0; i < istate->cache_nr; i++) {
const struct cache_entry *ce = istate->cache[i];
-+ if (ignore_skip_worktree && ce_skip_worktree(ce))
++ if (sw_action == PS_IGNORE_SKIP_WORKTREE && ce_skip_worktree(ce))
+ continue;
ce_path_match(istate, ce, pathspec, seen);
}
@@ pathspec.c: void add_pathspec_matches_against_index(const struct pathspec *paths
char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,
- const struct index_state *istate)
+ const struct index_state *istate,
-+ int ignore_skip_worktree)
++ enum ps_skip_worktree_action sw_action)
{
char *seen = xcalloc(pathspec->nr, 1);
- add_pathspec_matches_against_index(pathspec, istate, seen);
-+ add_pathspec_matches_against_index(pathspec, istate, seen,
-+ ignore_skip_worktree);
++ add_pathspec_matches_against_index(pathspec, istate, seen, sw_action);
return seen;
}
## pathspec.h ##
@@ pathspec.h: static inline int ps_strcmp(const struct pathspec_item *item,
+ return strcmp(s1, s2);
+ }
++enum ps_skip_worktree_action {
++ PS_HEED_SKIP_WORKTREE = 0,
++ PS_IGNORE_SKIP_WORKTREE = 1
++};
void add_pathspec_matches_against_index(const struct pathspec *pathspec,
const struct index_state *istate,
- char *seen);
-+ char *seen, int ignore_skip_worktree);
++ char *seen,
++ enum ps_skip_worktree_action sw_action);
char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,
- const struct index_state *istate);
+ const struct index_state *istate,
-+ int ignore_skip_worktree);
++ enum ps_skip_worktree_action sw_action);
int match_pathspec_attrs(const struct index_state *istate,
const char *name, int namelen,
const struct pathspec_item *item);
5: 584811e5c0 ! 5: 34a61a0d03 refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag
@@ Commit message
refresh_index() optionally takes a seen[] array to mark the pathspec
items that had matches in the index. This is used by `git add --refresh`
- to find out if there were any pathspec without matches, and display an
+ to find out if there was any pathspec without matches, and display an
error accordingly.
In the following patch, `git add` will also learn to warn about
- pathspecs that only match sparse entries (which are not updated). But
- for that, we will need a seen[] array marked exclusively with matches
- from dense entries. To avoid having to call ce_path_match() again for
- these entries after refresh_index() returns, add a flag that implements
- this restriction inside the function itself.
+ pathspecs that match no eligible path for updating, but do match sparse
+ entries. For that, we will need a seen[] array marked exclusively with
+ matches from dense entries. To avoid having to call ce_path_match()
+ again for these entries after refresh_index() returns, add a flag that
+ implements this restriction inside the function itself.
Note that refresh_index() does not update sparse entries, regardless of
passing the flag or not. The flag only controls whether matches with
@@ read-cache.c: int refresh_index(struct index_state *istate, unsigned int flags,
int in_porcelain = (flags & REFRESH_IN_PORCELAIN);
unsigned int options = (CE_MATCH_REFRESH |
@@ read-cache.c: int refresh_index(struct index_state *istate, unsigned int flags,
- int cache_errno = 0;
- int changed = 0;
int filtered = 0;
+ int t2_did_lstat = 0;
+ int t2_did_scan = 0;
+ char *cur_seen;
ce = istate->cache[i];
6: 15f399f4fa ! 6: 66d5c71182 add: warn when pathspec only matches SKIP_WORKTREE entries
@@ Metadata
Author: Matheus Tavares <matheus.bernardino@usp.br>
## Commit message ##
- add: warn when pathspec only matches SKIP_WORKTREE entries
+ add: warn when asked to update SKIP_WORKTREE entries
`git add` already refrains from updating SKIP_WORKTREE entries, but it
- silently exits with zero code when a pathspec only matches these
- entries. Instead, let's warn the user and display a hint on how to
- update these entries.
+ silently exits with zero code when it is asked to do so. Instead, let's
+ warn the user and display a hint on how to update these entries.
- Note that the warning is only shown if the pathspec matches no untracked
- paths in the working tree and only matches index entries with the
- SKIP_WORKTREE bit set. A warning message was chosen over erroring out
- right away to reproduce the same behavior `add` already exhibits with
- ignored files. This also allow users to continue their workflow without
- having to invoke `add` again with only the matching pathspecs, as the
- matched files will have already been added.
+ Note that we only warn the user whey they give a pathspec item that
+ matches no eligible path for updating, but it does match one or more
+ SKIP_WORKTREE entries. A warning was chosen over erroring out right away
+ to reproduce the same behavior `add` already exhibits with ignored
+ files. This also allow users to continue their workflow without having
+ to invoke `add` again with only the eligible paths (as those will have
+ already been added).
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
@@ Documentation/config/advice.txt: advice.*::
Advice shown if a user runs the add command without providing
the pathspec parameter.
+ updateSparsePath::
-+ Advice shown if the pathspec given to linkgit:git-add[1] only
-+ matches index entries outside the current sparse-checkout.
++ Advice shown when linkgit:git-add[1] is asked to update index
++ entries outside the current sparse checkout.
--
## advice.c ##
@@ advice.c: void NORETURN die_conclude_merge(void)
+ if (!pathspec_list->nr)
+ return;
+
-+ fprintf(stderr, _("The following pathspecs only matched index entries outside the current\n"
-+ "sparse checkout:\n"));
++ fprintf(stderr, _("The following pathspecs didn't match any"
++ " eligible path, but they do match index\n"
++ "entries outside the current sparse checkout:\n"));
+ for_each_string_list_item(item, pathspec_list)
+ fprintf(stderr, "%s\n", item->string);
+
+ advise_if_enabled(ADVICE_UPDATE_SPARSE_PATH,
-+ _("Disable or modify the sparsity rules if you intend to update such entries."));
-+
++ _("Disable or modify the sparsity rules if you intend"
++ " to update such entries."));
+}
+
void detach_advice(const char *new_name)
@@ advice.h: void advise_if_enabled(enum advice_type type, const char *advice, ...)
## builtin/add.c ##
@@ builtin/add.c: static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec,
- *dst++ = entry;
}
dir->nr = dst - dir->entries;
-- add_pathspec_matches_against_index(pathspec, &the_index, seen, 0);
-+ add_pathspec_matches_against_index(pathspec, &the_index, seen, 1);
+ add_pathspec_matches_against_index(pathspec, &the_index, seen,
+- PS_HEED_SKIP_WORKTREE);
++ PS_IGNORE_SKIP_WORKTREE);
return seen;
}
@@ builtin/add.c: int cmd_add(int argc, const char **argv, const char *prefix)
+ struct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;
if (!seen)
-- seen = find_pathspecs_matching_against_index(&pathspec, &the_index, 0);
-+ seen = find_pathspecs_matching_against_index(&pathspec, &the_index, 1);
+ seen = find_pathspecs_matching_against_index(&pathspec,
+- &the_index, PS_HEED_SKIP_WORKTREE);
++ &the_index, PS_IGNORE_SKIP_WORKTREE);
/*
* file_exists() assumes exact match
@@ builtin/add.c: int cmd_add(int argc, const char **argv, const char *prefix)
+ continue;
+ }
+
-+ /* Don't complain at 'git add .' inside empty repo. */
++ /* Don't complain at 'git add .' on empty repo */
+ if (!path[0])
+ continue;
+
@@ pathspec.h
@@ pathspec.h: void add_pathspec_matches_against_index(const struct pathspec *pathspec,
char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,
const struct index_state *istate,
- int ignore_skip_worktree);
+ enum ps_skip_worktree_action sw_action);
+char *find_pathspecs_matching_skip_worktree(const struct pathspec *pathspec);
+static inline int matches_skip_worktree(const struct pathspec *pathspec,
+ int item, char **seen_ptr)
@@ t/t3705-add-sparse-checkout.sh: setup_gitignore () {
EOF
}
-+test_expect_success 'setup' '
++test_expect_success 'setup' "
+ cat >sparse_error_header <<-EOF &&
-+ The following pathspecs only matched index entries outside the current
-+ sparse checkout:
++ The following pathspecs didn't match any eligible path, but they do match index
++ entries outside the current sparse checkout:
+ EOF
+
+ cat >sparse_hint <<-EOF &&
+ hint: Disable or modify the sparsity rules if you intend to update such entries.
-+ hint: Disable this message with "git config advice.updateSparsePath false"
++ hint: Disable this message with \"git config advice.updateSparsePath false\"
+ EOF
+
+ echo sparse_entry | cat sparse_error_header - >sparse_entry_error &&
+ cat sparse_entry_error sparse_hint >error_and_hint
-+'
++"
+
test_expect_success 'git add does not remove sparse entries' '
setup_sparse_entry &&
@@ t/t3705-add-sparse-checkout.sh: test_expect_success 'git add --renormalize does
- git add --renormalize sparse_entry &&
+ test_must_fail git add --renormalize sparse_entry 2>stderr &&
+ test_i18ncmp error_and_hint stderr &&
- test_sparse_entry_unchanged
- '
-
++ test_sparse_entry_unchanged
++'
++
+test_expect_success 'git add --dry-run --ignore-missing warn on sparse path' '
+ setup_sparse_entry &&
+ rm sparse_entry &&
+ test_must_fail git add --dry-run --ignore-missing sparse_entry 2>stderr &&
+ test_i18ncmp error_and_hint stderr &&
-+ test_sparse_entry_unchanged
-+'
-+
+ test_sparse_entry_unchanged
+ '
+
+test_expect_success 'do not advice about sparse entries when they do not match the pathspec' '
+ setup_sparse_entry &&
+ test_must_fail git add nonexistent 2>stderr &&
+ test_i18ngrep "fatal: pathspec .nonexistent. did not match any files" stderr &&
-+ test_i18ngrep ! "The following pathspecs only matched index entries" stderr
++ test_i18ngrep ! -F -f sparse_error_header stderr
+'
+
+test_expect_success 'do not warn when pathspec matches dense entries' '
7: bb29aa4d8f ! 7: d25ea10560 rm: honor sparse checkout patterns
@@ Metadata
## Commit message ##
rm: honor sparse checkout patterns
- `git add` refrains from adding or updating paths outside the sparsity
- rules, but `git rm` doesn't follow the same restrictions. This is
- somewhat counter-intuitive and inconsistent. So make `rm` honor the
- sparse checkout and advise on how to remove SKIP_WORKTREE entries, just
- like `add` does. Also add some tests for the new behavior.
+ `git add` refrains from adding or updating index entries that are
+ outside the current sparse checkout, but `git rm` doesn't follow the
+ same restriction. This is somewhat counter-intuitive and inconsistent.
+ So make `rm` honor the sparsity rules and advise on how to remove
+ SKIP_WORKTREE entries, just like `add` does. Also add some tests for the
+ new behavior.
Suggested-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
@@ Documentation/config/advice.txt: advice.*::
Advice shown if a user runs the add command without providing
the pathspec parameter.
updateSparsePath::
-- Advice shown if the pathspec given to linkgit:git-add[1] only
-- matches index entries outside the current sparse-checkout.
-+ Advice shown if the pathspec given to linkgit:git-add[1] or
-+ linkgit:git-rm[1] only matches index entries outside the
-+ current sparse-checkout.
+- Advice shown when linkgit:git-add[1] is asked to update index
+- entries outside the current sparse checkout.
++ Advice shown when either linkgit:git-add[1] or linkgit:git-rm[1]
++ is asked to update index entries outside the current sparse
++ checkout.
--
## Documentation/git-rm.txt ##
@@ t/t3602-rm-sparse-checkout.sh (new)
+
+. ./test-lib.sh
+
-+test_expect_success 'setup' '
++test_expect_success 'setup' "
+ mkdir -p sub/dir &&
+ touch a b c sub/d sub/dir/e &&
+ git add -A &&
+ git commit -m files &&
+
-+ cat >sparse_entry_b_error <<-EOF &&
-+ The following pathspecs only matched index entries outside the current
-+ sparse checkout:
-+ b
++ cat >sparse_error_header <<-EOF &&
++ The following pathspecs didn't match any eligible path, but they do match index
++ entries outside the current sparse checkout:
+ EOF
+
-+ cat >b_error_and_hint sparse_entry_b_error - <<-EOF
++ cat >sparse_hint <<-EOF &&
+ hint: Disable or modify the sparsity rules if you intend to update such entries.
-+ hint: Disable this message with "git config advice.updateSparsePath false"
++ hint: Disable this message with \"git config advice.updateSparsePath false\"
+ EOF
-+'
++
++ echo b | cat sparse_error_header - >sparse_entry_b_error &&
++ cat sparse_entry_b_error sparse_hint >b_error_and_hint
++"
+
+for opt in "" -f --dry-run
+do
@@ t/t3602-rm-sparse-checkout.sh (new)
+ git sparse-checkout set a &&
+ test_must_fail git rm nonexistent 2>stderr &&
+ test_i18ngrep "fatal: pathspec .nonexistent. did not match any files" stderr &&
-+ test_i18ngrep ! "The following pathspecs only matched index entries" stderr
++ test_i18ngrep ! -F -f sparse_error_header stderr
+'
+
+test_expect_success 'do not warn about sparse entries when pathspec matches dense entries' '-- 2.30.1
