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 it is asked to do so. Instead, let's warn the user and display a hint on how to update these entries. 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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Matheus Tavares committed Apr 8, 2021 at 17:41 UTC a20f70478ffcc66d30936920ebcc35ebfc12a7c7
7 files changed +172 -21
Documentation/config/advice.txt
+3
@@ -119,4 +119,7 @@ advice.*::
119 addEmptyPathspec::
120 Advice shown if a user runs the add command without providing
121 the pathspec parameter.
122 + updateSparsePath::
123 + Advice shown when linkgit:git-add[1] is asked to update index
124 + entries outside the current sparse checkout.
125 --
advice.c
+20
@@ -2,6 +2,7 @@
2 #include "config.h"
3 #include "color.h"
4 #include "help.h"
5 +#include "string-list.h"
6
7 int advice_fetch_show_forced_updates = 1;
8 int advice_push_update_rejected = 1;
@@ -136,6 +137,7 @@ static struct {
137 [ADVICE_STATUS_HINTS] = { "statusHints", 1 },
138 [ADVICE_STATUS_U_OPTION] = { "statusUoption", 1 },
139 [ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE] = { "submoduleAlternateErrorStrategyDie", 1 },
140 + [ADVICE_UPDATE_SPARSE_PATH] = { "updateSparsePath", 1 },
141 [ADVICE_WAITING_FOR_EDITOR] = { "waitingForEditor", 1 },
142 };
143
@@ -284,6 +286,24 @@ void NORETURN die_conclude_merge(void)
286 die(_("Exiting because of unfinished merge."));
287 }
288
289 +void advise_on_updating_sparse_paths(struct string_list *pathspec_list)
290 +{
291 + struct string_list_item *item;
292 +
293 + if (!pathspec_list->nr)
294 + return;
295 +
296 + fprintf(stderr, _("The following pathspecs didn't match any"
297 + " eligible path, but they do match index\n"
298 + "entries outside the current sparse checkout:\n"));
299 + for_each_string_list_item(item, pathspec_list)
300 + fprintf(stderr, "%s\n", item->string);
301 +
302 + advise_if_enabled(ADVICE_UPDATE_SPARSE_PATH,
303 + _("Disable or modify the sparsity rules if you intend"
304 + " to update such entries."));
305 +}
306 +
307 void detach_advice(const char *new_name)
308 {
309 const char *fmt =
advice.h
+4
@@ -3,6 +3,8 @@
3
4 #include "git-compat-util.h"
5
6 +struct string_list;
7 +
8 extern int advice_fetch_show_forced_updates;
9 extern int advice_push_update_rejected;
10 extern int advice_push_non_ff_current;
@@ -71,6 +73,7 @@ extern int advice_add_empty_pathspec;
73 ADVICE_STATUS_HINTS,
74 ADVICE_STATUS_U_OPTION,
75 ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,
76 + ADVICE_UPDATE_SPARSE_PATH,
77 ADVICE_WAITING_FOR_EDITOR,
78 };
79
@@ -92,6 +95,7 @@ void advise_if_enabled(enum advice_type type, const char *advice, ...);
95 int error_resolve_conflict(const char *me);
96 void NORETURN die_resolve_conflict(const char *me);
97 void NORETURN die_conclude_merge(void);
98 +void advise_on_updating_sparse_paths(struct string_list *pathspec_list);
99 void detach_advice(const char *new_name);
100
101 #endif /* ADVICE_H */
builtin/add.c
+56 -14
@@ -178,24 +178,43 @@ static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec,
178 }
179 dir->nr = dst - dir->entries;
180 add_pathspec_matches_against_index(pathspec, &the_index, seen,
181 - PS_HEED_SKIP_WORKTREE);
181 + PS_IGNORE_SKIP_WORKTREE);
182 return seen;
183 }
184
185 -static void refresh(int verbose, const struct pathspec *pathspec)
185 +static int refresh(int verbose, const struct pathspec *pathspec)
186 {
187 char *seen;
188 - int i;
188 + int i, ret = 0;
189 + char *skip_worktree_seen = NULL;
190 + struct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;
191 + int flags = REFRESH_IGNORE_SKIP_WORKTREE |
192 + (verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET);
193
194 seen = xcalloc(pathspec->nr, 1);
191 - refresh_index(&the_index, verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET,
192 - pathspec, seen, _("Unstaged changes after refreshing the index:"));
195 + refresh_index(&the_index, flags, pathspec, seen,
196 + _("Unstaged changes after refreshing the index:"));
197 for (i = 0; i < pathspec->nr; i++) {
194 - if (!seen[i])
195 - die(_("pathspec '%s' did not match any files"),
196 - pathspec->items[i].original);
198 + if (!seen[i]) {
199 + if (matches_skip_worktree(pathspec, i, &skip_worktree_seen)) {
200 + string_list_append(&only_match_skip_worktree,
201 + pathspec->items[i].original);
202 + } else {
203 + die(_("pathspec '%s' did not match any files"),
204 + pathspec->items[i].original);
205 + }
206 + }
207 + }
208 +
209 + if (only_match_skip_worktree.nr) {
210 + advise_on_updating_sparse_paths(&only_match_skip_worktree);
211 + ret = 1;
212 }
213 +
214 free(seen);
215 + free(skip_worktree_seen);
216 + string_list_clear(&only_match_skip_worktree, 0);
217 + return ret;
218 }
219
220 int run_add_interactive(const char *revision, const char *patch_mode,
@@ -571,16 +590,18 @@ int cmd_add(int argc, const char **argv, const char *prefix)
590 }
591
592 if (refresh_only) {
574 - refresh(verbose, &pathspec);
593 + exit_status |= refresh(verbose, &pathspec);
594 goto finish;
595 }
596
597 if (pathspec.nr) {
598 int i;
599 + char *skip_worktree_seen = NULL;
600 + struct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;
601
602 if (!seen)
603 seen = find_pathspecs_matching_against_index(&pathspec,
583 - &the_index, PS_HEED_SKIP_WORKTREE);
604 + &the_index, PS_IGNORE_SKIP_WORKTREE);
605
606 /*
607 * file_exists() assumes exact match
@@ -594,12 +615,24 @@ int cmd_add(int argc, const char **argv, const char *prefix)
615
616 for (i = 0; i < pathspec.nr; i++) {
617 const char *path = pathspec.items[i].match;
618 +
619 if (pathspec.items[i].magic & PATHSPEC_EXCLUDE)
620 continue;
599 - if (!seen[i] && path[0] &&
600 - ((pathspec.items[i].magic &
601 - (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||
602 - !file_exists(path))) {
621 + if (seen[i])
622 + continue;
623 +
624 + if (matches_skip_worktree(&pathspec, i, &skip_worktree_seen)) {
625 + string_list_append(&only_match_skip_worktree,
626 + pathspec.items[i].original);
627 + continue;
628 + }
629 +
630 + /* Don't complain at 'git add .' on empty repo */
631 + if (!path[0])
632 + continue;
633 +
634 + if ((pathspec.items[i].magic & (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||
635 + !file_exists(path)) {
636 if (ignore_missing) {
637 int dtype = DT_UNKNOWN;
638 if (is_excluded(&dir, &the_index, path, &dtype))
@@ -610,7 +643,16 @@ int cmd_add(int argc, const char **argv, const char *prefix)
643 pathspec.items[i].original);
644 }
645 }
646 +
647 +
648 + if (only_match_skip_worktree.nr) {
649 + advise_on_updating_sparse_paths(&only_match_skip_worktree);
650 + exit_status = 1;
651 + }
652 +
653 free(seen);
654 + free(skip_worktree_seen);
655 + string_list_clear(&only_match_skip_worktree, 0);
656 }
657
658 plug_bulk_checkin();
pathspec.c
+15
@@ -62,6 +62,21 @@ char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,
62 return seen;
63 }
64
65 +char *find_pathspecs_matching_skip_worktree(const struct pathspec *pathspec)
66 +{
67 + struct index_state *istate = the_repository->index;
68 + char *seen = xcalloc(pathspec->nr, 1);
69 + int i;
70 +
71 + for (i = 0; i < istate->cache_nr; i++) {
72 + struct cache_entry *ce = istate->cache[i];
73 + if (ce_skip_worktree(ce))
74 + ce_path_match(istate, ce, pathspec, seen);
75 + }
76 +
77 + return seen;
78 +}
79 +
80 /*
81 * Magic pathspec
82 *
pathspec.h
+8
@@ -160,6 +160,14 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,
160 char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,
161 const struct index_state *istate,
162 enum ps_skip_worktree_action sw_action);
163 +char *find_pathspecs_matching_skip_worktree(const struct pathspec *pathspec);
164 +static inline int matches_skip_worktree(const struct pathspec *pathspec,
165 + int item, char **seen_ptr)
166 +{
167 + if (!*seen_ptr)
168 + *seen_ptr = find_pathspecs_matching_skip_worktree(pathspec);
169 + return (*seen_ptr)[item];
170 +}
171 int match_pathspec_attrs(const struct index_state *istate,
172 const char *name, int namelen,
173 const struct pathspec_item *item);
t/t3705-add-sparse-checkout.sh
+66 -7
@@ -36,10 +36,26 @@ setup_gitignore () {
36 EOF
37 }
38
39 +test_expect_success 'setup' "
40 + cat >sparse_error_header <<-EOF &&
41 + The following pathspecs didn't match any eligible path, but they do match index
42 + entries outside the current sparse checkout:
43 + EOF
44 +
45 + cat >sparse_hint <<-EOF &&
46 + hint: Disable or modify the sparsity rules if you intend to update such entries.
47 + hint: Disable this message with \"git config advice.updateSparsePath false\"
48 + EOF
49 +
50 + echo sparse_entry | cat sparse_error_header - >sparse_entry_error &&
51 + cat sparse_entry_error sparse_hint >error_and_hint
52 +"
53 +
54 test_expect_success 'git add does not remove sparse entries' '
55 setup_sparse_entry &&
56 rm sparse_entry &&
42 - git add sparse_entry &&
57 + test_must_fail git add sparse_entry 2>stderr &&
58 + test_cmp error_and_hint stderr &&
59 test_sparse_entry_unchanged
60 '
61
@@ -47,7 +63,8 @@ test_expect_success 'git add -A does not remove sparse entries' '
63 setup_sparse_entry &&
64 rm sparse_entry &&
65 setup_gitignore &&
50 - git add -A &&
66 + git add -A 2>stderr &&
67 + test_must_be_empty stderr &&
68 test_sparse_entry_unchanged
69 '
70
@@ -55,7 +72,13 @@ test_expect_success 'git add . does not remove sparse entries' '
72 setup_sparse_entry &&
73 rm sparse_entry &&
74 setup_gitignore &&
58 - git add . &&
75 + test_must_fail git add . 2>stderr &&
76 +
77 + cat sparse_error_header >expect &&
78 + echo . >>expect &&
79 + cat sparse_hint >>expect &&
80 +
81 + test_cmp expect stderr &&
82 test_sparse_entry_unchanged
83 '
84
@@ -64,7 +87,8 @@ do
87 test_expect_success "git add${opt:+ $opt} does not update sparse entries" '
88 setup_sparse_entry &&
89 echo modified >sparse_entry &&
67 - git add $opt sparse_entry &&
90 + test_must_fail git add $opt sparse_entry 2>stderr &&
91 + test_cmp error_and_hint stderr &&
92 test_sparse_entry_unchanged
93 '
94 done
@@ -73,14 +97,16 @@ test_expect_success 'git add --refresh does not update sparse entries' '
97 setup_sparse_entry &&
98 git ls-files --debug sparse_entry | grep mtime >before &&
99 test-tool chmtime -60 sparse_entry &&
76 - git add --refresh sparse_entry &&
100 + test_must_fail git add --refresh sparse_entry 2>stderr &&
101 + test_cmp error_and_hint stderr &&
102 git ls-files --debug sparse_entry | grep mtime >after &&
103 test_cmp before after
104 '
105
106 test_expect_success 'git add --chmod does not update sparse entries' '
107 setup_sparse_entry &&
83 - git add --chmod=+x sparse_entry &&
108 + test_must_fail git add --chmod=+x sparse_entry 2>stderr &&
109 + test_cmp error_and_hint stderr &&
110 test_sparse_entry_unchanged &&
111 ! test -x sparse_entry
112 '
@@ -89,8 +115,41 @@ test_expect_success 'git add --renormalize does not update sparse entries' '
115 test_config core.autocrlf false &&
116 setup_sparse_entry "LINEONE\r\nLINETWO\r\n" &&
117 echo "sparse_entry text=auto" >.gitattributes &&
92 - git add --renormalize sparse_entry &&
118 + test_must_fail git add --renormalize sparse_entry 2>stderr &&
119 + test_cmp error_and_hint stderr &&
120 + test_sparse_entry_unchanged
121 +'
122 +
123 +test_expect_success 'git add --dry-run --ignore-missing warn on sparse path' '
124 + setup_sparse_entry &&
125 + rm sparse_entry &&
126 + test_must_fail git add --dry-run --ignore-missing sparse_entry 2>stderr &&
127 + test_cmp error_and_hint stderr &&
128 test_sparse_entry_unchanged
129 '
130
131 +test_expect_success 'do not advice about sparse entries when they do not match the pathspec' '
132 + setup_sparse_entry &&
133 + test_must_fail git add nonexistent 2>stderr &&
134 + grep "fatal: pathspec .nonexistent. did not match any files" stderr &&
135 + ! grep -F -f sparse_error_header stderr
136 +'
137 +
138 +test_expect_success 'do not warn when pathspec matches dense entries' '
139 + setup_sparse_entry &&
140 + echo modified >sparse_entry &&
141 + >dense_entry &&
142 + git add "*_entry" 2>stderr &&
143 + test_must_be_empty stderr &&
144 + test_sparse_entry_unchanged &&
145 + git ls-files --error-unmatch dense_entry
146 +'
147 +
148 +test_expect_success 'add obeys advice.updateSparsePath' '
149 + setup_sparse_entry &&
150 + test_must_fail git -c advice.updateSparsePath=false add sparse_entry 2>stderr &&
151 + test_cmp sparse_entry_error stderr
152 +
153 +'
154 +
155 test_done