threads / patch / 38076

patch, 3 partsls-tree fixes

Subject: [PATCH 0/3] ls-tree fixes

## tl;dr

11 messages between Nov 30, 2014 and Dec 8, 2014. Diffs are folded; open one to read it.

replies: 10people: 3as markdown or json

Nguyễn Thái Ngọc Duy· Nov 30, 2014, 09:04 UTC · lore

The first two fix ls-tree's unable to handle relative paths outside $PWD. The last one rejects negative pathspec. This is a resend from

http://thread.gmane.org/gmane.comp.version-control.git/259233/focus=259264
Nguyễn Thái Ngọc Duy (3):
  tree.c: update read_tree_recursive callback to pass strbuf as base
  ls-tree: remove path filtering logic in show_tree
  ls-tree: disable negative pathspec because it's not supported
 archive.c                    | 34 +++++++++++++++++++++-------------
 builtin/checkout.c           |  8 ++++----
 builtin/log.c                |  2 +-
 builtin/ls-tree.c            | 20 +++++++++++---------
 merge-recursive.c            | 15 ++++++---------
 quote.c                      | 21 ---------------------
 quote.h                      |  2 --
 t/t3102-ls-tree-wildcards.sh |  8 ++++++++
 tree.c                       | 16 +++++++++++-----
 tree.h                       |  3 ++-
 10 files changed, 64 insertions(+), 65 deletions(-)
-- 
2.2.0.60.gb7b3c64
Nguyễn Thái Ngọc Duy· Nov 30, 2014, 09:05 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base

This allows the callback to use 'base' as a temporary buffer to quickly assemble full path "without" extra allocation. The callback has to restore it afterwards of course.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 archive.c          | 34 +++++++++++++++++++++-------------
 builtin/checkout.c |  8 ++++----
 builtin/log.c      |  2 +-
 builtin/ls-tree.c  |  9 +++++----
 merge-recursive.c  | 15 ++++++---------
 tree.c             | 16 +++++++++++-----
 tree.h             |  3 ++-
 7 files changed, 50 insertions(+), 37 deletions(-)
Show changes to 7 files +50 −37

archive.c, builtin/checkout.c, builtin/log.c, builtin/ls-tree.c, merge-recursive.c, tree.c, tree.h

diff --git a/archive.c b/archive.c
index 94a9981..9e30246 100644
--- a/archive.c
+++ b/archive.c
@@ -157,18 +157,26 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
 	return write_entry(args, sha1, path.buf, path.len, mode);
 }
 
+static int write_archive_entry_buf(const unsigned char *sha1, struct strbuf *base,
+		const char *filename, unsigned mode, int stage,
+		void *context)
+{
+	return write_archive_entry(sha1, base->buf, base->len,
+				     filename, mode, stage, context);
+}
+
 static void queue_directory(const unsigned char *sha1,
-		const char *base, int baselen, const char *filename,
+		struct strbuf *base, const char *filename,
 		unsigned mode, int stage, struct archiver_context *c)
 {
 	struct directory *d;
-	d = xmallocz(sizeof(*d) + baselen + 1 + strlen(filename));
+	d = xmallocz(sizeof(*d) + base->len + 1 + strlen(filename));
 	d->up	   = c->bottom;
-	d->baselen = baselen;
+	d->baselen = base->len;
 	d->mode	   = mode;
 	d->stage   = stage;
 	c->bottom  = d;
-	d->len = sprintf(d->path, "%.*s%s/", baselen, base, filename);
+	d->len = sprintf(d->path, "%.*s%s/", (int)base->len, base->buf, filename);
 	hashcpy(d->sha1, sha1);
 }
 
