{"thread":{"id":"45913","subject":"[PATCH 0/8] convert pathspec.c to take an index parameter","startedAt":"2017-05-09T19:18:32Z","lastAt":"2017-05-12T18:45:03Z","messageCount":29,"participants":["Brandon Williams","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"319210","messageId":"20170509191805.176266-1-bmwill@google.com","threadId":"45913","inReplyTo":null,"subject":"[PATCH 0/8] convert pathspec.c to take an index parameter","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-09T19:17:57Z","receivedAt":"2017-05-09T19:18:32Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"This is another conversion series to convert the pathspec library code to take\nin an index parameter instead of relying on cache macros or on the global\nvariable 'the_index'.\n\nWhile I was working in the pathspec code I thought it would be good to do a\nlittle more cleanup and make the API cleaner. More specifically consolidating\nthe 'strip submodule slash' flags into a single flag while splitting out the\n'submodule leading path' check performed in the expensive case into its own\nflag.\n\nBrandon Williams (8):\n pathspec: provide a more descriptive die message\n submodule: add die_in_unpopulated_submodule function\n pathspec: change PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag\n pathspec: rename PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP\n pathspec: convert strip_submodule_slash to take an index\n pathspec: convert find_pathspecs_matching_against_index to take an\n index\n pathspec: convert init_pathspec_item to take an index\n pathspec: convert parse_pathspec to take an index\n\n archive.c | 4 +-\n builtin/add.c | 14 +++---\n builtin/blame.c | 2 +-\n builtin/check-ignore.c | 7 +--\n builtin/checkout.c | 2 +-\n builtin/clean.c | 2 +-\n builtin/commit.c | 4 +-\n builtin/grep.c | 2 +-\n builtin/ls-files.c | 6 +--\n builtin/ls-tree.c | 5 ++-\n builtin/rerere.c | 2 +-\n builtin/reset.c | 4 +-\n builtin/rm.c | 4 +-\n builtin/submodule--helper.c | 4 +-\n builtin/update-index.c | 2 +-\n line-log.c | 2 +-\n pathspec.c | 92 +++++++++++++++++-----------------------\n pathspec.h | 19 +++++----\n revision.c | 5 ++-\n submodule.c | 32 +++++++++++++-\n submodule.h | 2 +\n t/t6134-pathspec-in-submodule.sh | 6 +--\n tree-diff.c | 2 +-\n 23 files changed, 122 insertions(+), 102 deletions(-)\n\n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319211","messageId":"20170509191805.176266-3-bmwill@google.com","threadId":"45913","inReplyTo":"20170509191805.176266-1-bmwill@google.com","subject":"[PATCH 2/8] submodule: add die_in_unpopulated_submodule function","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-09T19:17:59Z","receivedAt":"2017-05-09T19:18:34Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Currently 'git add' is the only command which dies when launched from an\nunpopulated submodule (the place-holder directory for a submodule which\nhasn't been checked out). This is triggered implicitly by passing the\nPATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag to 'parse_pathspec()'.\n\nInstead make this desire more explicit by creating a function\n'die_in_unpopulated_submodule()' which dies if the provided 'prefix' has\na leading path component which matches a submodule in the the index.\n\nSigned-off-by: Brandon Williams \n---\n builtin/add.c | 3 +++\n pathspec.c | 29 -----------------------------\n submodule.c | 30 ++++++++++++++++++++++++++++++\n submodule.h | 2 ++\n t/t6134-pathspec-in-submodule.sh | 6 +-----\n 5 files changed, 36 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 9f53f020d..ec58e3679 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -17,6 +17,7 @@\n #include \"revision.h\"\n #include \"bulk-checkin.h\"\n #include \"argv-array.h\"\n+#include \"submodule.h\"\n \n static const char * const builtin_add_usage[] = {\n \tN_(\"git add [] [--] ...\"),\n@@ -379,6 +380,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tif (read_cache() < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n+\tdie_in_unpopulated_submodule(&the_index, prefix);\n+\n \t/*\n \t * Check the \"pathspec '%s' did not match any files\" block\n \t * below before enabling new magic.\ndiff --git a/pathspec.c b/pathspec.c\nindex 61b5b1273..e37256c3b 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -424,27 +424,6 @@ static void strip_submodule_slash_expensive(struct pathspec_item *item)\n \t}\n }\n \n-static void die_inside_submodule_path(struct pathspec_item *item)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < active_nr; i++) {\n-\t\tstruct cache_entry *ce = active_cache[i];\n-\t\tint ce_len = ce_namelen(ce);\n-\n-\t\tif (!S_ISGITLINK(ce->ce_mode))\n-\t\t\tcontinue;\n-\n-\t\tif (item->len < ce_len ||\n-\t\t !(item->match[ce_len] == '/' || item->match[ce_len] == '\\0') ||\n-\t\t memcmp(ce->name, item->match, ce_len))\n-\t\t\tcontinue;\n-\n-\t\tdie(_(\"Pathspec '%s' is in submodule '%.*s'\"),\n-\t\t item->original, ce_len, ce->name);\n-\t}\n-}\n-\n /*\n * Perform the initialization of a pathspec_item based on a pathspec element.\n */\n@@ -547,14 +526,6 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n \t/* sanity checks, pathspec matchers assume these are sane */\n \tif (item->nowildcard_len > item->len ||\n \t item->prefix > item->len) {\n-\t\t/*\n-\t\t * This case can be triggered by the user pointing us to a\n-\t\t * pathspec inside a submodule, which is an input error.\n-\t\t * Detect that here and complain, but fallback in the\n-\t\t * non-submodule case to a BUG, as we have no idea what\n-\t\t * would trigger that.\n-\t\t */\n-\t\tdie_inside_submodule_path(item);\n \t\tdie (\"BUG: error initializing pathspec_item\");\n \t}\n }\ndiff --git a/submodule.c b/submodule.c\nindex 7c3c4b17f..9e0502f25 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -281,6 +281,36 @@ int is_submodule_populated_gently(const char *path, int *return_error_code)\n \treturn ret;\n }\n \n+/*\n+ * Dies if the provided 'prefix' corresponds to an unpopulated submodule\n+ */\n+void die_in_unpopulated_submodule(const struct index_state *istate,\n+\t\t\t\t const char *prefix)\n+{\n+\tint i, prefixlen;\n+\n+\tif (!prefix)\n+\t\treturn;\n+\n+\tprefixlen = strlen(prefix);\n+\n+\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = istate->cache[i];\n+\t\tint ce_len = ce_namelen(ce);\n+\n+\t\tif (!S_ISGITLINK(ce->ce_mode))\n+\t\t\tcontinue;\n+\t\tif (prefixlen <= ce_len)\n+\t\t\tcontinue;\n+\t\tif (strncmp(ce->name, prefix, ce_len))\n+\t\t\tcontinue;\n+\t\tif (prefix[ce_len] != '/')\n+\t\t\tcontinue;\n+\n+\t\tdie(_(\"in unpopulated submodule '%s'\"), ce->name);\n+\t}\n+}\n+\n int parse_submodule_update_strategy(const char *value,\n \t\tstruct submodule_update_strategy *dst)\n {\ndiff --git a/submodule.h b/submodule.h\nindex 8a8bc49dc..f4fe6374d 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -48,6 +48,8 @@ extern int is_submodule_initialized(const char *path);\n * Otherwise the return error code is the same as of resolve_gitdir_gently.\n */\n extern int is_submodule_populated_gently(const char *path, int *return_error_code);\n+extern void die_in_unpopulated_submodule(const struct index_state *istate,\n+\t\t\t\t\t const char *prefix);\n extern int parse_submodule_update_strategy(const char *value,\n \t\tstruct submodule_update_strategy *dst);\n extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);\ndiff --git a/t/t6134-pathspec-in-submodule.sh b/t/t6134-pathspec-in-submodule.sh\nindex fd401ca60..0f1cb49ce 100755\n--- a/t/t6134-pathspec-in-submodule.sh\n+++ b/t/t6134-pathspec-in-submodule.sh\n@@ -24,13 +24,9 @@ test_expect_success 'error message for path inside submodule' '\n \ttest_cmp expect actual\n '\n \n-cat <expect\n-fatal: Pathspec '.' is in submodule 'sub'\n-EOF\n-\n test_expect_success 'error message for path inside submodule from within submodule' '\n \ttest_must_fail git -C sub add . 2>actual &&\n-\ttest_cmp expect actual\n+\ttest_i18ngrep \"in unpopulated submodule\" actual\n '\n \n test_done\n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319212","messageId":"20170509191805.176266-4-bmwill@google.com","threadId":"45913","inReplyTo":"20170509191805.176266-1-bmwill@google.com","subject":"[PATCH 3/8] pathspec: change PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-09T19:18:00Z","receivedAt":"2017-05-09T19:18:35Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"It's confusing to have two different 'strip submodule slash' flags which\ndo subtly different things. Both\nPATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE and\nPATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP will accomplish the same task of\nstriping a slash from a pathspec which matches a submodule entry in the\nindex. The only difference is that\nPATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE will perform additional checks\nand die if a pathspec has a leading path component which corresponds to\na submodule. This additional functionality should be split out into its\nown flag.\n\nTo this end, rename the PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag to\nPATHSPEC_SUBMODULE_LEADING_PATH and change its behavior to only die if a\npath descends into a submodule. In addition add the\nPATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag to callers which relied on the\nold slash stripping functionality.\n\nSigned-off-by: Brandon Williams \n---\n builtin/add.c | 3 ++-\n builtin/check-ignore.c | 3 ++-\n pathspec.c | 32 ++++++++++++++++----------------\n pathspec.h | 9 +++------\n 4 files changed, 23 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ec58e3679..2aa9aeab9 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -389,7 +389,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tparse_pathspec(&pathspec, 0,\n \t\t PATHSPEC_PREFER_FULL |\n \t\t PATHSPEC_SYMLINK_LEADING_PATH |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE,\n+\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP |\n+\t\t PATHSPEC_SUBMODULE_LEADING_PATH,\n \t\t prefix, argv);\n \n \tif (add_new_files) {\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex 1d73d3ca3..73237b2b1 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -87,7 +87,8 @@ static int check_ignore(struct dir_struct *dir,\n \tparse_pathspec(&pathspec,\n \t\t PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP,\n \t\t PATHSPEC_SYMLINK_LEADING_PATH |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE |\n+\t\t PATHSPEC_SUBMODULE_LEADING_PATH |\n+\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP |\n \t\t PATHSPEC_KEEP_ORDER,\n \t\t prefix, argv);\n \ndiff --git a/pathspec.c b/pathspec.c\nindex e37256c3b..f37d5769d 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -398,29 +398,29 @@ static void strip_submodule_slash_cheap(struct pathspec_item *item)\n \t}\n }\n \n-static void strip_submodule_slash_expensive(struct pathspec_item *item)\n+static void die_path_inside_submodule(const struct pathspec_item *item,\n+\t\t\t\t const struct index_state *istate)\n {\n \tint i;\n \n-\tfor (i = 0; i < active_nr; i++) {\n-\t\tstruct cache_entry *ce = active_cache[i];\n+\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = istate->cache[i];\n \t\tint ce_len = ce_namelen(ce);\n \n \t\tif (!S_ISGITLINK(ce->ce_mode))\n \t\t\tcontinue;\n \n-\t\tif (item->len <= ce_len || item->match[ce_len] != '/' ||\n-\t\t memcmp(ce->name, item->match, ce_len))\n+\t\tif (item->len <= ce_len)\n+\t\t\tcontinue;\n+\t\tif (item->match[ce_len] != '/')\n+\t\t\tcontinue;\n+\t\tif (strncmp(ce->name, item->match, ce_len))\n+\t\t\tcontinue;\n+\t\tif (item->len == ce_len + 1)\n \t\t\tcontinue;\n \n-\t\tif (item->len == ce_len + 1) {\n-\t\t\t/* strip trailing slash */\n-\t\t\titem->len--;\n-\t\t\titem->match[item->len] = '\\0';\n-\t\t} else {\n-\t\t\tdie(_(\"Pathspec '%s' is in submodule '%.*s'\"),\n-\t\t\t item->original, ce_len, ce->name);\n-\t\t}\n+\t\tdie(_(\"Pathspec '%s' is in submodule '%.*s'\"),\n+\t\t item->original, ce_len, ce->name);\n \t}\n }\n \n@@ -499,9 +499,6 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n \tif (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)\n \t\tstrip_submodule_slash_cheap(item);\n \n-\tif (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)\n-\t\tstrip_submodule_slash_expensive(item);\n-\n \tif (magic & PATHSPEC_LITERAL) {\n \t\titem->nowildcard_len = item->len;\n \t} else {\n@@ -639,6 +636,9 @@ void parse_pathspec(struct pathspec *pathspec,\n \t\t\tdie(_(\"pathspec '%s' is beyond a symbolic link\"), entry);\n \t\t}\n \n+\t\tif (flags & PATHSPEC_SUBMODULE_LEADING_PATH)\n+\t\t\tdie_path_inside_submodule(item + i, &the_index);\n+\n \t\tif (item[i].nowildcard_len < item[i].len)\n \t\t\tpathspec->has_wildcard = 1;\n \t\tpathspec->magic |= item[i].magic;\ndiff --git a/pathspec.h b/pathspec.h\nindex 55e976972..93a819cbf 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -63,13 +63,10 @@ struct pathspec {\n /* die if a symlink is part of the given path's directory */\n #define PATHSPEC_SYMLINK_LEADING_PATH (1<<4)\n /*\n- * This is like a combination of ..LEADING_PATH and .._SLASH_CHEAP\n- * (but not the same): it strips the trailing slash if the given path\n- * is a gitlink but also checks and dies if gitlink is part of the\n- * leading path (i.e. the given path goes beyond a submodule). It's\n- * safer than _SLASH_CHEAP and also more expensive.\n+ * Checks and dies if gitlink is part of the leading path\n+ * (i.e. the given path goes beyond a submodule).\n */\n-#define PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE (1<<5)\n+#define PATHSPEC_SUBMODULE_LEADING_PATH (1<<5)\n #define PATHSPEC_PREFIX_ORIGIN (1<<6)\n #define PATHSPEC_KEEP_ORDER (1<<7)\n /*\n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319213","messageId":"20170509191805.176266-2-bmwill@google.com","threadId":"45913","inReplyTo":"20170509191805.176266-1-bmwill@google.com","subject":"[PATCH 1/8] pathspec: provide a more descriptive die message","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-09T19:17:58Z","receivedAt":"2017-05-09T19:18:37Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"The current message displayed upon an internal error in\n'init_pathspec_item()' isn't very descriptive and doesn't provide much\ncontext to where the error occurred. Update the error message to\nprovide more context to where the error occured.\n\nSigned-off-by: Brandon Williams \n---\n pathspec.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/pathspec.c b/pathspec.c\nindex 303efda83..61b5b1273 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -555,7 +555,7 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n \t\t * would trigger that.\n \t\t */\n \t\tdie_inside_submodule_path(item);\n-\t\tdie (\"BUG: item->nowildcard_len > item->len || item->prefix > item->len)\");\n+\t\tdie (\"BUG: error initializing pathspec_item\");\n \t}\n }\n \n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319214","messageId":"20170509191805.176266-6-bmwill@google.com","threadId":"45913","inReplyTo":"20170509191805.176266-1-bmwill@google.com","subject":"[PATCH 5/8] pathspec: convert strip_submodule_slash to take an index","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-09T19:18:02Z","receivedAt":"2017-05-09T19:18:40Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Signed-off-by: Brandon Williams \n---\n pathspec.c | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/pathspec.c b/pathspec.c\nindex a1297ca02..cff069536 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -386,12 +386,13 @@ static const char *parse_element_magic(unsigned *magic, int *prefix_len,\n \t\treturn parse_short_magic(magic, elem);\n }\n \n-static void strip_submodule_slash(struct pathspec_item *item)\n+static void strip_submodule_slash(struct pathspec_item *item,\n+\t\t\t\t const struct index_state *istate)\n {\n \tif (item->len >= 1 && item->match[item->len - 1] == '/') {\n-\t\tint i = cache_name_pos(item->match, item->len - 1);\n+\t\tint i = index_name_pos(istate, item->match, item->len - 1);\n \n-\t\tif (i >= 0 && S_ISGITLINK(active_cache[i]->ce_mode)) {\n+\t\tif (i >= 0 && S_ISGITLINK(istate->cache[i]->ce_mode)) {\n \t\t\titem->len--;\n \t\t\titem->match[item->len] = '\\0';\n \t\t}\n@@ -497,7 +498,7 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n \t}\n \n \tif (flags & PATHSPEC_STRIP_SUBMODULE_SLASH)\n-\t\tstrip_submodule_slash(item);\n+\t\tstrip_submodule_slash(item, &the_index);\n \n \tif (magic & PATHSPEC_LITERAL) {\n \t\titem->nowildcard_len = item->len;\n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319215","messageId":"20170509191805.176266-8-bmwill@google.com","threadId":"45913","inReplyTo":"20170509191805.176266-1-bmwill@google.com","subject":"[PATCH 7/8] pathspec: convert init_pathspec_item to take an index","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-09T19:18:04Z","receivedAt":"2017-05-09T19:18:44Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Signed-off-by: Brandon Williams \n---\n pathspec.c | 11 +++++++----\n 1 file changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/pathspec.c b/pathspec.c\nindex bbd71b48b..c7ed8b3fb 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -430,7 +430,9 @@ static void die_path_inside_submodule(const struct pathspec_item *item,\n /*\n * Perform the initialization of a pathspec_item based on a pathspec element.\n */\n-static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n+static void init_pathspec_item(struct pathspec_item *item,\n+\t\t\t const struct index_state *istate,\n+\t\t\t unsigned flags,\n \t\t\t const char *prefix, int prefixlen,\n \t\t\t const char *elt)\n {\n@@ -500,7 +502,7 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n \t}\n \n \tif (flags & PATHSPEC_STRIP_SUBMODULE_SLASH)\n-\t\tstrip_submodule_slash(item, &the_index);\n+\t\tstrip_submodule_slash(item, istate);\n \n \tif (magic & PATHSPEC_LITERAL) {\n \t\titem->nowildcard_len = item->len;\n@@ -627,7 +629,8 @@ void parse_pathspec(struct pathspec *pathspec,\n \tfor (i = 0; i < n; i++) {\n \t\tentry = argv[i];\n \n-\t\tinit_pathspec_item(item + i, flags, prefix, prefixlen, entry);\n+\t\tinit_pathspec_item(item + i, &the_index, flags,\n+\t\t\t\t prefix, prefixlen, entry);\n \n \t\tif (item[i].magic & PATHSPEC_EXCLUDE)\n \t\t\tnr_exclude++;\n@@ -653,7 +656,7 @@ void parse_pathspec(struct pathspec *pathspec,\n \t */\n \tif (nr_exclude == n) {\n \t\tint plen = (!(flags & PATHSPEC_PREFER_CWD)) ? 0 : prefixlen;\n-\t\tinit_pathspec_item(item + n, 0, prefix, plen, \"\");\n+\t\tinit_pathspec_item(item + n, &the_index, 0, prefix, plen, \"\");\n \t\tpathspec->nr++;\n \t}\n \n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319216","messageId":"20170509191805.176266-7-bmwill@google.com","threadId":"45913","inReplyTo":"20170509191805.176266-1-bmwill@google.com","subject":"[PATCH 6/8] pathspec: convert find_pathspecs_matching_against_index to take an index","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-09T19:18:03Z","receivedAt":"2017-05-09T19:18:47Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Signed-off-by: Brandon Williams \n---\n builtin/add.c | 4 ++--\n builtin/check-ignore.c | 2 +-\n pathspec.c | 10 ++++++----\n pathspec.h | 7 +++++--\n 4 files changed, 14 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 51d7a5506..4e3bf20c2 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -136,7 +136,7 @@ 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, seen);\n+\tadd_pathspec_matches_against_index(pathspec, &the_index, seen);\n \treturn seen;\n }\n \n@@ -418,7 +418,7 @@ 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);\n+\t\t\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index);\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 694e4c61b..446b76dcf 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -97,7 +97,7 @@ 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);\n+\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index);\n \tfor (i = 0; i < pathspec.nr; i++) {\n \t\tfull_path = pathspec.items[i].match;\n \t\texclude = NULL;\ndiff --git a/pathspec.c b/pathspec.c\nindex cff069536..bbd71b48b 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -17,6 +17,7 @@\n * to use find_pathspecs_matching_against_index() instead.\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 {\n \tint num_unmatched = 0, i;\n@@ -32,8 +33,8 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n \t\t\tnum_unmatched++;\n \tif (!num_unmatched)\n \t\treturn;\n-\tfor (i = 0; i < active_nr; i++) {\n-\t\tconst struct cache_entry *ce = active_cache[i];\n+\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\tconst struct cache_entry *ce = istate->cache[i];\n \t\tce_path_match(ce, pathspec, seen);\n \t}\n }\n@@ -46,10 +47,11 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n * nature of the \"closest\" (i.e. most specific) matches which each of the\n * given pathspecs achieves against all items in the index.\n */\n-char *find_pathspecs_matching_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 {\n \tchar *seen = xcalloc(pathspec->nr, 1);\n-\tadd_pathspec_matches_against_index(pathspec, seen);\n+\tadd_pathspec_matches_against_index(pathspec, istate, seen);\n \treturn seen;\n }\n \ndiff --git a/pathspec.h b/pathspec.h\nindex dd6456d5d..dfb22440a 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -103,7 +103,10 @@ static inline int ps_strcmp(const struct pathspec_item *item,\n \t\treturn strcmp(s1, s2);\n }\n \n-extern char *find_pathspecs_matching_against_index(const struct pathspec *pathspec);\n-extern void add_pathspec_matches_against_index(const struct pathspec *pathspec, char *seen);\n+extern void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n+\t\t\t\t\t const struct index_state *istate,\n+\t\t\t\t\t char *seen);\n+extern char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,\n+\t\t\t\t\t\t const struct index_state *istate);\n \n #endif /* PATHSPEC_H */\n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319217","messageId":"20170509191805.176266-5-bmwill@google.com","threadId":"45913","inReplyTo":"20170509191805.176266-1-bmwill@google.com","subject":"[PATCH 4/8] pathspec: rename PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-09T19:18:01Z","receivedAt":"2017-05-09T19:18:49Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Now that there is only a single flag which strips a submodule slash,\nrename PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP to\nPATHSPEC_STRIP_SUBMODULE_SLASH.\n\nSigned-off-by: Brandon Williams \n---\n builtin/add.c | 2 +-\n builtin/check-ignore.c | 2 +-\n builtin/ls-files.c | 2 +-\n builtin/reset.c | 2 +-\n builtin/rm.c | 2 +-\n builtin/submodule--helper.c | 2 +-\n pathspec.c | 6 +++---\n pathspec.h | 2 +-\n 8 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 2aa9aeab9..51d7a5506 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -389,7 +389,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tparse_pathspec(&pathspec, 0,\n \t\t PATHSPEC_PREFER_FULL |\n \t\t PATHSPEC_SYMLINK_LEADING_PATH |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP |\n+\t\t PATHSPEC_STRIP_SUBMODULE_SLASH |\n \t\t PATHSPEC_SUBMODULE_LEADING_PATH,\n \t\t prefix, argv);\n \ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex 73237b2b1..694e4c61b 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -88,7 +88,7 @@ static int check_ignore(struct dir_struct *dir,\n \t\t PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP,\n \t\t PATHSPEC_SYMLINK_LEADING_PATH |\n \t\t PATHSPEC_SUBMODULE_LEADING_PATH |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP |\n+\t\t PATHSPEC_STRIP_SUBMODULE_SLASH |\n \t\t PATHSPEC_KEEP_ORDER,\n \t\t prefix, argv);\n \ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex d449e46db..e9dee2e41 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -618,7 +618,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \n \tparse_pathspec(&pathspec, 0,\n \t\t PATHSPEC_PREFER_CWD |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,\n+\t\t PATHSPEC_STRIP_SUBMODULE_SLASH,\n \t\t prefix, argv);\n \n \t/*\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex fc3b906c4..529f2f9be 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -236,7 +236,7 @@ static void parse_args(struct pathspec *pathspec,\n \n \tparse_pathspec(pathspec, 0,\n \t\t PATHSPEC_PREFER_FULL |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP |\n+\t\t PATHSPEC_STRIP_SUBMODULE_SLASH |\n \t\t (patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0),\n \t\t prefix, argv);\n }\ndiff --git a/builtin/rm.c b/builtin/rm.c\nindex fb79dcab1..8fe12d0a7 100644\n--- a/builtin/rm.c\n+++ b/builtin/rm.c\n@@ -272,7 +272,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \n \tparse_pathspec(&pathspec, 0,\n \t\t PATHSPEC_PREFER_CWD |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,\n+\t\t PATHSPEC_STRIP_SUBMODULE_SLASH,\n \t\t prefix, argv);\n \trefresh_index(&the_index, REFRESH_QUIET, &pathspec, NULL, NULL);\n \ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 85aafe46a..69149b557 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -234,7 +234,7 @@ static int module_list_compute(int argc, const char **argv,\n \tchar *ps_matched = NULL;\n \tparse_pathspec(pathspec, 0,\n \t\t PATHSPEC_PREFER_FULL |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,\n+\t\t PATHSPEC_STRIP_SUBMODULE_SLASH,\n \t\t prefix, argv);\n \n \tif (pathspec->nr)\ndiff --git a/pathspec.c b/pathspec.c\nindex f37d5769d..a1297ca02 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -386,7 +386,7 @@ static const char *parse_element_magic(unsigned *magic, int *prefix_len,\n \t\treturn parse_short_magic(magic, elem);\n }\n \n-static void strip_submodule_slash_cheap(struct pathspec_item *item)\n+static void strip_submodule_slash(struct pathspec_item *item)\n {\n \tif (item->len >= 1 && item->match[item->len - 1] == '/') {\n \t\tint i = cache_name_pos(item->match, item->len - 1);\n@@ -496,8 +496,8 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n \t\titem->original = xstrdup(elt);\n \t}\n \n-\tif (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)\n-\t\tstrip_submodule_slash_cheap(item);\n+\tif (flags & PATHSPEC_STRIP_SUBMODULE_SLASH)\n+\t\tstrip_submodule_slash(item);\n \n \tif (magic & PATHSPEC_LITERAL) {\n \t\titem->nowildcard_len = item->len;\ndiff --git a/pathspec.h b/pathspec.h\nindex 93a819cbf..dd6456d5d 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -59,7 +59,7 @@ struct pathspec {\n #define PATHSPEC_PREFER_FULL (1<<1) /* No args means match everything */\n #define PATHSPEC_MAXDEPTH_VALID (1<<2) /* max_depth field is valid */\n /* strip the trailing slash if the given path is a gitlink */\n-#define PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP (1<<3)\n+#define PATHSPEC_STRIP_SUBMODULE_SLASH (1<<3)\n /* die if a symlink is part of the given path's directory */\n #define PATHSPEC_SYMLINK_LEADING_PATH (1<<4)\n /*\n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319218","messageId":"20170509191805.176266-9-bmwill@google.com","threadId":"45913","inReplyTo":"20170509191805.176266-1-bmwill@google.com","subject":"[PATCH 8/8] pathspec: convert parse_pathspec to take an index","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-09T19:18:05Z","receivedAt":"2017-05-09T19:18:51Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Convert 'parse_pathspec()' to take an index parameter.\n\nSince the index is only needed when the PATHSPEC_SUBMODULE_LEADING_PATH\nand PATHSPEC_STRIP_SUBMODULE_SLASH flags are given, add a check\nrequiring a non-NULL index when either of these flags are given.\nConvert callers which use these two flags to pass in an index while\nhaving other callers pass in NULL.\n\nNow that pathspec.c does not use any cache macros and has no references\nto 'the_index', mark it by defining NO_THE_INDEX_COMPATIBILITY_MACROS.\n\nSigned-off-by: Brandon Williams \n---\n archive.c | 4 ++--\n builtin/add.c | 4 ++--\n builtin/blame.c | 2 +-\n builtin/check-ignore.c | 2 +-\n builtin/checkout.c | 2 +-\n builtin/clean.c | 2 +-\n builtin/commit.c | 4 ++--\n builtin/grep.c | 2 +-\n builtin/ls-files.c | 4 ++--\n builtin/ls-tree.c | 5 +++--\n builtin/rerere.c | 2 +-\n builtin/reset.c | 2 +-\n builtin/rm.c | 2 +-\n builtin/submodule--helper.c | 2 +-\n builtin/update-index.c | 2 +-\n line-log.c | 2 +-\n pathspec.c | 13 ++++++++++---\n pathspec.h | 1 +\n revision.c | 5 +++--\n submodule.c | 2 +-\n tree-diff.c | 2 +-\n 21 files changed, 38 insertions(+), 28 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 60b889198..ce9b30f2e 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -306,7 +306,7 @@ static int path_exists(struct tree *tree, const char *path)\n \tstruct pathspec pathspec;\n \tint ret;\n \n-\tparse_pathspec(&pathspec, 0, 0, \"\", paths);\n+\tparse_pathspec(&pathspec, NULL, 0, 0, \"\", paths);\n \tpathspec.recursive = 1;\n \tret = read_tree_recursive(tree, \"\", 0, 0, &pathspec,\n \t\t\t\t reject_entry, &pathspec);\n@@ -322,7 +322,7 @@ static void parse_pathspec_arg(const char **pathspec,\n \t * Also if pathspec patterns are dependent, we're in big\n \t * trouble as we test each one separately\n \t */\n-\tparse_pathspec(&ar_args->pathspec, 0,\n+\tparse_pathspec(&ar_args->pathspec, NULL, 0,\n \t\t PATHSPEC_PREFER_FULL,\n \t\t \"\", pathspec);\n \tar_args->pathspec.recursive = 1;\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 4e3bf20c2..23606db39 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -181,7 +181,7 @@ int interactive_add(int argc, const char **argv, const char *prefix, int patch)\n {\n \tstruct pathspec pathspec;\n \n-\tparse_pathspec(&pathspec, 0,\n+\tparse_pathspec(&pathspec, NULL, 0,\n \t\t PATHSPEC_PREFER_FULL |\n \t\t PATHSPEC_SYMLINK_LEADING_PATH |\n \t\t PATHSPEC_PREFIX_ORIGIN,\n@@ -386,7 +386,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t * Check the \"pathspec '%s' did not match any files\" block\n \t * below before enabling new magic.\n \t */\n-\tparse_pathspec(&pathspec, 0,\n+\tparse_pathspec(&pathspec, &the_index, 0,\n \t\t PATHSPEC_PREFER_FULL |\n \t\t PATHSPEC_SYMLINK_LEADING_PATH |\n \t\t PATHSPEC_STRIP_SUBMODULE_SLASH |\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex f7aa95f4b..e37837c17 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -557,7 +557,7 @@ static struct origin *find_origin(struct scoreboard *sb,\n \tpaths[0] = origin->path;\n \tpaths[1] = NULL;\n \n-\tparse_pathspec(&diff_opts.pathspec,\n+\tparse_pathspec(&diff_opts.pathspec, NULL,\n \t\t PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL,\n \t\t PATHSPEC_LITERAL_PATH, \"\", paths);\n \tdiff_setup_done(&diff_opts);\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex 446b76dcf..90169e79a 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -84,7 +84,7 @@ static int check_ignore(struct dir_struct *dir,\n \t * check-ignore just needs paths. Magic beyond :/ is really\n \t * irrelevant.\n \t */\n-\tparse_pathspec(&pathspec,\n+\tparse_pathspec(&pathspec, &the_index,\n \t\t PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP,\n \t\t PATHSPEC_SYMLINK_LEADING_PATH |\n \t\t PATHSPEC_SUBMODULE_LEADING_PATH |\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex bfa5419f3..cb8ed09f6 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1274,7 +1274,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (argc) {\n-\t\tparse_pathspec(&opts.pathspec, 0,\n+\t\tparse_pathspec(&opts.pathspec, NULL, 0,\n \t\t\t opts.patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0,\n \t\t\t prefix, argv);\n \ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex d861f836a..ebc980b4f 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -926,7 +926,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tfor (i = 0; i < exclude_list.nr; i++)\n \t\tadd_exclude(exclude_list.items[i].string, \"\", 0, el, -(i+1));\n \n-\tparse_pathspec(&pathspec, 0,\n+\tparse_pathspec(&pathspec, NULL, 0,\n \t\t PATHSPEC_PREFER_CWD,\n \t\t prefix, argv);\n \ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 4e288bc51..014193728 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -342,7 +342,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n \n \tif (is_status)\n \t\trefresh_flags |= REFRESH_UNMERGED;\n-\tparse_pathspec(&pathspec, 0,\n+\tparse_pathspec(&pathspec, NULL, 0,\n \t\t PATHSPEC_PREFER_FULL,\n \t\t prefix, argv);\n \n@@ -1373,7 +1373,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \thandle_untracked_files_arg(&s);\n \tif (show_ignored_in_status)\n \t\ts.show_ignored_files = 1;\n-\tparse_pathspec(&s.pathspec, 0,\n+\tparse_pathspec(&s.pathspec, NULL, 0,\n \t\t PATHSPEC_PREFER_FULL,\n \t\t prefix, argv);\n \ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 65070c52f..d330a85cb 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -1212,7 +1212,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\tverify_filename(prefix, argv[j], j == i && allow_revs);\n \t}\n \n-\tparse_pathspec(&pathspec, 0,\n+\tparse_pathspec(&pathspec, NULL, 0,\n \t\t PATHSPEC_PREFER_CWD |\n \t\t (opt.max_depth != -1 ? PATHSPEC_MAXDEPTH_VALID : 0),\n \t\t prefix, argv + i);\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex e9dee2e41..f5d81ab1d 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -429,7 +429,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)\n \tif (prefix) {\n \t\tstatic const char *(matchbuf[1]);\n \t\tmatchbuf[0] = NULL;\n-\t\tparse_pathspec(&pathspec, PATHSPEC_ALL_MAGIC,\n+\t\tparse_pathspec(&pathspec, NULL, PATHSPEC_ALL_MAGIC,\n \t\t\t PATHSPEC_PREFER_CWD, prefix, matchbuf);\n \t} else\n \t\tmemset(&pathspec, 0, sizeof(pathspec));\n@@ -616,7 +616,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \t\tdie(\"ls-files --recurse-submodules does not support \"\n \t\t \"--error-unmatch\");\n \n-\tparse_pathspec(&pathspec, 0,\n+\tparse_pathspec(&pathspec, &the_index, 0,\n \t\t PATHSPEC_PREFER_CWD |\n \t\t PATHSPEC_STRIP_SUBMODULE_SLASH,\n \t\t prefix, argv);\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex d7ebeb4ce..1d1083ed7 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -173,8 +173,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \t * cannot be lifted until it is converted to use\n \t * match_pathspec() or tree_entry_interesting()\n \t */\n-\tparse_pathspec(&pathspec, PATHSPEC_ALL_MAGIC &\n-\t\t\t\t ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),\n+\tparse_pathspec(&pathspec, NULL,\n+\t\t PATHSPEC_ALL_MAGIC &\n+\t\t ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),\n \t\t PATHSPEC_PREFER_CWD,\n \t\t prefix, argv + 1);\n \tfor (i = 0; i < pathspec.nr; i++)\ndiff --git a/builtin/rerere.c b/builtin/rerere.c\nindex 1bf72423b..749599a9c 100644\n--- a/builtin/rerere.c\n+++ b/builtin/rerere.c\n@@ -75,7 +75,7 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n \t\tstruct pathspec pathspec;\n \t\tif (argc < 2)\n \t\t\twarning(\"'git rerere forget' without paths is deprecated\");\n-\t\tparse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD,\n+\t\tparse_pathspec(&pathspec, NULL, 0, PATHSPEC_PREFER_CWD,\n \t\t\t prefix, argv + 1);\n \t\treturn rerere_forget(&pathspec);\n \t}\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 529f2f9be..f9da86622 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -234,7 +234,7 @@ static void parse_args(struct pathspec *pathspec,\n \tif (read_cache() < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n-\tparse_pathspec(pathspec, 0,\n+\tparse_pathspec(pathspec, &the_index, 0,\n \t\t PATHSPEC_PREFER_FULL |\n \t\t PATHSPEC_STRIP_SUBMODULE_SLASH |\n \t\t (patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0),\ndiff --git a/builtin/rm.c b/builtin/rm.c\nindex 8fe12d0a7..cffd27c20 100644\n--- a/builtin/rm.c\n+++ b/builtin/rm.c\n@@ -270,7 +270,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \tif (read_cache() < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n-\tparse_pathspec(&pathspec, 0,\n+\tparse_pathspec(&pathspec, &the_index, 0,\n \t\t PATHSPEC_PREFER_CWD |\n \t\t PATHSPEC_STRIP_SUBMODULE_SLASH,\n \t\t prefix, argv);\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 69149b557..b06d62d80 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -232,7 +232,7 @@ static int module_list_compute(int argc, const char **argv,\n {\n \tint i, result = 0;\n \tchar *ps_matched = NULL;\n-\tparse_pathspec(pathspec, 0,\n+\tparse_pathspec(pathspec, &the_index, 0,\n \t\t PATHSPEC_PREFER_FULL |\n \t\t PATHSPEC_STRIP_SUBMODULE_SLASH,\n \t\t prefix, argv);\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex ebfc09faa..0aaf7e0da 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -714,7 +714,7 @@ static int do_reupdate(int ac, const char **av,\n \tint has_head = 1;\n \tstruct pathspec pathspec;\n \n-\tparse_pathspec(&pathspec, 0,\n+\tparse_pathspec(&pathspec, NULL, 0,\n \t\t PATHSPEC_PREFER_CWD,\n \t\t prefix, av + 1);\n \ndiff --git a/line-log.c b/line-log.c\nindex a23b91047..4c3232639 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -754,7 +754,7 @@ void line_log_init(struct rev_info *rev, const char *prefix, struct string_list\n \t\t\targv_array_push(&array, r->path);\n \t\tpaths = argv_array_detach(&array);\n \n-\t\tparse_pathspec(&rev->diffopt.pathspec, 0,\n+\t\tparse_pathspec(&rev->diffopt.pathspec, NULL, 0,\n \t\t\t PATHSPEC_PREFER_FULL, \"\", paths);\n \t\t/* strings are now owned by pathspec */\n \t\tfree(paths);\ndiff --git a/pathspec.c b/pathspec.c\nindex c7ed8b3fb..b9c87b8d9 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -1,3 +1,4 @@\n+#define NO_THE_INDEX_COMPATIBILITY_MACROS\n #include \"cache.h\"\n #include \"dir.h\"\n #include \"pathspec.h\"\n@@ -573,6 +574,7 @@ static void NORETURN unsupported_magic(const char *pattern,\n * pathspec. die() if any magic in magic_mask is used.\n */\n void parse_pathspec(struct pathspec *pathspec,\n+\t\t const struct index_state *istate,\n \t\t unsigned magic_mask, unsigned flags,\n \t\t const char *prefix, const char **argv)\n {\n@@ -593,6 +595,11 @@ void parse_pathspec(struct pathspec *pathspec,\n \t (flags & PATHSPEC_PREFER_FULL))\n \t\tdie(\"BUG: PATHSPEC_PREFER_CWD and PATHSPEC_PREFER_FULL are incompatible\");\n \n+\tif (!istate && ((flags & PATHSPEC_STRIP_SUBMODULE_SLASH) ||\n+\t\t\t(flags & PATHSPEC_SUBMODULE_LEADING_PATH)))\n+\t die(\"BUG: PATHSPEC_STRIP_SUBMODULE_SLASH and \"\n+\t\t\"PATHSPEC_SUBMODULE_LEADING_PATH require an index\");\n+\n \t/* No arguments with prefix -> prefix pathspec */\n \tif (!entry) {\n \t\tif (flags & PATHSPEC_PREFER_FULL)\n@@ -629,7 +636,7 @@ void parse_pathspec(struct pathspec *pathspec,\n \tfor (i = 0; i < n; i++) {\n \t\tentry = argv[i];\n \n-\t\tinit_pathspec_item(item + i, &the_index, flags,\n+\t\tinit_pathspec_item(item + i, istate, flags,\n \t\t\t\t prefix, prefixlen, entry);\n \n \t\tif (item[i].magic & PATHSPEC_EXCLUDE)\n@@ -643,7 +650,7 @@ void parse_pathspec(struct pathspec *pathspec,\n \t\t}\n \n \t\tif (flags & PATHSPEC_SUBMODULE_LEADING_PATH)\n-\t\t\tdie_path_inside_submodule(item + i, &the_index);\n+\t\t\tdie_path_inside_submodule(item + i, istate);\n \n \t\tif (item[i].nowildcard_len < item[i].len)\n \t\t\tpathspec->has_wildcard = 1;\n@@ -656,7 +663,7 @@ void parse_pathspec(struct pathspec *pathspec,\n \t */\n \tif (nr_exclude == n) {\n \t\tint plen = (!(flags & PATHSPEC_PREFER_CWD)) ? 0 : prefixlen;\n-\t\tinit_pathspec_item(item + n, &the_index, 0, prefix, plen, \"\");\n+\t\tinit_pathspec_item(item + n, istate, 0, prefix, plen, \"\");\n \t\tpathspec->nr++;\n \t}\n \ndiff --git a/pathspec.h b/pathspec.h\nindex dfb22440a..4d1007572 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -78,6 +78,7 @@ struct pathspec {\n #define PATHSPEC_LITERAL_PATH (1<<8)\n \n extern void parse_pathspec(struct pathspec *pathspec,\n+\t\t\t const struct index_state *istate,\n \t\t\t unsigned magic_mask,\n \t\t\t unsigned flags,\n \t\t\t const char *prefix,\ndiff --git a/revision.c b/revision.c\nindex 7ff61ff5f..6efdb28ac 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1424,7 +1424,8 @@ static void prepare_show_merge(struct rev_info *revs)\n \t\t\ti++;\n \t}\n \tclear_pathspec(&revs->prune_data);\n-\tparse_pathspec(&revs->prune_data, PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL,\n+\tparse_pathspec(&revs->prune_data, NULL,\n+\t\t PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL,\n \t\t PATHSPEC_PREFER_FULL | PATHSPEC_LITERAL_PATH, \"\", prune);\n \trevs->limited = 1;\n }\n@@ -2276,7 +2277,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t */\n \t\tALLOC_GROW(prune_data.path, prune_data.nr + 1, prune_data.alloc);\n \t\tprune_data.path[prune_data.nr++] = NULL;\n-\t\tparse_pathspec(&revs->prune_data, 0, 0,\n+\t\tparse_pathspec(&revs->prune_data, NULL, 0, 0,\n \t\t\t revs->prefix, prune_data.path);\n \t}\n \ndiff --git a/submodule.c b/submodule.c\nindex 9e0502f25..ba9db9714 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -252,7 +252,7 @@ int is_submodule_initialized(const char *path)\n \t\t\targv_array_push(&args, item->string);\n \t\t}\n \n-\t\tparse_pathspec(&ps, 0, 0, NULL, args.argv);\n+\t\tparse_pathspec(&ps, NULL, 0, 0, NULL, args.argv);\n \t\tret = match_pathspec(&ps, path, strlen(path), 0, NULL, 1);\n \n \t\targv_array_clear(&args);\ndiff --git a/tree-diff.c b/tree-diff.c\nindex e164e532b..58c66b5bc 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -641,7 +641,7 @@ static void try_to_follow_renames(const unsigned char *old, const unsigned char\n \t\t\tpath[0] = p->one->path;\n \t\t\tpath[1] = NULL;\n \t\t\tclear_pathspec(&opt->pathspec);\n-\t\t\tparse_pathspec(&opt->pathspec,\n+\t\t\tparse_pathspec(&opt->pathspec, NULL,\n \t\t\t\t PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL,\n \t\t\t\t PATHSPEC_LITERAL_PATH, \"\", path);\n \n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319255","messageId":"xmqqefvxxnpq.fsf@gitster.mtv.corp.google.com","threadId":"45913","inReplyTo":"20170509191805.176266-4-bmwill@google.com","subject":"Re: [PATCH 3/8] pathspec: change PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-10T05:52:49Z","receivedAt":"2017-05-10T05:52:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Williams writes:\n\n> It's confusing to have two different 'strip submodule slash' flags which\n> do subtly different things. Both\n> PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE and\n> PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP will accomplish the same task of\n> striping a slash from a pathspec which matches a submodule entry in the\n> index. The only difference is that\n> PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE will perform additional checks\n> and die if a pathspec has a leading path component which corresponds to\n> a submodule. This additional functionality should be split out into its\n> own flag.\n>\n> To this end, rename the PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag to\n> PATHSPEC_SUBMODULE_LEADING_PATH and change its behavior to only die if a\n> path descends into a submodule. In addition add the\n> PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag to callers which relied on the\n> old slash stripping functionality.\n\n\"PATHSPEC_SUBMODULE_LEADING_PATH\" feels like an unfinished sentence\nto me. Do I understand your description correctly if I say it is\nabout \"checking\" the leading path to see if it overlaps with a\nsubmodule path? IOW, I am wondering if the name should have the\nword CHECK somewhere in it.\n\n"},{"id":"319256","messageId":"xmqqa86lxnoj.fsf@gitster.mtv.corp.google.com","threadId":"45913","inReplyTo":"20170509191805.176266-5-bmwill@google.com","subject":"Re: [PATCH 4/8] pathspec: rename PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-10T05:53:32Z","receivedAt":"2017-05-10T05:53:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Williams writes:\n\n> Now that there is only a single flag which strips a submodule slash,\n> rename PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP to\n> PATHSPEC_STRIP_SUBMODULE_SLASH.\n\nThis is a logical follow-up to the previous step.\n\n\n"},{"id":"319257","messageId":"xmqq60h9xnkp.fsf@gitster.mtv.corp.google.com","threadId":"45913","inReplyTo":"20170509191805.176266-6-bmwill@google.com","subject":"Re: [PATCH 5/8] pathspec: convert strip_submodule_slash to take an index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-10T05:55:50Z","receivedAt":"2017-05-10T05:55:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Williams writes:\n\n> Signed-off-by: Brandon Williams \n> ---\n> pathspec.c | 9 +++++----\n> 1 file changed, 5 insertions(+), 4 deletions(-)\n\nThis conversion is not wrong per-se, but I wonder if there is a\npractical situation where we want to validate a pathspec element\nagainst an in-core index that does not represent the index currently\nin effect (aka \"the_index\"). I do not think of any offhand myself.\n"},{"id":"319258","messageId":"xmqq1srxxn72.fsf@gitster.mtv.corp.google.com","threadId":"45913","inReplyTo":"20170509191805.176266-9-bmwill@google.com","subject":"Re: [PATCH 8/8] pathspec: convert parse_pathspec to take an index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-10T06:04:01Z","receivedAt":"2017-05-10T06:04:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Williams writes:\n\n> Convert 'parse_pathspec()' to take an index parameter.\n>\n> Since the index is only needed when the PATHSPEC_SUBMODULE_LEADING_PATH\n> and PATHSPEC_STRIP_SUBMODULE_SLASH flags are given, add a check\n> requiring a non-NULL index when either of these flags are given.\n> Convert callers which use these two flags to pass in an index while\n> having other callers pass in NULL.\n>\n> Now that pathspec.c does not use any cache macros and has no references\n> to 'the_index', mark it by defining NO_THE_INDEX_COMPATIBILITY_MACROS.\n>\n> Signed-off-by: Brandon Williams \n\nThe same comment as 5/8 applies to this change, but it is a bit\neasier to judge, because it has so many callers, and for some\nbuiltins, especially manipulator commands like add, checkout, and\ncommit, there may be a good reason why they want to keep the primary\nindex while playing with an additional in-core index in a distant\nfuture.\n\nDoes a pathspec parsed using one instance of index_state expected to\nwork when matching against a path in another instance of index_state?\nOtherwise, passing a non-NULL istate to parse_pathspec() would tie\nthe resulting pathspec to a particular index_state in some way and\nthere may need a mechanism to catch an attempt to match paths in\nanother index_state with such a pathspec as an error. Just\nspeculating out loud...\n\n"},{"id":"319277","messageId":"20170510161633.GA41649@google.com","threadId":"45913","inReplyTo":"xmqqefvxxnpq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/8] pathspec: change PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-10T16:16:33Z","receivedAt":"2017-05-10T16:16:41Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 05/10, Junio C Hamano wrote:\n> Brandon Williams writes:\n> \n> > It's confusing to have two different 'strip submodule slash' flags which\n> > do subtly different things. Both\n> > PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE and\n> > PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP will accomplish the same task of\n> > striping a slash from a pathspec which matches a submodule entry in the\n> > index. The only difference is that\n> > PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE will perform additional checks\n> > and die if a pathspec has a leading path component which corresponds to\n> > a submodule. This additional functionality should be split out into its\n> > own flag.\n> >\n> > To this end, rename the PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag to\n> > PATHSPEC_SUBMODULE_LEADING_PATH and change its behavior to only die if a\n> > path descends into a submodule. In addition add the\n> > PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag to callers which relied on the\n> > old slash stripping functionality.\n> \n> \"PATHSPEC_SUBMODULE_LEADING_PATH\" feels like an unfinished sentence\n> to me. Do I understand your description correctly if I say it is\n> about \"checking\" the leading path to see if it overlaps with a\n> submodule path? IOW, I am wondering if the name should have the\n> word CHECK somewhere in it.\n> \n\nYou're probably right, the name has something left to be desired. I\nchose it simply because it conforms to another flag\n\"PATHSPEC_SYMLINK_LEADING_PATH\" which dies if there is a symlink in the\nleading path.\n\nAnd you are correct, it checks if the leading path overlaps with a\nsubmodule path. If it strictly matches a submodule path that's alright\nthough. The point of this flag is to disallow paths which descend into\nsubmodules. One such use case is to prevent a user from trying to 'git\nadd' a file from a submodule.\n\n-- \nBrandon Williams\n"},{"id":"319281","messageId":"20170510170226.GB41649@google.com","threadId":"45913","inReplyTo":"xmqq1srxxn72.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 8/8] pathspec: convert parse_pathspec to take an index","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-10T17:02:26Z","receivedAt":"2017-05-10T17:02:34Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 05/10, Junio C Hamano wrote:\n> Brandon Williams writes:\n> \n> > Convert 'parse_pathspec()' to take an index parameter.\n> >\n> > Since the index is only needed when the PATHSPEC_SUBMODULE_LEADING_PATH\n> > and PATHSPEC_STRIP_SUBMODULE_SLASH flags are given, add a check\n> > requiring a non-NULL index when either of these flags are given.\n> > Convert callers which use these two flags to pass in an index while\n> > having other callers pass in NULL.\n> >\n> > Now that pathspec.c does not use any cache macros and has no references\n> > to 'the_index', mark it by defining NO_THE_INDEX_COMPATIBILITY_MACROS.\n> >\n> > Signed-off-by: Brandon Williams \n> \n> The same comment as 5/8 applies to this change, but it is a bit\n> easier to judge, because it has so many callers, and for some\n> builtins, especially manipulator commands like add, checkout, and\n> commit, there may be a good reason why they want to keep the primary\n> index while playing with an additional in-core index in a distant\n> future.\n> \n> Does a pathspec parsed using one instance of index_state expected to\n> work when matching against a path in another instance of index_state?\n> Otherwise, passing a non-NULL istate to parse_pathspec() would tie\n> the resulting pathspec to a particular index_state in some way and\n> there may need a mechanism to catch an attempt to match paths in\n> another index_state with such a pathspec as an error. Just\n> speculating out loud...\n> \n\nCurrently I don't believe this links a pathspec with a particular\nindex_state since an index is really just used to do some pre-processing\non the paths (check if the path goes into a submodule and die, or strip\na slash if the path matches a submodule), though I can see a future where\nthis is true.\n\nI did have another version of this series where I completely removed the\ntwo flags related to submodules. Since builtin/add is the only caller\nwhich needs to die if a path descends into a submodule, I had a function\nwhich did this after the parse_pathspec() call. Also, since (ae8d08242\npathspec: pass directory indicator to match_pathspec_item()) stripping\noff the slash from a submodule path really is no longer needed for the\npath matching logic, this means that we could potentially just drop the\nstrip slash flag. The only caveat is ls-files.\n\nls-files is the only command (that I know of) which does cache pruning\nbased on the common prefix of all pathspecs given. If there is a\nsubmodule named 'sub' and you provided a pathspec 'sub/', the matching\nlogic can handle this but the cache pruning logic would prune all\nentries from the index which don't have a leading 'sub/' which is the\ncommon prefix of all pathspecs (if you didn't strip off the slash).\nMeaning you'd prune the submodule 'sub' when you really didn't want to.\nThis could probably be fixed to have the cache pruning logic to prune by\nignoring the trailing slash always.\n\nSo there's another option here if you don't feel quite right about\npiping through an index into parse_pathspec just yet.\n\n-- \nBrandon Williams\n"},{"id":"319309","messageId":"xmqqh90sw4dr.fsf@gitster.mtv.corp.google.com","threadId":"45913","inReplyTo":"20170510170226.GB41649@google.com","subject":"Re: [PATCH 8/8] pathspec: convert parse_pathspec to take an index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-11T01:48:00Z","receivedAt":"2017-05-11T01:48:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Williams writes:\n\n> ls-files is the only command (that I know of) which does cache pruning\n> based on the common prefix of all pathspecs given. If there is a\n> submodule named 'sub' and you provided a pathspec 'sub/', the matching\n> logic can handle this but the cache pruning logic would prune all\n> entries from the index which don't have a leading 'sub/' which is the\n> common prefix of all pathspecs (if you didn't strip off the slash).\n> Meaning you'd prune the submodule 'sub' when you really didn't want to.\n> This could probably be fixed to have the cache pruning logic to prune by\n> ignoring the trailing slash always.\n>\n> So there's another option here if you don't feel quite right about\n> piping through an index into parse_pathspec just yet.\n\nOh, don't get me wrong---I do not think passing an index_state\ninstance throughout the callchain (and perhaps later we may pass an\ninstance of \"repository\" instead) is a wrong move in the longer term\nfor various APIs. I was just wondering if we have callers that can\nbenefit from this change immediately---manipulators like \"commit\" do\nalready use multiple instances of index_state structure.\n\nBut perhaps you are right---it may be wrong for the contents of the\ncurrent index (or any index) to affect how a pathspec element is\nparsed in the first place. If the current code (before this series)\nuses the_index only for error checking, we may want to separate that\nout of the parse_pathspec() callchain, so that it does not even look\nat any index (not just the_index). And that may be a better change\noverall.\n\nThanks.\n\n"},{"id":"319318","messageId":"dab1beaf-ca8d-2f3f-9fcc-22e6938a868a@kdbg.org","threadId":"45913","inReplyTo":"xmqqh90sw4dr.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 8/8] pathspec: convert parse_pathspec to take an index","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-05-11T05:04:46Z","receivedAt":"2017-05-11T05:04:55Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11.05.2017 um 03:48 schrieb Junio C Hamano:\n> But perhaps you are right---it may be wrong for the contents of the\n> current index (or any index) to affect how a pathspec element is\n> parsed in the first place. If the current code (before this series)\n> uses the_index only for error checking, we may want to separate that\n> out of the parse_pathspec() callchain, so that it does not even look\n> at any index (not just the_index). And that may be a better change\n> overall.\n\nJust a reminder: if core.ignoreCase is set, the variant of a path in the \nindex takes precedence over the variant found in the working tree. \nHence, pathspec must be matched against the index that corresponds to \nthe working tree. I guess that's the_index.\n\n-- Hannes\n"},{"id":"319319","messageId":"xmqqo9v0ascy.fsf@gitster.mtv.corp.google.com","threadId":"45913","inReplyTo":"dab1beaf-ca8d-2f3f-9fcc-22e6938a868a@kdbg.org","subject":"Re: [PATCH 8/8] pathspec: convert parse_pathspec to take an index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-11T05:13:17Z","receivedAt":"2017-05-11T05:13:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 11.05.2017 um 03:48 schrieb Junio C Hamano:\n>> But perhaps you are right---it may be wrong for the contents of the\n>> current index (or any index) to affect how a pathspec element is\n>> parsed in the first place. If the current code (before this series)\n>> uses the_index only for error checking, we may want to separate that\n>> out of the parse_pathspec() callchain, so that it does not even look\n>> at any index (not just the_index). And that may be a better change\n>> overall.\n>\n> Just a reminder: if core.ignoreCase is set, the variant of a path in\n> the index takes precedence over the variant found in the working\n> tree. Hence, pathspec must be matched against the index that\n> corresponds to the working tree. I guess that's the_index.\n\nYes, but that is what happens after a path from a working tree is\nfound to match a pathspec, to coerse its case into the one that is\nin the current index. The parse_pathspec() thing we are discussing\nwould have finished looooong time before that actual \"matching\" is\nattempted.\n\nThanks.\n"},{"id":"319425","messageId":"20170511173605.GB83655@google.com","threadId":"45913","inReplyTo":"xmqqh90sw4dr.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 8/8] pathspec: convert parse_pathspec to take an index","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-11T17:36:05Z","receivedAt":"2017-05-11T17:36:15Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 05/11, Junio C Hamano wrote:\n> Brandon Williams writes:\n> \n> > ls-files is the only command (that I know of) which does cache pruning\n> > based on the common prefix of all pathspecs given. If there is a\n> > submodule named 'sub' and you provided a pathspec 'sub/', the matching\n> > logic can handle this but the cache pruning logic would prune all\n> > entries from the index which don't have a leading 'sub/' which is the\n> > common prefix of all pathspecs (if you didn't strip off the slash).\n> > Meaning you'd prune the submodule 'sub' when you really didn't want to.\n> > This could probably be fixed to have the cache pruning logic to prune by\n> > ignoring the trailing slash always.\n> >\n> > So there's another option here if you don't feel quite right about\n> > piping through an index into parse_pathspec just yet.\n> \n> Oh, don't get me wrong---I do not think passing an index_state\n> instance throughout the callchain (and perhaps later we may pass an\n> instance of \"repository\" instead) is a wrong move in the longer term\n> for various APIs. I was just wondering if we have callers that can\n> benefit from this change immediately---manipulators like \"commit\" do\n> already use multiple instances of index_state structure.\n\nI didn't get the feeling you thought it was a bad change. I really\nappreciate your thoughts since they are things which I also was\nwondering about.\n\n> But perhaps you are right---it may be wrong for the contents of the\n> current index (or any index) to affect how a pathspec element is\n> parsed in the first place. If the current code (before this series)\n> uses the_index only for error checking, we may want to separate that\n> out of the parse_pathspec() callchain, so that it does not even look\n> at any index (not just the_index). And that may be a better change\n> overall.\n\nI'll polish up the other version of the series which does the processing\n(using an index) outside of parse_pathspec() and let you see how you\nfeel about those changes. Note that if we go with the route to not pass\nin an index now, it doesn't necessarily mean that down the line we won't\nhave to pass a 'repository' instance into parse_pathspec(). Just food\nfor thought.\n\n-- \nBrandon Williams\n"},{"id":"319476","messageId":"20170511220427.192627-1-bmwill@google.com","threadId":"45913","inReplyTo":"20170509191805.176266-1-bmwill@google.com","subject":"[PATCH v2 0/6] convert pathspec.c to take an index parameter","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-11T22:04:21Z","receivedAt":"2017-05-11T22:04:42Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"The main difference in v2 is that instead of piping through an index_state\nstruct into parse_pathspec, I ripped out the logic that needed to access the\nindex and either removed it completely if it wasn't needed anymore (stripping\nsubmodule slash) or factored it out into its own function which can be called\nafter initializing a pathspec object (dying if a path descends into a\nsubmodule).\n\nBrandon Williams (6):\n pathspec: provide a more descriptive die message\n submodule: add die_in_unpopulated_submodule function\n pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag\n ls-files: prevent prune_cache from overeagerly pruning submodules\n pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP\n pathspec: convert find_pathspecs_matching_against_index to take an\n index\n\n builtin/add.c | 12 ++++--\n builtin/check-ignore.c | 6 ++-\n builtin/ls-files.c | 31 ++++++++++++---\n builtin/reset.c | 1 -\n builtin/rm.c | 3 +-\n builtin/submodule--helper.c | 3 +-\n pathspec.c | 86 ++++------------------------------------\n pathspec.h | 25 +++++-------\n submodule.c | 63 +++++++++++++++++++++++++++++\n submodule.h | 4 ++\n t/t6134-pathspec-in-submodule.sh | 6 +--\n 11 files changed, 124 insertions(+), 116 deletions(-)\n\n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319477","messageId":"20170511220427.192627-3-bmwill@google.com","threadId":"45913","inReplyTo":"20170511220427.192627-1-bmwill@google.com","subject":"[PATCH v2 2/6] submodule: add die_in_unpopulated_submodule function","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-11T22:04:23Z","receivedAt":"2017-05-11T22:04:47Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Currently 'git add' is the only command which dies when launched from an\nunpopulated submodule (the place-holder directory for a submodule which\nhasn't been checked out). This is triggered implicitly by passing the\nPATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag to 'parse_pathspec()'.\n\nInstead make this desire more explicit by creating a function\n'die_in_unpopulated_submodule()' which dies if the provided 'prefix' has\na leading path component which matches a submodule in the the index.\n\nSigned-off-by: Brandon Williams \n---\n builtin/add.c | 3 +++\n pathspec.c | 29 -----------------------------\n submodule.c | 30 ++++++++++++++++++++++++++++++\n submodule.h | 2 ++\n t/t6134-pathspec-in-submodule.sh | 6 +-----\n 5 files changed, 36 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 9f53f020d..ec58e3679 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -17,6 +17,7 @@\n #include \"revision.h\"\n #include \"bulk-checkin.h\"\n #include \"argv-array.h\"\n+#include \"submodule.h\"\n \n static const char * const builtin_add_usage[] = {\n \tN_(\"git add [] [--] ...\"),\n@@ -379,6 +380,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tif (read_cache() < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n+\tdie_in_unpopulated_submodule(&the_index, prefix);\n+\n \t/*\n \t * Check the \"pathspec '%s' did not match any files\" block\n \t * below before enabling new magic.\ndiff --git a/pathspec.c b/pathspec.c\nindex 904cfb739..9b7634c5a 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -424,27 +424,6 @@ static void strip_submodule_slash_expensive(struct pathspec_item *item)\n \t}\n }\n \n-static void die_inside_submodule_path(struct pathspec_item *item)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < active_nr; i++) {\n-\t\tstruct cache_entry *ce = active_cache[i];\n-\t\tint ce_len = ce_namelen(ce);\n-\n-\t\tif (!S_ISGITLINK(ce->ce_mode))\n-\t\t\tcontinue;\n-\n-\t\tif (item->len < ce_len ||\n-\t\t !(item->match[ce_len] == '/' || item->match[ce_len] == '\\0') ||\n-\t\t memcmp(ce->name, item->match, ce_len))\n-\t\t\tcontinue;\n-\n-\t\tdie(_(\"Pathspec '%s' is in submodule '%.*s'\"),\n-\t\t item->original, ce_len, ce->name);\n-\t}\n-}\n-\n /*\n * Perform the initialization of a pathspec_item based on a pathspec element.\n */\n@@ -547,14 +526,6 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n \t/* sanity checks, pathspec matchers assume these are sane */\n \tif (item->nowildcard_len > item->len ||\n \t item->prefix > item->len) {\n-\t\t/*\n-\t\t * This case can be triggered by the user pointing us to a\n-\t\t * pathspec inside a submodule, which is an input error.\n-\t\t * Detect that here and complain, but fallback in the\n-\t\t * non-submodule case to a BUG, as we have no idea what\n-\t\t * would trigger that.\n-\t\t */\n-\t\tdie_inside_submodule_path(item);\n \t\tdie (\"BUG: error initializing pathspec_item\");\n \t}\n }\ndiff --git a/submodule.c b/submodule.c\nindex d3299e29c..885663c42 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -282,6 +282,36 @@ int is_submodule_populated_gently(const char *path, int *return_error_code)\n \treturn ret;\n }\n \n+/*\n+ * Dies if the provided 'prefix' corresponds to an unpopulated submodule\n+ */\n+void die_in_unpopulated_submodule(const struct index_state *istate,\n+\t\t\t\t const char *prefix)\n+{\n+\tint i, prefixlen;\n+\n+\tif (!prefix)\n+\t\treturn;\n+\n+\tprefixlen = strlen(prefix);\n+\n+\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = istate->cache[i];\n+\t\tint ce_len = ce_namelen(ce);\n+\n+\t\tif (!S_ISGITLINK(ce->ce_mode))\n+\t\t\tcontinue;\n+\t\tif (prefixlen <= ce_len)\n+\t\t\tcontinue;\n+\t\tif (strncmp(ce->name, prefix, ce_len))\n+\t\t\tcontinue;\n+\t\tif (prefix[ce_len] != '/')\n+\t\t\tcontinue;\n+\n+\t\tdie(_(\"in unpopulated submodule '%s'\"), ce->name);\n+\t}\n+}\n+\n int parse_submodule_update_strategy(const char *value,\n \t\tstruct submodule_update_strategy *dst)\n {\ndiff --git a/submodule.h b/submodule.h\nindex 1277480ad..d11b4da40 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -49,6 +49,8 @@ extern int is_submodule_initialized(const char *path);\n * Otherwise the return error code is the same as of resolve_gitdir_gently.\n */\n extern int is_submodule_populated_gently(const char *path, int *return_error_code);\n+extern void die_in_unpopulated_submodule(const struct index_state *istate,\n+\t\t\t\t\t const char *prefix);\n extern int parse_submodule_update_strategy(const char *value,\n \t\tstruct submodule_update_strategy *dst);\n extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);\ndiff --git a/t/t6134-pathspec-in-submodule.sh b/t/t6134-pathspec-in-submodule.sh\nindex fd401ca60..0f1cb49ce 100755\n--- a/t/t6134-pathspec-in-submodule.sh\n+++ b/t/t6134-pathspec-in-submodule.sh\n@@ -24,13 +24,9 @@ test_expect_success 'error message for path inside submodule' '\n \ttest_cmp expect actual\n '\n \n-cat <expect\n-fatal: Pathspec '.' is in submodule 'sub'\n-EOF\n-\n test_expect_success 'error message for path inside submodule from within submodule' '\n \ttest_must_fail git -C sub add . 2>actual &&\n-\ttest_cmp expect actual\n+\ttest_i18ngrep \"in unpopulated submodule\" actual\n '\n \n test_done\n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319478","messageId":"20170511220427.192627-5-bmwill@google.com","threadId":"45913","inReplyTo":"20170511220427.192627-1-bmwill@google.com","subject":"[PATCH v2 4/6] ls-files: prevent prune_cache from overeagerly pruning submodules","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-11T22:04:25Z","receivedAt":"2017-05-11T22:04:53Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Since (ae8d08242 pathspec: pass directory indicator to\nmatch_pathspec_item()) the path matching logic has been able to cope\nwith submodules without needing to strip off a trailing slash if a path\nrefers to a submodule.\n\nls-files is the only caller of 'parse_pathspec()' which relies on the\nbehavior of the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag because it\nuses the result to construct a common prefix of all provided pathspecs\nwhich is then used to prune the index of all entries which don't have\nthat prefix. Since submodules entries in the index don't have a\ntrailing slash 'prune_cache()' will be overeager and prune a submodule\n'sub' if the common prefix is 'sub/'. To correct this behavior, only\nprune entries which don't match up to, but not including, a trailing\nslash of the common prefix.\n\nThis is in preparation to remove the\nPATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag in a later patch.\n\nSigned-off-by: Brandon Williams \n---\n builtin/ls-files.c | 31 +++++++++++++++++++++++++------\n 1 file changed, 25 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex a6c70dbe9..1f3d47844 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -97,7 +97,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)\n {\n \tint len = max_prefix_len;\n \n-\tif (len >= ent->len)\n+\tif (len > ent->len)\n \t\tdie(\"git ls-files: internal error - directory entry not superset of prefix\");\n \n \tif (!dir_path_match(ent, &pathspec, len, ps_matched))\n@@ -238,7 +238,7 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)\n \t\tstrbuf_addstr(&name, super_prefix);\n \tstrbuf_addstr(&name, ce->name);\n \n-\tif (len >= ce_namelen(ce))\n+\tif (len > ce_namelen(ce))\n \t\tdie(\"git ls-files: internal error - cache entry not superset of prefix\");\n \n \tif (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&\n@@ -403,6 +403,25 @@ static void prune_cache(const char *prefix, size_t prefixlen)\n \tactive_nr = last - pos;\n }\n \n+static int get_common_prefix_len(const char *common_prefix)\n+{\n+\tint common_prefix_len;\n+\n+\tif (!common_prefix)\n+\t\treturn 0;\n+\n+\tcommon_prefix_len = strlen(common_prefix);\n+\n+\t/*\n+\t * If the prefix has a trailing slash, strip it so that submodules wont\n+\t * be pruned from the index.\n+\t */\n+\tif (common_prefix[common_prefix_len - 1] == '/')\n+\t\tcommon_prefix_len--;\n+\n+\treturn common_prefix_len;\n+}\n+\n /*\n * Read the tree specified with --with-tree option\n * (typically, HEAD) into stage #1 and then\n@@ -624,8 +643,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \t\t \"--error-unmatch\");\n \n \tparse_pathspec(&pathspec, 0,\n-\t\t PATHSPEC_PREFER_CWD |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,\n+\t\t PATHSPEC_PREFER_CWD,\n \t\t prefix, argv);\n \n \t/*\n@@ -637,7 +655,9 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \t\tmax_prefix = NULL;\n \telse\n \t\tmax_prefix = common_prefix(&pathspec);\n-\tmax_prefix_len = max_prefix ? strlen(max_prefix) : 0;\n+\tmax_prefix_len = get_common_prefix_len(max_prefix);\n+\n+\tprune_cache(max_prefix, max_prefix_len);\n \n \t/* Treat unmatching pathspec elements as errors */\n \tif (pathspec.nr && error_unmatch)\n@@ -651,7 +671,6 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \t show_killed || show_modified || show_resolve_undo))\n \t\tshow_cached = 1;\n \n-\tprune_cache(max_prefix, max_prefix_len);\n \tif (with_tree) {\n \t\t/*\n \t\t * Basic sanity check; show-stages and show-unmerged\n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319479","messageId":"20170511220427.192627-4-bmwill@google.com","threadId":"45913","inReplyTo":"20170511220427.192627-1-bmwill@google.com","subject":"[PATCH v2 3/6] pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-11T22:04:24Z","receivedAt":"2017-05-11T22:04:58Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Since (ae8d08242 pathspec: pass directory indicator to\nmatch_pathspec_item()) the path matching logic has been able to cope\nwith submodules without needing to strip off a trailing slash if a path\nrefers to a submodule.\n\nSince the stripping the trailing slash is no longer necessary, remove\nthe PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag. In addition, factor\nout the logic which dies if a path decends into a submodule so that it\ncan still be used as a check after a pathspec struct has been\ninitialized.\n\nSigned-off-by: Brandon Williams \n---\n builtin/add.c | 5 +++--\n builtin/check-ignore.c | 4 +++-\n pathspec.c | 29 -----------------------------\n pathspec.h | 14 +++-----------\n submodule.c | 33 +++++++++++++++++++++++++++++++++\n submodule.h | 2 ++\n 6 files changed, 44 insertions(+), 43 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ec58e3679..86770d6af 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -388,10 +388,11 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t */\n \tparse_pathspec(&pathspec, 0,\n \t\t PATHSPEC_PREFER_FULL |\n-\t\t PATHSPEC_SYMLINK_LEADING_PATH |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE,\n+\t\t PATHSPEC_SYMLINK_LEADING_PATH,\n \t\t prefix, argv);\n \n+\tdie_path_inside_submodule(&the_index, &pathspec);\n+\n \tif (add_new_files) {\n \t\tint baselen;\n \ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex 1d73d3ca3..91040a4b0 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -4,6 +4,7 @@\n #include \"quote.h\"\n #include \"pathspec.h\"\n #include \"parse-options.h\"\n+#include \"submodule.h\"\n \n static int quiet, verbose, stdin_paths, show_non_matching, no_index;\n static const char * const check_ignore_usage[] = {\n@@ -87,10 +88,11 @@ static int check_ignore(struct dir_struct *dir,\n \tparse_pathspec(&pathspec,\n \t\t PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP,\n \t\t PATHSPEC_SYMLINK_LEADING_PATH |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE |\n \t\t PATHSPEC_KEEP_ORDER,\n \t\t prefix, argv);\n \n+\tdie_path_inside_submodule(&the_index, &pathspec);\n+\n \t/*\n \t * look for pathspecs matching entries in the index, since these\n \t * should not be ignored, in order to be consistent with\ndiff --git a/pathspec.c b/pathspec.c\nindex 9b7634c5a..e42431278 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -398,32 +398,6 @@ static void strip_submodule_slash_cheap(struct pathspec_item *item)\n \t}\n }\n \n-static void strip_submodule_slash_expensive(struct pathspec_item *item)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < active_nr; i++) {\n-\t\tstruct cache_entry *ce = active_cache[i];\n-\t\tint ce_len = ce_namelen(ce);\n-\n-\t\tif (!S_ISGITLINK(ce->ce_mode))\n-\t\t\tcontinue;\n-\n-\t\tif (item->len <= ce_len || item->match[ce_len] != '/' ||\n-\t\t memcmp(ce->name, item->match, ce_len))\n-\t\t\tcontinue;\n-\n-\t\tif (item->len == ce_len + 1) {\n-\t\t\t/* strip trailing slash */\n-\t\t\titem->len--;\n-\t\t\titem->match[item->len] = '\\0';\n-\t\t} else {\n-\t\t\tdie(_(\"Pathspec '%s' is in submodule '%.*s'\"),\n-\t\t\t item->original, ce_len, ce->name);\n-\t\t}\n-\t}\n-}\n-\n /*\n * Perform the initialization of a pathspec_item based on a pathspec element.\n */\n@@ -499,9 +473,6 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n \tif (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)\n \t\tstrip_submodule_slash_cheap(item);\n \n-\tif (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)\n-\t\tstrip_submodule_slash_expensive(item);\n-\n \tif (magic & PATHSPEC_LITERAL) {\n \t\titem->nowildcard_len = item->len;\n \t} else {\ndiff --git a/pathspec.h b/pathspec.h\nindex 55e976972..3729efa85 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -62,23 +62,15 @@ struct pathspec {\n #define PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP (1<<3)\n /* die if a symlink is part of the given path's directory */\n #define PATHSPEC_SYMLINK_LEADING_PATH (1<<4)\n-/*\n- * This is like a combination of ..LEADING_PATH and .._SLASH_CHEAP\n- * (but not the same): it strips the trailing slash if the given path\n- * is a gitlink but also checks and dies if gitlink is part of the\n- * leading path (i.e. the given path goes beyond a submodule). It's\n- * safer than _SLASH_CHEAP and also more expensive.\n- */\n-#define PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE (1<<5)\n-#define PATHSPEC_PREFIX_ORIGIN (1<<6)\n-#define PATHSPEC_KEEP_ORDER (1<<7)\n+#define PATHSPEC_PREFIX_ORIGIN (1<<5)\n+#define PATHSPEC_KEEP_ORDER (1<<6)\n /*\n * For the callers that just need pure paths from somewhere else, not\n * from command line. Global --*-pathspecs options are ignored. No\n * magic is parsed in each pathspec either. If PATHSPEC_LITERAL is\n * allowed, then it will automatically set for every pathspec.\n */\n-#define PATHSPEC_LITERAL_PATH (1<<8)\n+#define PATHSPEC_LITERAL_PATH (1<<7)\n \n extern void parse_pathspec(struct pathspec *pathspec,\n \t\t\t unsigned magic_mask,\ndiff --git a/submodule.c b/submodule.c\nindex 885663c42..80851d044 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -312,6 +312,39 @@ void die_in_unpopulated_submodule(const struct index_state *istate,\n \t}\n }\n \n+/*\n+ * Dies if any paths in the provided pathspec descends into a submodule\n+ */\n+void die_path_inside_submodule(const struct index_state *istate,\n+\t\t\t const struct pathspec *ps)\n+{\n+\tint i, j;\n+\n+\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = istate->cache[i];\n+\t\tint ce_len = ce_namelen(ce);\n+\n+\t\tif (!S_ISGITLINK(ce->ce_mode))\n+\t\t\tcontinue;\n+\n+\t\tfor (j = 0; j < ps->nr ; j++) {\n+\t\t\tconst struct pathspec_item *item = &ps->items[j];\n+\n+\t\t\tif (item->len <= ce_len)\n+\t\t\t\tcontinue;\n+\t\t\tif (item->match[ce_len] != '/')\n+\t\t\t\tcontinue;\n+\t\t\tif (strncmp(ce->name, item->match, ce_len))\n+\t\t\t\tcontinue;\n+\t\t\tif (item->len == ce_len + 1)\n+\t\t\t\tcontinue;\n+\n+\t\t\tdie(_(\"Pathspec '%s' is in submodule '%.*s'\"),\n+\t\t\t item->original, ce_len, ce->name);\n+\t\t}\n+\t}\n+}\n+\n int parse_submodule_update_strategy(const char *value,\n \t\tstruct submodule_update_strategy *dst)\n {\ndiff --git a/submodule.h b/submodule.h\nindex d11b4da40..266d81f1c 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -51,6 +51,8 @@ extern int is_submodule_initialized(const char *path);\n extern int is_submodule_populated_gently(const char *path, int *return_error_code);\n extern void die_in_unpopulated_submodule(const struct index_state *istate,\n \t\t\t\t\t const char *prefix);\n+extern void die_path_inside_submodule(const struct index_state *istate,\n+\t\t\t\t const struct pathspec *ps);\n extern int parse_submodule_update_strategy(const char *value,\n \t\tstruct submodule_update_strategy *dst);\n extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);\n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319480","messageId":"20170511220427.192627-7-bmwill@google.com","threadId":"45913","inReplyTo":"20170511220427.192627-1-bmwill@google.com","subject":"[PATCH v2 6/6] pathspec: convert find_pathspecs_matching_against_index to take an index","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-11T22:04:27Z","receivedAt":"2017-05-11T22:05:02Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Convert find_pathspecs_matching_against_index to take an index\nparameter.\n\nIn addition mark pathspec.c with NO_THE_INDEX_COMPATIBILITY_MACROS now\nthat it doesn't use any cache macros or reference 'the_index'.\n\nSigned-off-by: Brandon Williams \n---\n builtin/add.c | 4 ++--\n builtin/check-ignore.c | 2 +-\n pathspec.c | 11 +++++++----\n pathspec.h | 7 +++++--\n 4 files changed, 15 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 86770d6af..0365a5209 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -136,7 +136,7 @@ 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, seen);\n+\tadd_pathspec_matches_against_index(pathspec, &the_index, seen);\n \treturn seen;\n }\n \n@@ -418,7 +418,7 @@ 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);\n+\t\t\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index);\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 91040a4b0..7629018a5 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -98,7 +98,7 @@ 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);\n+\tseen = find_pathspecs_matching_against_index(&pathspec, &the_index);\n \tfor (i = 0; i < pathspec.nr; i++) {\n \t\tfull_path = pathspec.items[i].match;\n \t\texclude = NULL;\ndiff --git a/pathspec.c b/pathspec.c\nindex 1e5df2316..828405021 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -1,3 +1,4 @@\n+#define NO_THE_INDEX_COMPATIBILITY_MACROS\n #include \"cache.h\"\n #include \"dir.h\"\n #include \"pathspec.h\"\n@@ -17,6 +18,7 @@\n * to use find_pathspecs_matching_against_index() instead.\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 {\n \tint num_unmatched = 0, i;\n@@ -32,8 +34,8 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n \t\t\tnum_unmatched++;\n \tif (!num_unmatched)\n \t\treturn;\n-\tfor (i = 0; i < active_nr; i++) {\n-\t\tconst struct cache_entry *ce = active_cache[i];\n+\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\tconst struct cache_entry *ce = istate->cache[i];\n \t\tce_path_match(ce, pathspec, seen);\n \t}\n }\n@@ -46,10 +48,11 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n * nature of the \"closest\" (i.e. most specific) matches which each of the\n * given pathspecs achieves against all items in the index.\n */\n-char *find_pathspecs_matching_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 {\n \tchar *seen = xcalloc(pathspec->nr, 1);\n-\tadd_pathspec_matches_against_index(pathspec, seen);\n+\tadd_pathspec_matches_against_index(pathspec, istate, seen);\n \treturn seen;\n }\n \ndiff --git a/pathspec.h b/pathspec.h\nindex 6671b9577..60e650040 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -96,7 +96,10 @@ static inline int ps_strcmp(const struct pathspec_item *item,\n \t\treturn strcmp(s1, s2);\n }\n \n-extern char *find_pathspecs_matching_against_index(const struct pathspec *pathspec);\n-extern void add_pathspec_matches_against_index(const struct pathspec *pathspec, char *seen);\n+extern void add_pathspec_matches_against_index(const struct pathspec *pathspec,\n+\t\t\t\t\t const struct index_state *istate,\n+\t\t\t\t\t char *seen);\n+extern char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,\n+\t\t\t\t\t\t const struct index_state *istate);\n \n #endif /* PATHSPEC_H */\n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319481","messageId":"20170511220427.192627-6-bmwill@google.com","threadId":"45913","inReplyTo":"20170511220427.192627-1-bmwill@google.com","subject":"[PATCH v2 5/6] pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-11T22:04:26Z","receivedAt":"2017-05-11T22:05:03Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Since (ae8d08242 pathspec: pass directory indicator to\nmatch_pathspec_item()) the path matching logic has been able to cope\nwith submodules without needing to strip off a trailing slash if a path\nrefers to a submodule.\n\nSince stripping the slash is no longer necessary, remove the\nPATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag.\n\nSigned-off-by: Brandon Williams \n---\n builtin/reset.c | 1 -\n builtin/rm.c | 3 +--\n builtin/submodule--helper.c | 3 +--\n pathspec.c | 15 ---------------\n pathspec.h | 10 ++++------\n 5 files changed, 6 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex fc3b906c4..5db2adc4c 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -236,7 +236,6 @@ static void parse_args(struct pathspec *pathspec,\n \n \tparse_pathspec(pathspec, 0,\n \t\t PATHSPEC_PREFER_FULL |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP |\n \t\t (patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0),\n \t\t prefix, argv);\n }\ndiff --git a/builtin/rm.c b/builtin/rm.c\nindex fb79dcab1..7c323d012 100644\n--- a/builtin/rm.c\n+++ b/builtin/rm.c\n@@ -271,8 +271,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"index file corrupt\"));\n \n \tparse_pathspec(&pathspec, 0,\n-\t\t PATHSPEC_PREFER_CWD |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,\n+\t\t PATHSPEC_PREFER_CWD,\n \t\t prefix, argv);\n \trefresh_index(&the_index, REFRESH_QUIET, &pathspec, NULL, NULL);\n \ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 566a5b6a6..8cc648d85 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -233,8 +233,7 @@ static int module_list_compute(int argc, const char **argv,\n \tint i, result = 0;\n \tchar *ps_matched = NULL;\n \tparse_pathspec(pathspec, 0,\n-\t\t PATHSPEC_PREFER_FULL |\n-\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,\n+\t\t PATHSPEC_PREFER_FULL,\n \t\t prefix, argv);\n \n \tif (pathspec->nr)\ndiff --git a/pathspec.c b/pathspec.c\nindex e42431278..1e5df2316 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -386,18 +386,6 @@ static const char *parse_element_magic(unsigned *magic, int *prefix_len,\n \t\treturn parse_short_magic(magic, elem);\n }\n \n-static void strip_submodule_slash_cheap(struct pathspec_item *item)\n-{\n-\tif (item->len >= 1 && item->match[item->len - 1] == '/') {\n-\t\tint i = cache_name_pos(item->match, item->len - 1);\n-\n-\t\tif (i >= 0 && S_ISGITLINK(active_cache[i]->ce_mode)) {\n-\t\t\titem->len--;\n-\t\t\titem->match[item->len] = '\\0';\n-\t\t}\n-\t}\n-}\n-\n /*\n * Perform the initialization of a pathspec_item based on a pathspec element.\n */\n@@ -470,9 +458,6 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n \t\titem->original = xstrdup(elt);\n \t}\n \n-\tif (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)\n-\t\tstrip_submodule_slash_cheap(item);\n-\n \tif (magic & PATHSPEC_LITERAL) {\n \t\titem->nowildcard_len = item->len;\n \t} else {\ndiff --git a/pathspec.h b/pathspec.h\nindex 3729efa85..6671b9577 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -58,19 +58,17 @@ struct pathspec {\n #define PATHSPEC_PREFER_CWD (1<<0) /* No args means match cwd */\n #define PATHSPEC_PREFER_FULL (1<<1) /* No args means match everything */\n #define PATHSPEC_MAXDEPTH_VALID (1<<2) /* max_depth field is valid */\n-/* strip the trailing slash if the given path is a gitlink */\n-#define PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP (1<<3)\n /* die if a symlink is part of the given path's directory */\n-#define PATHSPEC_SYMLINK_LEADING_PATH (1<<4)\n-#define PATHSPEC_PREFIX_ORIGIN (1<<5)\n-#define PATHSPEC_KEEP_ORDER (1<<6)\n+#define PATHSPEC_SYMLINK_LEADING_PATH (1<<3)\n+#define PATHSPEC_PREFIX_ORIGIN (1<<4)\n+#define PATHSPEC_KEEP_ORDER (1<<5)\n /*\n * For the callers that just need pure paths from somewhere else, not\n * from command line. Global --*-pathspecs options are ignored. No\n * magic is parsed in each pathspec either. If PATHSPEC_LITERAL is\n * allowed, then it will automatically set for every pathspec.\n */\n-#define PATHSPEC_LITERAL_PATH (1<<7)\n+#define PATHSPEC_LITERAL_PATH (1<<6)\n \n extern void parse_pathspec(struct pathspec *pathspec,\n \t\t\t unsigned magic_mask,\n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319482","messageId":"20170511220427.192627-2-bmwill@google.com","threadId":"45913","inReplyTo":"20170511220427.192627-1-bmwill@google.com","subject":"[PATCH v2 1/6] pathspec: provide a more descriptive die message","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-11T22:04:22Z","receivedAt":"2017-05-11T22:05:05Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"The current message displayed upon an internal error in\n'init_pathspec_item()' isn't very descriptive and doesn't provide much\ncontext to where the error occurred. Update the error message to\nprovide more context to where the error occured.\n\nSigned-off-by: Brandon Williams \n---\n pathspec.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/pathspec.c b/pathspec.c\nindex 50f76fff4..904cfb739 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -555,7 +555,7 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n \t\t * would trigger that.\n \t\t */\n \t\tdie_inside_submodule_path(item);\n-\t\tdie (\"BUG: item->nowildcard_len > item->len || item->prefix > item->len)\");\n+\t\tdie (\"BUG: error initializing pathspec_item\");\n \t}\n }\n \n-- \n2.13.0.rc2.291.g57267f2277-goog\n\n"},{"id":"319491","messageId":"xmqqshka99nt.fsf@gitster.mtv.corp.google.com","threadId":"45913","inReplyTo":"20170511173605.GB83655@google.com","subject":"Re: [PATCH 8/8] pathspec: convert parse_pathspec to take an index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-12T00:54:46Z","receivedAt":"2017-05-12T00:54:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Williams writes:\n\n> ... Note that if we go with the route to not pass\n> in an index now, it doesn't necessarily mean that down the line we won't\n> have to pass a 'repository' instance into parse_pathspec().\n\nCorrect. \n\nAn attribute annotated pathspec may want to check if the attributes\nused in it are used in the repository at all for validation or\noptimization purposes, for example.\n"},{"id":"319500","messageId":"20170512172920.GB98586@google.com","threadId":"45913","inReplyTo":"xmqqh90q7ige.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/6] convert pathspec.c to take an index parameter","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-05-12T17:29:20Z","receivedAt":"2017-05-12T18:44:29Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 05/12, Junio C Hamano wrote:\n> Brandon Williams writes:\n> \n> > The main difference in v2 is that instead of piping through an index_state\n> > struct into parse_pathspec, I ripped out the logic that needed to access the\n> > index and either removed it completely if it wasn't needed anymore (stripping\n> > submodule slash) or factored it out into its own function which can be called\n> > after initializing a pathspec object (dying if a path descends into a\n> > submodule).\n> >\n> > Brandon Williams (6):\n> > pathspec: provide a more descriptive die message\n> > submodule: add die_in_unpopulated_submodule function\n> > pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag\n> > ls-files: prevent prune_cache from overeagerly pruning submodules\n> > pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP\n> > pathspec: convert find_pathspecs_matching_against_index to take an\n> > index\n> \n> This was (perhaps surprisingly) quite a pleasant read, even compared\n> to the other approach ;-).\n\nGlad you think so! Thanks!\n\n-- \nBrandon Williams\n"},{"id":"319518","messageId":"xmqqh90q7ige.fsf@gitster.mtv.corp.google.com","threadId":"45913","inReplyTo":"20170511220427.192627-1-bmwill@google.com","subject":"Re: [PATCH v2 0/6] convert pathspec.c to take an index parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-12T05:27:45Z","receivedAt":"2017-05-12T18:45:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Williams writes:\n\n> The main difference in v2 is that instead of piping through an index_state\n> struct into parse_pathspec, I ripped out the logic that needed to access the\n> index and either removed it completely if it wasn't needed anymore (stripping\n> submodule slash) or factored it out into its own function which can be called\n> after initializing a pathspec object (dying if a path descends into a\n> submodule).\n>\n> Brandon Williams (6):\n> pathspec: provide a more descriptive die message\n> submodule: add die_in_unpopulated_submodule function\n> pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag\n> ls-files: prevent prune_cache from overeagerly pruning submodules\n> pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP\n> pathspec: convert find_pathspecs_matching_against_index to take an\n> index\n\nThis was (perhaps surprisingly) quite a pleasant read, even compared\nto the other approach ;-).\n"}]}