git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 4/6] ls-files: prevent prune_cache from overeagerly pruning submodules

From
BWBrandon Williams <bmwill@google.com>
Date
May 11, 2017, 22:04 UTC
Message-ID
<20170511220427.192627-5-bmwill@google.com>
In-Reply-To
<20170511220427.192627-1-bmwill@google.com>

Since (ae8d08242 pathspec: pass directory indicator to match_pathspec_item()) the path matching logic has been able to cope with submodules without needing to strip off a trailing slash if a path refers to a submodule.

ls-files is the only caller of 'parse_pathspec()' which relies on the behavior of the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag because it uses the result to construct a common prefix of all provided pathspecs which is then used to prune the index of all entries which don't have that prefix. Since submodules entries in the index don't have a trailing slash 'prune_cache()' will be overeager and prune a submodule 'sub' if the common prefix is 'sub/'. To correct this behavior, only prune entries which don't match up to, but not including, a trailing slash of the common prefix.

This is in preparation to remove the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag in a later patch.

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/ls-files.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index a6c70dbe9..1f3d47844 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -97,7 +97,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)
 {
 	int len = max_prefix_len;
 
-	if (len >= ent->len)
+	if (len > ent->len)
 		die("git ls-files: internal error - directory entry not superset of prefix");
 
 	if (!dir_path_match(ent, &pathspec, len, ps_matched))
@@ -238,7 +238,7 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
 		strbuf_addstr(&name, super_prefix);
 	strbuf_addstr(&name, ce->name);
 
-	if (len >= ce_namelen(ce))
+	if (len > ce_namelen(ce))
 		die("git ls-files: internal error - cache entry not superset of prefix");
 
 	if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
@@ -403,6 +403,25 @@ static void prune_cache(const char *prefix, size_t prefixlen)
 	active_nr = last - pos;
 }
 
+static int get_common_prefix_len(const char *common_prefix)
+{
+	int common_prefix_len;
+
+	if (!common_prefix)
+		return 0;
+
+	common_prefix_len = strlen(common_prefix);
+
+	/*
+	 * If the prefix has a trailing slash, strip it so that submodules wont
+	 * be pruned from the index.
+	 */
+	if (common_prefix[common_prefix_len - 1] == '/')
+		common_prefix_len--;
+
+	return common_prefix_len;
+}
+
 /*
  * Read the tree specified with --with-tree option
  * (typically, HEAD) into stage #1 and then
@@ -624,8 +643,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 		    "--error-unmatch");
 
 	parse_pathspec(&pathspec, 0,
-		       PATHSPEC_PREFER_CWD |
-		       PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+		       PATHSPEC_PREFER_CWD,
 		       prefix, argv);
 
 	/*
@@ -637,7 +655,9 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 		max_prefix = NULL;
 	else
 		max_prefix = common_prefix(&pathspec);
-	max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
+	max_prefix_len = get_common_prefix_len(max_prefix);
+
+	prune_cache(max_prefix, max_prefix_len);
 
 	/* Treat unmatching pathspec elements as errors */
 	if (pathspec.nr && error_unmatch)
@@ -651,7 +671,6 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 	      show_killed || show_modified || show_resolve_undo))
 		show_cached = 1;
 
-	prune_cache(max_prefix, max_prefix_len);
 	if (with_tree) {
 		/*
 		 * Basic sanity check; show-stages and show-unmerged
-- 
2.13.0.rc2.291.g57267f2277-goog
Previous: Brandon WilliamsNext: Brandon Williams
Message 23 of 29 in “convert pathspec.c to take an index parameter”
  1. 0/8 convert pathspec.c to take an index parameterBrandon Williams, May 9, 2017
  2. 2/8 submodule: add die_in_unpopulated_submodule functionBrandon Williams, May 9, 2017
  3. 3/8 pathspec: change PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flagBrandon Williams, May 9, 2017
  4. Junio C HamanoMay 10, 2017
  5. Brandon WilliamsMay 10, 2017
  6. 1/8 pathspec: provide a more descriptive die messageBrandon Williams, May 9, 2017
  7. 5/8 pathspec: convert strip_submodule_slash to take an indexBrandon Williams, May 9, 2017
  8. Junio C HamanoMay 10, 2017
  9. 7/8 pathspec: convert init_pathspec_item to take an indexBrandon Williams, May 9, 2017
  10. 6/8 pathspec: convert find_pathspecs_matching_against_index to take an indexBrandon Williams, May 9, 2017
  11. 4/8 pathspec: rename PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAPBrandon Williams, May 9, 2017
  12. Junio C HamanoMay 10, 2017
  13. 8/8 pathspec: convert parse_pathspec to take an indexBrandon Williams, May 9, 2017
  14. Junio C HamanoMay 10, 2017
  15. Brandon WilliamsMay 10, 2017
  16. Junio C HamanoMay 11, 2017
  17. Johannes SixtMay 11, 2017
  18. Junio C HamanoMay 11, 2017
  19. Brandon WilliamsMay 11, 2017
  20. Junio C HamanoMay 12, 2017
  21. 0/6 convert pathspec.c to take an index parameterBrandon Williams, May 11, 2017
  22. 2/6 submodule: add die_in_unpopulated_submodule functionBrandon Williams, May 11, 2017
  23. 4/6 ls-files: prevent prune_cache from overeagerly pruning submodulesBrandon Williams, May 11, 2017
  24. 3/6 pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flagBrandon Williams, May 11, 2017
  25. 6/6 pathspec: convert find_pathspecs_matching_against_index to take an indexBrandon Williams, May 11, 2017
  26. 5/6 pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAPBrandon Williams, May 11, 2017
  27. 1/6 pathspec: provide a more descriptive die messageBrandon Williams, May 11, 2017
  28. Junio C HamanoMay 12, 2017
  29. Brandon WilliamsMay 12, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.