threads / rfc / 19494

RFC patchConvert ce_path_match() use to match_pathspec()

Subject: [PATCH RFC] Convert ce_path_match() use to match_pathspec()

## tl;dr

3 messages between May 25, 2009 and May 26, 2009. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Nguyễn Thái Ngọc Duy· May 25, 2009, 08:42 UTC · lore

Back in history, ce_path_match() was first introduced in commit fdee7d07ba6c79b3e5125e96adbe1d9c3e75ce1d, in diff-cache.c. It seems to be used to handle pathspec that we have today. But it did not support wildcards.

About one year later, match_pathspec() was introduced as match() in commit 0d78153952e70c21e94dc6b7eefcb2ac5337a902, builtin-add.c. This version supported wildcards.

For some reasons diff code did not get converted to use match_pathspec(). So diff commands do not understand wildcards. I was not here that time to know the reasons. But I find it quite handy to do "git diff -- '*.sh'", just like the rest of git commands.

Hence this patch, which simply calls match_pathspec() inside ce_path_match(). With this, "git diff-files" and "git diff-index" now support wildcards. "git diff-tree" does not because it does not use ce_path_match().

"git update-index --again" is also affected (in a good way hopefully)
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 So.. comments?
 read-cache.c |   20 +-------------------
 1 files changed, 1 insertions(+), 19 deletions(-)
Show changes to read-cache.c +1 −19
diff --git a/read-cache.c b/read-cache.c
index 3f58711..d2daf01 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -677,28 +677,10 @@ int ce_same_name(struct cache_entry *a, struct cache_entry *b)
 
 int ce_path_match(const struct cache_entry *ce, const char **pathspec)
 {
-	const char *match, *name;
-	int len;
-
 	if (!pathspec)
 		return 1;
 
-	len = ce_namelen(ce);
-	name = ce->name;
-	while ((match = *pathspec++) != NULL) {
-		int matchlen = strlen(match);
-		if (matchlen > len)
-			continue;
-		if (memcmp(name, match, matchlen))
-			continue;
-		if (matchlen && name[matchlen-1] == '/')
-			return 1;
-		if (name[matchlen] == '/' || !name[matchlen])
-			return 1;
-		if (!matchlen)
-			return 1;
-	}
-	return 0;
+	return match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL);
 }
 
 /*
-- 
test
Junio C Hamano· May 25, 2009, 22:14 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH RFC] Convert ce_path_match() use to match_pathspec()

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> For some reasons diff code did not get converted to use
> match_pathspec(). So diff commands do not understand wildcards.

ce_path_match() is not just about diffs; there may be places that do not expect pathspecs to match cruft with globs. Have you checked all the callsites and they are Ok with globbing?

I think using glob in diff-files should be Ok, but that would make it inconsistent with diff-tree (and possibly diff-index but I didn't check). The correct operation of diff-tree (and path pruning in "git log" family) heavily relies on an early-exit optimization not to recurse into a directory when we can detect that none of the paths in that directory will ever match any of the given pathspecs, and this is done based on the non-globbing (iow "leading path") semantics; you need to be extra careful about this.

Nguyen Thai Ngoc Duy· May 26, 2009, 11:04 UTC · re: Junio C Hamano · lore

Re: [PATCH RFC] Convert ce_path_match() use to match_pathspec()

2009/5/26 Junio C Hamano <gitster@pobox.com>:
Show 8 quoted lines
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> For some reasons diff code did not get converted to use
>> match_pathspec(). So diff commands do not understand wildcards.
>
> ce_path_match() is not just about diffs; there may be places that do not
> expect pathspecs to match cruft with globs.  Have you checked all the
> callsites and they are Ok with globbing?

I'm pretty sure except preload-index. I have checked again, all read_cache_preload() callsites seem to prefer files and leading path over globbing.

Show 8 quoted lines
> I think using glob in diff-files should be Ok, but that would make it
> inconsistent with diff-tree (and possibly diff-index but I didn't check).
> The correct operation of diff-tree (and path pruning in "git log" family)
> heavily relies on an early-exit optimization not to recurse into a
> directory when we can detect that none of the paths in that directory will
> ever match any of the given pathspecs, and this is done based on the
> non-globbing (iow "leading path") semantics; you need to be extra careful
> about this.
Now I know the reason. Thanks.
-- 
Duy

← back to recent threads