@@ -191,28 +199,28 @@ static int write_directory(struct archiver_context *c)
 }
 
 static int queue_or_write_archive_entry(const unsigned char *sha1,
-		const char *base, int baselen, const char *filename,
+		struct strbuf *base, const char *filename,
 		unsigned mode, int stage, void *context)
 {
 	struct archiver_context *c = context;
 
 	while (c->bottom &&
-	       !(baselen >= c->bottom->len &&
-		 !strncmp(base, c->bottom->path, c->bottom->len))) {
+	       !(base->len >= c->bottom->len &&
+		 !strncmp(base->buf, c->bottom->path, c->bottom->len))) {
 		struct directory *next = c->bottom->up;
 		free(c->bottom);
 		c->bottom = next;
 	}
 
 	if (S_ISDIR(mode)) {
-		queue_directory(sha1, base, baselen, filename,
+		queue_directory(sha1, base, filename,
 				mode, stage, c);
 		return READ_TREE_RECURSIVE;
 	}
 
 	if (write_directory(c))
 		return -1;
-	return write_archive_entry(sha1, base, baselen, filename, mode,
+	return write_archive_entry(sha1, base->buf, base->len, filename, mode,
 				   stage, context);
 }
 
@@ -260,7 +268,7 @@ int write_archive_entries(struct archiver_args *args,
 	err = read_tree_recursive(args->tree, "", 0, 0, &args->pathspec,
 				  args->pathspec.has_wildcard ?
 				  queue_or_write_archive_entry :
-				  write_archive_entry,
+				  write_archive_entry_buf,
 				  &context);
 	if (err == READ_TREE_RECURSIVE)
 		err = 0;
@@ -286,14 +294,14 @@ static const struct archiver *lookup_archiver(const char *name)
 	return NULL;
 }
 
-static int reject_entry(const unsigned char *sha1, const char *base,
-			int baselen, const char *filename, unsigned mode,
+static int reject_entry(const unsigned char *sha1, struct strbuf *base,
+			const char *filename, unsigned mode,
 			int stage, void *context)
 {
 	int ret = -1;
 	if (S_ISDIR(mode)) {
 		struct strbuf sb = STRBUF_INIT;
-		strbuf_addstr(&sb, base);
+		strbuf_addbuf(&sb, base);
 		strbuf_addstr(&sb, filename);
 		if (!match_pathspec(context, sb.buf, sb.len, 0, NULL, 1))
 			ret = READ_TREE_RECURSIVE;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5410dac..8adf48d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -62,7 +62,7 @@ static int post_checkout_hook(struct commit *old, struct commit *new,
 
 }
 
-static int update_some(const unsigned char *sha1, const char *base, int baselen,
+static int update_some(const unsigned char *sha1, struct strbuf *base,
 		const char *pathname, unsigned mode, int stage, void *context)
 {
 	int len;
@@ -71,11 +71,11 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,
 	if (S_ISDIR(mode))
 		return READ_TREE_RECURSIVE;
 
-	len = baselen + strlen(pathname);
+	len = base->len + strlen(pathname);
 	ce = xcalloc(1, cache_entry_size(len));
 	hashcpy(ce->sha1, sha1);
-	memcpy(ce->name, base, baselen);
-	memcpy(ce->name + baselen, pathname, len - baselen);
+	memcpy(ce->name, base->buf, base->len);
+	memcpy(ce->name + base->len, pathname, len - base->len);
 	ce->ce_flags = create_ce_flags(0) | CE_UPDATE;
 	ce->ce_namelen = len;
 	ce->ce_mode = create_ce_mode(mode);
diff --git a/builtin/log.c b/builtin/log.c
index 734aab3..f2a9f01 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -489,7 +489,7 @@ static int show_tag_object(const unsigned char *sha1, struct rev_info *rev)
 }
 
 static int show_tree_object(const unsigned char *sha1,
-		const char *base, int baselen,
+		struct strbuf *base,
 		const char *pathname, unsigned mode, int stage, void *context)
 {
 	printf("%s%s\n", pathname, S_ISDIR(mode) ? "/" : "");
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 51184df..1ab0381 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -61,7 +61,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
 	}
 }
 
-static int show_tree(const unsigned char *sha1, const char *base, int baselen,
+static int show_tree(const unsigned char *sha1, struct strbuf *base,
 		const char *pathname, unsigned mode, int stage, void *context)
 {
 	int retval = 0;
@@ -79,7 +79,7 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,
 		 */
 		type = commit_type;
 	} else if (S_ISDIR(mode)) {
-		if (show_recursive(base, baselen, pathname)) {
+		if (show_recursive(base->buf, base->len, pathname)) {
 			retval = READ_TREE_RECURSIVE;
 			if (!(ls_options & LS_SHOW_TREES))
 				return retval;
@@ -90,7 +90,8 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,
 		return 0;
 
 	if (chomp_prefix &&
-	    (baselen < chomp_prefix || memcmp(ls_tree_prefix, base, chomp_prefix)))
+	    (base->len < chomp_prefix ||
+	     memcmp(ls_tree_prefix, base->buf, chomp_prefix)))
 		return 0;
 
 	if (!(ls_options & LS_NAME_ONLY)) {
@@ -112,7 +113,7 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,
 			printf("%06o %s %s\t", mode, type,
 			       find_unique_abbrev(sha1, abbrev));
 	}
-	write_name_quotedpfx(base + chomp_prefix, baselen - chomp_prefix,
+	write_name_quotedpfx(base->buf + chomp_prefix, base->len - chomp_prefix,
 			  pathname, stdout, line_termination);
 	return retval;
 }
diff --git a/merge-recursive.c b/merge-recursive.c
index fdb7d0f..25c067e 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -275,23 +275,20 @@ struct tree *write_tree_from_memory(struct merge_options *o)
 }
 
 static int save_files_dirs(const unsigned char *sha1,
-		const char *base, int baselen, const char *path,
+		struct strbuf *base, const char *path,
 		unsigned int mode, int stage, void *context)
 {
-	int len = strlen(path);
-	char *newpath = xmalloc(baselen + len + 1);
+	int baselen = base->len;
 	struct merge_options *o = context;
 
-	memcpy(newpath, base, baselen);
-	memcpy(newpath + baselen, path, len);
-	newpath[baselen + len] = '\0';
+	strbuf_addstr(base, path);
 
 	if (S_ISDIR(mode))
-		string_list_insert(&o->current_directory_set, newpath);
+		string_list_insert(&o->current_directory_set, base->buf);
 	else
-		string_list_insert(&o->current_file_set, newpath);
-	free(newpath);
+		string_list_insert(&o->current_file_set, base->buf);
 
+	strbuf_setlen(base, baselen);
 	return (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
 }
 
diff --git a/tree.c b/tree.c
index bb02c1c..58ebfce 100644
--- a/tree.c
+++ b/tree.c
@@ -30,9 +30,12 @@ static int read_one_entry_opt(const unsigned char *sha1, const char *base, int b
 	return add_cache_entry(ce, opt);
 }
 
-static int read_one_entry(const unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage, void *context)
+static int read_one_entry(const unsigned char *sha1, struct strbuf *base,
+			  const char *pathname, unsigned mode, int stage,
+			  void *context)
 {
-	return read_one_entry_opt(sha1, base, baselen, pathname, mode, stage,
+	return read_one_entry_opt(sha1, base->buf, base->len, pathname,
+				  mode, stage,
 				  ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK);
 }
 
@@ -40,9 +43,12 @@ static int read_one_entry(const unsigned char *sha1, const char *base, int basel
  * This is used when the caller knows there is no existing entries at
  * the stage that will conflict with the entry being added.
  */
-static int read_one_entry_quick(const unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage, void *context)
+static int read_one_entry_quick(const unsigned char *sha1, struct strbuf *base,
+				const char *pathname, unsigned mode, int stage,
+				void *context)
 {
-	return read_one_entry_opt(sha1, base, baselen, pathname, mode, stage,
+	return read_one_entry_opt(sha1, base->buf, base->len, pathname,
+				  mode, stage,
 				  ADD_CACHE_JUST_APPEND);
 }
 
@@ -70,7 +76,7 @@ static int read_tree_1(struct tree *tree, struct strbuf *base,
 				continue;
 		}
 
-		switch (fn(entry.sha1, base->buf, base->len,
+		switch (fn(entry.sha1, base,
 			   entry.path, entry.mode, stage, context)) {
 		case 0:
 			continue;
diff --git a/tree.h b/tree.h
index d84ac63..d24125f 100644
--- a/tree.h
+++ b/tree.h
@@ -4,6 +4,7 @@
 #include "object.h"
 
 extern const char *tree_type;
+struct strbuf;
 
 struct tree {
 	struct object object;
@@ -22,7 +23,7 @@ void free_tree_buffer(struct tree *tree);
 struct tree *parse_tree_indirect(const unsigned char *sha1);
 
 #define READ_TREE_RECURSIVE 1
-typedef int (*read_tree_fn_t)(const unsigned char *, const char *, int, const char *, unsigned int, int, void *);
+typedef int (*read_tree_fn_t)(const unsigned char *, struct strbuf *, const char *, unsigned int, int, void *);
 
 extern int read_tree_recursive(struct tree *tree,
 			       const char *base, int baselen,

-- 
2.2.0.60.gb7b3c64
Junio C Hamano· Dec 1, 2014, 19:32 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> This allows the callback to use 'base' as a temporary buffer to
> quickly assemble full path "without" extra allocation. The callback
> has to restore it afterwards of course.
Hmph, what's the quote around 'without' doing there?
Duy Nguyen· Dec 2, 2014, 12:11 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base

On Tue, Dec 2, 2014 at 2:32 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 7 quoted lines
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> This allows the callback to use 'base' as a temporary buffer to
>> quickly assemble full path "without" extra allocation. The callback
>> has to restore it afterwards of course.
>
> Hmph, what's the quote around 'without' doing there?

because it's only true if you haven't used up all preallocated space in strbuf. If someone passes an empty strbuf, then underneath strbuf may do a few realloc until the buffer is large enough.

-- 
Duy
Eric Sunshine· Dec 3, 2014, 00:11 UTC · re: Duy Nguyen · lore

Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base

On Tue, Dec 2, 2014 at 7:11 AM, Duy Nguyen <pclouds@gmail.com> wrote:
Show 12 quoted lines
> On Tue, Dec 2, 2014 at 2:32 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>
>>> This allows the callback to use 'base' as a temporary buffer to
>>> quickly assemble full path "without" extra allocation. The callback
>>> has to restore it afterwards of course.
>>
>> Hmph, what's the quote around 'without' doing there?
>
> because it's only true if you haven't used up all preallocated space
> in strbuf. If someone passes an empty strbuf, then underneath strbuf
> may do a few realloc until the buffer is large enough.
Would it be easier to understand if written like this?
    This allows the callback to use 'base' as a temporary buffer to
    quickly assemble full path, thus avoiding allocation/deallocation
    for each iteration step.
Junio C Hamano· Dec 3, 2014, 16:13 UTC · re: Eric Sunshine · lore

Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base

Eric Sunshine <sunshine@sunshineco.com> writes:
Show 19 quoted lines
> On Tue, Dec 2, 2014 at 7:11 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Tue, Dec 2, 2014 at 2:32 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>>
>>>> This allows the callback to use 'base' as a temporary buffer to
>>>> quickly assemble full path "without" extra allocation. The callback
>>>> has to restore it afterwards of course.
>>>
>>> Hmph, what's the quote around 'without' doing there?
>>
>> because it's only true if you haven't used up all preallocated space
>> in strbuf. If someone passes an empty strbuf, then underneath strbuf
>> may do a few realloc until the buffer is large enough.
>
> Would it be easier to understand if written like this?
>
>     This allows the callback to use 'base' as a temporary buffer to
>     quickly assemble full path, thus avoiding allocation/deallocation
>     for each iteration step.
Thanks.

I am not Duy so I do not know if that matches what he wanted to say, or if it matches what the change gives us. Your phrasing wouldn't have made me say "Hmph...?" and it is an improvement that goes in the right direction, I think.

A question during the review, especially on proposed log messages and documentation changes, is rarely just a request to explain it to the questioner in the discussion. It is an indication that what is being commented on needs tweaking to be understood.

Duy Nguyen· Dec 8, 2014, 13:30 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base

On Wed, Dec 3, 2014 at 11:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
> A question during the review, especially on proposed log messages
> and documentation changes, is rarely just a request to explain it to
> the questioner in the discussion. It is an indication that what is
> being commented on needs tweaking to be understood.

I do the same at work and somehow forgot to apply the same principle here :D Do you want to me resend with Eric's wording?

-- 
Duy
Nguyễn Thái Ngọc Duy· Nov 30, 2014, 09:05 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 2/3] ls-tree: remove path filtering logic in show_tree

ls-tree uses read_tree_recursive() which already does path filtering using pathspec. No need to filter one more time based on prefix only. "ls-tree ../somewhere" does not work because of this. write_name_quotedpfx() can now be retired because nobody else uses it.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/ls-tree.c            | 14 +++++++-------
 quote.c                      | 21 ---------------------
 quote.h                      |  2 --
 t/t3102-ls-tree-wildcards.sh |  8 ++++++++
 4 files changed, 15 insertions(+), 30 deletions(-)
Show changes to 4 files +15 −30

builtin/ls-tree.c, quote.c, quote.h, t/t3102-ls-tree-wildcards.sh

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 1ab0381..053edb2 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -65,6 +65,7 @@ static int show_tree(const unsigned char *sha1, struct strbuf *base,
 		const char *pathname, unsigned mode, int stage, void *context)
 {
 	int retval = 0;
+	int baselen;
 	const char *type = blob_type;
 
 	if (S_ISGITLINK(mode)) {
@@ -89,11 +90,6 @@ static int show_tree(const unsigned char *sha1, struct strbuf *base,
 	else if (ls_options & LS_TREE_ONLY)
 		return 0;
 
-	if (chomp_prefix &&
-	    (base->len < chomp_prefix ||
-	     memcmp(ls_tree_prefix, base->buf, chomp_prefix)))
-		return 0;
-
 	if (!(ls_options & LS_NAME_ONLY)) {
 		if (ls_options & LS_SHOW_SIZE) {
 			char size_text[24];
@@ -113,8 +109,12 @@ static int show_tree(const unsigned char *sha1, struct strbuf *base,
 			printf("%06o %s %s\t", mode, type,
 			       find_unique_abbrev(sha1, abbrev));
 	}
-	write_name_quotedpfx(base->buf + chomp_prefix, base->len - chomp_prefix,
-			  pathname, stdout, line_termination);
+	baselen = base->len;
+	strbuf_addstr(base, pathname);
+	write_name_quoted_relative(base->buf,
+				   chomp_prefix ? ls_tree_prefix : NULL,
+				   stdout, line_termination);
+	strbuf_setlen(base, baselen);
 	return retval;
 }
 
diff --git a/quote.c b/quote.c
index 45e3db1..7920e18 100644
--- a/quote.c
+++ b/quote.c
@@ -274,27 +274,6 @@ void write_name_quoted(const char *name, FILE *fp, int terminator)
 	fputc(terminator, fp);
 }
 
-void write_name_quotedpfx(const char *pfx, size_t pfxlen,
-			  const char *name, FILE *fp, int terminator)
-{
-	int needquote = 0;
-
-	if (terminator) {
-		needquote = next_quote_pos(pfx, pfxlen) < pfxlen
-			|| name[next_quote_pos(name, -1)];
-	}
-	if (needquote) {
-		fputc('"', fp);
-		quote_c_style_counted(pfx, pfxlen, NULL, fp, 1);
-		quote_c_style(name, NULL, fp, 1);
-		fputc('"', fp);
-	} else {
-		fwrite(pfx, pfxlen, 1, fp);
-		fputs(name, fp);
-	}
-	fputc(terminator, fp);
-}
-
 void write_name_quoted_relative(const char *name, const char *prefix,
 				FILE *fp, int terminator)
 {
diff --git a/quote.h b/quote.h
index 71dcc3a..99e04d3 100644
--- a/quote.h
+++ b/quote.h
@@ -56,8 +56,6 @@ extern size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq
 extern void quote_two_c_style(struct strbuf *, const char *, const char *, int);
 
 extern void write_name_quoted(const char *name, FILE *, int terminator);
-extern void write_name_quotedpfx(const char *pfx, size_t pfxlen,
-                                 const char *name, FILE *, int terminator);
 extern void write_name_quoted_relative(const char *name, const char *prefix,
 		FILE *fp, int terminator);
 
diff --git a/t/t3102-ls-tree-wildcards.sh b/t/t3102-ls-tree-wildcards.sh
index c286854..83fca8d 100755
--- a/t/t3102-ls-tree-wildcards.sh
+++ b/t/t3102-ls-tree-wildcards.sh
@@ -19,4 +19,12 @@ EOF
 	test_cmp expected actual
 '
 
+test_expect_success 'ls-tree outside prefix' '
+	cat >expected <<EOF &&
+100644 blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391	../a[a]/three
+EOF
+	( cd aa && git ls-tree -r HEAD "../a[a]"; ) >actual &&
+	test_cmp expected actual
+'
+
 test_done

-- 
2.2.0.60.gb7b3c64
Nguyễn Thái Ngọc Duy· Nov 30, 2014, 09:05 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 3/3] ls-tree: disable negative pathspec because it's not supported

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/ls-tree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Show changes to builtin/ls-tree.c +2 −1
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 053edb2..3b04a0f 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -174,7 +174,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 	 * cannot be lifted until it is converted to use
 	 * match_pathspec() or tree_entry_interesting()
 	 */
-	parse_pathspec(&pathspec, PATHSPEC_GLOB | PATHSPEC_ICASE,
+	parse_pathspec(&pathspec, PATHSPEC_GLOB | PATHSPEC_ICASE |
+				  PATHSPEC_EXCLUDE,
 		       PATHSPEC_PREFER_CWD,
 		       prefix, argv + 1);
 	for (i = 0; i < pathspec.nr; i++)

-- 
2.2.0.60.gb7b3c64
Junio C Hamano· Dec 1, 2014, 19:40 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 3/3] ls-tree: disable negative pathspec because it's not supported

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Hmph, that's sad. Should the below say "test_expect_failure" without "test_must_fail", anticipating a fix later?

 t/t3102-ls-tree-wildcards.sh | 4 ++++
 1 file changed, 4 insertions(+)
Show changes to t/t3102-ls-tree-wildcards.sh +4 −0
diff --git a/t/t3102-ls-tree-wildcards.sh b/t/t3102-ls-tree-wildcards.sh
index 83fca8d..93127a0 100755
--- a/t/t3102-ls-tree-wildcards.sh
+++ b/t/t3102-ls-tree-wildcards.sh
@@ -27,4 +27,8 @@ EOF
 	test_cmp expected actual
 '
 
+test_expect_success 'ls-tree rejects negated pathspec' '
+	test_must_fail git ls-tree -r HEAD ":(exclude)a" "a*"
+'
+
 test_done
Duy Nguyen· Dec 2, 2014, 12:14 UTC · re: Junio C Hamano · lore

Re: [PATCH 3/3] ls-tree: disable negative pathspec because it's not supported

On Tue, Dec 2, 2014 at 2:40 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 6 quoted lines
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>
> Hmph, that's sad.  Should the below say "test_expect_failure"
> without "test_must_fail", anticipating a fix later?

Not a fix from me any time soon (I still need to improve pathspec support in git-mv). If the git-list-files series goes well, I do plan to make it list trees and it should support all pathspec without the fear of subtly breaking a plumbing. But I will change it to expect_failure unless you change your mind.

-- 
Duy

← back to recent threads