{"thread":{"id":"38076","subject":"[PATCH 0/3] ls-tree fixes","startedAt":"2014-11-30T09:04:59Z","lastAt":"2014-12-08T13:30:09Z","messageCount":11,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"252739","messageId":"1417338302-8208-1-git-send-email-pclouds@gmail.com","threadId":"38076","inReplyTo":null,"subject":"[PATCH 0/3] ls-tree fixes","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-11-30T09:04:59Z","receivedAt":"2014-11-30T09:04:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The first two fix ls-tree's unable to handle relative paths outside\n$PWD. The last one rejects negative pathspec. This is a resend from\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/259233/focus=259264\n\nNguyễn Thái Ngọc Duy (3):\n tree.c: update read_tree_recursive callback to pass strbuf as base\n ls-tree: remove path filtering logic in show_tree\n ls-tree: disable negative pathspec because it's not supported\n\n archive.c | 34 +++++++++++++++++++++-------------\n builtin/checkout.c | 8 ++++----\n builtin/log.c | 2 +-\n builtin/ls-tree.c | 20 +++++++++++---------\n merge-recursive.c | 15 ++++++---------\n quote.c | 21 ---------------------\n quote.h | 2 --\n t/t3102-ls-tree-wildcards.sh | 8 ++++++++\n tree.c | 16 +++++++++++-----\n tree.h | 3 ++-\n 10 files changed, 64 insertions(+), 65 deletions(-)\n\n-- \n2.2.0.60.gb7b3c64\n"},{"id":"252740","messageId":"1417338302-8208-2-git-send-email-pclouds@gmail.com","threadId":"38076","inReplyTo":"1417338302-8208-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-11-30T09:05:00Z","receivedAt":"2014-11-30T09:05:00Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This allows the callback to use 'base' as a temporary buffer to\nquickly assemble full path \"without\" extra allocation. The callback\nhas to restore it afterwards of course.\n\nHelped-by: Eric Sunshine \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n archive.c | 34 +++++++++++++++++++++-------------\n builtin/checkout.c | 8 ++++----\n builtin/log.c | 2 +-\n builtin/ls-tree.c | 9 +++++----\n merge-recursive.c | 15 ++++++---------\n tree.c | 16 +++++++++++-----\n tree.h | 3 ++-\n 7 files changed, 50 insertions(+), 37 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 94a9981..9e30246 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -157,18 +157,26 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n \treturn write_entry(args, sha1, path.buf, path.len, mode);\n }\n \n+static int write_archive_entry_buf(const unsigned char *sha1, struct strbuf *base,\n+\t\tconst char *filename, unsigned mode, int stage,\n+\t\tvoid *context)\n+{\n+\treturn write_archive_entry(sha1, base->buf, base->len,\n+\t\t\t\t filename, mode, stage, context);\n+}\n+\n static void queue_directory(const unsigned char *sha1,\n-\t\tconst char *base, int baselen, const char *filename,\n+\t\tstruct strbuf *base, const char *filename,\n \t\tunsigned mode, int stage, struct archiver_context *c)\n {\n \tstruct directory *d;\n-\td = xmallocz(sizeof(*d) + baselen + 1 + strlen(filename));\n+\td = xmallocz(sizeof(*d) + base->len + 1 + strlen(filename));\n \td->up\t = c->bottom;\n-\td->baselen = baselen;\n+\td->baselen = base->len;\n \td->mode\t = mode;\n \td->stage = stage;\n \tc->bottom = d;\n-\td->len = sprintf(d->path, \"%.*s%s/\", baselen, base, filename);\n+\td->len = sprintf(d->path, \"%.*s%s/\", (int)base->len, base->buf, filename);\n \thashcpy(d->sha1, sha1);\n }\n \n@@ -191,28 +199,28 @@ static int write_directory(struct archiver_context *c)\n }\n \n static int queue_or_write_archive_entry(const unsigned char *sha1,\n-\t\tconst char *base, int baselen, const char *filename,\n+\t\tstruct strbuf *base, const char *filename,\n \t\tunsigned mode, int stage, void *context)\n {\n \tstruct archiver_context *c = context;\n \n \twhile (c->bottom &&\n-\t !(baselen >= c->bottom->len &&\n-\t\t !strncmp(base, c->bottom->path, c->bottom->len))) {\n+\t !(base->len >= c->bottom->len &&\n+\t\t !strncmp(base->buf, c->bottom->path, c->bottom->len))) {\n \t\tstruct directory *next = c->bottom->up;\n \t\tfree(c->bottom);\n \t\tc->bottom = next;\n \t}\n \n \tif (S_ISDIR(mode)) {\n-\t\tqueue_directory(sha1, base, baselen, filename,\n+\t\tqueue_directory(sha1, base, filename,\n \t\t\t\tmode, stage, c);\n \t\treturn READ_TREE_RECURSIVE;\n \t}\n \n \tif (write_directory(c))\n \t\treturn -1;\n-\treturn write_archive_entry(sha1, base, baselen, filename, mode,\n+\treturn write_archive_entry(sha1, base->buf, base->len, filename, mode,\n \t\t\t\t stage, context);\n }\n \n@@ -260,7 +268,7 @@ int write_archive_entries(struct archiver_args *args,\n \terr = read_tree_recursive(args->tree, \"\", 0, 0, &args->pathspec,\n \t\t\t\t args->pathspec.has_wildcard ?\n \t\t\t\t queue_or_write_archive_entry :\n-\t\t\t\t write_archive_entry,\n+\t\t\t\t write_archive_entry_buf,\n \t\t\t\t &context);\n \tif (err == READ_TREE_RECURSIVE)\n \t\terr = 0;\n@@ -286,14 +294,14 @@ static const struct archiver *lookup_archiver(const char *name)\n \treturn NULL;\n }\n \n-static int reject_entry(const unsigned char *sha1, const char *base,\n-\t\t\tint baselen, const char *filename, unsigned mode,\n+static int reject_entry(const unsigned char *sha1, struct strbuf *base,\n+\t\t\tconst char *filename, unsigned mode,\n \t\t\tint stage, void *context)\n {\n \tint ret = -1;\n \tif (S_ISDIR(mode)) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n-\t\tstrbuf_addstr(&sb, base);\n+\t\tstrbuf_addbuf(&sb, base);\n \t\tstrbuf_addstr(&sb, filename);\n \t\tif (!match_pathspec(context, sb.buf, sb.len, 0, NULL, 1))\n \t\t\tret = READ_TREE_RECURSIVE;\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 5410dac..8adf48d 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -62,7 +62,7 @@ static int post_checkout_hook(struct commit *old, struct commit *new,\n \n }\n \n-static int update_some(const unsigned char *sha1, const char *base, int baselen,\n+static int update_some(const unsigned char *sha1, struct strbuf *base,\n \t\tconst char *pathname, unsigned mode, int stage, void *context)\n {\n \tint len;\n@@ -71,11 +71,11 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,\n \tif (S_ISDIR(mode))\n \t\treturn READ_TREE_RECURSIVE;\n \n-\tlen = baselen + strlen(pathname);\n+\tlen = base->len + strlen(pathname);\n \tce = xcalloc(1, cache_entry_size(len));\n \thashcpy(ce->sha1, sha1);\n-\tmemcpy(ce->name, base, baselen);\n-\tmemcpy(ce->name + baselen, pathname, len - baselen);\n+\tmemcpy(ce->name, base->buf, base->len);\n+\tmemcpy(ce->name + base->len, pathname, len - base->len);\n \tce->ce_flags = create_ce_flags(0) | CE_UPDATE;\n \tce->ce_namelen = len;\n \tce->ce_mode = create_ce_mode(mode);\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 734aab3..f2a9f01 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -489,7 +489,7 @@ static int show_tag_object(const unsigned char *sha1, struct rev_info *rev)\n }\n \n static int show_tree_object(const unsigned char *sha1,\n-\t\tconst char *base, int baselen,\n+\t\tstruct strbuf *base,\n \t\tconst char *pathname, unsigned mode, int stage, void *context)\n {\n \tprintf(\"%s%s\\n\", pathname, S_ISDIR(mode) ? \"/\" : \"\");\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 51184df..1ab0381 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -61,7 +61,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)\n \t}\n }\n \n-static int show_tree(const unsigned char *sha1, const char *base, int baselen,\n+static int show_tree(const unsigned char *sha1, struct strbuf *base,\n \t\tconst char *pathname, unsigned mode, int stage, void *context)\n {\n \tint retval = 0;\n@@ -79,7 +79,7 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,\n \t\t */\n \t\ttype = commit_type;\n \t} else if (S_ISDIR(mode)) {\n-\t\tif (show_recursive(base, baselen, pathname)) {\n+\t\tif (show_recursive(base->buf, base->len, pathname)) {\n \t\t\tretval = READ_TREE_RECURSIVE;\n \t\t\tif (!(ls_options & LS_SHOW_TREES))\n \t\t\t\treturn retval;\n@@ -90,7 +90,8 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,\n \t\treturn 0;\n \n \tif (chomp_prefix &&\n-\t (baselen < chomp_prefix || memcmp(ls_tree_prefix, base, chomp_prefix)))\n+\t (base->len < chomp_prefix ||\n+\t memcmp(ls_tree_prefix, base->buf, chomp_prefix)))\n \t\treturn 0;\n \n \tif (!(ls_options & LS_NAME_ONLY)) {\n@@ -112,7 +113,7 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,\n \t\t\tprintf(\"%06o %s %s\\t\", mode, type,\n \t\t\t find_unique_abbrev(sha1, abbrev));\n \t}\n-\twrite_name_quotedpfx(base + chomp_prefix, baselen - chomp_prefix,\n+\twrite_name_quotedpfx(base->buf + chomp_prefix, base->len - chomp_prefix,\n \t\t\t pathname, stdout, line_termination);\n \treturn retval;\n }\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex fdb7d0f..25c067e 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -275,23 +275,20 @@ struct tree *write_tree_from_memory(struct merge_options *o)\n }\n \n static int save_files_dirs(const unsigned char *sha1,\n-\t\tconst char *base, int baselen, const char *path,\n+\t\tstruct strbuf *base, const char *path,\n \t\tunsigned int mode, int stage, void *context)\n {\n-\tint len = strlen(path);\n-\tchar *newpath = xmalloc(baselen + len + 1);\n+\tint baselen = base->len;\n \tstruct merge_options *o = context;\n \n-\tmemcpy(newpath, base, baselen);\n-\tmemcpy(newpath + baselen, path, len);\n-\tnewpath[baselen + len] = '\\0';\n+\tstrbuf_addstr(base, path);\n \n \tif (S_ISDIR(mode))\n-\t\tstring_list_insert(&o->current_directory_set, newpath);\n+\t\tstring_list_insert(&o->current_directory_set, base->buf);\n \telse\n-\t\tstring_list_insert(&o->current_file_set, newpath);\n-\tfree(newpath);\n+\t\tstring_list_insert(&o->current_file_set, base->buf);\n \n+\tstrbuf_setlen(base, baselen);\n \treturn (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);\n }\n \ndiff --git a/tree.c b/tree.c\nindex bb02c1c..58ebfce 100644\n--- a/tree.c\n+++ b/tree.c\n@@ -30,9 +30,12 @@ static int read_one_entry_opt(const unsigned char *sha1, const char *base, int b\n \treturn add_cache_entry(ce, opt);\n }\n \n-static int read_one_entry(const unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage, void *context)\n+static int read_one_entry(const unsigned char *sha1, struct strbuf *base,\n+\t\t\t const char *pathname, unsigned mode, int stage,\n+\t\t\t void *context)\n {\n-\treturn read_one_entry_opt(sha1, base, baselen, pathname, mode, stage,\n+\treturn read_one_entry_opt(sha1, base->buf, base->len, pathname,\n+\t\t\t\t mode, stage,\n \t\t\t\t ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK);\n }\n \n@@ -40,9 +43,12 @@ static int read_one_entry(const unsigned char *sha1, const char *base, int basel\n * This is used when the caller knows there is no existing entries at\n * the stage that will conflict with the entry being added.\n */\n-static int read_one_entry_quick(const unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage, void *context)\n+static int read_one_entry_quick(const unsigned char *sha1, struct strbuf *base,\n+\t\t\t\tconst char *pathname, unsigned mode, int stage,\n+\t\t\t\tvoid *context)\n {\n-\treturn read_one_entry_opt(sha1, base, baselen, pathname, mode, stage,\n+\treturn read_one_entry_opt(sha1, base->buf, base->len, pathname,\n+\t\t\t\t mode, stage,\n \t\t\t\t ADD_CACHE_JUST_APPEND);\n }\n \n@@ -70,7 +76,7 @@ static int read_tree_1(struct tree *tree, struct strbuf *base,\n \t\t\t\tcontinue;\n \t\t}\n \n-\t\tswitch (fn(entry.sha1, base->buf, base->len,\n+\t\tswitch (fn(entry.sha1, base,\n \t\t\t entry.path, entry.mode, stage, context)) {\n \t\tcase 0:\n \t\t\tcontinue;\ndiff --git a/tree.h b/tree.h\nindex d84ac63..d24125f 100644\n--- a/tree.h\n+++ b/tree.h\n@@ -4,6 +4,7 @@\n #include \"object.h\"\n \n extern const char *tree_type;\n+struct strbuf;\n \n struct tree {\n \tstruct object object;\n@@ -22,7 +23,7 @@ void free_tree_buffer(struct tree *tree);\n struct tree *parse_tree_indirect(const unsigned char *sha1);\n \n #define READ_TREE_RECURSIVE 1\n-typedef int (*read_tree_fn_t)(const unsigned char *, const char *, int, const char *, unsigned int, int, void *);\n+typedef int (*read_tree_fn_t)(const unsigned char *, struct strbuf *, const char *, unsigned int, int, void *);\n \n extern int read_tree_recursive(struct tree *tree,\n \t\t\t const char *base, int baselen,\n-- \n2.2.0.60.gb7b3c64\n"},{"id":"252741","messageId":"1417338302-8208-3-git-send-email-pclouds@gmail.com","threadId":"38076","inReplyTo":"1417338302-8208-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/3] ls-tree: remove path filtering logic in show_tree","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-11-30T09:05:01Z","receivedAt":"2014-11-30T09:05:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"ls-tree uses read_tree_recursive() which already does path filtering\nusing pathspec. No need to filter one more time based on prefix\nonly. \"ls-tree ../somewhere\" does not work because of\nthis. write_name_quotedpfx() can now be retired because nobody else\nuses it.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/ls-tree.c | 14 +++++++-------\n quote.c | 21 ---------------------\n quote.h | 2 --\n t/t3102-ls-tree-wildcards.sh | 8 ++++++++\n 4 files changed, 15 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 1ab0381..053edb2 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -65,6 +65,7 @@ static int show_tree(const unsigned char *sha1, struct strbuf *base,\n \t\tconst char *pathname, unsigned mode, int stage, void *context)\n {\n \tint retval = 0;\n+\tint baselen;\n \tconst char *type = blob_type;\n \n \tif (S_ISGITLINK(mode)) {\n@@ -89,11 +90,6 @@ static int show_tree(const unsigned char *sha1, struct strbuf *base,\n \telse if (ls_options & LS_TREE_ONLY)\n \t\treturn 0;\n \n-\tif (chomp_prefix &&\n-\t (base->len < chomp_prefix ||\n-\t memcmp(ls_tree_prefix, base->buf, chomp_prefix)))\n-\t\treturn 0;\n-\n \tif (!(ls_options & LS_NAME_ONLY)) {\n \t\tif (ls_options & LS_SHOW_SIZE) {\n \t\t\tchar size_text[24];\n@@ -113,8 +109,12 @@ static int show_tree(const unsigned char *sha1, struct strbuf *base,\n \t\t\tprintf(\"%06o %s %s\\t\", mode, type,\n \t\t\t find_unique_abbrev(sha1, abbrev));\n \t}\n-\twrite_name_quotedpfx(base->buf + chomp_prefix, base->len - chomp_prefix,\n-\t\t\t pathname, stdout, line_termination);\n+\tbaselen = base->len;\n+\tstrbuf_addstr(base, pathname);\n+\twrite_name_quoted_relative(base->buf,\n+\t\t\t\t chomp_prefix ? ls_tree_prefix : NULL,\n+\t\t\t\t stdout, line_termination);\n+\tstrbuf_setlen(base, baselen);\n \treturn retval;\n }\n \ndiff --git a/quote.c b/quote.c\nindex 45e3db1..7920e18 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -274,27 +274,6 @@ void write_name_quoted(const char *name, FILE *fp, int terminator)\n \tfputc(terminator, fp);\n }\n \n-void write_name_quotedpfx(const char *pfx, size_t pfxlen,\n-\t\t\t const char *name, FILE *fp, int terminator)\n-{\n-\tint needquote = 0;\n-\n-\tif (terminator) {\n-\t\tneedquote = next_quote_pos(pfx, pfxlen) < pfxlen\n-\t\t\t|| name[next_quote_pos(name, -1)];\n-\t}\n-\tif (needquote) {\n-\t\tfputc('\"', fp);\n-\t\tquote_c_style_counted(pfx, pfxlen, NULL, fp, 1);\n-\t\tquote_c_style(name, NULL, fp, 1);\n-\t\tfputc('\"', fp);\n-\t} else {\n-\t\tfwrite(pfx, pfxlen, 1, fp);\n-\t\tfputs(name, fp);\n-\t}\n-\tfputc(terminator, fp);\n-}\n-\n void write_name_quoted_relative(const char *name, const char *prefix,\n \t\t\t\tFILE *fp, int terminator)\n {\ndiff --git a/quote.h b/quote.h\nindex 71dcc3a..99e04d3 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -56,8 +56,6 @@ extern size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq\n extern void quote_two_c_style(struct strbuf *, const char *, const char *, int);\n \n extern void write_name_quoted(const char *name, FILE *, int terminator);\n-extern void write_name_quotedpfx(const char *pfx, size_t pfxlen,\n- const char *name, FILE *, int terminator);\n extern void write_name_quoted_relative(const char *name, const char *prefix,\n \t\tFILE *fp, int terminator);\n \ndiff --git a/t/t3102-ls-tree-wildcards.sh b/t/t3102-ls-tree-wildcards.sh\nindex c286854..83fca8d 100755\n--- a/t/t3102-ls-tree-wildcards.sh\n+++ b/t/t3102-ls-tree-wildcards.sh\n@@ -19,4 +19,12 @@ EOF\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'ls-tree outside prefix' '\n+\tcat >expected <actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.2.0.60.gb7b3c64\n"},{"id":"252742","messageId":"1417338302-8208-4-git-send-email-pclouds@gmail.com","threadId":"38076","inReplyTo":"1417338302-8208-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 3/3] ls-tree: disable negative pathspec because it's not supported","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-11-30T09:05:02Z","receivedAt":"2014-11-30T09:05:02Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/ls-tree.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 053edb2..3b04a0f 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -174,7 +174,8 @@ 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_GLOB | PATHSPEC_ICASE,\n+\tparse_pathspec(&pathspec, PATHSPEC_GLOB | PATHSPEC_ICASE |\n+\t\t\t\t PATHSPEC_EXCLUDE,\n \t\t PATHSPEC_PREFER_CWD,\n \t\t prefix, argv + 1);\n \tfor (i = 0; i < pathspec.nr; i++)\n-- \n2.2.0.60.gb7b3c64\n"},{"id":"252803","messageId":"xmqqk32bp3nk.fsf@gitster.dls.corp.google.com","threadId":"38076","inReplyTo":"1417338302-8208-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-01T19:32:15Z","receivedAt":"2014-12-01T19:32:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> This allows the callback to use 'base' as a temporary buffer to\n> quickly assemble full path \"without\" extra allocation. The callback\n> has to restore it afterwards of course.\n\nHmph, what's the quote around 'without' doing there?\n"},{"id":"252804","messageId":"xmqqfvczp39n.fsf@gitster.dls.corp.google.com","threadId":"38076","inReplyTo":"1417338302-8208-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 3/3] ls-tree: disable negative pathspec because it's not supported","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-01T19:40:36Z","receivedAt":"2014-12-01T19:40:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n\nHmph, that's sad. Should the below say \"test_expect_failure\"\nwithout \"test_must_fail\", anticipating a fix later?\n\n t/t3102-ls-tree-wildcards.sh | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/t/t3102-ls-tree-wildcards.sh b/t/t3102-ls-tree-wildcards.sh\nindex 83fca8d..93127a0 100755\n--- a/t/t3102-ls-tree-wildcards.sh\n+++ b/t/t3102-ls-tree-wildcards.sh\n@@ -27,4 +27,8 @@ EOF\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'ls-tree rejects negated pathspec' '\n+\ttest_must_fail git ls-tree -r HEAD \":(exclude)a\" \"a*\"\n+'\n+\n test_done\n"},{"id":"252854","messageId":"CACsJy8B7G9xQerZ+6GTPQuG2v1DuJLXfsKFobuz_vyFahnXuYQ@mail.gmail.com","threadId":"38076","inReplyTo":"xmqqk32bp3nk.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-12-02T12:11:15Z","receivedAt":"2014-12-02T12:11:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Dec 2, 2014 at 2:32 AM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> This allows the callback to use 'base' as a temporary buffer to\n>> quickly assemble full path \"without\" extra allocation. The callback\n>> has to restore it afterwards of course.\n>\n> Hmph, what's the quote around 'without' doing there?\n\nbecause it's only true if you haven't used up all preallocated space\nin strbuf. If someone passes an empty strbuf, then underneath strbuf\nmay do a few realloc until the buffer is large enough.\n-- \nDuy\n"},{"id":"252858","messageId":"CACsJy8CY0r0V7syGbtor2t+eOM4Tmzj38qL4EaL+QiE-z7B+AQ@mail.gmail.com","threadId":"38076","inReplyTo":"xmqqfvczp39n.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 3/3] ls-tree: disable negative pathspec because it's not supported","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-12-02T12:14:37Z","receivedAt":"2014-12-02T12:14:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Dec 2, 2014 at 2:40 AM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> Signed-off-by: Nguyễn Thái Ngọc Duy \n>\n> Hmph, that's sad. Should the below say \"test_expect_failure\"\n> without \"test_must_fail\", anticipating a fix later?\n\nNot a fix from me any time soon (I still need to improve pathspec\nsupport in git-mv). If the git-list-files series goes well, I do plan\nto make it list trees and it should support all pathspec without the\nfear of subtly breaking a plumbing. But I will change it to\nexpect_failure unless you change your mind.\n-- \nDuy\n"},{"id":"252903","messageId":"CAPig+cS1OK6pv5A0vuJf=j6eFrNv70=AYgVz3zQny-md15_xKg@mail.gmail.com","threadId":"38076","inReplyTo":"CACsJy8B7G9xQerZ+6GTPQuG2v1DuJLXfsKFobuz_vyFahnXuYQ@mail.gmail.com","subject":"Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-03T00:11:44Z","receivedAt":"2014-12-03T00:11:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Dec 2, 2014 at 7:11 AM, Duy Nguyen wrote:\n> On Tue, Dec 2, 2014 at 2:32 AM, Junio C Hamano wrote:\n>> Nguyễn Thái Ngọc Duy writes:\n>>\n>>> This allows the callback to use 'base' as a temporary buffer to\n>>> quickly assemble full path \"without\" extra allocation. The callback\n>>> has to restore it afterwards of course.\n>>\n>> Hmph, what's the quote around 'without' doing there?\n>\n> because it's only true if you haven't used up all preallocated space\n> in strbuf. If someone passes an empty strbuf, then underneath strbuf\n> may do a few realloc until the buffer is large enough.\n\nWould it be easier to understand if written like this?\n\n This allows the callback to use 'base' as a temporary buffer to\n quickly assemble full path, thus avoiding allocation/deallocation\n for each iteration step.\n"},{"id":"252951","messageId":"xmqq1toglnjf.fsf@gitster.dls.corp.google.com","threadId":"38076","inReplyTo":"CAPig+cS1OK6pv5A0vuJf=j6eFrNv70=AYgVz3zQny-md15_xKg@mail.gmail.com","subject":"Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-03T16:13:08Z","receivedAt":"2014-12-03T16:13:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Tue, Dec 2, 2014 at 7:11 AM, Duy Nguyen wrote:\n>> On Tue, Dec 2, 2014 at 2:32 AM, Junio C Hamano wrote:\n>>> Nguyễn Thái Ngọc Duy writes:\n>>>\n>>>> This allows the callback to use 'base' as a temporary buffer to\n>>>> quickly assemble full path \"without\" extra allocation. The callback\n>>>> has to restore it afterwards of course.\n>>>\n>>> Hmph, what's the quote around 'without' doing there?\n>>\n>> because it's only true if you haven't used up all preallocated space\n>> in strbuf. If someone passes an empty strbuf, then underneath strbuf\n>> may do a few realloc until the buffer is large enough.\n>\n> Would it be easier to understand if written like this?\n>\n> This allows the callback to use 'base' as a temporary buffer to\n> quickly assemble full path, thus avoiding allocation/deallocation\n> for each iteration step.\n\nThanks.\n\nI am not Duy so I do not know if that matches what he wanted to say,\nor if it matches what the change gives us. Your phrasing wouldn't\nhave made me say \"Hmph...?\" and it is an improvement that goes in\nthe right direction, I think.\n\nA question during the review, especially on proposed log messages\nand documentation changes, is rarely just a request to explain it to\nthe questioner in the discussion. It is an indication that what is\nbeing commented on needs tweaking to be understood.\n"},{"id":"253306","messageId":"CACsJy8BXjDqB0vGMw0QijaBKq0hsC8ZR4Teu=P7xUZFG7rjvag@mail.gmail.com","threadId":"38076","inReplyTo":"xmqq1toglnjf.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/3] tree.c: update read_tree_recursive callback to pass strbuf as base","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-12-08T13:30:09Z","receivedAt":"2014-12-08T13:30:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Dec 3, 2014 at 11:13 PM, Junio C Hamano wrote:\n> A question during the review, especially on proposed log messages\n> and documentation changes, is rarely just a request to explain it to\n> the questioner in the discussion. It is an indication that what is\n> being commented on needs tweaking to be understood.\n\nI do the same at work and somehow forgot to apply the same principle\nhere :D Do you want to me resend with Eric's wording?\n-- \nDuy\n"}]}