{"thread":{"id":"55302","subject":"[PATCH v3 2/7] t3705: add tests for `git add` in sparse checkouts","startedAt":"2021-03-12T22:49:10Z","lastAt":"2021-04-16T23:17:21Z","messageCount":32,"participants":["Matheus Tavares","Elijah Newren","Junio C Hamano","Matheus Tavares Bernardino","Ævar Arnfjörð Bjarmason","Derrick Stolee"],"isPatch":true,"patchVersion":3,"patchTotal":7},"messages":[{"id":"419005","messageId":"d93c3f51465a3e409819db63bd81802e7ef20ea9.1615588108.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1615588108.git.matheus.bernardino@usp.br","subject":"[PATCH v3 2/7] t3705: add tests for `git add` in sparse checkouts","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-03-12T22:47:57Z","receivedAt":"2021-03-12T22:49:10Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"We already have a couple tests for `add` with SKIP_WORKTREE entries in\nt7012, but these only cover the most basic scenarios. As we will be\nchanging how `add` deals with sparse paths in the subsequent commits,\nlet's move these two tests to their own file and add more test cases\nfor different `add` options and situations. This also demonstrates two\noptions that don't currently respect SKIP_WORKTREE entries: `--chmod`\nand `--renormalize`.\n\nSigned-off-by: Matheus Tavares \n---\n t/t3705-add-sparse-checkout.sh | 96 ++++++++++++++++++++++++++++++++\n t/t7012-skip-worktree-writing.sh | 19 -------\n 2 files changed, 96 insertions(+), 19 deletions(-)\n create mode 100755 t/t3705-add-sparse-checkout.sh\n\ndiff --git a/t/t3705-add-sparse-checkout.sh b/t/t3705-add-sparse-checkout.sh\nnew file mode 100755\nindex 0000000000..6c5b8be863\n--- /dev/null\n+++ b/t/t3705-add-sparse-checkout.sh\n@@ -0,0 +1,96 @@\n+#!/bin/sh\n+\n+test_description='git add in sparse checked out working trees'\n+\n+. ./test-lib.sh\n+\n+SPARSE_ENTRY_BLOB=\"\"\n+\n+# Optionally take a printf format string to write to the sparse_entry file\n+setup_sparse_entry () {\n+\t# 'sparse_entry' might already be in the index with the skip-worktree\n+\t# bit set. Remove it so that the subsequent git add can update it.\n+\tgit update-index --force-remove sparse_entry &&\n+\tif test $# -eq 1\n+\tthen\n+\t\tprintf \"$1\" >sparse_entry\n+\telse\n+\t\t>sparse_entry\n+\tfi &&\n+\tgit add sparse_entry &&\n+\tgit update-index --skip-worktree sparse_entry &&\n+\tSPARSE_ENTRY_BLOB=$(git rev-parse :sparse_entry)\n+}\n+\n+test_sparse_entry_unchanged () {\n+\techo \"100644 $SPARSE_ENTRY_BLOB 0\tsparse_entry\" >expected &&\n+\tgit ls-files --stage sparse_entry >actual &&\n+\ttest_cmp expected actual\n+}\n+\n+setup_gitignore () {\n+\ttest_when_finished rm -f .gitignore &&\n+\tcat >.gitignore <<-EOF\n+\t*\n+\t!/sparse_entry\n+\tEOF\n+}\n+\n+test_expect_success 'git add does not remove sparse entries' '\n+\tsetup_sparse_entry &&\n+\trm sparse_entry &&\n+\tgit add sparse_entry &&\n+\ttest_sparse_entry_unchanged\n+'\n+\n+test_expect_success 'git add -A does not remove sparse entries' '\n+\tsetup_sparse_entry &&\n+\trm sparse_entry &&\n+\tsetup_gitignore &&\n+\tgit add -A &&\n+\ttest_sparse_entry_unchanged\n+'\n+\n+test_expect_success 'git add . does not remove sparse entries' '\n+\tsetup_sparse_entry &&\n+\trm sparse_entry &&\n+\tsetup_gitignore &&\n+\tgit add . &&\n+\ttest_sparse_entry_unchanged\n+'\n+\n+for opt in \"\" -f -u --ignore-removal --dry-run\n+do\n+\ttest_expect_success \"git add${opt:+ $opt} does not update sparse entries\" '\n+\t\tsetup_sparse_entry &&\n+\t\techo modified >sparse_entry &&\n+\t\tgit add $opt sparse_entry &&\n+\t\ttest_sparse_entry_unchanged\n+\t'\n+done\n+\n+test_expect_success 'git add --refresh does not update sparse entries' '\n+\tsetup_sparse_entry &&\n+\tgit ls-files --debug sparse_entry | grep mtime >before &&\n+\ttest-tool chmtime -60 sparse_entry &&\n+\tgit add --refresh sparse_entry &&\n+\tgit ls-files --debug sparse_entry | grep mtime >after &&\n+\ttest_cmp before after\n+'\n+\n+test_expect_failure 'git add --chmod does not update sparse entries' '\n+\tsetup_sparse_entry &&\n+\tgit add --chmod=+x sparse_entry &&\n+\ttest_sparse_entry_unchanged &&\n+\t! test -x sparse_entry\n+'\n+\n+test_expect_failure 'git add --renormalize does not update sparse entries' '\n+\ttest_config core.autocrlf false &&\n+\tsetup_sparse_entry \"LINEONE\\r\\nLINETWO\\r\\n\" &&\n+\techo \"sparse_entry text=auto\" >.gitattributes &&\n+\tgit add --renormalize sparse_entry &&\n+\ttest_sparse_entry_unchanged\n+'\n+\n+test_done\ndiff --git a/t/t7012-skip-worktree-writing.sh b/t/t7012-skip-worktree-writing.sh\nindex f2a8e76511..a1080b94e3 100755\n--- a/t/t7012-skip-worktree-writing.sh\n+++ b/t/t7012-skip-worktree-writing.sh\n@@ -60,13 +60,6 @@ setup_absent() {\n \tgit update-index --skip-worktree 1\n }\n \n-test_absent() {\n-\techo \"100644 $EMPTY_BLOB 0\t1\" > expected &&\n-\tgit ls-files --stage 1 > result &&\n-\ttest_cmp expected result &&\n-\ttest ! -f 1\n-}\n-\n setup_dirty() {\n \tgit update-index --force-remove 1 &&\n \techo dirty > 1 &&\n@@ -100,18 +93,6 @@ test_expect_success 'index setup' '\n \ttest_cmp expected result\n '\n \n-test_expect_success 'git-add ignores worktree content' '\n-\tsetup_absent &&\n-\tgit add 1 &&\n-\ttest_absent\n-'\n-\n-test_expect_success 'git-add ignores worktree content' '\n-\tsetup_dirty &&\n-\tgit add 1 &&\n-\ttest_dirty\n-'\n-\n test_expect_success 'git-rm fails if worktree is dirty' '\n \tsetup_dirty &&\n \ttest_must_fail git rm 1 &&\n-- \n2.30.1\n\n"},{"id":"419006","messageId":"cover.1615588108.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1614138107.git.matheus.bernardino@usp.br","subject":"[PATCH v3 0/7] add/rm: honor sparse checkout and warn on sparse paths","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-03-12T22:47:55Z","receivedAt":"2021-03-12T22:49:10Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Make `rm` honor sparse checkouts, and make both `rm` and `add` warn\nwhen they are asked to update sparse entries.\n\nMain changes since v2:\n\nPatch 2:\n- Removed unnecessary `rm` in t3705's auxiliary function and added\n comment about\twhy `update-index --force-remove` is used there.\n\nPatch 4:\n- Replaced the boolean function parameter with a more descriptive enum.\n This way, readers don't have to go to the function definition to\n understand what the caller wants.\n\nPatches 5 and 6:\n- Reworded both commit messages and the advice.updateSparsePath docs to\n avoid saying \"pathspecs that *only* match sparse entries\", as the\n situations to which they refer might also include matches among\n ignored paths.\n- Rephrased the warning message about sparse paths so that it doesn't\n contradict the message on ignored paths when they are displayed\n together.\n\nPatch 7:\n- Adjusted the tests to check for the new sparse warning message.\n\nMatheus Tavares (7):\n add: include magic part of pathspec on --refresh error\n t3705: add tests for `git add` in sparse checkouts\n add: make --chmod and --renormalize honor sparse checkouts\n pathspec: allow to ignore SKIP_WORKTREE entries on index matching\n refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag\n add: warn when asked to update SKIP_WORKTREE entries\n rm: honor sparse checkout patterns\n\n Documentation/config/advice.txt | 4 +\n Documentation/git-rm.txt | 4 +-\n advice.c | 20 ++++\n advice.h | 4 +\n builtin/add.c | 77 ++++++++++++---\n builtin/check-ignore.c | 3 +-\n builtin/rm.c | 35 ++++---\n cache.h | 15 +--\n pathspec.c | 25 ++++-\n pathspec.h | 18 +++-\n read-cache.c | 5 +-\n t/t3602-rm-sparse-checkout.sh | 78 ++++++++++++++++\n t/t3700-add.sh | 6 ++\n t/t3705-add-sparse-checkout.sh | 155 +++++++++++++++++++++++++++++++\n t/t7011-skip-worktree-reading.sh | 5 -\n t/t7012-skip-worktree-writing.sh | 19 ----\n 16 files changed, 409 insertions(+), 64 deletions(-)\n create mode 100755 t/t3602-rm-sparse-checkout.sh\n create mode 100755 t/t3705-add-sparse-checkout.sh\n\nRange-diff against v2:\n1: 46147a8637 = 1: 05b66a8c2b add: include magic part of pathspec on --refresh error\n2: 2de55420fa ! 2: d93c3f5146 t3705: add tests for `git add` in sparse checkouts\n @@ t/t3705-add-sparse-checkout.sh (new)\n +\n +# Optionally take a printf format string to write to the sparse_entry file\n +setup_sparse_entry () {\n -+\trm -f sparse_entry &&\n ++\t# 'sparse_entry' might already be in the index with the skip-worktree\n ++\t# bit set. Remove it so that the subsequent git add can update it.\n +\tgit update-index --force-remove sparse_entry &&\n -+\n +\tif test $# -eq 1\n +\tthen\n +\t\tprintf \"$1\" >sparse_entry\n3: 604480ade9 = 3: c90d5e3c23 add: make --chmod and --renormalize honor sparse checkouts\n4: 1494e65316 ! 4: 69d51579b3 pathspec: allow to ignore SKIP_WORKTREE entries on index matching\n @@ Metadata\n ## Commit message ##\n pathspec: allow to ignore SKIP_WORKTREE entries on index matching\n \n - Add the 'ignore_skip_worktree' boolean parameter to both\n - add_pathspec_matches_against_index() and\n - find_pathspecs_matching_against_index(). When true, these functions will\n - not try to match the given pathspec with SKIP_WORKTREE entries. This\n - will be used in a future patch to make `git add` display a hint\n - when the pathspec matches only sparse paths.\n + Add a new enum parameter to `add_pathspec_matches_against_index()` and\n + `find_pathspecs_matching_against_index()`, allowing callers to specify\n + whether these function should attempt to match SKIP_WORKTREE entries or\n + not. This will be used in a future patch to make `git add` display a\n + hint when it is asked to update SKIP_WORKTREE entries.\n \n Signed-off-by: Matheus Tavares \n \n @@ builtin/add.c: static char *prune_directory(struct dir_struct *dir, struct paths\n \t}\n \tdir->nr = dst - dir->entries;\n -\tadd_pathspec_matches_against_index(pathspec, &the_index, seen);\n -+\tadd_pathspec_matches_against_index(pathspec, &the_index, seen, 0);\n ++\tadd_pathspec_matches_against_index(pathspec, &the_index, seen,\n ++\t\t\t\t\t PS_HEED_SKIP_WORKTREE);\n \treturn seen;\n }\n \n @@ builtin/add.c: int cmd_add(int argc, const char **argv, const char *prefix)\n \n \t\tif (!seen)\n -\t\t\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index);\n -+\t\t\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index, 0);\n ++\t\t\tseen = find_pathspecs_matching_against_index(&pathspec,\n ++\t\t\t\t\t&the_index, PS_HEED_SKIP_WORKTREE);\n \n \t\t/*\n \t\t * file_exists() assumes exact match\n @@ builtin/check-ignore.c: static int check_ignore(struct dir_struct *dir,\n \t * 'git status', 'git add' etc.\n \t */\n -\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index);\n -+\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index, 0);\n ++\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index,\n ++\t\t\t\t\t\t PS_HEED_SKIP_WORKTREE);\n \tfor (i = 0; i < pathspec.nr; i++) {\n \t\tfull_path = pathspec.items[i].match;\n \t\tpattern = NULL;\n @@ pathspec.c\n void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n \t\t\t\t\tconst struct index_state *istate,\n -\t\t\t\t\tchar *seen)\n -+\t\t\t\t\tchar *seen, int ignore_skip_worktree)\n ++\t\t\t\t\tchar *seen,\n ++\t\t\t\t\tenum ps_skip_worktree_action sw_action)\n {\n \tint num_unmatched = 0, i;\n \n @@ pathspec.c: void add_pathspec_matches_against_index(const struct pathspec *paths\n \t\treturn;\n \tfor (i = 0; i < istate->cache_nr; i++) {\n \t\tconst struct cache_entry *ce = istate->cache[i];\n -+\t\tif (ignore_skip_worktree && ce_skip_worktree(ce))\n ++\t\tif (sw_action == PS_IGNORE_SKIP_WORKTREE && ce_skip_worktree(ce))\n +\t\t\tcontinue;\n \t\tce_path_match(istate, ce, pathspec, seen);\n \t}\n @@ pathspec.c: void add_pathspec_matches_against_index(const struct pathspec *paths\n char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,\n -\t\t\t\t\t const struct index_state *istate)\n +\t\t\t\t\t const struct index_state *istate,\n -+\t\t\t\t\t int ignore_skip_worktree)\n ++\t\t\t\t\t enum ps_skip_worktree_action sw_action)\n {\n \tchar *seen = xcalloc(pathspec->nr, 1);\n -\tadd_pathspec_matches_against_index(pathspec, istate, seen);\n -+\tadd_pathspec_matches_against_index(pathspec, istate, seen,\n -+\t\t\t\t\t ignore_skip_worktree);\n ++\tadd_pathspec_matches_against_index(pathspec, istate, seen, sw_action);\n \treturn seen;\n }\n \n \n ## pathspec.h ##\n @@ pathspec.h: static inline int ps_strcmp(const struct pathspec_item *item,\n + \t\treturn strcmp(s1, s2);\n + }\n \n ++enum ps_skip_worktree_action {\n ++ PS_HEED_SKIP_WORKTREE = 0,\n ++ PS_IGNORE_SKIP_WORKTREE = 1\n ++};\n void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n \t\t\t\t\tconst struct index_state *istate,\n -\t\t\t\t\tchar *seen);\n -+\t\t\t\t\tchar *seen, int ignore_skip_worktree);\n ++\t\t\t\t\tchar *seen,\n ++\t\t\t\t\tenum ps_skip_worktree_action sw_action);\n char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,\n -\t\t\t\t\t const struct index_state *istate);\n +\t\t\t\t\t const struct index_state *istate,\n -+\t\t\t\t\t int ignore_skip_worktree);\n ++\t\t\t\t\t enum ps_skip_worktree_action sw_action);\n int match_pathspec_attrs(const struct index_state *istate,\n \t\t\t const char *name, int namelen,\n \t\t\t const struct pathspec_item *item);\n5: 584811e5c0 ! 5: 34a61a0d03 refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag\n @@ Commit message\n \n refresh_index() optionally takes a seen[] array to mark the pathspec\n items that had matches in the index. This is used by `git add --refresh`\n - to find out if there were any pathspec without matches, and display an\n + to find out if there was any pathspec without matches, and display an\n error accordingly.\n \n In the following patch, `git add` will also learn to warn about\n - pathspecs that only match sparse entries (which are not updated). But\n - for that, we will need a seen[] array marked exclusively with matches\n - from dense entries. To avoid having to call ce_path_match() again for\n - these entries after refresh_index() returns, add a flag that implements\n - this restriction inside the function itself.\n + pathspecs that match no eligible path for updating, but do match sparse\n + entries. For that, we will need a seen[] array marked exclusively with\n + matches from dense entries. To avoid having to call ce_path_match()\n + again for these entries after refresh_index() returns, add a flag that\n + implements this restriction inside the function itself.\n \n Note that refresh_index() does not update sparse entries, regardless of\n passing the flag or not. The flag only controls whether matches with\n @@ read-cache.c: int refresh_index(struct index_state *istate, unsigned int flags,\n \tint in_porcelain = (flags & REFRESH_IN_PORCELAIN);\n \tunsigned int options = (CE_MATCH_REFRESH |\n @@ read-cache.c: int refresh_index(struct index_state *istate, unsigned int flags,\n - \t\tint cache_errno = 0;\n - \t\tint changed = 0;\n \t\tint filtered = 0;\n + \t\tint t2_did_lstat = 0;\n + \t\tint t2_did_scan = 0;\n +\t\tchar *cur_seen;\n \n \t\tce = istate->cache[i];\n6: 15f399f4fa ! 6: 66d5c71182 add: warn when pathspec only matches SKIP_WORKTREE entries\n @@ Metadata\n Author: Matheus Tavares \n \n ## Commit message ##\n - add: warn when pathspec only matches SKIP_WORKTREE entries\n + add: warn when asked to update SKIP_WORKTREE entries\n \n `git add` already refrains from updating SKIP_WORKTREE entries, but it\n - silently exits with zero code when a pathspec only matches these\n - entries. Instead, let's warn the user and display a hint on how to\n - update these entries.\n + silently exits with zero code when it is asked to do so. Instead, let's\n + warn the user and display a hint on how to update these entries.\n \n - Note that the warning is only shown if the pathspec matches no untracked\n - paths in the working tree and only matches index entries with the\n - SKIP_WORKTREE bit set. A warning message was chosen over erroring out\n - right away to reproduce the same behavior `add` already exhibits with\n - ignored files. This also allow users to continue their workflow without\n - having to invoke `add` again with only the matching pathspecs, as the\n - matched files will have already been added.\n + Note that we only warn the user whey they give a pathspec item that\n + matches no eligible path for updating, but it does match one or more\n + SKIP_WORKTREE entries. A warning was chosen over erroring out right away\n + to reproduce the same behavior `add` already exhibits with ignored\n + files. This also allow users to continue their workflow without having\n + to invoke `add` again with only the eligible paths (as those will have\n + already been added).\n \n Signed-off-by: Matheus Tavares \n \n @@ Documentation/config/advice.txt: advice.*::\n \t\tAdvice shown if a user runs the add command without providing\n \t\tthe pathspec parameter.\n +\tupdateSparsePath::\n -+\t\tAdvice shown if the pathspec given to linkgit:git-add[1] only\n -+\t\tmatches index entries outside the current sparse-checkout.\n ++\t\tAdvice shown when linkgit:git-add[1] is asked to update index\n ++\t\tentries outside the current sparse checkout.\n --\n \n ## advice.c ##\n @@ advice.c: void NORETURN die_conclude_merge(void)\n +\tif (!pathspec_list->nr)\n +\t\treturn;\n +\n -+\tfprintf(stderr, _(\"The following pathspecs only matched index entries outside the current\\n\"\n -+\t\t\t \"sparse checkout:\\n\"));\n ++\tfprintf(stderr, _(\"The following pathspecs didn't match any\"\n ++\t\t\t \" eligible path, but they do match index\\n\"\n ++\t\t\t \"entries outside the current sparse checkout:\\n\"));\n +\tfor_each_string_list_item(item, pathspec_list)\n +\t\tfprintf(stderr, \"%s\\n\", item->string);\n +\n +\tadvise_if_enabled(ADVICE_UPDATE_SPARSE_PATH,\n -+\t\t\t _(\"Disable or modify the sparsity rules if you intend to update such entries.\"));\n -+\n ++\t\t\t _(\"Disable or modify the sparsity rules if you intend\"\n ++\t\t\t \" to update such entries.\"));\n +}\n +\n void detach_advice(const char *new_name)\n @@ advice.h: void advise_if_enabled(enum advice_type type, const char *advice, ...)\n \n ## builtin/add.c ##\n @@ builtin/add.c: static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec,\n - \t\t\t*dst++ = entry;\n \t}\n \tdir->nr = dst - dir->entries;\n --\tadd_pathspec_matches_against_index(pathspec, &the_index, seen, 0);\n -+\tadd_pathspec_matches_against_index(pathspec, &the_index, seen, 1);\n + \tadd_pathspec_matches_against_index(pathspec, &the_index, seen,\n +-\t\t\t\t\t PS_HEED_SKIP_WORKTREE);\n ++\t\t\t\t\t PS_IGNORE_SKIP_WORKTREE);\n \treturn seen;\n }\n \n @@ builtin/add.c: int cmd_add(int argc, const char **argv, const char *prefix)\n +\t\tstruct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;\n \n \t\tif (!seen)\n --\t\t\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index, 0);\n -+\t\t\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index, 1);\n + \t\t\tseen = find_pathspecs_matching_against_index(&pathspec,\n +-\t\t\t\t\t&the_index, PS_HEED_SKIP_WORKTREE);\n ++\t\t\t\t\t&the_index, PS_IGNORE_SKIP_WORKTREE);\n \n \t\t/*\n \t\t * file_exists() assumes exact match\n @@ builtin/add.c: int cmd_add(int argc, const char **argv, const char *prefix)\n +\t\t\t\tcontinue;\n +\t\t\t}\n +\n -+\t\t\t/* Don't complain at 'git add .' inside empty repo. */\n ++\t\t\t/* Don't complain at 'git add .' on empty repo */\n +\t\t\tif (!path[0])\n +\t\t\t\tcontinue;\n +\n @@ pathspec.h\n @@ pathspec.h: void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,\n \t\t\t\t\t const struct index_state *istate,\n - \t\t\t\t\t int ignore_skip_worktree);\n + \t\t\t\t\t enum ps_skip_worktree_action sw_action);\n +char *find_pathspecs_matching_skip_worktree(const struct pathspec *pathspec);\n +static inline int matches_skip_worktree(const struct pathspec *pathspec,\n +\t\t\t\t\tint item, char **seen_ptr)\n @@ t/t3705-add-sparse-checkout.sh: setup_gitignore () {\n \tEOF\n }\n \n -+test_expect_success 'setup' '\n ++test_expect_success 'setup' \"\n +\tcat >sparse_error_header <<-EOF &&\n -+\tThe following pathspecs only matched index entries outside the current\n -+\tsparse checkout:\n ++\tThe following pathspecs didn't match any eligible path, but they do match index\n ++\tentries outside the current sparse checkout:\n +\tEOF\n +\n +\tcat >sparse_hint <<-EOF &&\n +\thint: Disable or modify the sparsity rules if you intend to update such entries.\n -+\thint: Disable this message with \"git config advice.updateSparsePath false\"\n ++\thint: Disable this message with \\\"git config advice.updateSparsePath false\\\"\n +\tEOF\n +\n +\techo sparse_entry | cat sparse_error_header - >sparse_entry_error &&\n +\tcat sparse_entry_error sparse_hint >error_and_hint\n -+'\n ++\"\n +\n test_expect_success 'git add does not remove sparse entries' '\n \tsetup_sparse_entry &&\n @@ t/t3705-add-sparse-checkout.sh: test_expect_success 'git add --renormalize does\n -\tgit add --renormalize sparse_entry &&\n +\ttest_must_fail git add --renormalize sparse_entry 2>stderr &&\n +\ttest_i18ncmp error_and_hint stderr &&\n - \ttest_sparse_entry_unchanged\n - '\n - \n ++\ttest_sparse_entry_unchanged\n ++'\n ++\n +test_expect_success 'git add --dry-run --ignore-missing warn on sparse path' '\n +\tsetup_sparse_entry &&\n +\trm sparse_entry &&\n +\ttest_must_fail git add --dry-run --ignore-missing sparse_entry 2>stderr &&\n +\ttest_i18ncmp error_and_hint stderr &&\n -+\ttest_sparse_entry_unchanged\n -+'\n -+\n + \ttest_sparse_entry_unchanged\n + '\n + \n +test_expect_success 'do not advice about sparse entries when they do not match the pathspec' '\n +\tsetup_sparse_entry &&\n +\ttest_must_fail git add nonexistent 2>stderr &&\n +\ttest_i18ngrep \"fatal: pathspec .nonexistent. did not match any files\" stderr &&\n -+\ttest_i18ngrep ! \"The following pathspecs only matched index entries\" stderr\n ++\ttest_i18ngrep ! -F -f sparse_error_header stderr\n +'\n +\n +test_expect_success 'do not warn when pathspec matches dense entries' '\n7: bb29aa4d8f ! 7: d25ea10560 rm: honor sparse checkout patterns\n @@ Metadata\n ## Commit message ##\n rm: honor sparse checkout patterns\n \n - `git add` refrains from adding or updating paths outside the sparsity\n - rules, but `git rm` doesn't follow the same restrictions. This is\n - somewhat counter-intuitive and inconsistent. So make `rm` honor the\n - sparse checkout and advise on how to remove SKIP_WORKTREE entries, just\n - like `add` does. Also add some tests for the new behavior.\n + `git add` refrains from adding or updating index entries that are\n + outside the current sparse checkout, but `git rm` doesn't follow the\n + same restriction. This is somewhat counter-intuitive and inconsistent.\n + So make `rm` honor the sparsity rules and advise on how to remove\n + SKIP_WORKTREE entries, just like `add` does. Also add some tests for the\n + new behavior.\n \n Suggested-by: Elijah Newren \n Signed-off-by: Matheus Tavares \n @@ Documentation/config/advice.txt: advice.*::\n \t\tAdvice shown if a user runs the add command without providing\n \t\tthe pathspec parameter.\n \tupdateSparsePath::\n --\t\tAdvice shown if the pathspec given to linkgit:git-add[1] only\n --\t\tmatches index entries outside the current sparse-checkout.\n -+\t\tAdvice shown if the pathspec given to linkgit:git-add[1] or\n -+\t\tlinkgit:git-rm[1] only matches index entries outside the\n -+\t\tcurrent sparse-checkout.\n +-\t\tAdvice shown when linkgit:git-add[1] is asked to update index\n +-\t\tentries outside the current sparse checkout.\n ++\t\tAdvice shown when either linkgit:git-add[1] or linkgit:git-rm[1]\n ++\t\tis asked to update index entries outside the current sparse\n ++\t\tcheckout.\n --\n \n ## Documentation/git-rm.txt ##\n @@ t/t3602-rm-sparse-checkout.sh (new)\n +\n +. ./test-lib.sh\n +\n -+test_expect_success 'setup' '\n ++test_expect_success 'setup' \"\n +\tmkdir -p sub/dir &&\n +\ttouch a b c sub/d sub/dir/e &&\n +\tgit add -A &&\n +\tgit commit -m files &&\n +\n -+\tcat >sparse_entry_b_error <<-EOF &&\n -+\tThe following pathspecs only matched index entries outside the current\n -+\tsparse checkout:\n -+\tb\n ++\tcat >sparse_error_header <<-EOF &&\n ++\tThe following pathspecs didn't match any eligible path, but they do match index\n ++\tentries outside the current sparse checkout:\n +\tEOF\n +\n -+\tcat >b_error_and_hint sparse_entry_b_error - <<-EOF\n ++\tcat >sparse_hint <<-EOF &&\n +\thint: Disable or modify the sparsity rules if you intend to update such entries.\n -+\thint: Disable this message with \"git config advice.updateSparsePath false\"\n ++\thint: Disable this message with \\\"git config advice.updateSparsePath false\\\"\n +\tEOF\n -+'\n ++\n ++\techo b | cat sparse_error_header - >sparse_entry_b_error &&\n ++\tcat sparse_entry_b_error sparse_hint >b_error_and_hint\n ++\"\n +\n +for opt in \"\" -f --dry-run\n +do\n @@ t/t3602-rm-sparse-checkout.sh (new)\n +\tgit sparse-checkout set a &&\n +\ttest_must_fail git rm nonexistent 2>stderr &&\n +\ttest_i18ngrep \"fatal: pathspec .nonexistent. did not match any files\" stderr &&\n -+\ttest_i18ngrep ! \"The following pathspecs only matched index entries\" stderr\n ++\ttest_i18ngrep ! -F -f sparse_error_header stderr\n +'\n +\n +test_expect_success 'do not warn about sparse entries when pathspec matches dense entries' '\n-- \n2.30.1\n\n"},{"id":"419007","messageId":"05b66a8c2b0a11c2b4695f8bd49ecf4ad6bfc438.1615588108.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1615588108.git.matheus.bernardino@usp.br","subject":"[PATCH v3 1/7] add: include magic part of pathspec on --refresh error","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-03-12T22:47:56Z","receivedAt":"2021-03-12T22:49:10Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"When `git add --refresh ` doesn't find any matches for the\ngiven pathspec, it prints an error message using the `match` field of\nthe `struct pathspec_item`. However, this field doesn't contain the\nmagic part of the pathspec. Instead, let's use the `original` field.\n\nSigned-off-by: Matheus Tavares \n---\n builtin/add.c | 2 +-\n t/t3700-add.sh | 6 ++++++\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ea762a41e3..24ed7e25f3 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -187,7 +187,7 @@ static void refresh(int verbose, const struct pathspec *pathspec)\n \tfor (i = 0; i < pathspec->nr; i++) {\n \t\tif (!seen[i])\n \t\t\tdie(_(\"pathspec '%s' did not match any files\"),\n-\t\t\t pathspec->items[i].match);\n+\t\t\t pathspec->items[i].original);\n \t}\n \tfree(seen);\n }\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex b3b122ff97..cd78dd65e5 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -196,6 +196,12 @@ test_expect_success 'git add --refresh with pathspec' '\n \tgrep baz actual\n '\n \n+test_expect_success 'git add --refresh correctly reports no match error' \"\n+\techo \\\"fatal: pathspec ':(icase)nonexistent' did not match any files\\\" >expect &&\n+\ttest_must_fail git add --refresh ':(icase)nonexistent' 2>actual &&\n+\ttest_i18ncmp expect actual\n+\"\n+\n test_expect_success POSIXPERM,SANITY 'git add should fail atomically upon an unreadable file' '\n \tgit reset --hard &&\n \tdate >foo1 &&\n-- \n2.30.1\n\n"},{"id":"419008","messageId":"69d51579b36defbcac2eedaef19465c2a973c041.1615588109.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1615588108.git.matheus.bernardino@usp.br","subject":"[PATCH v3 4/7] pathspec: allow to ignore SKIP_WORKTREE entries on index matching","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-03-12T22:47:59Z","receivedAt":"2021-03-12T22:49:10Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Add a new enum parameter to `add_pathspec_matches_against_index()` and\n`find_pathspecs_matching_against_index()`, allowing callers to specify\nwhether these function should attempt to match SKIP_WORKTREE entries or\nnot. This will be used in a future patch to make `git add` display a\nwarning when it is asked to update SKIP_WORKTREE entries.\n\nSigned-off-by: Matheus Tavares \n---\n builtin/add.c | 6 ++++--\n builtin/check-ignore.c | 3 ++-\n pathspec.c | 10 +++++++---\n pathspec.h | 10 ++++++++--\n 4 files changed, 21 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 5fec21a792..050cb8af30 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -177,7 +177,8 @@ static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec,\n \t\t\t*dst++ = entry;\n \t}\n \tdir->nr = dst - dir->entries;\n-\tadd_pathspec_matches_against_index(pathspec, &the_index, seen);\n+\tadd_pathspec_matches_against_index(pathspec, &the_index, seen,\n+\t\t\t\t\t PS_HEED_SKIP_WORKTREE);\n \treturn seen;\n }\n \n@@ -578,7 +579,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tint i;\n \n \t\tif (!seen)\n-\t\t\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index);\n+\t\t\tseen = find_pathspecs_matching_against_index(&pathspec,\n+\t\t\t\t\t&the_index, PS_HEED_SKIP_WORKTREE);\n \n \t\t/*\n \t\t * file_exists() assumes exact match\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex 3c652748d5..0f4480a11b 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -100,7 +100,8 @@ static int check_ignore(struct dir_struct *dir,\n \t * should not be ignored, in order to be consistent with\n \t * 'git status', 'git add' etc.\n \t */\n-\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index);\n+\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index,\n+\t\t\t\t\t\t PS_HEED_SKIP_WORKTREE);\n \tfor (i = 0; i < pathspec.nr; i++) {\n \t\tfull_path = pathspec.items[i].match;\n \t\tpattern = NULL;\ndiff --git a/pathspec.c b/pathspec.c\nindex 7a229d8d22..6d502e64e5 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -21,7 +21,8 @@\n */\n void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n \t\t\t\t\tconst struct index_state *istate,\n-\t\t\t\t\tchar *seen)\n+\t\t\t\t\tchar *seen,\n+\t\t\t\t\tenum ps_skip_worktree_action sw_action)\n {\n \tint num_unmatched = 0, i;\n \n@@ -38,6 +39,8 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n \t\treturn;\n \tfor (i = 0; i < istate->cache_nr; i++) {\n \t\tconst struct cache_entry *ce = istate->cache[i];\n+\t\tif (sw_action == PS_IGNORE_SKIP_WORKTREE && ce_skip_worktree(ce))\n+\t\t\tcontinue;\n \t\tce_path_match(istate, ce, pathspec, seen);\n \t}\n }\n@@ -51,10 +54,11 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n * given pathspecs achieves against all items in the index.\n */\n char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,\n-\t\t\t\t\t const struct index_state *istate)\n+\t\t\t\t\t const struct index_state *istate,\n+\t\t\t\t\t enum ps_skip_worktree_action sw_action)\n {\n \tchar *seen = xcalloc(pathspec->nr, 1);\n-\tadd_pathspec_matches_against_index(pathspec, istate, seen);\n+\tadd_pathspec_matches_against_index(pathspec, istate, seen, sw_action);\n \treturn seen;\n }\n \ndiff --git a/pathspec.h b/pathspec.h\nindex 454ce364fa..0feb8e9f67 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -149,11 +149,17 @@ static inline int ps_strcmp(const struct pathspec_item *item,\n \t\treturn strcmp(s1, s2);\n }\n \n+enum ps_skip_worktree_action {\n+ PS_HEED_SKIP_WORKTREE = 0,\n+ PS_IGNORE_SKIP_WORKTREE = 1\n+};\n void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n \t\t\t\t\tconst struct index_state *istate,\n-\t\t\t\t\tchar *seen);\n+\t\t\t\t\tchar *seen,\n+\t\t\t\t\tenum ps_skip_worktree_action sw_action);\n char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,\n-\t\t\t\t\t const struct index_state *istate);\n+\t\t\t\t\t const struct index_state *istate,\n+\t\t\t\t\t enum ps_skip_worktree_action sw_action);\n int match_pathspec_attrs(const struct index_state *istate,\n \t\t\t const char *name, int namelen,\n \t\t\t const struct pathspec_item *item);\n-- \n2.30.1\n\n"},{"id":"419009","messageId":"34a61a0d03868c43d68a04bca8d86dd98de2aa28.1615588109.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1615588108.git.matheus.bernardino@usp.br","subject":"[PATCH v3 5/7] refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-03-12T22:48:00Z","receivedAt":"2021-03-12T22:49:10Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"refresh_index() optionally takes a seen[] array to mark the pathspec\nitems that had matches in the index. This is used by `git add --refresh`\nto find out if there was any pathspec without matches, and display an\nerror accordingly.\n\nIn the following patch, `git add` will also learn to warn about\npathspecs that match no eligible path for updating, but do match sparse\nentries. For that, we will need a seen[] array marked exclusively with\nmatches from dense entries. To avoid having to call ce_path_match()\nagain for these entries after refresh_index() returns, add a flag that\nimplements this restriction inside the function itself.\n\nNote that refresh_index() does not update sparse entries, regardless of\npassing the flag or not. The flag only controls whether matches with\nthese entries should appear in the seen[] array.\n\nWhile we are here, also realign the REFRESH_* flags and convert the hex\nvalues to the more natural bit shift format, which makes it easier to\nspot holes.\n\nSigned-off-by: Matheus Tavares \n---\n cache.h | 15 ++++++++-------\n read-cache.c | 5 ++++-\n 2 files changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 6fda8091f1..0b6074d49e 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -879,13 +879,14 @@ int match_stat_data_racy(const struct index_state *istate,\n \n void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, struct stat *st);\n \n-#define REFRESH_REALLY\t\t0x0001\t/* ignore_valid */\n-#define REFRESH_UNMERGED\t0x0002\t/* allow unmerged */\n-#define REFRESH_QUIET\t\t0x0004\t/* be quiet about it */\n-#define REFRESH_IGNORE_MISSING\t0x0008\t/* ignore non-existent */\n-#define REFRESH_IGNORE_SUBMODULES\t0x0010\t/* ignore submodules */\n-#define REFRESH_IN_PORCELAIN\t0x0020\t/* user friendly output, not \"needs update\" */\n-#define REFRESH_PROGRESS\t0x0040 /* show progress bar if stderr is tty */\n+#define REFRESH_REALLY (1 << 0) /* ignore_valid */\n+#define REFRESH_UNMERGED (1 << 1) /* allow unmerged */\n+#define REFRESH_QUIET (1 << 2) /* be quiet about it */\n+#define REFRESH_IGNORE_MISSING (1 << 3) /* ignore non-existent */\n+#define REFRESH_IGNORE_SUBMODULES (1 << 4) /* ignore submodules */\n+#define REFRESH_IN_PORCELAIN (1 << 5) /* user friendly output, not \"needs update\" */\n+#define REFRESH_PROGRESS (1 << 6) /* show progress bar if stderr is tty */\n+#define REFRESH_DONT_MARK_SPARSE_MATCHES (1 << 7) /* don't mark sparse entries' matches on seen[] */\n int refresh_index(struct index_state *, unsigned int flags, const struct pathspec *pathspec, char *seen, const char *header_msg);\n /*\n * Refresh the index and write it to disk.\ndiff --git a/read-cache.c b/read-cache.c\nindex 1e9a50c6c7..0bf7c64eff 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1514,6 +1514,7 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \tint quiet = (flags & REFRESH_QUIET) != 0;\n \tint not_new = (flags & REFRESH_IGNORE_MISSING) != 0;\n \tint ignore_submodules = (flags & REFRESH_IGNORE_SUBMODULES) != 0;\n+\tint no_sparse_on_seen = (flags & REFRESH_DONT_MARK_SPARSE_MATCHES) != 0;\n \tint first = 1;\n \tint in_porcelain = (flags & REFRESH_IN_PORCELAIN);\n \tunsigned int options = (CE_MATCH_REFRESH |\n@@ -1552,12 +1553,14 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \t\tint filtered = 0;\n \t\tint t2_did_lstat = 0;\n \t\tint t2_did_scan = 0;\n+\t\tchar *cur_seen;\n \n \t\tce = istate->cache[i];\n \t\tif (ignore_submodules && S_ISGITLINK(ce->ce_mode))\n \t\t\tcontinue;\n \n-\t\tif (pathspec && !ce_path_match(istate, ce, pathspec, seen))\n+\t\tcur_seen = no_sparse_on_seen && ce_skip_worktree(ce) ? NULL : seen;\n+\t\tif (pathspec && !ce_path_match(istate, ce, pathspec, cur_seen))\n \t\t\tfiltered = 1;\n \n \t\tif (ce_stage(ce)) {\n-- \n2.30.1\n\n"},{"id":"419010","messageId":"c90d5e3c2327d0ea43afc3f1b98dca76cf4973c2.1615588108.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1615588108.git.matheus.bernardino@usp.br","subject":"[PATCH v3 3/7] add: make --chmod and --renormalize honor sparse checkouts","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-03-12T22:47:58Z","receivedAt":"2021-03-12T22:49:11Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Signed-off-by: Matheus Tavares \n---\n builtin/add.c | 5 +++++\n t/t3705-add-sparse-checkout.sh | 4 ++--\n 2 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 24ed7e25f3..5fec21a792 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -46,6 +46,9 @@ static int chmod_pathspec(struct pathspec *pathspec, char flip, int show_only)\n \t\tstruct cache_entry *ce = active_cache[i];\n \t\tint err;\n \n+\t\tif (ce_skip_worktree(ce))\n+\t\t\tcontinue;\n+\n \t\tif (pathspec && !ce_path_match(&the_index, ce, pathspec, NULL))\n \t\t\tcontinue;\n \n@@ -144,6 +147,8 @@ static int renormalize_tracked_files(const struct pathspec *pathspec, int flags)\n \tfor (i = 0; i < active_nr; i++) {\n \t\tstruct cache_entry *ce = active_cache[i];\n \n+\t\tif (ce_skip_worktree(ce))\n+\t\t\tcontinue;\n \t\tif (ce_stage(ce))\n \t\t\tcontinue; /* do not touch unmerged paths */\n \t\tif (!S_ISREG(ce->ce_mode) && !S_ISLNK(ce->ce_mode))\ndiff --git a/t/t3705-add-sparse-checkout.sh b/t/t3705-add-sparse-checkout.sh\nindex 6c5b8be863..00b10ac877 100755\n--- a/t/t3705-add-sparse-checkout.sh\n+++ b/t/t3705-add-sparse-checkout.sh\n@@ -78,14 +78,14 @@ test_expect_success 'git add --refresh does not update sparse entries' '\n \ttest_cmp before after\n '\n \n-test_expect_failure 'git add --chmod does not update sparse entries' '\n+test_expect_success 'git add --chmod does not update sparse entries' '\n \tsetup_sparse_entry &&\n \tgit add --chmod=+x sparse_entry &&\n \ttest_sparse_entry_unchanged &&\n \t! test -x sparse_entry\n '\n \n-test_expect_failure 'git add --renormalize does not update sparse entries' '\n+test_expect_success 'git add --renormalize does not update sparse entries' '\n \ttest_config core.autocrlf false &&\n \tsetup_sparse_entry \"LINEONE\\r\\nLINETWO\\r\\n\" &&\n \techo \"sparse_entry text=auto\" >.gitattributes &&\n-- \n2.30.1\n\n"},{"id":"419011","messageId":"66d5c71182274c78e1fcfe84e77deb17e4f0d7e6.1615588109.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1615588108.git.matheus.bernardino@usp.br","subject":"[PATCH v3 6/7] add: warn when asked to update SKIP_WORKTREE entries","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-03-12T22:48:01Z","receivedAt":"2021-03-12T22:49:11Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"`git add` already refrains from updating SKIP_WORKTREE entries, but it\nsilently exits with zero code when it is asked to do so. Instead, let's\nwarn the user and display a hint on how to update these entries.\n\nNote that we only warn the user whey they give a pathspec item that\nmatches no eligible path for updating, but it does match one or more\nSKIP_WORKTREE entries. A warning was chosen over erroring out right away\nto reproduce the same behavior `add` already exhibits with ignored\nfiles. This also allow users to continue their workflow without having\nto invoke `add` again with only the eligible paths (as those will have\nalready been added).\n\nSigned-off-by: Matheus Tavares \n---\n Documentation/config/advice.txt | 3 ++\n advice.c | 20 +++++++++\n advice.h | 4 ++\n builtin/add.c | 70 ++++++++++++++++++++++++-------\n pathspec.c | 15 +++++++\n pathspec.h | 8 ++++\n t/t3705-add-sparse-checkout.sh | 73 +++++++++++++++++++++++++++++----\n 7 files changed, 172 insertions(+), 21 deletions(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex acbd0c09aa..851b83cf30 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -119,4 +119,7 @@ advice.*::\n \taddEmptyPathspec::\n \t\tAdvice shown if a user runs the add command without providing\n \t\tthe pathspec parameter.\n+\tupdateSparsePath::\n+\t\tAdvice shown when linkgit:git-add[1] is asked to update index\n+\t\tentries outside the current sparse checkout.\n --\ndiff --git a/advice.c b/advice.c\nindex 164742305f..0b9c89c48a 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -2,6 +2,7 @@\n #include \"config.h\"\n #include \"color.h\"\n #include \"help.h\"\n+#include \"string-list.h\"\n \n int advice_fetch_show_forced_updates = 1;\n int advice_push_update_rejected = 1;\n@@ -136,6 +137,7 @@ static struct {\n \t[ADVICE_STATUS_HINTS]\t\t\t\t= { \"statusHints\", 1 },\n \t[ADVICE_STATUS_U_OPTION]\t\t\t= { \"statusUoption\", 1 },\n \t[ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE] = { \"submoduleAlternateErrorStrategyDie\", 1 },\n+\t[ADVICE_UPDATE_SPARSE_PATH]\t\t\t= { \"updateSparsePath\", 1 },\n \t[ADVICE_WAITING_FOR_EDITOR]\t\t\t= { \"waitingForEditor\", 1 },\n };\n \n@@ -284,6 +286,24 @@ void NORETURN die_conclude_merge(void)\n \tdie(_(\"Exiting because of unfinished merge.\"));\n }\n \n+void advise_on_updating_sparse_paths(struct string_list *pathspec_list)\n+{\n+\tstruct string_list_item *item;\n+\n+\tif (!pathspec_list->nr)\n+\t\treturn;\n+\n+\tfprintf(stderr, _(\"The following pathspecs didn't match any\"\n+\t\t\t \" eligible path, but they do match index\\n\"\n+\t\t\t \"entries outside the current sparse checkout:\\n\"));\n+\tfor_each_string_list_item(item, pathspec_list)\n+\t\tfprintf(stderr, \"%s\\n\", item->string);\n+\n+\tadvise_if_enabled(ADVICE_UPDATE_SPARSE_PATH,\n+\t\t\t _(\"Disable or modify the sparsity rules if you intend\"\n+\t\t\t \" to update such entries.\"));\n+}\n+\n void detach_advice(const char *new_name)\n {\n \tconst char *fmt =\ndiff --git a/advice.h b/advice.h\nindex bc2432980a..bd26c385d0 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -3,6 +3,8 @@\n \n #include \"git-compat-util.h\"\n \n+struct string_list;\n+\n extern int advice_fetch_show_forced_updates;\n extern int advice_push_update_rejected;\n extern int advice_push_non_ff_current;\n@@ -71,6 +73,7 @@ extern int advice_add_empty_pathspec;\n \tADVICE_STATUS_HINTS,\n \tADVICE_STATUS_U_OPTION,\n \tADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,\n+\tADVICE_UPDATE_SPARSE_PATH,\n \tADVICE_WAITING_FOR_EDITOR,\n };\n \n@@ -92,6 +95,7 @@ void advise_if_enabled(enum advice_type type, const char *advice, ...);\n int error_resolve_conflict(const char *me);\n void NORETURN die_resolve_conflict(const char *me);\n void NORETURN die_conclude_merge(void);\n+void advise_on_updating_sparse_paths(struct string_list *pathspec_list);\n void detach_advice(const char *new_name);\n \n #endif /* ADVICE_H */\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 050cb8af30..d453313735 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -178,24 +178,43 @@ static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec,\n \t}\n \tdir->nr = dst - dir->entries;\n \tadd_pathspec_matches_against_index(pathspec, &the_index, seen,\n-\t\t\t\t\t PS_HEED_SKIP_WORKTREE);\n+\t\t\t\t\t PS_IGNORE_SKIP_WORKTREE);\n \treturn seen;\n }\n \n-static void refresh(int verbose, const struct pathspec *pathspec)\n+static int refresh(int verbose, const struct pathspec *pathspec)\n {\n \tchar *seen;\n-\tint i;\n+\tint i, ret = 0;\n+\tchar *skip_worktree_seen = NULL;\n+\tstruct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;\n+\tint flags = REFRESH_DONT_MARK_SPARSE_MATCHES |\n+\t\t (verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET);\n \n \tseen = xcalloc(pathspec->nr, 1);\n-\trefresh_index(&the_index, verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET,\n-\t\t pathspec, seen, _(\"Unstaged changes after refreshing the index:\"));\n+\trefresh_index(&the_index, flags, pathspec, seen,\n+\t\t _(\"Unstaged changes after refreshing the index:\"));\n \tfor (i = 0; i < pathspec->nr; i++) {\n-\t\tif (!seen[i])\n-\t\t\tdie(_(\"pathspec '%s' did not match any files\"),\n-\t\t\t pathspec->items[i].original);\n+\t\tif (!seen[i]) {\n+\t\t\tif (matches_skip_worktree(pathspec, i, &skip_worktree_seen)) {\n+\t\t\t\tstring_list_append(&only_match_skip_worktree,\n+\t\t\t\t\t\t pathspec->items[i].original);\n+\t\t\t} else {\n+\t\t\t\tdie(_(\"pathspec '%s' did not match any files\"),\n+\t\t\t\t pathspec->items[i].original);\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif (only_match_skip_worktree.nr) {\n+\t\tadvise_on_updating_sparse_paths(&only_match_skip_worktree);\n+\t\tret = 1;\n \t}\n+\n \tfree(seen);\n+\tfree(skip_worktree_seen);\n+\tstring_list_clear(&only_match_skip_worktree, 0);\n+\treturn ret;\n }\n \n int run_add_interactive(const char *revision, const char *patch_mode,\n@@ -571,16 +590,18 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (refresh_only) {\n-\t\trefresh(verbose, &pathspec);\n+\t\texit_status |= refresh(verbose, &pathspec);\n \t\tgoto finish;\n \t}\n \n \tif (pathspec.nr) {\n \t\tint i;\n+\t\tchar *skip_worktree_seen = NULL;\n+\t\tstruct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;\n \n \t\tif (!seen)\n \t\t\tseen = find_pathspecs_matching_against_index(&pathspec,\n-\t\t\t\t\t&the_index, PS_HEED_SKIP_WORKTREE);\n+\t\t\t\t\t&the_index, PS_IGNORE_SKIP_WORKTREE);\n \n \t\t/*\n \t\t * file_exists() assumes exact match\n@@ -594,12 +615,24 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \t\tfor (i = 0; i < pathspec.nr; i++) {\n \t\t\tconst char *path = pathspec.items[i].match;\n+\n \t\t\tif (pathspec.items[i].magic & PATHSPEC_EXCLUDE)\n \t\t\t\tcontinue;\n-\t\t\tif (!seen[i] && path[0] &&\n-\t\t\t ((pathspec.items[i].magic &\n-\t\t\t (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||\n-\t\t\t !file_exists(path))) {\n+\t\t\tif (seen[i])\n+\t\t\t\tcontinue;\n+\n+\t\t\tif (matches_skip_worktree(&pathspec, i, &skip_worktree_seen)) {\n+\t\t\t\tstring_list_append(&only_match_skip_worktree,\n+\t\t\t\t\t\t pathspec.items[i].original);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\t/* Don't complain at 'git add .' on empty repo */\n+\t\t\tif (!path[0])\n+\t\t\t\tcontinue;\n+\n+\t\t\tif ((pathspec.items[i].magic & (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||\n+\t\t\t !file_exists(path)) {\n \t\t\t\tif (ignore_missing) {\n \t\t\t\t\tint dtype = DT_UNKNOWN;\n \t\t\t\t\tif (is_excluded(&dir, &the_index, path, &dtype))\n@@ -610,7 +643,16 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t pathspec.items[i].original);\n \t\t\t}\n \t\t}\n+\n+\n+\t\tif (only_match_skip_worktree.nr) {\n+\t\t\tadvise_on_updating_sparse_paths(&only_match_skip_worktree);\n+\t\t\texit_status = 1;\n+\t\t}\n+\n \t\tfree(seen);\n+\t\tfree(skip_worktree_seen);\n+\t\tstring_list_clear(&only_match_skip_worktree, 0);\n \t}\n \n \tplug_bulk_checkin();\ndiff --git a/pathspec.c b/pathspec.c\nindex 6d502e64e5..8247a65ec8 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -62,6 +62,21 @@ char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,\n \treturn seen;\n }\n \n+char *find_pathspecs_matching_skip_worktree(const struct pathspec *pathspec)\n+{\n+\tstruct index_state *istate = the_repository->index;\n+\tchar *seen = xcalloc(pathspec->nr, 1);\n+\tint i;\n+\n+\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = istate->cache[i];\n+\t\tif (ce_skip_worktree(ce))\n+\t\t ce_path_match(istate, ce, pathspec, seen);\n+\t}\n+\n+\treturn seen;\n+}\n+\n /*\n * Magic pathspec\n *\ndiff --git a/pathspec.h b/pathspec.h\nindex 0feb8e9f67..5b4c6614bf 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -160,6 +160,14 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,\n \t\t\t\t\t const struct index_state *istate,\n \t\t\t\t\t enum ps_skip_worktree_action sw_action);\n+char *find_pathspecs_matching_skip_worktree(const struct pathspec *pathspec);\n+static inline int matches_skip_worktree(const struct pathspec *pathspec,\n+\t\t\t\t\tint item, char **seen_ptr)\n+{\n+\tif (!*seen_ptr)\n+\t\t*seen_ptr = find_pathspecs_matching_skip_worktree(pathspec);\n+\treturn (*seen_ptr)[item];\n+}\n int match_pathspec_attrs(const struct index_state *istate,\n \t\t\t const char *name, int namelen,\n \t\t\t const struct pathspec_item *item);\ndiff --git a/t/t3705-add-sparse-checkout.sh b/t/t3705-add-sparse-checkout.sh\nindex 00b10ac877..c8db764058 100755\n--- a/t/t3705-add-sparse-checkout.sh\n+++ b/t/t3705-add-sparse-checkout.sh\n@@ -36,10 +36,26 @@ setup_gitignore () {\n \tEOF\n }\n \n+test_expect_success 'setup' \"\n+\tcat >sparse_error_header <<-EOF &&\n+\tThe following pathspecs didn't match any eligible path, but they do match index\n+\tentries outside the current sparse checkout:\n+\tEOF\n+\n+\tcat >sparse_hint <<-EOF &&\n+\thint: Disable or modify the sparsity rules if you intend to update such entries.\n+\thint: Disable this message with \\\"git config advice.updateSparsePath false\\\"\n+\tEOF\n+\n+\techo sparse_entry | cat sparse_error_header - >sparse_entry_error &&\n+\tcat sparse_entry_error sparse_hint >error_and_hint\n+\"\n+\n test_expect_success 'git add does not remove sparse entries' '\n \tsetup_sparse_entry &&\n \trm sparse_entry &&\n-\tgit add sparse_entry &&\n+\ttest_must_fail git add sparse_entry 2>stderr &&\n+\ttest_i18ncmp error_and_hint stderr &&\n \ttest_sparse_entry_unchanged\n '\n \n@@ -47,7 +63,8 @@ test_expect_success 'git add -A does not remove sparse entries' '\n \tsetup_sparse_entry &&\n \trm sparse_entry &&\n \tsetup_gitignore &&\n-\tgit add -A &&\n+\tgit add -A 2>stderr &&\n+\ttest_must_be_empty stderr &&\n \ttest_sparse_entry_unchanged\n '\n \n@@ -55,7 +72,13 @@ test_expect_success 'git add . does not remove sparse entries' '\n \tsetup_sparse_entry &&\n \trm sparse_entry &&\n \tsetup_gitignore &&\n-\tgit add . &&\n+\ttest_must_fail git add . 2>stderr &&\n+\n+\tcat sparse_error_header >expect &&\n+\techo . >>expect &&\n+\tcat sparse_hint >>expect &&\n+\n+\ttest_i18ncmp expect stderr &&\n \ttest_sparse_entry_unchanged\n '\n \n@@ -64,7 +87,8 @@ do\n \ttest_expect_success \"git add${opt:+ $opt} does not update sparse entries\" '\n \t\tsetup_sparse_entry &&\n \t\techo modified >sparse_entry &&\n-\t\tgit add $opt sparse_entry &&\n+\t\ttest_must_fail git add $opt sparse_entry 2>stderr &&\n+\t\ttest_i18ncmp error_and_hint stderr &&\n \t\ttest_sparse_entry_unchanged\n \t'\n done\n@@ -73,14 +97,16 @@ test_expect_success 'git add --refresh does not update sparse entries' '\n \tsetup_sparse_entry &&\n \tgit ls-files --debug sparse_entry | grep mtime >before &&\n \ttest-tool chmtime -60 sparse_entry &&\n-\tgit add --refresh sparse_entry &&\n+\ttest_must_fail git add --refresh sparse_entry 2>stderr &&\n+\ttest_i18ncmp error_and_hint stderr &&\n \tgit ls-files --debug sparse_entry | grep mtime >after &&\n \ttest_cmp before after\n '\n \n test_expect_success 'git add --chmod does not update sparse entries' '\n \tsetup_sparse_entry &&\n-\tgit add --chmod=+x sparse_entry &&\n+\ttest_must_fail git add --chmod=+x sparse_entry 2>stderr &&\n+\ttest_i18ncmp error_and_hint stderr &&\n \ttest_sparse_entry_unchanged &&\n \t! test -x sparse_entry\n '\n@@ -89,8 +115,41 @@ test_expect_success 'git add --renormalize does not update sparse entries' '\n \ttest_config core.autocrlf false &&\n \tsetup_sparse_entry \"LINEONE\\r\\nLINETWO\\r\\n\" &&\n \techo \"sparse_entry text=auto\" >.gitattributes &&\n-\tgit add --renormalize sparse_entry &&\n+\ttest_must_fail git add --renormalize sparse_entry 2>stderr &&\n+\ttest_i18ncmp error_and_hint stderr &&\n+\ttest_sparse_entry_unchanged\n+'\n+\n+test_expect_success 'git add --dry-run --ignore-missing warn on sparse path' '\n+\tsetup_sparse_entry &&\n+\trm sparse_entry &&\n+\ttest_must_fail git add --dry-run --ignore-missing sparse_entry 2>stderr &&\n+\ttest_i18ncmp error_and_hint stderr &&\n \ttest_sparse_entry_unchanged\n '\n \n+test_expect_success 'do not advice about sparse entries when they do not match the pathspec' '\n+\tsetup_sparse_entry &&\n+\ttest_must_fail git add nonexistent 2>stderr &&\n+\ttest_i18ngrep \"fatal: pathspec .nonexistent. did not match any files\" stderr &&\n+\ttest_i18ngrep ! -F -f sparse_error_header stderr\n+'\n+\n+test_expect_success 'do not warn when pathspec matches dense entries' '\n+\tsetup_sparse_entry &&\n+\techo modified >sparse_entry &&\n+\t>dense_entry &&\n+\tgit add \"*_entry\" 2>stderr &&\n+\ttest_must_be_empty stderr &&\n+\ttest_sparse_entry_unchanged &&\n+\tgit ls-files --error-unmatch dense_entry\n+'\n+\n+test_expect_success 'add obeys advice.updateSparsePath' '\n+\tsetup_sparse_entry &&\n+\ttest_must_fail git -c advice.updateSparsePath=false add sparse_entry 2>stderr &&\n+\ttest_i18ncmp sparse_entry_error stderr\n+\n+'\n+\n test_done\n-- \n2.30.1\n\n"},{"id":"419012","messageId":"d25ea10560b501d2ca77b567927a0bb9bc874a1e.1615588109.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1615588108.git.matheus.bernardino@usp.br","subject":"[PATCH v3 7/7] rm: honor sparse checkout patterns","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-03-12T22:48:02Z","receivedAt":"2021-03-12T22:49:42Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"`git add` refrains from adding or updating index entries that are\noutside the current sparse checkout, but `git rm` doesn't follow the\nsame restriction. This is somewhat counter-intuitive and inconsistent.\nSo make `rm` honor the sparsity rules and advise on how to remove\nSKIP_WORKTREE entries just like `add` does. Also add some tests for the\nnew behavior.\n\nSuggested-by: Elijah Newren \nSigned-off-by: Matheus Tavares \n---\n Documentation/config/advice.txt | 5 +-\n Documentation/git-rm.txt | 4 +-\n builtin/rm.c | 35 +++++++++-----\n t/t3602-rm-sparse-checkout.sh | 78 ++++++++++++++++++++++++++++++++\n t/t7011-skip-worktree-reading.sh | 5 --\n 5 files changed, 108 insertions(+), 19 deletions(-)\n create mode 100755 t/t3602-rm-sparse-checkout.sh\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex 851b83cf30..8b2849ff7b 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -120,6 +120,7 @@ advice.*::\n \t\tAdvice shown if a user runs the add command without providing\n \t\tthe pathspec parameter.\n \tupdateSparsePath::\n-\t\tAdvice shown when linkgit:git-add[1] is asked to update index\n-\t\tentries outside the current sparse checkout.\n+\t\tAdvice shown when either linkgit:git-add[1] or linkgit:git-rm[1]\n+\t\tis asked to update index entries outside the current sparse\n+\t\tcheckout.\n --\ndiff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt\nindex ab750367fd..26e9b28470 100644\n--- a/Documentation/git-rm.txt\n+++ b/Documentation/git-rm.txt\n@@ -23,7 +23,9 @@ branch, and no updates to their contents can be staged in the index,\n though that default behavior can be overridden with the `-f` option.\n When `--cached` is given, the staged content has to\n match either the tip of the branch or the file on disk,\n-allowing the file to be removed from just the index.\n+allowing the file to be removed from just the index. When\n+sparse-checkouts are in use (see linkgit:git-sparse-checkout[1]),\n+`git rm` will only remove paths within the sparse-checkout patterns.\n \n \n OPTIONS\ndiff --git a/builtin/rm.c b/builtin/rm.c\nindex 4858631e0f..d23a3b2164 100644\n--- a/builtin/rm.c\n+++ b/builtin/rm.c\n@@ -5,6 +5,7 @@\n */\n #define USE_THE_INDEX_COMPATIBILITY_MACROS\n #include \"builtin.h\"\n+#include \"advice.h\"\n #include \"config.h\"\n #include \"lockfile.h\"\n #include \"dir.h\"\n@@ -254,7 +255,7 @@ static struct option builtin_rm_options[] = {\n int cmd_rm(int argc, const char **argv, const char *prefix)\n {\n \tstruct lock_file lock_file = LOCK_INIT;\n-\tint i;\n+\tint i, ret = 0;\n \tstruct pathspec pathspec;\n \tchar *seen;\n \n@@ -295,6 +296,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \n \tfor (i = 0; i < active_nr; i++) {\n \t\tconst struct cache_entry *ce = active_cache[i];\n+\t\tif (ce_skip_worktree(ce))\n+\t\t\tcontinue;\n \t\tif (!ce_path_match(&the_index, ce, &pathspec, seen))\n \t\t\tcontinue;\n \t\tALLOC_GROW(list.entry, list.nr + 1, list.alloc);\n@@ -308,24 +311,34 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \tif (pathspec.nr) {\n \t\tconst char *original;\n \t\tint seen_any = 0;\n+\t\tchar *skip_worktree_seen = NULL;\n+\t\tstruct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;\n+\n \t\tfor (i = 0; i < pathspec.nr; i++) {\n \t\t\toriginal = pathspec.items[i].original;\n-\t\t\tif (!seen[i]) {\n-\t\t\t\tif (!ignore_unmatch) {\n-\t\t\t\t\tdie(_(\"pathspec '%s' did not match any files\"),\n-\t\t\t\t\t original);\n-\t\t\t\t}\n-\t\t\t}\n-\t\t\telse {\n+\t\t\tif (seen[i])\n \t\t\t\tseen_any = 1;\n-\t\t\t}\n+\t\t\telse if (ignore_unmatch)\n+\t\t\t\tcontinue;\n+\t\t\telse if (matches_skip_worktree(&pathspec, i, &skip_worktree_seen))\n+\t\t\t\tstring_list_append(&only_match_skip_worktree, original);\n+\t\t\telse\n+\t\t\t\tdie(_(\"pathspec '%s' did not match any files\"), original);\n+\n \t\t\tif (!recursive && seen[i] == MATCHED_RECURSIVELY)\n \t\t\t\tdie(_(\"not removing '%s' recursively without -r\"),\n \t\t\t\t *original ? original : \".\");\n \t\t}\n \n+\t\tif (only_match_skip_worktree.nr) {\n+\t\t\tadvise_on_updating_sparse_paths(&only_match_skip_worktree);\n+\t\t\tret = 1;\n+\t\t}\n+\t\tfree(skip_worktree_seen);\n+\t\tstring_list_clear(&only_match_skip_worktree, 0);\n+\n \t\tif (!seen_any)\n-\t\t\texit(0);\n+\t\t\texit(ret);\n \t}\n \n \tif (!index_only)\n@@ -405,5 +418,5 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \t\t\t COMMIT_LOCK | SKIP_IF_UNCHANGED))\n \t\tdie(_(\"Unable to write new index file\"));\n \n-\treturn 0;\n+\treturn ret;\n }\ndiff --git a/t/t3602-rm-sparse-checkout.sh b/t/t3602-rm-sparse-checkout.sh\nnew file mode 100755\nindex 0000000000..c83b026ed8\n--- /dev/null\n+++ b/t/t3602-rm-sparse-checkout.sh\n@@ -0,0 +1,78 @@\n+#!/bin/sh\n+\n+test_description='git rm in sparse checked out working trees'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' \"\n+\tmkdir -p sub/dir &&\n+\ttouch a b c sub/d sub/dir/e &&\n+\tgit add -A &&\n+\tgit commit -m files &&\n+\n+\tcat >sparse_error_header <<-EOF &&\n+\tThe following pathspecs didn't match any eligible path, but they do match index\n+\tentries outside the current sparse checkout:\n+\tEOF\n+\n+\tcat >sparse_hint <<-EOF &&\n+\thint: Disable or modify the sparsity rules if you intend to update such entries.\n+\thint: Disable this message with \\\"git config advice.updateSparsePath false\\\"\n+\tEOF\n+\n+\techo b | cat sparse_error_header - >sparse_entry_b_error &&\n+\tcat sparse_entry_b_error sparse_hint >b_error_and_hint\n+\"\n+\n+for opt in \"\" -f --dry-run\n+do\n+\ttest_expect_success \"rm${opt:+ $opt} does not remove sparse entries\" '\n+\t\tgit sparse-checkout set a &&\n+\t\ttest_must_fail git rm $opt b 2>stderr &&\n+\t\ttest_i18ncmp b_error_and_hint stderr &&\n+\t\tgit ls-files --error-unmatch b\n+\t'\n+done\n+\n+test_expect_success 'recursive rm does not remove sparse entries' '\n+\tgit reset --hard &&\n+\tgit sparse-checkout set sub/dir &&\n+\tgit rm -r sub &&\n+\tgit status --porcelain -uno >actual &&\n+\techo \"D sub/dir/e\" >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'rm obeys advice.updateSparsePath' '\n+\tgit reset --hard &&\n+\tgit sparse-checkout set a &&\n+\ttest_must_fail git -c advice.updateSparsePath=false rm b 2>stderr &&\n+\ttest_i18ncmp sparse_entry_b_error stderr\n+'\n+\n+test_expect_success 'do not advice about sparse entries when they do not match the pathspec' '\n+\tgit reset --hard &&\n+\tgit sparse-checkout set a &&\n+\ttest_must_fail git rm nonexistent 2>stderr &&\n+\ttest_i18ngrep \"fatal: pathspec .nonexistent. did not match any files\" stderr &&\n+\ttest_i18ngrep ! -F -f sparse_error_header stderr\n+'\n+\n+test_expect_success 'do not warn about sparse entries when pathspec matches dense entries' '\n+\tgit reset --hard &&\n+\tgit sparse-checkout set a &&\n+\tgit rm \"[ba]\" 2>stderr &&\n+\ttest_must_be_empty stderr &&\n+\tgit ls-files --error-unmatch b &&\n+\ttest_must_fail git ls-files --error-unmatch a\n+'\n+\n+test_expect_success 'do not warn about sparse entries with --ignore-unmatch' '\n+\tgit reset --hard &&\n+\tgit sparse-checkout set a &&\n+\tgit rm --ignore-unmatch b 2>stderr &&\n+\ttest_must_be_empty stderr &&\n+\tgit ls-files --error-unmatch b\n+'\n+\n+test_done\ndiff --git a/t/t7011-skip-worktree-reading.sh b/t/t7011-skip-worktree-reading.sh\nindex 26852586ac..1761a2b1b9 100755\n--- a/t/t7011-skip-worktree-reading.sh\n+++ b/t/t7011-skip-worktree-reading.sh\n@@ -132,11 +132,6 @@ test_expect_success 'diff-files does not examine skip-worktree dirty entries' '\n \ttest -z \"$(git diff-files -- one)\"\n '\n \n-test_expect_success 'git-rm succeeds on skip-worktree absent entries' '\n-\tsetup_absent &&\n-\tgit rm 1\n-'\n-\n test_expect_success 'commit on skip-worktree absent entries' '\n \tgit reset &&\n \tsetup_absent &&\n-- \n2.30.1\n\n"},{"id":"419025","messageId":"CABPp-BEriPaDY=84=j0tT62+U93jKePTyTKPZA4+dYtSFJaSTg@mail.gmail.com","threadId":"55302","inReplyTo":"cover.1615588108.git.matheus.bernardino@usp.br","subject":"Re: [PATCH v3 0/7] add/rm: honor sparse checkout and warn on sparse paths","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-03-13T07:07:21Z","receivedAt":"2021-03-13T07:09:01Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Mar 12, 2021 at 2:48 PM Matheus Tavares\n wrote:\n>\n> Make `rm` honor sparse checkouts, and make both `rm` and `add` warn\n> when they are asked to update sparse entries.\n>\n> Main changes since v2:\n>\n> Patch 2:\n> - Removed unnecessary `rm` in t3705's auxiliary function and added\n> comment about why `update-index --force-remove` is used there.\n>\n> Patch 4:\n> - Replaced the boolean function parameter with a more descriptive enum.\n> This way, readers don't have to go to the function definition to\n> understand what the caller wants.\n>\n> Patches 5 and 6:\n> - Reworded both commit messages and the advice.updateSparsePath docs to\n> avoid saying \"pathspecs that *only* match sparse entries\", as the\n> situations to which they refer might also include matches among\n> ignored paths.\n> - Rephrased the warning message about sparse paths so that it doesn't\n> contradict the message on ignored paths when they are displayed\n> together.\n>\n> Patch 7:\n> - Adjusted the tests to check for the new sparse warning message.\n\nv2 already looked pretty good to me; even if I still had lots of\nlittle comments on that series. This version addresses all my\nremaining concerns with v2. So this gets my:\n\nReviewed-by: Elijah Newren \n\n\nThanks for all your work on this!\n"},{"id":"419670","messageId":"xmqqsg4sowks.fsf@gitster.g","threadId":"55302","inReplyTo":"34a61a0d03868c43d68a04bca8d86dd98de2aa28.1615588109.git.matheus.bernardino@usp.br","subject":"Re: [PATCH v3 5/7] refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-18T23:45:23Z","receivedAt":"2021-03-18T23:46:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matheus Tavares writes:\n\n> refresh_index() optionally takes a seen[] array to mark the pathspec\n> items that had matches in the index. This is used by `git add --refresh`\n> to find out if there was any pathspec without matches, and display an\n> error accordingly.\n\nIt smells a bit iffy in that this allows a path to \"match\" a pattern\nbut yet not result in seen[] to record that the pattern participated\nin the match. If the series is working towards certain operations\nnot to touch paths that are outside the sparse checkout, shouldn't\nit be making these paths not to match the pattern, and by doing so\nit would automatically cause the pattern to be considered \"not yet\nmatching any path\" when the matcher attempts to match the pattern to\nsuch a path?\n\n"},{"id":"419671","messageId":"xmqqo8fgovuz.fsf@gitster.g","threadId":"55302","inReplyTo":"xmqqsg4sowks.fsf@gitster.g","subject":"Re: [PATCH v3 5/7] refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-19T00:00:52Z","receivedAt":"2021-03-19T00:01:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Matheus Tavares writes:\n>\n>> refresh_index() optionally takes a seen[] array to mark the pathspec\n>> items that had matches in the index. This is used by `git add --refresh`\n>> to find out if there was any pathspec without matches, and display an\n>> error accordingly.\n>\n> It smells a bit iffy in that this allows a path to \"match\" a pattern\n> but yet not result in seen[] to record that the pattern participated\n> in the match. If the series is working towards certain operations\n> not to touch paths that are outside the sparse checkout, shouldn't\n> it be making these paths not to match the pattern, and by doing so\n> it would automatically cause the pattern to be considered \"not yet\n> matching any path\" when the matcher attempts to match the pattern to\n> such a path?\n\nIn other words, the change makes me wonder why we are not adding a\nflag that says \"do we or do we not want to match paths outside the\nsparse checkout cone?\", with which the seen[] would automatically\nrecord the right thing.\n"},{"id":"419699","messageId":"CAHd-oW4V4_XrY7XfQO1gLEZZmzcLbG0_M2ys+Meh4Ysu0Psz0Q@mail.gmail.com","threadId":"55302","inReplyTo":"xmqqo8fgovuz.fsf@gitster.g","subject":"Re: [PATCH v3 5/7] refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-03-19T12:23:03Z","receivedAt":"2021-03-19T12:23:54Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On Thu, Mar 18, 2021 at 9:00 PM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> > Matheus Tavares writes:\n> >\n> >> refresh_index() optionally takes a seen[] array to mark the pathspec\n> >> items that had matches in the index. This is used by `git add --refresh`\n> >> to find out if there was any pathspec without matches, and display an\n> >> error accordingly.\n> >\n> > It smells a bit iffy in that this allows a path to \"match\" a pattern\n> > but yet not result in seen[] to record that the pattern participated\n> > in the match. If the series is working towards certain operations\n> > not to touch paths that are outside the sparse checkout, shouldn't\n> > it be making these paths not to match the pattern, and by doing so\n> > it would automatically cause the pattern to be considered \"not yet\n> > matching any path\" when the matcher attempts to match the pattern to\n> > such a path?\n>\n> In other words, the change makes me wonder why we are not adding a\n> flag that says \"do we or do we not want to match paths outside the\n> sparse checkout cone?\", with which the seen[] would automatically\n> record the right thing.\n\nYeah, makes sense. I didn't want to make the flag skip the sparse\npaths unconditionally (i.e. without matching them) because then we\nwould also skip the ce_stage() checkings below and the later\nce_mark_uptodate(). And I wasn't sure whether this could cause any\nunwanted side effects.\n\nBut thinking more carefully about this now, unmerged paths should\nnever have the SKIP_WORKTREE bit set anyway, right? What about the\nCE_UPTODATE mark, would it be safe to skip it? I'm not very familiar\nwith this code, but I'll try to investigate more later.\n"},{"id":"419719","messageId":"xmqqlfajnn6w.fsf@gitster.g","threadId":"55302","inReplyTo":"CAHd-oW4V4_XrY7XfQO1gLEZZmzcLbG0_M2ys+Meh4Ysu0Psz0Q@mail.gmail.com","subject":"Re: [PATCH v3 5/7] refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-19T16:05:43Z","receivedAt":"2021-03-19T16:06:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matheus Tavares Bernardino writes:\n\n>> In other words, the change makes me wonder why we are not adding a\n>> flag that says \"do we or do we not want to match paths outside the\n>> sparse checkout cone?\", with which the seen[] would automatically\n>> record the right thing.\n>\n> Yeah, makes sense. I didn't want to make the flag skip the sparse\n> paths unconditionally (i.e. without matching them) because then we\n> would also skip the ce_stage() checkings below and the later\n> ce_mark_uptodate(). And I wasn't sure whether this could cause any\n> unwanted side effects.\n>\n> But thinking more carefully about this now, unmerged paths should\n> never have the SKIP_WORKTREE bit set anyway, right? What about the\n> CE_UPTODATE mark, would it be safe to skip it? I'm not very familiar\n> with this code, but I'll try to investigate more later.\n\nI do not offhand know the answers to these questions you ask in the\nlast paragraph myself, and it could turn out to be that the \"let\nprocessing go as before, matching what those excluded paths by the\nmatcher, but making the caller responsible for filtering out the\npaths excluded by sparcity out of the result from the matcher, but\ntweak seen[]\" approach, however ugly, is the most practical, after\nyour investigation. But I'd prefer to see how bad the situation is\nfirst ;-).\n\nThanks.\n"},{"id":"419896","messageId":"87h7l4qfdn.fsf@evledraar.gmail.com","threadId":"55302","inReplyTo":"d25ea10560b501d2ca77b567927a0bb9bc874a1e.1615588109.git.matheus.bernardino@usp.br","subject":"Re: [PATCH v3 7/7] rm: honor sparse checkout patterns","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-03-21T23:03:00Z","receivedAt":"2021-03-21T23:03:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Mar 12 2021, Matheus Tavares wrote:\n\n> +\ttest_expect_success \"rm${opt:+ $opt} does not remove sparse entries\" '\n> +\t\tgit sparse-checkout set a &&\n> +\t\ttest_must_fail git rm $opt b 2>stderr &&\n> +\t\ttest_i18ncmp b_error_and_hint stderr &&\n> +\t\tgit ls-files --error-unmatch b\n> +\t'\n> +done\n> +\n> +test_expect_success 'recursive rm does not remove sparse entries' '\n> +\tgit reset --hard &&\n> +\tgit sparse-checkout set sub/dir &&\n> +\tgit rm -r sub &&\n> +\tgit status --porcelain -uno >actual &&\n> +\techo \"D sub/dir/e\" >expected &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> +test_expect_success 'rm obeys advice.updateSparsePath' '\n> +\tgit reset --hard &&\n> +\tgit sparse-checkout set a &&\n> +\ttest_must_fail git -c advice.updateSparsePath=false rm b 2>stderr &&\n> +\ttest_i18ncmp sparse_entry_b_error stderr\n\nCalls to the test_i18ncmp function should just be test_cmp now. See\n1108cea7f8e (tests: remove most uses of test_i18ncmp, 2021-02-11).\n"},{"id":"419899","messageId":"CAHd-oW7bycPCnAtme1+LnKxnBbUFdaoi11ff7J5OPgcfqExktw@mail.gmail.com","threadId":"55302","inReplyTo":"87h7l4qfdn.fsf@evledraar.gmail.com","subject":"Re: [PATCH v3 7/7] rm: honor sparse checkout patterns","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-03-22T01:08:27Z","receivedAt":"2021-03-22T01:09:23Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On Sun, Mar 21, 2021 at 8:03 PM Ævar Arnfjörð Bjarmason\n wrote:\n>\n>\n> On Fri, Mar 12 2021, Matheus Tavares wrote:\n>\n> > + test_expect_success \"rm${opt:+ $opt} does not remove sparse entries\" '\n> > + git sparse-checkout set a &&\n> > + test_must_fail git rm $opt b 2>stderr &&\n> > + test_i18ncmp b_error_and_hint stderr &&\n> > + git ls-files --error-unmatch b\n> > + '\n> > +done\n> > +\n> > +test_expect_success 'recursive rm does not remove sparse entries' '\n> > + git reset --hard &&\n> > + git sparse-checkout set sub/dir &&\n> > + git rm -r sub &&\n> > + git status --porcelain -uno >actual &&\n> > + echo \"D sub/dir/e\" >expected &&\n> > + test_cmp expected actual\n> > +'\n> > +\n> > +test_expect_success 'rm obeys advice.updateSparsePath' '\n> > + git reset --hard &&\n> > + git sparse-checkout set a &&\n> > + test_must_fail git -c advice.updateSparsePath=false rm b 2>stderr &&\n> > + test_i18ncmp sparse_entry_b_error stderr\n>\n> Calls to the test_i18ncmp function should just be test_cmp now. See\n> 1108cea7f8e (tests: remove most uses of test_i18ncmp, 2021-02-11).\n\nThanks for letting me know, I'll fix it for the next version.\n"},{"id":"420057","messageId":"085184fb-d637-696d-085c-0b456c7e9568@gmail.com","threadId":"55302","inReplyTo":"d93c3f51465a3e409819db63bd81802e7ef20ea9.1615588108.git.matheus.bernardino@usp.br","subject":"Re: [PATCH v3 2/7] t3705: add tests for `git add` in sparse checkouts","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-03-23T20:00:11Z","receivedAt":"2021-03-23T20:01:13Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/12/21 5:47 PM, Matheus Tavares wrote:\n> We already have a couple tests for `add` with SKIP_WORKTREE entries in\n> t7012, but these only cover the most basic scenarios. As we will be\n> changing how `add` deals with sparse paths in the subsequent commits,\n> let's move these two tests to their own file and add more test cases\n> for different `add` options and situations. This also demonstrates two\n> options that don't currently respect SKIP_WORKTREE entries: `--chmod`\n> and `--renormalize`.\n\nI'm super-late to taking a look at this series, but I wanted\nto point out how much I like this new test structure. In\nparticular, I appreciate that you point out undesirable\nbehavior with a failing test here that changes with your\ncorrection in the next patch.\n\nThanks!\n-Stolee\n\n"},{"id":"420066","messageId":"3f5739f2-8d3a-3410-3907-f0cb022397b4@gmail.com","threadId":"55302","inReplyTo":"d25ea10560b501d2ca77b567927a0bb9bc874a1e.1615588109.git.matheus.bernardino@usp.br","subject":"Re: [PATCH v3 7/7] rm: honor sparse checkout patterns","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-03-23T20:47:16Z","receivedAt":"2021-03-23T20:48:15Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/12/2021 5:48 PM, Matheus Tavares wrote:\n> `git add` refrains from adding or updating index entries that are\n> outside the current sparse checkout, but `git rm` doesn't follow the\n> same restriction. This is somewhat counter-intuitive and inconsistent.\n> So make `rm` honor the sparsity rules and advise on how to remove\n> SKIP_WORKTREE entries just like `add` does. Also add some tests for the\n> new behavior.\n> \n> Suggested-by: Elijah Newren \n> Signed-off-by: Matheus Tavares \n> ---\n> Documentation/config/advice.txt | 5 +-\n> Documentation/git-rm.txt | 4 +-\n> builtin/rm.c | 35 +++++++++-----\n> t/t3602-rm-sparse-checkout.sh | 78 ++++++++++++++++++++++++++++++++\n> t/t7011-skip-worktree-reading.sh | 5 --\n> 5 files changed, 108 insertions(+), 19 deletions(-)\n> create mode 100755 t/t3602-rm-sparse-checkout.sh\n> \n> diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\n> index 851b83cf30..8b2849ff7b 100644\n> --- a/Documentation/config/advice.txt\n> +++ b/Documentation/config/advice.txt\n> @@ -120,6 +120,7 @@ advice.*::\n> \t\tAdvice shown if a user runs the add command without providing\n> \t\tthe pathspec parameter.\n> \tupdateSparsePath::\n> -\t\tAdvice shown when linkgit:git-add[1] is asked to update index\n> -\t\tentries outside the current sparse checkout.\n> +\t\tAdvice shown when either linkgit:git-add[1] or linkgit:git-rm[1]\n> +\t\tis asked to update index entries outside the current sparse\n> +\t\tcheckout.\n\nI wonder if this is going to need to be updated forever, or if in\nthe _next_ iteration we just say \"Advice shown when a command is\nasked to update...\" (using the \"three is many\" rule).\n\n> diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt\n> index ab750367fd..26e9b28470 100644\n> --- a/Documentation/git-rm.txt\n> +++ b/Documentation/git-rm.txt\n> @@ -23,7 +23,9 @@ branch, and no updates to their contents can be staged in the index,\n> though that default behavior can be overridden with the `-f` option.\n> When `--cached` is given, the staged content has to\n> match either the tip of the branch or the file on disk,\n> -allowing the file to be removed from just the index.\n> +allowing the file to be removed from just the index. When\n> +sparse-checkouts are in use (see linkgit:git-sparse-checkout[1]),\n> +`git rm` will only remove paths within the sparse-checkout patterns.\n\nGood to update in the correct spot.\n\nImplementation and tests look good, too.\n\nI just wanted to chime in and voice my support of this general\ndirection. Thank you!\n\n-Stolee\n"},{"id":"420638","messageId":"CAHd-oW4kRLjV9Sq3CFt-V1Ot9pYFzJggU1zPp3Hcuw=qWfq7Mg@mail.gmail.com","threadId":"55302","inReplyTo":"xmqqlfajnn6w.fsf@gitster.g","subject":"Re: [PATCH v3 5/7] refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-03-30T18:51:17Z","receivedAt":"2021-03-30T18:52:30Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On Fri, Mar 19, 2021 at 1:05 PM Junio C Hamano wrote:\n>\n> Matheus Tavares Bernardino writes:\n>\n> >> In other words, the change makes me wonder why we are not adding a\n> >> flag that says \"do we or do we not want to match paths outside the\n> >> sparse checkout cone?\", with which the seen[] would automatically\n> >> record the right thing.\n> >\n> > Yeah, makes sense. I didn't want to make the flag skip the sparse\n> > paths unconditionally (i.e. without matching them) because then we\n> > would also skip the ce_stage() checkings below and the later\n> > ce_mark_uptodate(). And I wasn't sure whether this could cause any\n> > unwanted side effects.\n> >\n> > But thinking more carefully about this now, unmerged paths should\n> > never have the SKIP_WORKTREE bit set anyway, right? What about the\n> > CE_UPTODATE mark, would it be safe to skip it? I'm not very familiar\n> > with this code, but I'll try to investigate more later.\n\nSorry I haven't given any update on this yet. From what I could see so\nfar, it seems OK to ignore the skip_worktree entries in\nrefresh_index() when it is called from `git add --refresh`. But\nbecause we would no longer mark the skip_worktree entries that match\nthe pathspec with CE_UPTODATE, do_write_index() would start checking\nif they are racy clean (this is only done when `!ce_uptodate(ce)`),\nwhich could add some lstat() overhead.\n\nHowever, this made me think what happens today if we do have a racy\nclean entry with the skip_worktree bit set... `git add --refresh` will\nprobably update the index without noticing that the entry is racy\nclean (because it won't check CE_UPTODATE entries, and skip_worktree\nentries will have this bit set in refresh_index()). Thus the entries'\nsize won't be truncated to zero when writing the index, and the entry\nwill appear unchanged even if we later unset the skip_worktree bit.\n\nBut can we have a \"racy clean skip_worktree entry\"? Yes, this seems\npossible e.g. if the following sequence happens fast enough for mtime\nto be the same before and after the update:\n\n echo x >file\n git update-index --refresh --skip-worktree file\n echo y>file\n\nHere is a more complete example which artificially creates a \"racy\nclean skip_worktree entry\", runs `git add --refresh`, and shows that\nthe racy clean entry was not detected:\n\n# Setup\necho sparse >sparse\necho dense >dense\ngit add .\ngit commit -m files\n\n# Emulate a racy clean situation\ntouch -d yesterday date\ntouch -r date sparse\ngit update-index --refresh --skip-worktree sparse\ntouch -r date .git/index\necho xparse >sparse\ntouch -r date sparse\n\n# `git add` will now write a new index without checking if\n# `sparse` is racy clean nor truncating its size\ntouch -r date dense\ngit add --refresh .\n\ngit update-index --no-skip-worktree sparse\ngit status\n\n\nThis situation feels rather uncommon, but perhaps the same could\nhappen with `git sparse-checkout set` instead of `git update-index\n--refresh --skip-worktree`? IDK. This made me think whether\nrefresh_index() should really mark skip_worktree entries with\nCE_UPTODATE, in the first place.\n\nAny thoughts?\n"},{"id":"420692","messageId":"CABPp-BGjuz1ZEriCOhrpakQCQ8AZ12ovir5Vm233nadvywcWcA@mail.gmail.com","threadId":"55302","inReplyTo":"CAHd-oW4kRLjV9Sq3CFt-V1Ot9pYFzJggU1zPp3Hcuw=qWfq7Mg@mail.gmail.com","subject":"Re: [PATCH v3 5/7] refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-03-31T09:14:29Z","receivedAt":"2021-03-31T09:15:34Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Tue, Mar 30, 2021 at 11:51 AM Matheus Tavares Bernardino\n wrote:\n>\n> On Fri, Mar 19, 2021 at 1:05 PM Junio C Hamano wrote:\n> >\n> > Matheus Tavares Bernardino writes:\n> >\n> > >> In other words, the change makes me wonder why we are not adding a\n> > >> flag that says \"do we or do we not want to match paths outside the\n> > >> sparse checkout cone?\", with which the seen[] would automatically\n> > >> record the right thing.\n> > >\n> > > Yeah, makes sense. I didn't want to make the flag skip the sparse\n> > > paths unconditionally (i.e. without matching them) because then we\n> > > would also skip the ce_stage() checkings below and the later\n> > > ce_mark_uptodate(). And I wasn't sure whether this could cause any\n> > > unwanted side effects.\n> > >\n> > > But thinking more carefully about this now, unmerged paths should\n> > > never have the SKIP_WORKTREE bit set anyway, right? What about the\n> > > CE_UPTODATE mark, would it be safe to skip it? I'm not very familiar\n> > > with this code, but I'll try to investigate more later.\n>\n> Sorry I haven't given any update on this yet. From what I could see so\n> far, it seems OK to ignore the skip_worktree entries in\n> refresh_index() when it is called from `git add --refresh`. But\n> because we would no longer mark the skip_worktree entries that match\n> the pathspec with CE_UPTODATE, do_write_index() would start checking\n> if they are racy clean (this is only done when `!ce_uptodate(ce)`),\n> which could add some lstat() overhead.\n>\n> However, this made me think what happens today if we do have a racy\n> clean entry with the skip_worktree bit set... `git add --refresh` will\n> probably update the index without noticing that the entry is racy\n> clean (because it won't check CE_UPTODATE entries, and skip_worktree\n> entries will have this bit set in refresh_index()). Thus the entries'\n> size won't be truncated to zero when writing the index, and the entry\n> will appear unchanged even if we later unset the skip_worktree bit.\n>\n> But can we have a \"racy clean skip_worktree entry\"? Yes, this seems\n> possible e.g. if the following sequence happens fast enough for mtime\n> to be the same before and after the update:\n>\n> echo x >file\n> git update-index --refresh --skip-worktree file\n> echo y>file\n>\n> Here is a more complete example which artificially creates a \"racy\n> clean skip_worktree entry\", runs `git add --refresh`, and shows that\n> the racy clean entry was not detected:\n>\n> # Setup\n> echo sparse >sparse\n> echo dense >dense\n> git add .\n> git commit -m files\n>\n> # Emulate a racy clean situation\n> touch -d yesterday date\n> touch -r date sparse\n> git update-index --refresh --skip-worktree sparse\n> touch -r date .git/index\n> echo xparse >sparse\n> touch -r date sparse\n>\n> # `git add` will now write a new index without checking if\n> # `sparse` is racy clean nor truncating its size\n> touch -r date dense\n> git add --refresh .\n>\n> git update-index --no-skip-worktree sparse\n> git status\n> \n>\n> This situation feels rather uncommon, but perhaps the same could\n> happen with `git sparse-checkout set` instead of `git update-index\n> --refresh --skip-worktree`? IDK. This made me think whether\n> refresh_index() should really mark skip_worktree entries with\n> CE_UPTODATE, in the first place.\n>\n> Any thoughts?\n\nWow, that's a weird one. Nice digging. I don't understand the racily\nclean stuff that well, or the CE_UPTODATE handling...but based on my\nmeager understanding of these things I'd say that marking\nskip_worktree entries with CE_UPTODATE seems wrong and I'd agree with\nyour hunch that we shouldn't be updating it for those files. If an\nentry is skip_worktree, it's assumed to not be present in the working\ntree and that we'll treat it as \"unchanged\". So, when the filehappens\nto be present despite that bit being set, checking the files' stat\ninformation or pretending to have done so so we can claim it is up to\ndate seems wrong to me. In fact, I'd say the mere recognition that\nthe file is present in the working tree despite being SKIP_WORKTREE to\nme implies it's not \"up-to-date\" for at least one definition of that\nterm.\n\nI suspect that if someone flips the skip-worktree bit on and off for a\nfile without removing it from the working tree, not marking\nskip_worktree entries with CE_UPTODATE as you mention above might\nforce us to incur the cost of an extra stat on said file when we run\n\"git status\" later. If so, I think that's a totally reasonable cost,\nso if that was your worry, I'd say that this is shifting the cost\nwhere it belongs.\n\nBut, like I said, I'm not very familiar with the racily clean code or\nCE_UPTODATE, so it's possible I said something above that doesn't even\nmake sense. Does my reasoning help at all?\n"},{"id":"421263","messageId":"cover.1617914011.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1615588108.git.matheus.bernardino@usp.br","subject":"[PATCH v4 0/7] add/rm: honor sparse checkout and warn on sparse paths","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-08T20:41:21Z","receivedAt":"2021-04-08T20:41:38Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Make `rm` honor sparse checkouts, and make both `rm` and `add` warn\nwhen asked to update sparse entries.\n\nThere are two changes since v3:\n\n- `test_i18ncmp` and `test_i18ngrep` were replaced by `test_cmp` and\n `grep`\n\n- The flag added in patch 5 now makes refresh_index() completely ignore\n skip_worktree entries, instead of just suppressing their matches on\n the seen[] array. The previous implementation was not necessarily\n wrong but, as Junio pointed out, it was rather odd to keep matching\n the entries if we no longer want to use the matches.\n\n As \"side effects\", the new version of the flag also makes\n refresh_index() refrain from both:\n\n (1) checking and warning if skip_worktree entries matching the given\n pathspec are unmerged.\n\n (2) marking skip_worktree entries matching the given pathspec with\n CE_UPTODATE.\n\n The change (1) is actually interesting because `git add` doesn't\n update skip_worktree entries, and thus, it doesn't make much sense to\n warn if they are unmerged. Besides, we will already warn if the user\n requests to update such entries, anyway. And finally, unmerged\n entries should not have the skip_worktree bit set in the first place.\n (`git merge` should clean this bit when writing the new index, and\n neither `git sparse-checkout` nor `git update-index` allow to set the\n bit on an unmerged entry.)\n\n Change (2) is perhaps not very beneficial, but it is also not harmful.\n The only practical difference we get by not setting the CE_UPTODATE\n flag in the skip_worktree entries is that, when writing a new index at\n the end of `git add --refresh`, do_write_index() will start checking\n if these entries are racy clean. Note that it already does that for\n all the skip_worktree entries that do not match the user-given\n pathspecs. And, in fact, this behavior distinction based on the\n pathspec only happens with `--refresh`. Plain `git add` and other\n options don't mark any skip_worktree entry with CE_UPTODATE\n (regardless of the pathspecs) and thus, all these entries are checked\n when writing the index. So `git add --refresh` will only do what the\n other options already do. \n\n (Additionally, as I mentioned in [1], there might actually be at least\n one advantage in checking if the skip_worktree entries are racy clean.\n But this is a very specific case, and it's probably a topic for a\n another thread :)\n\n[1]: https://lore.kernel.org/git/CAHd-oW4kRLjV9Sq3CFt-V1Ot9pYFzJggU1zPp3Hcuw=qWfq7Mg@mail.gmail.com/\n\nMatheus Tavares (7):\n add: include magic part of pathspec on --refresh error\n t3705: add tests for `git add` in sparse checkouts\n add: make --chmod and --renormalize honor sparse checkouts\n pathspec: allow to ignore SKIP_WORKTREE entries on index matching\n refresh_index(): add flag to ignore SKIP_WORKTREE entries\n add: warn when asked to update SKIP_WORKTREE entries\n rm: honor sparse checkout patterns\n\n Documentation/config/advice.txt | 4 +\n Documentation/git-rm.txt | 4 +-\n advice.c | 20 ++++\n advice.h | 4 +\n builtin/add.c | 77 ++++++++++++---\n builtin/check-ignore.c | 3 +-\n builtin/rm.c | 35 ++++---\n cache.h | 15 +--\n pathspec.c | 25 ++++-\n pathspec.h | 18 +++-\n read-cache.c | 3 +\n t/t3602-rm-sparse-checkout.sh | 78 ++++++++++++++++\n t/t3700-add.sh | 6 ++\n t/t3705-add-sparse-checkout.sh | 155 +++++++++++++++++++++++++++++++\n t/t7011-skip-worktree-reading.sh | 5 -\n t/t7012-skip-worktree-writing.sh | 19 ----\n 16 files changed, 408 insertions(+), 63 deletions(-)\n create mode 100755 t/t3602-rm-sparse-checkout.sh\n create mode 100755 t/t3705-add-sparse-checkout.sh\n\nRange-diff against v3:\n1: 216e3e1af0 ! 1: 65f241b5c7 add: include magic part of pathspec on --refresh error\n @@ t/t3700-add.sh: test_expect_success 'git add --refresh with pathspec' '\n +test_expect_success 'git add --refresh correctly reports no match error' \"\n +\techo \\\"fatal: pathspec ':(icase)nonexistent' did not match any files\\\" >expect &&\n +\ttest_must_fail git add --refresh ':(icase)nonexistent' 2>actual &&\n -+\ttest_i18ncmp expect actual\n ++\ttest_cmp expect actual\n +\"\n +\n test_expect_success POSIXPERM,SANITY 'git add should fail atomically upon an unreadable file' '\n2: fd250e29b3 = 2: 5d0cdb3718 t3705: add tests for `git add` in sparse checkouts\n3: 3ab45d387a = 3: 565c7d3460 add: make --chmod and --renormalize honor sparse checkouts\n4: df4509268a = 4: a6e98b17eb pathspec: allow to ignore SKIP_WORKTREE entries on index matching\n5: ceb6b508a2 ! 5: 4330f50930 refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag\n @@ Metadata\n Author: Matheus Tavares \n \n ## Commit message ##\n - refresh_index(): add REFRESH_DONT_MARK_SPARSE_MATCHES flag\n + refresh_index(): add flag to ignore SKIP_WORKTREE entries\n \n - refresh_index() optionally takes a seen[] array to mark the pathspec\n - items that had matches in the index. This is used by `git add --refresh`\n - to find out if there was any pathspec without matches, and display an\n - error accordingly.\n -\n - In the following patch, `git add` will also learn to warn about\n - pathspecs that match no eligible path for updating, but do match sparse\n - entries. For that, we will need a seen[] array marked exclusively with\n - matches from dense entries. To avoid having to call ce_path_match()\n - again for these entries after refresh_index() returns, add a flag that\n - implements this restriction inside the function itself.\n -\n - Note that refresh_index() does not update sparse entries, regardless of\n - passing the flag or not. The flag only controls whether matches with\n - these entries should appear in the seen[] array.\n + refresh_index() doesn't update SKIP_WORKTREE entries, but it still\n + matches them against the given pathspecs, marks the matches on the\n + seen[] array, check if unmerged, etc. In the following patch, one caller\n + will need refresh_index() to ignore SKIP_WORKTREE entries entirely, so\n + add a flag that implements this behavior.\n \n While we are here, also realign the REFRESH_* flags and convert the hex\n values to the more natural bit shift format, which makes it easier to\n @@ cache.h: int match_stat_data_racy(const struct index_state *istate,\n +#define REFRESH_IGNORE_SUBMODULES (1 << 4) /* ignore submodules */\n +#define REFRESH_IN_PORCELAIN (1 << 5) /* user friendly output, not \"needs update\" */\n +#define REFRESH_PROGRESS (1 << 6) /* show progress bar if stderr is tty */\n -+#define REFRESH_DONT_MARK_SPARSE_MATCHES (1 << 7) /* don't mark sparse entries' matches on seen[] */\n ++#define REFRESH_IGNORE_SKIP_WORKTREE (1 << 7) /* ignore skip_worktree entries */\n int refresh_index(struct index_state *, unsigned int flags, const struct pathspec *pathspec, char *seen, const char *header_msg);\n /*\n * Refresh the index and write it to disk.\n @@ read-cache.c: int refresh_index(struct index_state *istate, unsigned int flags,\n \tint quiet = (flags & REFRESH_QUIET) != 0;\n \tint not_new = (flags & REFRESH_IGNORE_MISSING) != 0;\n \tint ignore_submodules = (flags & REFRESH_IGNORE_SUBMODULES) != 0;\n -+\tint no_sparse_on_seen = (flags & REFRESH_DONT_MARK_SPARSE_MATCHES) != 0;\n ++\tint ignore_skip_worktree = (flags & REFRESH_IGNORE_SKIP_WORKTREE) != 0;\n \tint first = 1;\n \tint in_porcelain = (flags & REFRESH_IN_PORCELAIN);\n \tunsigned int options = (CE_MATCH_REFRESH |\n @@ read-cache.c: int refresh_index(struct index_state *istate, unsigned int flags,\n - \t\tint filtered = 0;\n - \t\tint t2_did_lstat = 0;\n - \t\tint t2_did_scan = 0;\n -+\t\tchar *cur_seen;\n - \n \t\tce = istate->cache[i];\n \t\tif (ignore_submodules && S_ISGITLINK(ce->ce_mode))\n \t\t\tcontinue;\n ++\t\tif (ignore_skip_worktree && ce_skip_worktree(ce))\n ++\t\t\tcontinue;\n \n --\t\tif (pathspec && !ce_path_match(istate, ce, pathspec, seen))\n -+\t\tcur_seen = no_sparse_on_seen && ce_skip_worktree(ce) ? NULL : seen;\n -+\t\tif (pathspec && !ce_path_match(istate, ce, pathspec, cur_seen))\n + \t\tif (pathspec && !ce_path_match(istate, ce, pathspec, seen))\n \t\t\tfiltered = 1;\n - \n - \t\tif (ce_stage(ce)) {\n6: 048b406fa3 ! 6: da9cd3e9f6 add: warn when asked to update SKIP_WORKTREE entries\n @@ builtin/add.c: static char *prune_directory(struct dir_struct *dir, struct paths\n +\tint i, ret = 0;\n +\tchar *skip_worktree_seen = NULL;\n +\tstruct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;\n -+\tint flags = REFRESH_DONT_MARK_SPARSE_MATCHES |\n ++\tint flags = REFRESH_IGNORE_SKIP_WORKTREE |\n +\t\t (verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET);\n \n \tseen = xcalloc(pathspec->nr, 1);\n @@ t/t3705-add-sparse-checkout.sh: setup_gitignore () {\n \trm sparse_entry &&\n -\tgit add sparse_entry &&\n +\ttest_must_fail git add sparse_entry 2>stderr &&\n -+\ttest_i18ncmp error_and_hint stderr &&\n ++\ttest_cmp error_and_hint stderr &&\n \ttest_sparse_entry_unchanged\n '\n \n @@ t/t3705-add-sparse-checkout.sh: test_expect_success 'git add . does not remove s\n +\techo . >>expect &&\n +\tcat sparse_hint >>expect &&\n +\n -+\ttest_i18ncmp expect stderr &&\n ++\ttest_cmp expect stderr &&\n \ttest_sparse_entry_unchanged\n '\n \n @@ t/t3705-add-sparse-checkout.sh: do\n \t\techo modified >sparse_entry &&\n -\t\tgit add $opt sparse_entry &&\n +\t\ttest_must_fail git add $opt sparse_entry 2>stderr &&\n -+\t\ttest_i18ncmp error_and_hint stderr &&\n ++\t\ttest_cmp error_and_hint stderr &&\n \t\ttest_sparse_entry_unchanged\n \t'\n done\n @@ t/t3705-add-sparse-checkout.sh: test_expect_success 'git add --refresh does not\n \ttest-tool chmtime -60 sparse_entry &&\n -\tgit add --refresh sparse_entry &&\n +\ttest_must_fail git add --refresh sparse_entry 2>stderr &&\n -+\ttest_i18ncmp error_and_hint stderr &&\n ++\ttest_cmp error_and_hint stderr &&\n \tgit ls-files --debug sparse_entry | grep mtime >after &&\n \ttest_cmp before after\n '\n @@ t/t3705-add-sparse-checkout.sh: test_expect_success 'git add --refresh does not\n \tsetup_sparse_entry &&\n -\tgit add --chmod=+x sparse_entry &&\n +\ttest_must_fail git add --chmod=+x sparse_entry 2>stderr &&\n -+\ttest_i18ncmp error_and_hint stderr &&\n ++\ttest_cmp error_and_hint stderr &&\n \ttest_sparse_entry_unchanged &&\n \t! test -x sparse_entry\n '\n @@ t/t3705-add-sparse-checkout.sh: test_expect_success 'git add --renormalize does\n \techo \"sparse_entry text=auto\" >.gitattributes &&\n -\tgit add --renormalize sparse_entry &&\n +\ttest_must_fail git add --renormalize sparse_entry 2>stderr &&\n -+\ttest_i18ncmp error_and_hint stderr &&\n ++\ttest_cmp error_and_hint stderr &&\n +\ttest_sparse_entry_unchanged\n +'\n +\n @@ t/t3705-add-sparse-checkout.sh: test_expect_success 'git add --renormalize does\n +\tsetup_sparse_entry &&\n +\trm sparse_entry &&\n +\ttest_must_fail git add --dry-run --ignore-missing sparse_entry 2>stderr &&\n -+\ttest_i18ncmp error_and_hint stderr &&\n ++\ttest_cmp error_and_hint stderr &&\n \ttest_sparse_entry_unchanged\n '\n \n +test_expect_success 'do not advice about sparse entries when they do not match the pathspec' '\n +\tsetup_sparse_entry &&\n +\ttest_must_fail git add nonexistent 2>stderr &&\n -+\ttest_i18ngrep \"fatal: pathspec .nonexistent. did not match any files\" stderr &&\n -+\ttest_i18ngrep ! -F -f sparse_error_header stderr\n ++\tgrep \"fatal: pathspec .nonexistent. did not match any files\" stderr &&\n ++\t! grep -F -f sparse_error_header stderr\n +'\n +\n +test_expect_success 'do not warn when pathspec matches dense entries' '\n @@ t/t3705-add-sparse-checkout.sh: test_expect_success 'git add --renormalize does\n +test_expect_success 'add obeys advice.updateSparsePath' '\n +\tsetup_sparse_entry &&\n +\ttest_must_fail git -c advice.updateSparsePath=false add sparse_entry 2>stderr &&\n -+\ttest_i18ncmp sparse_entry_error stderr\n ++\ttest_cmp sparse_entry_error stderr\n +\n +'\n +\n7: dd0fe93eb7 ! 7: 4459976ae9 rm: honor sparse checkout patterns\n @@ t/t3602-rm-sparse-checkout.sh (new)\n +\ttest_expect_success \"rm${opt:+ $opt} does not remove sparse entries\" '\n +\t\tgit sparse-checkout set a &&\n +\t\ttest_must_fail git rm $opt b 2>stderr &&\n -+\t\ttest_i18ncmp b_error_and_hint stderr &&\n ++\t\ttest_cmp b_error_and_hint stderr &&\n +\t\tgit ls-files --error-unmatch b\n +\t'\n +done\n @@ t/t3602-rm-sparse-checkout.sh (new)\n +\tgit reset --hard &&\n +\tgit sparse-checkout set a &&\n +\ttest_must_fail git -c advice.updateSparsePath=false rm b 2>stderr &&\n -+\ttest_i18ncmp sparse_entry_b_error stderr\n ++\ttest_cmp sparse_entry_b_error stderr\n +'\n +\n +test_expect_success 'do not advice about sparse entries when they do not match the pathspec' '\n +\tgit reset --hard &&\n +\tgit sparse-checkout set a &&\n +\ttest_must_fail git rm nonexistent 2>stderr &&\n -+\ttest_i18ngrep \"fatal: pathspec .nonexistent. did not match any files\" stderr &&\n -+\ttest_i18ngrep ! -F -f sparse_error_header stderr\n ++\tgrep \"fatal: pathspec .nonexistent. did not match any files\" stderr &&\n ++\t! grep -F -f sparse_error_header stderr\n +'\n +\n +test_expect_success 'do not warn about sparse entries when pathspec matches dense entries' '\n-- \n2.30.1\n\n"},{"id":"421264","messageId":"65f241b5c71a000a515b39c2159becb970d2cfdb.1617914011.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1617914011.git.matheus.bernardino@usp.br","subject":"[PATCH v4 1/7] add: include magic part of pathspec on --refresh error","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-08T20:41:22Z","receivedAt":"2021-04-08T20:41:40Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"When `git add --refresh ` doesn't find any matches for the\ngiven pathspec, it prints an error message using the `match` field of\nthe `struct pathspec_item`. However, this field doesn't contain the\nmagic part of the pathspec. Instead, let's use the `original` field.\n\nSigned-off-by: Matheus Tavares \n---\n builtin/add.c | 2 +-\n t/t3700-add.sh | 6 ++++++\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ea762a41e3..24ed7e25f3 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -187,7 +187,7 @@ static void refresh(int verbose, const struct pathspec *pathspec)\n \tfor (i = 0; i < pathspec->nr; i++) {\n \t\tif (!seen[i])\n \t\t\tdie(_(\"pathspec '%s' did not match any files\"),\n-\t\t\t pathspec->items[i].match);\n+\t\t\t pathspec->items[i].original);\n \t}\n \tfree(seen);\n }\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex b3b122ff97..dd3011430d 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -196,6 +196,12 @@ test_expect_success 'git add --refresh with pathspec' '\n \tgrep baz actual\n '\n \n+test_expect_success 'git add --refresh correctly reports no match error' \"\n+\techo \\\"fatal: pathspec ':(icase)nonexistent' did not match any files\\\" >expect &&\n+\ttest_must_fail git add --refresh ':(icase)nonexistent' 2>actual &&\n+\ttest_cmp expect actual\n+\"\n+\n test_expect_success POSIXPERM,SANITY 'git add should fail atomically upon an unreadable file' '\n \tgit reset --hard &&\n \tdate >foo1 &&\n-- \n2.30.1\n\n"},{"id":"421265","messageId":"5d0cdb3718ec0230fe199441b6a4089ce828a3e9.1617914011.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1617914011.git.matheus.bernardino@usp.br","subject":"[PATCH v4 2/7] t3705: add tests for `git add` in sparse checkouts","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-08T20:41:23Z","receivedAt":"2021-04-08T20:41:42Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"We already have a couple tests for `add` with SKIP_WORKTREE entries in\nt7012, but these only cover the most basic scenarios. As we will be\nchanging how `add` deals with sparse paths in the subsequent commits,\nlet's move these two tests to their own file and add more test cases\nfor different `add` options and situations. This also demonstrates two\noptions that don't currently respect SKIP_WORKTREE entries: `--chmod`\nand `--renormalize`.\n\nSigned-off-by: Matheus Tavares \n---\n t/t3705-add-sparse-checkout.sh | 96 ++++++++++++++++++++++++++++++++\n t/t7012-skip-worktree-writing.sh | 19 -------\n 2 files changed, 96 insertions(+), 19 deletions(-)\n create mode 100755 t/t3705-add-sparse-checkout.sh\n\ndiff --git a/t/t3705-add-sparse-checkout.sh b/t/t3705-add-sparse-checkout.sh\nnew file mode 100755\nindex 0000000000..6c5b8be863\n--- /dev/null\n+++ b/t/t3705-add-sparse-checkout.sh\n@@ -0,0 +1,96 @@\n+#!/bin/sh\n+\n+test_description='git add in sparse checked out working trees'\n+\n+. ./test-lib.sh\n+\n+SPARSE_ENTRY_BLOB=\"\"\n+\n+# Optionally take a printf format string to write to the sparse_entry file\n+setup_sparse_entry () {\n+\t# 'sparse_entry' might already be in the index with the skip-worktree\n+\t# bit set. Remove it so that the subsequent git add can update it.\n+\tgit update-index --force-remove sparse_entry &&\n+\tif test $# -eq 1\n+\tthen\n+\t\tprintf \"$1\" >sparse_entry\n+\telse\n+\t\t>sparse_entry\n+\tfi &&\n+\tgit add sparse_entry &&\n+\tgit update-index --skip-worktree sparse_entry &&\n+\tSPARSE_ENTRY_BLOB=$(git rev-parse :sparse_entry)\n+}\n+\n+test_sparse_entry_unchanged () {\n+\techo \"100644 $SPARSE_ENTRY_BLOB 0\tsparse_entry\" >expected &&\n+\tgit ls-files --stage sparse_entry >actual &&\n+\ttest_cmp expected actual\n+}\n+\n+setup_gitignore () {\n+\ttest_when_finished rm -f .gitignore &&\n+\tcat >.gitignore <<-EOF\n+\t*\n+\t!/sparse_entry\n+\tEOF\n+}\n+\n+test_expect_success 'git add does not remove sparse entries' '\n+\tsetup_sparse_entry &&\n+\trm sparse_entry &&\n+\tgit add sparse_entry &&\n+\ttest_sparse_entry_unchanged\n+'\n+\n+test_expect_success 'git add -A does not remove sparse entries' '\n+\tsetup_sparse_entry &&\n+\trm sparse_entry &&\n+\tsetup_gitignore &&\n+\tgit add -A &&\n+\ttest_sparse_entry_unchanged\n+'\n+\n+test_expect_success 'git add . does not remove sparse entries' '\n+\tsetup_sparse_entry &&\n+\trm sparse_entry &&\n+\tsetup_gitignore &&\n+\tgit add . &&\n+\ttest_sparse_entry_unchanged\n+'\n+\n+for opt in \"\" -f -u --ignore-removal --dry-run\n+do\n+\ttest_expect_success \"git add${opt:+ $opt} does not update sparse entries\" '\n+\t\tsetup_sparse_entry &&\n+\t\techo modified >sparse_entry &&\n+\t\tgit add $opt sparse_entry &&\n+\t\ttest_sparse_entry_unchanged\n+\t'\n+done\n+\n+test_expect_success 'git add --refresh does not update sparse entries' '\n+\tsetup_sparse_entry &&\n+\tgit ls-files --debug sparse_entry | grep mtime >before &&\n+\ttest-tool chmtime -60 sparse_entry &&\n+\tgit add --refresh sparse_entry &&\n+\tgit ls-files --debug sparse_entry | grep mtime >after &&\n+\ttest_cmp before after\n+'\n+\n+test_expect_failure 'git add --chmod does not update sparse entries' '\n+\tsetup_sparse_entry &&\n+\tgit add --chmod=+x sparse_entry &&\n+\ttest_sparse_entry_unchanged &&\n+\t! test -x sparse_entry\n+'\n+\n+test_expect_failure 'git add --renormalize does not update sparse entries' '\n+\ttest_config core.autocrlf false &&\n+\tsetup_sparse_entry \"LINEONE\\r\\nLINETWO\\r\\n\" &&\n+\techo \"sparse_entry text=auto\" >.gitattributes &&\n+\tgit add --renormalize sparse_entry &&\n+\ttest_sparse_entry_unchanged\n+'\n+\n+test_done\ndiff --git a/t/t7012-skip-worktree-writing.sh b/t/t7012-skip-worktree-writing.sh\nindex f2a8e76511..a1080b94e3 100755\n--- a/t/t7012-skip-worktree-writing.sh\n+++ b/t/t7012-skip-worktree-writing.sh\n@@ -60,13 +60,6 @@ setup_absent() {\n \tgit update-index --skip-worktree 1\n }\n \n-test_absent() {\n-\techo \"100644 $EMPTY_BLOB 0\t1\" > expected &&\n-\tgit ls-files --stage 1 > result &&\n-\ttest_cmp expected result &&\n-\ttest ! -f 1\n-}\n-\n setup_dirty() {\n \tgit update-index --force-remove 1 &&\n \techo dirty > 1 &&\n@@ -100,18 +93,6 @@ test_expect_success 'index setup' '\n \ttest_cmp expected result\n '\n \n-test_expect_success 'git-add ignores worktree content' '\n-\tsetup_absent &&\n-\tgit add 1 &&\n-\ttest_absent\n-'\n-\n-test_expect_success 'git-add ignores worktree content' '\n-\tsetup_dirty &&\n-\tgit add 1 &&\n-\ttest_dirty\n-'\n-\n test_expect_success 'git-rm fails if worktree is dirty' '\n \tsetup_dirty &&\n \ttest_must_fail git rm 1 &&\n-- \n2.30.1\n\n"},{"id":"421266","messageId":"565c7d34606da681fbf6cf4e292efe7134616c79.1617914011.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1617914011.git.matheus.bernardino@usp.br","subject":"[PATCH v4 3/7] add: make --chmod and --renormalize honor sparse checkouts","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-08T20:41:24Z","receivedAt":"2021-04-08T20:41:45Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Signed-off-by: Matheus Tavares \n---\n builtin/add.c | 5 +++++\n t/t3705-add-sparse-checkout.sh | 4 ++--\n 2 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 24ed7e25f3..5fec21a792 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -46,6 +46,9 @@ static int chmod_pathspec(struct pathspec *pathspec, char flip, int show_only)\n \t\tstruct cache_entry *ce = active_cache[i];\n \t\tint err;\n \n+\t\tif (ce_skip_worktree(ce))\n+\t\t\tcontinue;\n+\n \t\tif (pathspec && !ce_path_match(&the_index, ce, pathspec, NULL))\n \t\t\tcontinue;\n \n@@ -144,6 +147,8 @@ static int renormalize_tracked_files(const struct pathspec *pathspec, int flags)\n \tfor (i = 0; i < active_nr; i++) {\n \t\tstruct cache_entry *ce = active_cache[i];\n \n+\t\tif (ce_skip_worktree(ce))\n+\t\t\tcontinue;\n \t\tif (ce_stage(ce))\n \t\t\tcontinue; /* do not touch unmerged paths */\n \t\tif (!S_ISREG(ce->ce_mode) && !S_ISLNK(ce->ce_mode))\ndiff --git a/t/t3705-add-sparse-checkout.sh b/t/t3705-add-sparse-checkout.sh\nindex 6c5b8be863..00b10ac877 100755\n--- a/t/t3705-add-sparse-checkout.sh\n+++ b/t/t3705-add-sparse-checkout.sh\n@@ -78,14 +78,14 @@ test_expect_success 'git add --refresh does not update sparse entries' '\n \ttest_cmp before after\n '\n \n-test_expect_failure 'git add --chmod does not update sparse entries' '\n+test_expect_success 'git add --chmod does not update sparse entries' '\n \tsetup_sparse_entry &&\n \tgit add --chmod=+x sparse_entry &&\n \ttest_sparse_entry_unchanged &&\n \t! test -x sparse_entry\n '\n \n-test_expect_failure 'git add --renormalize does not update sparse entries' '\n+test_expect_success 'git add --renormalize does not update sparse entries' '\n \ttest_config core.autocrlf false &&\n \tsetup_sparse_entry \"LINEONE\\r\\nLINETWO\\r\\n\" &&\n \techo \"sparse_entry text=auto\" >.gitattributes &&\n-- \n2.30.1\n\n"},{"id":"421267","messageId":"a6e98b17eb5f4770367cf336832d92c5017e0af7.1617914011.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1617914011.git.matheus.bernardino@usp.br","subject":"[PATCH v4 4/7] pathspec: allow to ignore SKIP_WORKTREE entries on index matching","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-08T20:41:25Z","receivedAt":"2021-04-08T20:41:49Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Add a new enum parameter to `add_pathspec_matches_against_index()` and\n`find_pathspecs_matching_against_index()`, allowing callers to specify\nwhether these function should attempt to match SKIP_WORKTREE entries or\nnot. This will be used in a future patch to make `git add` display a\nwarning when it is asked to update SKIP_WORKTREE entries.\n\nSigned-off-by: Matheus Tavares \n---\n builtin/add.c | 6 ++++--\n builtin/check-ignore.c | 3 ++-\n pathspec.c | 10 +++++++---\n pathspec.h | 10 ++++++++--\n 4 files changed, 21 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 5fec21a792..050cb8af30 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -177,7 +177,8 @@ static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec,\n \t\t\t*dst++ = entry;\n \t}\n \tdir->nr = dst - dir->entries;\n-\tadd_pathspec_matches_against_index(pathspec, &the_index, seen);\n+\tadd_pathspec_matches_against_index(pathspec, &the_index, seen,\n+\t\t\t\t\t PS_HEED_SKIP_WORKTREE);\n \treturn seen;\n }\n \n@@ -578,7 +579,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tint i;\n \n \t\tif (!seen)\n-\t\t\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index);\n+\t\t\tseen = find_pathspecs_matching_against_index(&pathspec,\n+\t\t\t\t\t&the_index, PS_HEED_SKIP_WORKTREE);\n \n \t\t/*\n \t\t * file_exists() assumes exact match\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex 3c652748d5..0f4480a11b 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -100,7 +100,8 @@ static int check_ignore(struct dir_struct *dir,\n \t * should not be ignored, in order to be consistent with\n \t * 'git status', 'git add' etc.\n \t */\n-\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index);\n+\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index,\n+\t\t\t\t\t\t PS_HEED_SKIP_WORKTREE);\n \tfor (i = 0; i < pathspec.nr; i++) {\n \t\tfull_path = pathspec.items[i].match;\n \t\tpattern = NULL;\ndiff --git a/pathspec.c b/pathspec.c\nindex 18b3be362a..8b05575110 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -21,7 +21,8 @@\n */\n void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n \t\t\t\t\tconst struct index_state *istate,\n-\t\t\t\t\tchar *seen)\n+\t\t\t\t\tchar *seen,\n+\t\t\t\t\tenum ps_skip_worktree_action sw_action)\n {\n \tint num_unmatched = 0, i;\n \n@@ -38,6 +39,8 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n \t\treturn;\n \tfor (i = 0; i < istate->cache_nr; i++) {\n \t\tconst struct cache_entry *ce = istate->cache[i];\n+\t\tif (sw_action == PS_IGNORE_SKIP_WORKTREE && ce_skip_worktree(ce))\n+\t\t\tcontinue;\n \t\tce_path_match(istate, ce, pathspec, seen);\n \t}\n }\n@@ -51,10 +54,11 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n * given pathspecs achieves against all items in the index.\n */\n char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,\n-\t\t\t\t\t const struct index_state *istate)\n+\t\t\t\t\t const struct index_state *istate,\n+\t\t\t\t\t enum ps_skip_worktree_action sw_action)\n {\n \tchar *seen = xcalloc(pathspec->nr, 1);\n-\tadd_pathspec_matches_against_index(pathspec, istate, seen);\n+\tadd_pathspec_matches_against_index(pathspec, istate, seen, sw_action);\n \treturn seen;\n }\n \ndiff --git a/pathspec.h b/pathspec.h\nindex 454ce364fa..0feb8e9f67 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -149,11 +149,17 @@ static inline int ps_strcmp(const struct pathspec_item *item,\n \t\treturn strcmp(s1, s2);\n }\n \n+enum ps_skip_worktree_action {\n+ PS_HEED_SKIP_WORKTREE = 0,\n+ PS_IGNORE_SKIP_WORKTREE = 1\n+};\n void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n \t\t\t\t\tconst struct index_state *istate,\n-\t\t\t\t\tchar *seen);\n+\t\t\t\t\tchar *seen,\n+\t\t\t\t\tenum ps_skip_worktree_action sw_action);\n char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,\n-\t\t\t\t\t const struct index_state *istate);\n+\t\t\t\t\t const struct index_state *istate,\n+\t\t\t\t\t enum ps_skip_worktree_action sw_action);\n int match_pathspec_attrs(const struct index_state *istate,\n \t\t\t const char *name, int namelen,\n \t\t\t const struct pathspec_item *item);\n-- \n2.30.1\n\n"},{"id":"421268","messageId":"4330f509303c76521ab3e4d342065ce4025ec6f1.1617914011.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1617914011.git.matheus.bernardino@usp.br","subject":"[PATCH v4 5/7] refresh_index(): add flag to ignore SKIP_WORKTREE entries","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-08T20:41:26Z","receivedAt":"2021-04-08T20:41:51Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"refresh_index() doesn't update SKIP_WORKTREE entries, but it still\nmatches them against the given pathspecs, marks the matches on the\nseen[] array, check if unmerged, etc. In the following patch, one caller\nwill need refresh_index() to ignore SKIP_WORKTREE entries entirely, so\nadd a flag that implements this behavior.\n\nWhile we are here, also realign the REFRESH_* flags and convert the hex\nvalues to the more natural bit shift format, which makes it easier to\nspot holes.\n\nSigned-off-by: Matheus Tavares \n---\n cache.h | 15 ++++++++-------\n read-cache.c | 3 +++\n 2 files changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 148d9ab5f1..19c03fb6e7 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -879,13 +879,14 @@ int match_stat_data_racy(const struct index_state *istate,\n \n void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, struct stat *st);\n \n-#define REFRESH_REALLY\t\t0x0001\t/* ignore_valid */\n-#define REFRESH_UNMERGED\t0x0002\t/* allow unmerged */\n-#define REFRESH_QUIET\t\t0x0004\t/* be quiet about it */\n-#define REFRESH_IGNORE_MISSING\t0x0008\t/* ignore non-existent */\n-#define REFRESH_IGNORE_SUBMODULES\t0x0010\t/* ignore submodules */\n-#define REFRESH_IN_PORCELAIN\t0x0020\t/* user friendly output, not \"needs update\" */\n-#define REFRESH_PROGRESS\t0x0040 /* show progress bar if stderr is tty */\n+#define REFRESH_REALLY (1 << 0) /* ignore_valid */\n+#define REFRESH_UNMERGED (1 << 1) /* allow unmerged */\n+#define REFRESH_QUIET (1 << 2) /* be quiet about it */\n+#define REFRESH_IGNORE_MISSING (1 << 3) /* ignore non-existent */\n+#define REFRESH_IGNORE_SUBMODULES (1 << 4) /* ignore submodules */\n+#define REFRESH_IN_PORCELAIN (1 << 5) /* user friendly output, not \"needs update\" */\n+#define REFRESH_PROGRESS (1 << 6) /* show progress bar if stderr is tty */\n+#define REFRESH_IGNORE_SKIP_WORKTREE (1 << 7) /* ignore skip_worktree entries */\n int refresh_index(struct index_state *, unsigned int flags, const struct pathspec *pathspec, char *seen, const char *header_msg);\n /*\n * Refresh the index and write it to disk.\ndiff --git a/read-cache.c b/read-cache.c\nindex 5a907af2fb..8381935b84 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1514,6 +1514,7 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \tint quiet = (flags & REFRESH_QUIET) != 0;\n \tint not_new = (flags & REFRESH_IGNORE_MISSING) != 0;\n \tint ignore_submodules = (flags & REFRESH_IGNORE_SUBMODULES) != 0;\n+\tint ignore_skip_worktree = (flags & REFRESH_IGNORE_SKIP_WORKTREE) != 0;\n \tint first = 1;\n \tint in_porcelain = (flags & REFRESH_IN_PORCELAIN);\n \tunsigned int options = (CE_MATCH_REFRESH |\n@@ -1556,6 +1557,8 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \t\tce = istate->cache[i];\n \t\tif (ignore_submodules && S_ISGITLINK(ce->ce_mode))\n \t\t\tcontinue;\n+\t\tif (ignore_skip_worktree && ce_skip_worktree(ce))\n+\t\t\tcontinue;\n \n \t\tif (pathspec && !ce_path_match(istate, ce, pathspec, seen))\n \t\t\tfiltered = 1;\n-- \n2.30.1\n\n"},{"id":"421269","messageId":"da9cd3e9f6c10e624e46774e8b19f3caaf1ef60f.1617914011.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1617914011.git.matheus.bernardino@usp.br","subject":"[PATCH v4 6/7] add: warn when asked to update SKIP_WORKTREE entries","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-08T20:41:27Z","receivedAt":"2021-04-08T20:41:54Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"`git add` already refrains from updating SKIP_WORKTREE entries, but it\nsilently exits with zero code when it is asked to do so. Instead, let's\nwarn the user and display a hint on how to update these entries.\n\nNote that we only warn the user whey they give a pathspec item that\nmatches no eligible path for updating, but it does match one or more\nSKIP_WORKTREE entries. A warning was chosen over erroring out right away\nto reproduce the same behavior `add` already exhibits with ignored\nfiles. This also allow users to continue their workflow without having\nto invoke `add` again with only the eligible paths (as those will have\nalready been added).\n\nSigned-off-by: Matheus Tavares \n---\n Documentation/config/advice.txt | 3 ++\n advice.c | 20 +++++++++\n advice.h | 4 ++\n builtin/add.c | 70 ++++++++++++++++++++++++-------\n pathspec.c | 15 +++++++\n pathspec.h | 8 ++++\n t/t3705-add-sparse-checkout.sh | 73 +++++++++++++++++++++++++++++----\n 7 files changed, 172 insertions(+), 21 deletions(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex acbd0c09aa..851b83cf30 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -119,4 +119,7 @@ advice.*::\n \taddEmptyPathspec::\n \t\tAdvice shown if a user runs the add command without providing\n \t\tthe pathspec parameter.\n+\tupdateSparsePath::\n+\t\tAdvice shown when linkgit:git-add[1] is asked to update index\n+\t\tentries outside the current sparse checkout.\n --\ndiff --git a/advice.c b/advice.c\nindex 164742305f..0b9c89c48a 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -2,6 +2,7 @@\n #include \"config.h\"\n #include \"color.h\"\n #include \"help.h\"\n+#include \"string-list.h\"\n \n int advice_fetch_show_forced_updates = 1;\n int advice_push_update_rejected = 1;\n@@ -136,6 +137,7 @@ static struct {\n \t[ADVICE_STATUS_HINTS]\t\t\t\t= { \"statusHints\", 1 },\n \t[ADVICE_STATUS_U_OPTION]\t\t\t= { \"statusUoption\", 1 },\n \t[ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE] = { \"submoduleAlternateErrorStrategyDie\", 1 },\n+\t[ADVICE_UPDATE_SPARSE_PATH]\t\t\t= { \"updateSparsePath\", 1 },\n \t[ADVICE_WAITING_FOR_EDITOR]\t\t\t= { \"waitingForEditor\", 1 },\n };\n \n@@ -284,6 +286,24 @@ void NORETURN die_conclude_merge(void)\n \tdie(_(\"Exiting because of unfinished merge.\"));\n }\n \n+void advise_on_updating_sparse_paths(struct string_list *pathspec_list)\n+{\n+\tstruct string_list_item *item;\n+\n+\tif (!pathspec_list->nr)\n+\t\treturn;\n+\n+\tfprintf(stderr, _(\"The following pathspecs didn't match any\"\n+\t\t\t \" eligible path, but they do match index\\n\"\n+\t\t\t \"entries outside the current sparse checkout:\\n\"));\n+\tfor_each_string_list_item(item, pathspec_list)\n+\t\tfprintf(stderr, \"%s\\n\", item->string);\n+\n+\tadvise_if_enabled(ADVICE_UPDATE_SPARSE_PATH,\n+\t\t\t _(\"Disable or modify the sparsity rules if you intend\"\n+\t\t\t \" to update such entries.\"));\n+}\n+\n void detach_advice(const char *new_name)\n {\n \tconst char *fmt =\ndiff --git a/advice.h b/advice.h\nindex bc2432980a..bd26c385d0 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -3,6 +3,8 @@\n \n #include \"git-compat-util.h\"\n \n+struct string_list;\n+\n extern int advice_fetch_show_forced_updates;\n extern int advice_push_update_rejected;\n extern int advice_push_non_ff_current;\n@@ -71,6 +73,7 @@ extern int advice_add_empty_pathspec;\n \tADVICE_STATUS_HINTS,\n \tADVICE_STATUS_U_OPTION,\n \tADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,\n+\tADVICE_UPDATE_SPARSE_PATH,\n \tADVICE_WAITING_FOR_EDITOR,\n };\n \n@@ -92,6 +95,7 @@ void advise_if_enabled(enum advice_type type, const char *advice, ...);\n int error_resolve_conflict(const char *me);\n void NORETURN die_resolve_conflict(const char *me);\n void NORETURN die_conclude_merge(void);\n+void advise_on_updating_sparse_paths(struct string_list *pathspec_list);\n void detach_advice(const char *new_name);\n \n #endif /* ADVICE_H */\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 050cb8af30..6980311aaa 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -178,24 +178,43 @@ static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec,\n \t}\n \tdir->nr = dst - dir->entries;\n \tadd_pathspec_matches_against_index(pathspec, &the_index, seen,\n-\t\t\t\t\t PS_HEED_SKIP_WORKTREE);\n+\t\t\t\t\t PS_IGNORE_SKIP_WORKTREE);\n \treturn seen;\n }\n \n-static void refresh(int verbose, const struct pathspec *pathspec)\n+static int refresh(int verbose, const struct pathspec *pathspec)\n {\n \tchar *seen;\n-\tint i;\n+\tint i, ret = 0;\n+\tchar *skip_worktree_seen = NULL;\n+\tstruct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;\n+\tint flags = REFRESH_IGNORE_SKIP_WORKTREE |\n+\t\t (verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET);\n \n \tseen = xcalloc(pathspec->nr, 1);\n-\trefresh_index(&the_index, verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET,\n-\t\t pathspec, seen, _(\"Unstaged changes after refreshing the index:\"));\n+\trefresh_index(&the_index, flags, pathspec, seen,\n+\t\t _(\"Unstaged changes after refreshing the index:\"));\n \tfor (i = 0; i < pathspec->nr; i++) {\n-\t\tif (!seen[i])\n-\t\t\tdie(_(\"pathspec '%s' did not match any files\"),\n-\t\t\t pathspec->items[i].original);\n+\t\tif (!seen[i]) {\n+\t\t\tif (matches_skip_worktree(pathspec, i, &skip_worktree_seen)) {\n+\t\t\t\tstring_list_append(&only_match_skip_worktree,\n+\t\t\t\t\t\t pathspec->items[i].original);\n+\t\t\t} else {\n+\t\t\t\tdie(_(\"pathspec '%s' did not match any files\"),\n+\t\t\t\t pathspec->items[i].original);\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif (only_match_skip_worktree.nr) {\n+\t\tadvise_on_updating_sparse_paths(&only_match_skip_worktree);\n+\t\tret = 1;\n \t}\n+\n \tfree(seen);\n+\tfree(skip_worktree_seen);\n+\tstring_list_clear(&only_match_skip_worktree, 0);\n+\treturn ret;\n }\n \n int run_add_interactive(const char *revision, const char *patch_mode,\n@@ -571,16 +590,18 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (refresh_only) {\n-\t\trefresh(verbose, &pathspec);\n+\t\texit_status |= refresh(verbose, &pathspec);\n \t\tgoto finish;\n \t}\n \n \tif (pathspec.nr) {\n \t\tint i;\n+\t\tchar *skip_worktree_seen = NULL;\n+\t\tstruct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;\n \n \t\tif (!seen)\n \t\t\tseen = find_pathspecs_matching_against_index(&pathspec,\n-\t\t\t\t\t&the_index, PS_HEED_SKIP_WORKTREE);\n+\t\t\t\t\t&the_index, PS_IGNORE_SKIP_WORKTREE);\n \n \t\t/*\n \t\t * file_exists() assumes exact match\n@@ -594,12 +615,24 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \t\tfor (i = 0; i < pathspec.nr; i++) {\n \t\t\tconst char *path = pathspec.items[i].match;\n+\n \t\t\tif (pathspec.items[i].magic & PATHSPEC_EXCLUDE)\n \t\t\t\tcontinue;\n-\t\t\tif (!seen[i] && path[0] &&\n-\t\t\t ((pathspec.items[i].magic &\n-\t\t\t (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||\n-\t\t\t !file_exists(path))) {\n+\t\t\tif (seen[i])\n+\t\t\t\tcontinue;\n+\n+\t\t\tif (matches_skip_worktree(&pathspec, i, &skip_worktree_seen)) {\n+\t\t\t\tstring_list_append(&only_match_skip_worktree,\n+\t\t\t\t\t\t pathspec.items[i].original);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\t/* Don't complain at 'git add .' on empty repo */\n+\t\t\tif (!path[0])\n+\t\t\t\tcontinue;\n+\n+\t\t\tif ((pathspec.items[i].magic & (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||\n+\t\t\t !file_exists(path)) {\n \t\t\t\tif (ignore_missing) {\n \t\t\t\t\tint dtype = DT_UNKNOWN;\n \t\t\t\t\tif (is_excluded(&dir, &the_index, path, &dtype))\n@@ -610,7 +643,16 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t pathspec.items[i].original);\n \t\t\t}\n \t\t}\n+\n+\n+\t\tif (only_match_skip_worktree.nr) {\n+\t\t\tadvise_on_updating_sparse_paths(&only_match_skip_worktree);\n+\t\t\texit_status = 1;\n+\t\t}\n+\n \t\tfree(seen);\n+\t\tfree(skip_worktree_seen);\n+\t\tstring_list_clear(&only_match_skip_worktree, 0);\n \t}\n \n \tplug_bulk_checkin();\ndiff --git a/pathspec.c b/pathspec.c\nindex 8b05575110..ab67ff7c5c 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -62,6 +62,21 @@ char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,\n \treturn seen;\n }\n \n+char *find_pathspecs_matching_skip_worktree(const struct pathspec *pathspec)\n+{\n+\tstruct index_state *istate = the_repository->index;\n+\tchar *seen = xcalloc(pathspec->nr, 1);\n+\tint i;\n+\n+\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = istate->cache[i];\n+\t\tif (ce_skip_worktree(ce))\n+\t\t ce_path_match(istate, ce, pathspec, seen);\n+\t}\n+\n+\treturn seen;\n+}\n+\n /*\n * Magic pathspec\n *\ndiff --git a/pathspec.h b/pathspec.h\nindex 0feb8e9f67..5b4c6614bf 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -160,6 +160,14 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,\n \t\t\t\t\t const struct index_state *istate,\n \t\t\t\t\t enum ps_skip_worktree_action sw_action);\n+char *find_pathspecs_matching_skip_worktree(const struct pathspec *pathspec);\n+static inline int matches_skip_worktree(const struct pathspec *pathspec,\n+\t\t\t\t\tint item, char **seen_ptr)\n+{\n+\tif (!*seen_ptr)\n+\t\t*seen_ptr = find_pathspecs_matching_skip_worktree(pathspec);\n+\treturn (*seen_ptr)[item];\n+}\n int match_pathspec_attrs(const struct index_state *istate,\n \t\t\t const char *name, int namelen,\n \t\t\t const struct pathspec_item *item);\ndiff --git a/t/t3705-add-sparse-checkout.sh b/t/t3705-add-sparse-checkout.sh\nindex 00b10ac877..2b1fd0d0ee 100755\n--- a/t/t3705-add-sparse-checkout.sh\n+++ b/t/t3705-add-sparse-checkout.sh\n@@ -36,10 +36,26 @@ setup_gitignore () {\n \tEOF\n }\n \n+test_expect_success 'setup' \"\n+\tcat >sparse_error_header <<-EOF &&\n+\tThe following pathspecs didn't match any eligible path, but they do match index\n+\tentries outside the current sparse checkout:\n+\tEOF\n+\n+\tcat >sparse_hint <<-EOF &&\n+\thint: Disable or modify the sparsity rules if you intend to update such entries.\n+\thint: Disable this message with \\\"git config advice.updateSparsePath false\\\"\n+\tEOF\n+\n+\techo sparse_entry | cat sparse_error_header - >sparse_entry_error &&\n+\tcat sparse_entry_error sparse_hint >error_and_hint\n+\"\n+\n test_expect_success 'git add does not remove sparse entries' '\n \tsetup_sparse_entry &&\n \trm sparse_entry &&\n-\tgit add sparse_entry &&\n+\ttest_must_fail git add sparse_entry 2>stderr &&\n+\ttest_cmp error_and_hint stderr &&\n \ttest_sparse_entry_unchanged\n '\n \n@@ -47,7 +63,8 @@ test_expect_success 'git add -A does not remove sparse entries' '\n \tsetup_sparse_entry &&\n \trm sparse_entry &&\n \tsetup_gitignore &&\n-\tgit add -A &&\n+\tgit add -A 2>stderr &&\n+\ttest_must_be_empty stderr &&\n \ttest_sparse_entry_unchanged\n '\n \n@@ -55,7 +72,13 @@ test_expect_success 'git add . does not remove sparse entries' '\n \tsetup_sparse_entry &&\n \trm sparse_entry &&\n \tsetup_gitignore &&\n-\tgit add . &&\n+\ttest_must_fail git add . 2>stderr &&\n+\n+\tcat sparse_error_header >expect &&\n+\techo . >>expect &&\n+\tcat sparse_hint >>expect &&\n+\n+\ttest_cmp expect stderr &&\n \ttest_sparse_entry_unchanged\n '\n \n@@ -64,7 +87,8 @@ do\n \ttest_expect_success \"git add${opt:+ $opt} does not update sparse entries\" '\n \t\tsetup_sparse_entry &&\n \t\techo modified >sparse_entry &&\n-\t\tgit add $opt sparse_entry &&\n+\t\ttest_must_fail git add $opt sparse_entry 2>stderr &&\n+\t\ttest_cmp error_and_hint stderr &&\n \t\ttest_sparse_entry_unchanged\n \t'\n done\n@@ -73,14 +97,16 @@ test_expect_success 'git add --refresh does not update sparse entries' '\n \tsetup_sparse_entry &&\n \tgit ls-files --debug sparse_entry | grep mtime >before &&\n \ttest-tool chmtime -60 sparse_entry &&\n-\tgit add --refresh sparse_entry &&\n+\ttest_must_fail git add --refresh sparse_entry 2>stderr &&\n+\ttest_cmp error_and_hint stderr &&\n \tgit ls-files --debug sparse_entry | grep mtime >after &&\n \ttest_cmp before after\n '\n \n test_expect_success 'git add --chmod does not update sparse entries' '\n \tsetup_sparse_entry &&\n-\tgit add --chmod=+x sparse_entry &&\n+\ttest_must_fail git add --chmod=+x sparse_entry 2>stderr &&\n+\ttest_cmp error_and_hint stderr &&\n \ttest_sparse_entry_unchanged &&\n \t! test -x sparse_entry\n '\n@@ -89,8 +115,41 @@ test_expect_success 'git add --renormalize does not update sparse entries' '\n \ttest_config core.autocrlf false &&\n \tsetup_sparse_entry \"LINEONE\\r\\nLINETWO\\r\\n\" &&\n \techo \"sparse_entry text=auto\" >.gitattributes &&\n-\tgit add --renormalize sparse_entry &&\n+\ttest_must_fail git add --renormalize sparse_entry 2>stderr &&\n+\ttest_cmp error_and_hint stderr &&\n+\ttest_sparse_entry_unchanged\n+'\n+\n+test_expect_success 'git add --dry-run --ignore-missing warn on sparse path' '\n+\tsetup_sparse_entry &&\n+\trm sparse_entry &&\n+\ttest_must_fail git add --dry-run --ignore-missing sparse_entry 2>stderr &&\n+\ttest_cmp error_and_hint stderr &&\n \ttest_sparse_entry_unchanged\n '\n \n+test_expect_success 'do not advice about sparse entries when they do not match the pathspec' '\n+\tsetup_sparse_entry &&\n+\ttest_must_fail git add nonexistent 2>stderr &&\n+\tgrep \"fatal: pathspec .nonexistent. did not match any files\" stderr &&\n+\t! grep -F -f sparse_error_header stderr\n+'\n+\n+test_expect_success 'do not warn when pathspec matches dense entries' '\n+\tsetup_sparse_entry &&\n+\techo modified >sparse_entry &&\n+\t>dense_entry &&\n+\tgit add \"*_entry\" 2>stderr &&\n+\ttest_must_be_empty stderr &&\n+\ttest_sparse_entry_unchanged &&\n+\tgit ls-files --error-unmatch dense_entry\n+'\n+\n+test_expect_success 'add obeys advice.updateSparsePath' '\n+\tsetup_sparse_entry &&\n+\ttest_must_fail git -c advice.updateSparsePath=false add sparse_entry 2>stderr &&\n+\ttest_cmp sparse_entry_error stderr\n+\n+'\n+\n test_done\n-- \n2.30.1\n\n"},{"id":"421270","messageId":"4459976ae9c508509c3989ba6684eecc7e051fc9.1617914011.git.matheus.bernardino@usp.br","threadId":"55302","inReplyTo":"cover.1617914011.git.matheus.bernardino@usp.br","subject":"[PATCH v4 7/7] rm: honor sparse checkout patterns","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-08T20:41:28Z","receivedAt":"2021-04-08T20:41:58Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"`git add` refrains from adding or updating index entries that are\noutside the current sparse checkout, but `git rm` doesn't follow the\nsame restriction. This is somewhat counter-intuitive and inconsistent.\nSo make `rm` honor the sparsity rules and advise on how to remove\nSKIP_WORKTREE entries just like `add` does. Also add some tests for the\nnew behavior.\n\nSuggested-by: Elijah Newren \nSigned-off-by: Matheus Tavares \n---\n Documentation/config/advice.txt | 5 +-\n Documentation/git-rm.txt | 4 +-\n builtin/rm.c | 35 +++++++++-----\n t/t3602-rm-sparse-checkout.sh | 78 ++++++++++++++++++++++++++++++++\n t/t7011-skip-worktree-reading.sh | 5 --\n 5 files changed, 108 insertions(+), 19 deletions(-)\n create mode 100755 t/t3602-rm-sparse-checkout.sh\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex 851b83cf30..8b2849ff7b 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -120,6 +120,7 @@ advice.*::\n \t\tAdvice shown if a user runs the add command without providing\n \t\tthe pathspec parameter.\n \tupdateSparsePath::\n-\t\tAdvice shown when linkgit:git-add[1] is asked to update index\n-\t\tentries outside the current sparse checkout.\n+\t\tAdvice shown when either linkgit:git-add[1] or linkgit:git-rm[1]\n+\t\tis asked to update index entries outside the current sparse\n+\t\tcheckout.\n --\ndiff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt\nindex ab750367fd..26e9b28470 100644\n--- a/Documentation/git-rm.txt\n+++ b/Documentation/git-rm.txt\n@@ -23,7 +23,9 @@ branch, and no updates to their contents can be staged in the index,\n though that default behavior can be overridden with the `-f` option.\n When `--cached` is given, the staged content has to\n match either the tip of the branch or the file on disk,\n-allowing the file to be removed from just the index.\n+allowing the file to be removed from just the index. When\n+sparse-checkouts are in use (see linkgit:git-sparse-checkout[1]),\n+`git rm` will only remove paths within the sparse-checkout patterns.\n \n \n OPTIONS\ndiff --git a/builtin/rm.c b/builtin/rm.c\nindex 4858631e0f..d23a3b2164 100644\n--- a/builtin/rm.c\n+++ b/builtin/rm.c\n@@ -5,6 +5,7 @@\n */\n #define USE_THE_INDEX_COMPATIBILITY_MACROS\n #include \"builtin.h\"\n+#include \"advice.h\"\n #include \"config.h\"\n #include \"lockfile.h\"\n #include \"dir.h\"\n@@ -254,7 +255,7 @@ static struct option builtin_rm_options[] = {\n int cmd_rm(int argc, const char **argv, const char *prefix)\n {\n \tstruct lock_file lock_file = LOCK_INIT;\n-\tint i;\n+\tint i, ret = 0;\n \tstruct pathspec pathspec;\n \tchar *seen;\n \n@@ -295,6 +296,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \n \tfor (i = 0; i < active_nr; i++) {\n \t\tconst struct cache_entry *ce = active_cache[i];\n+\t\tif (ce_skip_worktree(ce))\n+\t\t\tcontinue;\n \t\tif (!ce_path_match(&the_index, ce, &pathspec, seen))\n \t\t\tcontinue;\n \t\tALLOC_GROW(list.entry, list.nr + 1, list.alloc);\n@@ -308,24 +311,34 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \tif (pathspec.nr) {\n \t\tconst char *original;\n \t\tint seen_any = 0;\n+\t\tchar *skip_worktree_seen = NULL;\n+\t\tstruct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;\n+\n \t\tfor (i = 0; i < pathspec.nr; i++) {\n \t\t\toriginal = pathspec.items[i].original;\n-\t\t\tif (!seen[i]) {\n-\t\t\t\tif (!ignore_unmatch) {\n-\t\t\t\t\tdie(_(\"pathspec '%s' did not match any files\"),\n-\t\t\t\t\t original);\n-\t\t\t\t}\n-\t\t\t}\n-\t\t\telse {\n+\t\t\tif (seen[i])\n \t\t\t\tseen_any = 1;\n-\t\t\t}\n+\t\t\telse if (ignore_unmatch)\n+\t\t\t\tcontinue;\n+\t\t\telse if (matches_skip_worktree(&pathspec, i, &skip_worktree_seen))\n+\t\t\t\tstring_list_append(&only_match_skip_worktree, original);\n+\t\t\telse\n+\t\t\t\tdie(_(\"pathspec '%s' did not match any files\"), original);\n+\n \t\t\tif (!recursive && seen[i] == MATCHED_RECURSIVELY)\n \t\t\t\tdie(_(\"not removing '%s' recursively without -r\"),\n \t\t\t\t *original ? original : \".\");\n \t\t}\n \n+\t\tif (only_match_skip_worktree.nr) {\n+\t\t\tadvise_on_updating_sparse_paths(&only_match_skip_worktree);\n+\t\t\tret = 1;\n+\t\t}\n+\t\tfree(skip_worktree_seen);\n+\t\tstring_list_clear(&only_match_skip_worktree, 0);\n+\n \t\tif (!seen_any)\n-\t\t\texit(0);\n+\t\t\texit(ret);\n \t}\n \n \tif (!index_only)\n@@ -405,5 +418,5 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \t\t\t COMMIT_LOCK | SKIP_IF_UNCHANGED))\n \t\tdie(_(\"Unable to write new index file\"));\n \n-\treturn 0;\n+\treturn ret;\n }\ndiff --git a/t/t3602-rm-sparse-checkout.sh b/t/t3602-rm-sparse-checkout.sh\nnew file mode 100755\nindex 0000000000..e9e9a15c74\n--- /dev/null\n+++ b/t/t3602-rm-sparse-checkout.sh\n@@ -0,0 +1,78 @@\n+#!/bin/sh\n+\n+test_description='git rm in sparse checked out working trees'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' \"\n+\tmkdir -p sub/dir &&\n+\ttouch a b c sub/d sub/dir/e &&\n+\tgit add -A &&\n+\tgit commit -m files &&\n+\n+\tcat >sparse_error_header <<-EOF &&\n+\tThe following pathspecs didn't match any eligible path, but they do match index\n+\tentries outside the current sparse checkout:\n+\tEOF\n+\n+\tcat >sparse_hint <<-EOF &&\n+\thint: Disable or modify the sparsity rules if you intend to update such entries.\n+\thint: Disable this message with \\\"git config advice.updateSparsePath false\\\"\n+\tEOF\n+\n+\techo b | cat sparse_error_header - >sparse_entry_b_error &&\n+\tcat sparse_entry_b_error sparse_hint >b_error_and_hint\n+\"\n+\n+for opt in \"\" -f --dry-run\n+do\n+\ttest_expect_success \"rm${opt:+ $opt} does not remove sparse entries\" '\n+\t\tgit sparse-checkout set a &&\n+\t\ttest_must_fail git rm $opt b 2>stderr &&\n+\t\ttest_cmp b_error_and_hint stderr &&\n+\t\tgit ls-files --error-unmatch b\n+\t'\n+done\n+\n+test_expect_success 'recursive rm does not remove sparse entries' '\n+\tgit reset --hard &&\n+\tgit sparse-checkout set sub/dir &&\n+\tgit rm -r sub &&\n+\tgit status --porcelain -uno >actual &&\n+\techo \"D sub/dir/e\" >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'rm obeys advice.updateSparsePath' '\n+\tgit reset --hard &&\n+\tgit sparse-checkout set a &&\n+\ttest_must_fail git -c advice.updateSparsePath=false rm b 2>stderr &&\n+\ttest_cmp sparse_entry_b_error stderr\n+'\n+\n+test_expect_success 'do not advice about sparse entries when they do not match the pathspec' '\n+\tgit reset --hard &&\n+\tgit sparse-checkout set a &&\n+\ttest_must_fail git rm nonexistent 2>stderr &&\n+\tgrep \"fatal: pathspec .nonexistent. did not match any files\" stderr &&\n+\t! grep -F -f sparse_error_header stderr\n+'\n+\n+test_expect_success 'do not warn about sparse entries when pathspec matches dense entries' '\n+\tgit reset --hard &&\n+\tgit sparse-checkout set a &&\n+\tgit rm \"[ba]\" 2>stderr &&\n+\ttest_must_be_empty stderr &&\n+\tgit ls-files --error-unmatch b &&\n+\ttest_must_fail git ls-files --error-unmatch a\n+'\n+\n+test_expect_success 'do not warn about sparse entries with --ignore-unmatch' '\n+\tgit reset --hard &&\n+\tgit sparse-checkout set a &&\n+\tgit rm --ignore-unmatch b 2>stderr &&\n+\ttest_must_be_empty stderr &&\n+\tgit ls-files --error-unmatch b\n+'\n+\n+test_done\ndiff --git a/t/t7011-skip-worktree-reading.sh b/t/t7011-skip-worktree-reading.sh\nindex 26852586ac..1761a2b1b9 100755\n--- a/t/t7011-skip-worktree-reading.sh\n+++ b/t/t7011-skip-worktree-reading.sh\n@@ -132,11 +132,6 @@ test_expect_success 'diff-files does not examine skip-worktree dirty entries' '\n \ttest -z \"$(git diff-files -- one)\"\n '\n \n-test_expect_success 'git-rm succeeds on skip-worktree absent entries' '\n-\tsetup_absent &&\n-\tgit rm 1\n-'\n-\n test_expect_success 'commit on skip-worktree absent entries' '\n \tgit reset &&\n \tsetup_absent &&\n-- \n2.30.1\n\n"},{"id":"421972","messageId":"CABPp-BGFfqOLg4tt85f-F-TJEXTDQFfAvRuui6VCctcd0FB5sQ@mail.gmail.com","threadId":"55302","inReplyTo":"cover.1617914011.git.matheus.bernardino@usp.br","subject":"Re: [PATCH v4 0/7] add/rm: honor sparse checkout and warn on sparse paths","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-04-14T16:36:19Z","receivedAt":"2021-04-14T16:36:33Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Apr 8, 2021 at 1:41 PM Matheus Tavares\n wrote:\n>\n> Make `rm` honor sparse checkouts, and make both `rm` and `add` warn\n> when asked to update sparse entries.\n>\n> There are two changes since v3:\n>\n> - `test_i18ncmp` and `test_i18ngrep` were replaced by `test_cmp` and\n> `grep`\n>\n> - The flag added in patch 5 now makes refresh_index() completely ignore\n> skip_worktree entries, instead of just suppressing their matches on\n> the seen[] array. The previous implementation was not necessarily\n> wrong but, as Junio pointed out, it was rather odd to keep matching\n> the entries if we no longer want to use the matches.\n>\n> As \"side effects\", the new version of the flag also makes\n> refresh_index() refrain from both:\n>\n> (1) checking and warning if skip_worktree entries matching the given\n> pathspec are unmerged.\n>\n> (2) marking skip_worktree entries matching the given pathspec with\n> CE_UPTODATE.\n>\n> The change (1) is actually interesting because `git add` doesn't\n> update skip_worktree entries, and thus, it doesn't make much sense to\n> warn if they are unmerged. Besides, we will already warn if the user\n> requests to update such entries, anyway. And finally, unmerged\n> entries should not have the skip_worktree bit set in the first place.\n> (`git merge` should clean this bit when writing the new index, and\n> neither `git sparse-checkout` nor `git update-index` allow to set the\n> bit on an unmerged entry.)\n>\n> Change (2) is perhaps not very beneficial, but it is also not harmful.\n> The only practical difference we get by not setting the CE_UPTODATE\n> flag in the skip_worktree entries is that, when writing a new index at\n> the end of `git add --refresh`, do_write_index() will start checking\n> if these entries are racy clean. Note that it already does that for\n> all the skip_worktree entries that do not match the user-given\n> pathspecs. And, in fact, this behavior distinction based on the\n> pathspec only happens with `--refresh`. Plain `git add` and other\n> options don't mark any skip_worktree entry with CE_UPTODATE\n> (regardless of the pathspecs) and thus, all these entries are checked\n> when writing the index. So `git add --refresh` will only do what the\n> other options already do.\n\nSorry for the delay. These two changes sound good to me, and the\nrange-diff looks reasonable.\n\n> (Additionally, as I mentioned in [1], there might actually be at least\n> one advantage in checking if the skip_worktree entries are racy clean.\n> But this is a very specific case, and it's probably a topic for a\n> another thread :)\n>\n> [1]: https://lore.kernel.org/git/CAHd-oW4kRLjV9Sq3CFt-V1Ot9pYFzJggU1zPp3Hcuw=qWfq7Mg@mail.gmail.com/\n\nThis I'm a bit surprised by. I thought the outcome there was that you\ndidn't want to mark skip_worktree entries as CE_UPTODATE in order to\nforce them to be stat'd in the future when someone clears the\nskip_worktree bit. (And that we'd expect the sparse-checkout\nadd/disable command to be the likely one to handle that, though if\nusers use `git update-index --no-skip-worktree PATH` then they'd also\nbe responsible for any refreshing as well.) Am I misunderstanding?\n"},{"id":"421973","messageId":"69e2d520-3caf-a64d-8bb4-c2c9723be9de@gmail.com","threadId":"55302","inReplyTo":"5d0cdb3718ec0230fe199441b6a4089ce828a3e9.1617914011.git.matheus.bernardino@usp.br","subject":"Re: [PATCH v4 2/7] t3705: add tests for `git add` in sparse checkouts","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-04-14T16:39:42Z","receivedAt":"2021-04-14T16:39:47Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/8/2021 4:41 PM, Matheus Tavares wrote:\n> We already have a couple tests for `add` with SKIP_WORKTREE entries in\n> t7012, but these only cover the most basic scenarios. As we will be\n> changing how `add` deals with sparse paths in the subsequent commits,\n> let's move these two tests to their own file and add more test cases\n> for different `add` options and situations. This also demonstrates two\n> options that don't currently respect SKIP_WORKTREE entries: `--chmod`\n> and `--renormalize`.\n\nThese are good tests. I will adapt these for my sparse-index\nseries to ensure 'git add' works correctly in that environment.\n\nThanks,\n-Stolee\n"},{"id":"421977","messageId":"CAHd-oW5e29Q8=4M-bxUT7mmWi1VyjQ1rCy+zWcw0_WETwB87_A@mail.gmail.com","threadId":"55302","inReplyTo":"CABPp-BGFfqOLg4tt85f-F-TJEXTDQFfAvRuui6VCctcd0FB5sQ@mail.gmail.com","subject":"Re: [PATCH v4 0/7] add/rm: honor sparse checkout and warn on sparse paths","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-04-14T18:04:59Z","receivedAt":"2021-04-14T18:05:19Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On Wed, Apr 14, 2021 at 1:36 PM Elijah Newren wrote:\n>\n> On Thu, Apr 8, 2021 at 1:41 PM Matheus Tavares\n> wrote:\n> >\n> > Make `rm` honor sparse checkouts, and make both `rm` and `add` warn\n> > when asked to update sparse entries.\n> >\n> > There are two changes since v3:\n> >\n> > - `test_i18ncmp` and `test_i18ngrep` were replaced by `test_cmp` and\n> > `grep`\n> >\n> > - The flag added in patch 5 now makes refresh_index() completely ignore\n> > skip_worktree entries, instead of just suppressing their matches on\n> > the seen[] array. The previous implementation was not necessarily\n> > wrong but, as Junio pointed out, it was rather odd to keep matching\n> > the entries if we no longer want to use the matches.\n> >\n> > As \"side effects\", the new version of the flag also makes\n> > refresh_index() refrain from both:\n> >\n> > (1) checking and warning if skip_worktree entries matching the given\n> > pathspec are unmerged.\n> >\n> > (2) marking skip_worktree entries matching the given pathspec with\n> > CE_UPTODATE.\n> >\n> > The change (1) is actually interesting because `git add` doesn't\n> > update skip_worktree entries, and thus, it doesn't make much sense to\n> > warn if they are unmerged. Besides, we will already warn if the user\n> > requests to update such entries, anyway. And finally, unmerged\n> > entries should not have the skip_worktree bit set in the first place.\n> > (`git merge` should clean this bit when writing the new index, and\n> > neither `git sparse-checkout` nor `git update-index` allow to set the\n> > bit on an unmerged entry.)\n> >\n> > Change (2) is perhaps not very beneficial, but it is also not harmful.\n> > The only practical difference we get by not setting the CE_UPTODATE\n> > flag in the skip_worktree entries is that, when writing a new index at\n> > the end of `git add --refresh`, do_write_index() will start checking\n> > if these entries are racy clean. Note that it already does that for\n> > all the skip_worktree entries that do not match the user-given\n> > pathspecs. And, in fact, this behavior distinction based on the\n> > pathspec only happens with `--refresh`. Plain `git add` and other\n> > options don't mark any skip_worktree entry with CE_UPTODATE\n> > (regardless of the pathspecs) and thus, all these entries are checked\n> > when writing the index. So `git add --refresh` will only do what the\n> > other options already do.\n>\n> Sorry for the delay. These two changes sound good to me, and the\n> range-diff looks reasonable.\n>\n> > (Additionally, as I mentioned in [1], there might actually be at least\n> > one advantage in checking if the skip_worktree entries are racy clean.\n> > But this is a very specific case, and it's probably a topic for a\n> > another thread :)\n> >\n> > [1]: https://lore.kernel.org/git/CAHd-oW4kRLjV9Sq3CFt-V1Ot9pYFzJggU1zPp3Hcuw=qWfq7Mg@mail.gmail.com/\n>\n> This I'm a bit surprised by. I thought the outcome there was that you\n> didn't want to mark skip_worktree entries as CE_UPTODATE in order to\n> force them to be stat'd in the future when someone clears the\n> skip_worktree bit.\n\nHmm, not exactly. This situation is a bit tricky and I probably got\nlost when trying to communicate my thoughts about it.\n\nIn short, the outcome of not marking skip_worktree entries as\nCE_UPTODATE (which is an in-memory-only flag) is that, when writing\nthe updated index at the end of `git status --refresh` , we now\nproperly detect and mark skip_worktree entries whose associated files\nare present in the working tree and are modified in relation to the\nrespective blobs. (This whole process is skipped for CE_UPTODATE\nentries.)\n\nThis doesn't have any effect while the skip_worktree bit is set. But\nit makes it possible for a later `git status` to properly show the\nfiles as modified when the skip_worktree bit gets unset. If we don't\ndo this, the later `git status` will wrongly think these entries are\nclean.\n\nThis is because of the way git detects racily clean entries.\nParaphrasing `Documentation/technical/racy-git.txt`, we take two\nactions to diagnose these entries:\n\n1) When we want to know if an entry is up-to-date: if the entry's\ntimestamp is equal to, or newer than, the index timestamp, we not only\ncompare the cached stat info with the filesystem stat info but we also\ncompare the actual contents.\n\n2) When writing a new index: if the index contains racily clean\nentries, their `st_size` is truncated to zero.\n\nItem 2) is important because, otherwise, the subsequent operations\nwouldn't be able to detect the racily clean entries using 1) as the\nindex timestamp would have been updated.\n\nAnd that's what happens with skip_worktree entries on `git status\n--refresh`. We mark them as CE_UPTODATE even if the file exists in the\nworking tree, so we don't check if the cached entry is racily clean,\nand thus we don't truncate `st_size` to 0, hiding the racily clean\nentry.\n\nWith all that said, I think this whole situation must be quite rare\nand not very important in practice...\n"},{"id":"422125","messageId":"xmqqzgxxkj8v.fsf@gitster.g","threadId":"55302","inReplyTo":"cover.1617914011.git.matheus.bernardino@usp.br","subject":"Re: [PATCH v4 0/7] add/rm: honor sparse checkout and warn on sparse paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-16T21:33:04Z","receivedAt":"2021-04-16T21:33:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matheus Tavares writes:\n\n> Make `rm` honor sparse checkouts, and make both `rm` and `add` warn\n> when asked to update sparse entries.\n\nOK, has the back-and-forth concluded and this is now ready to be\nmerged down to 'next'?\n"},{"id":"422133","messageId":"CABPp-BE=KfQTj0ad3uBS90MA1EDkHFV8kXi7xbEMDcnoi-MiOg@mail.gmail.com","threadId":"55302","inReplyTo":"xmqqzgxxkj8v.fsf@gitster.g","subject":"Re: [PATCH v4 0/7] add/rm: honor sparse checkout and warn on sparse paths","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-04-16T23:17:02Z","receivedAt":"2021-04-16T23:17:21Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Apr 16, 2021 at 2:33 PM Junio C Hamano wrote:\n>\n> Matheus Tavares writes:\n>\n> > Make `rm` honor sparse checkouts, and make both `rm` and `add` warn\n> > when asked to update sparse entries.\n>\n> OK, has the back-and-forth concluded and this is now ready to be\n> merged down to 'next'?\n\nYes, let's merge it.\n"}]}