{"thread":{"id":"37475","subject":"git archive and glob pathspecs","startedAt":"2014-09-02T22:17:38Z","lastAt":"2014-09-23T16:57:35Z","messageCount":9,"participants":["Peter Wu","Duy Nguyen","Nguyễn Thái Ngọc Duy","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"248765","messageId":"7752133.8a4hJ4krff@al","threadId":"37475","inReplyTo":null,"subject":"git archive and glob pathspecs","fromName":"Peter Wu","fromEmail":"peter@lekensteyn.nl","sentAt":"2014-09-02T22:17:38Z","receivedAt":"2014-09-02T22:17:38Z","isPatch":false,"sender":{"key":"peter@lekensteyn.nl","avatar":"https://gravatar.com/avatar/efcf30db53f30b15e7c905ef37bd4dc44d87573bd92baf97cd707115d3fcd0c0?d=mp&s=160"},"body":"Hi,\n\nThe `git archive` seems to accept a pathspec judging from the error message (git\nversion 2.1.0):\n\n git archive HEAD -- :x\n fatal: pathspec 'x' did not match any files\n\nWhen I try to use deeper glob specs however, it throws an error (this also\nhappens if I use `:(glob)**/Makefile`, tested in the git source tree):\n\n $ git archive HEAD -- ':(glob)*/Makefile'\n fatal: pathspec '*/Makefile' did not match any files\n\nStrange enough, command `git log -- ':(glob)*/Makefile'` works. Any idea what is\nwrong?\n\nKind regards,\nPeter\nhttps://lekensteyn.nl\n"},{"id":"248772","messageId":"CACsJy8BZBe1XV-uM8VHeGcDntn6vqJrNsWi6wTdQ1+fWAAsGFg@mail.gmail.com","threadId":"37475","inReplyTo":"7752133.8a4hJ4krff@al","subject":"Re: git archive and glob pathspecs","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-09-03T06:21:06Z","receivedAt":"2014-09-03T06:21:06Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Sep 3, 2014 at 5:17 AM, Peter Wu wrote:\n> Hi,\n>\n> The `git archive` seems to accept a pathspec judging from the error message (git\n> version 2.1.0):\n>\n> git archive HEAD -- :x\n> fatal: pathspec 'x' did not match any files\n>\n> When I try to use deeper glob specs however, it throws an error (this also\n> happens if I use `:(glob)**/Makefile`, tested in the git source tree):\n>\n> $ git archive HEAD -- ':(glob)*/Makefile'\n> fatal: pathspec '*/Makefile' did not match any files\n>\n> Strange enough, command `git log -- ':(glob)*/Makefile'` works. Any idea what is\n> wrong?\n\nThere may be something wrong. This patch seems to make it work for me,\nbut it includes lots of empty directories. I'll have a closer look\nlater (btw it's surprising that negative pathspec works too..)\n\ndiff --git a/archive.c b/archive.c\nindex 3fc0fb2..a5be58d 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -221,6 +221,7 @@ static int path_exists(struct tree *tree, const char *path)\n int ret;\n\n parse_pathspec(&pathspec, 0, 0, \"\", paths);\n+ pathspec.recursive = 1;\n ret = read_tree_recursive(tree, \"\", 0, 0, &pathspec, reject_entry, NULL);\n free_pathspec(&pathspec);\n return ret != 0;\n@@ -237,6 +238,7 @@ static void parse_pathspec_arg(const char **pathspec,\n parse_pathspec(&ar_args->pathspec, 0,\n PATHSPEC_PREFER_FULL,\n \"\", pathspec);\n+ ar_args->pathspec.recursive = 1;\n if (pathspec) {\n while (*pathspec) {\n if (**pathspec && !path_exists(ar_args->tree, *pathspec))\n\n-- \nDuy\n"},{"id":"248833","messageId":"1409837872-20837-1-git-send-email-pclouds@gmail.com","threadId":"37475","inReplyTo":"7752133.8a4hJ4krff@al","subject":"[PATCH] archive: support filtering paths with glob","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-09-04T13:37:52Z","receivedAt":"2014-09-04T13:37:52Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This patch fixes two problems with using :(glob) (or even \"*.c\"\nwithout \":(glob)\").\n\nThe first one is we forgot to turn on the 'recursive' flag in struct\npathspec. Without that, tree_entry_interesting() will not mark\npotential directories \"interesting\" so that it can confirm whether\nthose directories have anything matching the pathspec.\n\nThe marking directories interesting has a side effect that we need to\nwalk inside a directory to realize that there's nothing interested in\nthere. By that time, 'archive' code has already written the (empty)\ndirectory down. That means lots of empty directories in the result\narchive.\n\nThis problem is fixed by lazily writing directories down when we know\nthey are actually needed. There is a theoretical bug in this\nimplementation: we can't write empty trees/directories that match that\npathspec.\n\nNoticed-by: Peter Wu \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Similar approach could probably be used for teaching ls-tree about pathspec..\n\n archive.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++-\n t/t5000-tar-tree.sh | 10 +++++++\n 2 files changed, 91 insertions(+), 1 deletion(-)\n\ndiff --git a/archive.c b/archive.c\nindex 3fc0fb2..9e62bf4 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -98,9 +98,19 @@ static void setup_archive_check(struct git_attr_check *check)\n \tcheck[1].attr = attr_export_subst;\n }\n \n+struct directory {\n+\tstruct directory *up;\n+\tunsigned char sha1[20];\n+\tint baselen, len;\n+\tunsigned mode;\n+\tint stage;\n+\tchar path[FLEX_ARRAY];\n+};\n+\n struct archiver_context {\n \tstruct archiver_args *args;\n \twrite_archive_entry_fn_t write_entry;\n+\tstruct directory *bottom;\n };\n \n static int write_archive_entry(const unsigned char *sha1, const char *base,\n@@ -146,6 +156,65 @@ 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 void queue_directory(const unsigned char *sha1,\n+\t\tconst char *base, int baselen, 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->up\t = c->bottom;\n+\td->baselen = baselen;\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+\thashcpy(d->sha1, sha1);\n+}\n+\n+static int write_directory(struct archiver_context *c)\n+{\n+\tstruct directory *d = c->bottom;\n+\tint ret;\n+\n+\tif (!d)\n+\t\treturn 0;\n+\tc->bottom = d->up;\n+\td->path[d->len - 1] = '\\0'; /* no trailing slash */\n+\tret =\n+\t\twrite_directory(c) ||\n+\t\twrite_archive_entry(d->sha1, d->path, d->baselen,\n+\t\t\t\t d->path + d->baselen, d->mode,\n+\t\t\t\t d->stage, c) != READ_TREE_RECURSIVE;\n+\tfree(d);\n+\treturn ret ? -1 : 0;\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\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\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\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+\t\t\t\t stage, context);\n+}\n+\n int write_archive_entries(struct archiver_args *args,\n \t\twrite_archive_entry_fn_t write_entry)\n {\n@@ -167,6 +236,7 @@ int write_archive_entries(struct archiver_args *args,\n \t\t\treturn err;\n \t}\n \n+\tmemset(&context, 0, sizeof(context));\n \tcontext.args = args;\n \tcontext.write_entry = write_entry;\n \n@@ -187,9 +257,17 @@ int write_archive_entries(struct archiver_args *args,\n \t}\n \n \terr = read_tree_recursive(args->tree, \"\", 0, 0, &args->pathspec,\n-\t\t\t\t write_archive_entry, &context);\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 &context);\n \tif (err == READ_TREE_RECURSIVE)\n \t\terr = 0;\n+\twhile (context.bottom) {\n+\t\tstruct directory *next = context.bottom->up;\n+\t\tfree(context.bottom);\n+\t\tcontext.bottom = next;\n+\t}\n \treturn err;\n }\n \n@@ -221,6 +299,7 @@ static int path_exists(struct tree *tree, const char *path)\n \tint ret;\n \n \tparse_pathspec(&pathspec, 0, 0, \"\", paths);\n+\tpathspec.recursive = 1;\n \tret = read_tree_recursive(tree, \"\", 0, 0, &pathspec, reject_entry, NULL);\n \tfree_pathspec(&pathspec);\n \treturn ret != 0;\n@@ -237,6 +316,7 @@ static void parse_pathspec_arg(const char **pathspec,\n \tparse_pathspec(&ar_args->pathspec, 0,\n \t\t PATHSPEC_PREFER_FULL,\n \t\t \"\", pathspec);\n+\tar_args->pathspec.recursive = 1;\n \tif (pathspec) {\n \t\twhile (*pathspec) {\n \t\t\tif (**pathspec && !path_exists(ar_args->tree, *pathspec))\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 7b8babd..ade7a7f 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -305,4 +305,14 @@ test_expect_success GZIP 'remote tar.gz can be disabled' '\n \t\t>remote.tar.gz\n '\n \n+test_expect_success 'archive and :(glob)' '\n+\tgit archive -v HEAD -- \":(glob)**/sh\" >/dev/null 2>actual &&\n+\tcat >expect < On Wed, Sep 3, 2014 at 5:17 AM, Peter Wu wrote:\n> > Hi,\n> >\n> > The `git archive` seems to accept a pathspec judging from the error message (git\n> > version 2.1.0):\n> >\n> > git archive HEAD -- :x\n> > fatal: pathspec 'x' did not match any files\n> >\n> > When I try to use deeper glob specs however, it throws an error (this also\n> > happens if I use `:(glob)**/Makefile`, tested in the git source tree):\n> >\n> > $ git archive HEAD -- ':(glob)*/Makefile'\n> > fatal: pathspec '*/Makefile' did not match any files\n> >\n> > Strange enough, command `git log -- ':(glob)*/Makefile'` works. Any idea what is\n> > wrong?\n> \n> There may be something wrong. This patch seems to make it work for me,\n> but it includes lots of empty directories. I'll have a closer look\n> later (btw it's surprising that negative pathspec works too..)\n\nI can confirm that this patch shows Makefile's, but also includes a lot of empty\ndirectories.\n\nAs for why this happens, my guess is that write_archive_entries() recurses the\nfull tree and adds every encountered directory (via read_tree_1, via\nwrite_archive_entry()).\n\nTo fix this, write_archive (write_tar_archive, etc.) should be taught to handle\nglob patterns, or parse_pathspec should expand globs (and then\nparse_pathspec_arg might have to validate the remaining patterns).\n\nKind regards,\nPeter\n\n> diff --git a/archive.c b/archive.c\n> index 3fc0fb2..a5be58d 100644\n> --- a/archive.c\n> +++ b/archive.c\n> @@ -221,6 +221,7 @@ static int path_exists(struct tree *tree, const char *path)\n> int ret;\n> \n> parse_pathspec(&pathspec, 0, 0, \"\", paths);\n> + pathspec.recursive = 1;\n> ret = read_tree_recursive(tree, \"\", 0, 0, &pathspec, reject_entry, NULL);\n> free_pathspec(&pathspec);\n> return ret != 0;\n> @@ -237,6 +238,7 @@ static void parse_pathspec_arg(const char **pathspec,\n> parse_pathspec(&ar_args->pathspec, 0,\n> PATHSPEC_PREFER_FULL,\n> \"\", pathspec);\n> + ar_args->pathspec.recursive = 1;\n> if (pathspec) {\n> while (*pathspec) {\n> if (**pathspec && !path_exists(ar_args->tree, *pathspec))\n"},{"id":"249343","messageId":"1433062.XhRCATRB3s@al","threadId":"37475","inReplyTo":"1409837872-20837-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] archive: support filtering paths with glob","fromName":"Peter Wu","fromEmail":"peter@lekensteyn.nl","sentAt":"2014-09-13T10:52:49Z","receivedAt":"2014-09-13T10:52:49Z","isPatch":true,"sender":{"key":"peter@lekensteyn.nl","avatar":"https://gravatar.com/avatar/efcf30db53f30b15e7c905ef37bd4dc44d87573bd92baf97cd707115d3fcd0c0?d=mp&s=160"},"body":"On Thursday 04 September 2014 20:37:52 Nguyễn Thái Ngọc Duy wrote:\n> This patch fixes two problems with using :(glob) (or even \"*.c\"\n> without \":(glob)\").\n> \n> The first one is we forgot to turn on the 'recursive' flag in struct\n> pathspec. Without that, tree_entry_interesting() will not mark\n> potential directories \"interesting\" so that it can confirm whether\n> those directories have anything matching the pathspec.\n> \n> The marking directories interesting has a side effect that we need to\n> walk inside a directory to realize that there's nothing interested in\n> there. By that time, 'archive' code has already written the (empty)\n> directory down. That means lots of empty directories in the result\n> archive.\n> \n> This problem is fixed by lazily writing directories down when we know\n> they are actually needed. There is a theoretical bug in this\n> implementation: we can't write empty trees/directories that match that\n> pathspec.\n> \n> Noticed-by: Peter Wu \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n\nAh, ignore my last mail, I just noticed this one. This patch works fine. By the\nway, the glob pattern is treated as a 'nullglob'. If you specify a glob pattern\nthat matches nothing, an empty archive is created. Perhaps an error message\nshould be raised for that as it is unlikely that a user wants that?\n\nTested-by: Peter Wu \n\n> ---\n> Similar approach could probably be used for teaching ls-tree about pathspec..\n> \n> archive.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++-\n> t/t5000-tar-tree.sh | 10 +++++++\n> 2 files changed, 91 insertions(+), 1 deletion(-)\n> \n> diff --git a/archive.c b/archive.c\n> index 3fc0fb2..9e62bf4 100644\n> --- a/archive.c\n> +++ b/archive.c\n> @@ -98,9 +98,19 @@ static void setup_archive_check(struct git_attr_check *check)\n> \tcheck[1].attr = attr_export_subst;\n> }\n> \n> +struct directory {\n> +\tstruct directory *up;\n> +\tunsigned char sha1[20];\n> +\tint baselen, len;\n> +\tunsigned mode;\n> +\tint stage;\n> +\tchar path[FLEX_ARRAY];\n> +};\n> +\n> struct archiver_context {\n> \tstruct archiver_args *args;\n> \twrite_archive_entry_fn_t write_entry;\n> +\tstruct directory *bottom;\n> };\n> \n> static int write_archive_entry(const unsigned char *sha1, const char *base,\n> @@ -146,6 +156,65 @@ 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 void queue_directory(const unsigned char *sha1,\n> +\t\tconst char *base, int baselen, 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->up\t = c->bottom;\n> +\td->baselen = baselen;\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> +\thashcpy(d->sha1, sha1);\n> +}\n> +\n> +static int write_directory(struct archiver_context *c)\n> +{\n> +\tstruct directory *d = c->bottom;\n> +\tint ret;\n> +\n> +\tif (!d)\n> +\t\treturn 0;\n> +\tc->bottom = d->up;\n> +\td->path[d->len - 1] = '\\0'; /* no trailing slash */\n> +\tret =\n> +\t\twrite_directory(c) ||\n> +\t\twrite_archive_entry(d->sha1, d->path, d->baselen,\n> +\t\t\t\t d->path + d->baselen, d->mode,\n> +\t\t\t\t d->stage, c) != READ_TREE_RECURSIVE;\n> +\tfree(d);\n> +\treturn ret ? -1 : 0;\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\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\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\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> +\t\t\t\t stage, context);\n> +}\n> +\n> int write_archive_entries(struct archiver_args *args,\n> \t\twrite_archive_entry_fn_t write_entry)\n> {\n> @@ -167,6 +236,7 @@ int write_archive_entries(struct archiver_args *args,\n> \t\t\treturn err;\n> \t}\n> \n> +\tmemset(&context, 0, sizeof(context));\n> \tcontext.args = args;\n> \tcontext.write_entry = write_entry;\n> \n> @@ -187,9 +257,17 @@ int write_archive_entries(struct archiver_args *args,\n> \t}\n> \n> \terr = read_tree_recursive(args->tree, \"\", 0, 0, &args->pathspec,\n> -\t\t\t\t write_archive_entry, &context);\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 &context);\n> \tif (err == READ_TREE_RECURSIVE)\n> \t\terr = 0;\n> +\twhile (context.bottom) {\n> +\t\tstruct directory *next = context.bottom->up;\n> +\t\tfree(context.bottom);\n> +\t\tcontext.bottom = next;\n> +\t}\n> \treturn err;\n> }\n> \n> @@ -221,6 +299,7 @@ static int path_exists(struct tree *tree, const char *path)\n> \tint ret;\n> \n> \tparse_pathspec(&pathspec, 0, 0, \"\", paths);\n> +\tpathspec.recursive = 1;\n> \tret = read_tree_recursive(tree, \"\", 0, 0, &pathspec, reject_entry, NULL);\n> \tfree_pathspec(&pathspec);\n> \treturn ret != 0;\n> @@ -237,6 +316,7 @@ static void parse_pathspec_arg(const char **pathspec,\n> \tparse_pathspec(&ar_args->pathspec, 0,\n> \t\t PATHSPEC_PREFER_FULL,\n> \t\t \"\", pathspec);\n> +\tar_args->pathspec.recursive = 1;\n> \tif (pathspec) {\n> \t\twhile (*pathspec) {\n> \t\t\tif (**pathspec && !path_exists(ar_args->tree, *pathspec))\n> diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\n> index 7b8babd..ade7a7f 100755\n> --- a/t/t5000-tar-tree.sh\n> +++ b/t/t5000-tar-tree.sh\n> @@ -305,4 +305,14 @@ test_expect_success GZIP 'remote tar.gz can be disabled' '\n> \t\t>remote.tar.gz\n> '\n> \n> +test_expect_success 'archive and :(glob)' '\n> +\tgit archive -v HEAD -- \":(glob)**/sh\" >/dev/null 2>actual &&\n> +\tcat >expect < +a/\n> +a/bin/\n> +a/bin/sh\n> +EOF\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_done\n"},{"id":"249711","messageId":"1411271706-1151-1-git-send-email-pclouds@gmail.com","threadId":"37475","inReplyTo":"1433062.XhRCATRB3s@al","subject":"[PATCH v2] archive: support filtering paths with glob","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-09-21T03:55:06Z","receivedAt":"2014-09-21T03:55:06Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This patch fixes two problems with using :(glob) (or even \"*.c\"\nwithout \":(glob)\").\n\nThe first one is we forgot to turn on the 'recursive' flag in struct\npathspec. Without that, tree_entry_interesting() will not mark\npotential directories \"interesting\" so that it can confirm whether\nthose directories have anything matching the pathspec.\n\nThe marking directories interesting has a side effect that we need to\nwalk inside a directory to realize that there's nothing interested in\nthere. By that time, 'archive' code has already written the (empty)\ndirectory down. That means lots of empty directories in the result\narchive.\n\nThis problem is fixed by lazily writing directories down when we know\nthey are actually needed. There is a theoretical bug in this\nimplementation: we can't write empty trees/directories that match that\npathspec.\n\npath_exists() is also made stricter in order to detect non-matching\npathspec because when this 'recursive' flag is on, we most likely\nmatch some directories. The easiest way is not consider any\ndirectories \"matched\".\n\nNoticed-by: Peter Wu \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n On Sat, Sep 13, 2014 at 5:52 PM, Peter Wu wrote:\n > By the\n > way, the glob pattern is treated as a 'nullglob'. If you specify a glob pattern\n > that matches nothing, an empty archive is created. Perhaps an error message\n > should be raised for that as it is unlikely that a user wants that?\n\n If you do \"git archive HEAD non-existent-path\", you get an error.\n There's no reason why glob is an exception. Fixed in this version.\n\n archive.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++--\n t/t5000-tar-tree.sh | 14 ++++++++\n 2 files changed, 108 insertions(+), 3 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 952a659..94a9981 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -5,6 +5,7 @@\n #include \"archive.h\"\n #include \"parse-options.h\"\n #include \"unpack-trees.h\"\n+#include \"dir.h\"\n \n static char const * const archive_usage[] = {\n \tN_(\"git archive [options] [...]\"),\n@@ -98,9 +99,19 @@ static void setup_archive_check(struct git_attr_check *check)\n \tcheck[1].attr = attr_export_subst;\n }\n \n+struct directory {\n+\tstruct directory *up;\n+\tunsigned char sha1[20];\n+\tint baselen, len;\n+\tunsigned mode;\n+\tint stage;\n+\tchar path[FLEX_ARRAY];\n+};\n+\n struct archiver_context {\n \tstruct archiver_args *args;\n \twrite_archive_entry_fn_t write_entry;\n+\tstruct directory *bottom;\n };\n \n static int write_archive_entry(const unsigned char *sha1, const char *base,\n@@ -146,6 +157,65 @@ 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 void queue_directory(const unsigned char *sha1,\n+\t\tconst char *base, int baselen, 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->up\t = c->bottom;\n+\td->baselen = baselen;\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+\thashcpy(d->sha1, sha1);\n+}\n+\n+static int write_directory(struct archiver_context *c)\n+{\n+\tstruct directory *d = c->bottom;\n+\tint ret;\n+\n+\tif (!d)\n+\t\treturn 0;\n+\tc->bottom = d->up;\n+\td->path[d->len - 1] = '\\0'; /* no trailing slash */\n+\tret =\n+\t\twrite_directory(c) ||\n+\t\twrite_archive_entry(d->sha1, d->path, d->baselen,\n+\t\t\t\t d->path + d->baselen, d->mode,\n+\t\t\t\t d->stage, c) != READ_TREE_RECURSIVE;\n+\tfree(d);\n+\treturn ret ? -1 : 0;\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\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\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\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+\t\t\t\t stage, context);\n+}\n+\n int write_archive_entries(struct archiver_args *args,\n \t\twrite_archive_entry_fn_t write_entry)\n {\n@@ -167,6 +237,7 @@ int write_archive_entries(struct archiver_args *args,\n \t\t\treturn err;\n \t}\n \n+\tmemset(&context, 0, sizeof(context));\n \tcontext.args = args;\n \tcontext.write_entry = write_entry;\n \n@@ -187,9 +258,17 @@ int write_archive_entries(struct archiver_args *args,\n \t}\n \n \terr = read_tree_recursive(args->tree, \"\", 0, 0, &args->pathspec,\n-\t\t\t\t write_archive_entry, &context);\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 &context);\n \tif (err == READ_TREE_RECURSIVE)\n \t\terr = 0;\n+\twhile (context.bottom) {\n+\t\tstruct directory *next = context.bottom->up;\n+\t\tfree(context.bottom);\n+\t\tcontext.bottom = next;\n+\t}\n \treturn err;\n }\n \n@@ -211,7 +290,16 @@ static int reject_entry(const unsigned char *sha1, const char *base,\n \t\t\tint baselen, const char *filename, unsigned mode,\n \t\t\tint stage, void *context)\n {\n-\treturn -1;\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_addstr(&sb, filename);\n+\t\tif (!match_pathspec(context, sb.buf, sb.len, 0, NULL, 1))\n+\t\t\tret = READ_TREE_RECURSIVE;\n+\t\tstrbuf_release(&sb);\n+\t}\n+\treturn ret;\n }\n \n static int path_exists(struct tree *tree, const char *path)\n@@ -221,7 +309,9 @@ static int path_exists(struct tree *tree, const char *path)\n \tint ret;\n \n \tparse_pathspec(&pathspec, 0, 0, \"\", paths);\n-\tret = read_tree_recursive(tree, \"\", 0, 0, &pathspec, reject_entry, NULL);\n+\tpathspec.recursive = 1;\n+\tret = read_tree_recursive(tree, \"\", 0, 0, &pathspec,\n+\t\t\t\t reject_entry, &pathspec);\n \tfree_pathspec(&pathspec);\n \treturn ret != 0;\n }\n@@ -237,6 +327,7 @@ static void parse_pathspec_arg(const char **pathspec,\n \tparse_pathspec(&ar_args->pathspec, 0,\n \t\t PATHSPEC_PREFER_FULL,\n \t\t \"\", pathspec);\n+\tar_args->pathspec.recursive = 1;\n \tif (pathspec) {\n \t\twhile (*pathspec) {\n \t\t\tif (**pathspec && !path_exists(ar_args->tree, *pathspec))\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 7b8babd..d01bbdc 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -305,4 +305,18 @@ test_expect_success GZIP 'remote tar.gz can be disabled' '\n \t\t>remote.tar.gz\n '\n \n+test_expect_success 'archive and :(glob)' '\n+\tgit archive -v HEAD -- \":(glob)**/sh\" >/dev/null 2>actual &&\n+\tcat >expect </dev/null\n+'\n+\n test_done\n-- \n2.1.0.rc0.78.gc0d8480\n"},{"id":"249748","messageId":"xmqq4mvzlb5c.fsf@gitster.dls.corp.google.com","threadId":"37475","inReplyTo":"1411271706-1151-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2] archive: support filtering paths with glob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-22T19:15:27Z","receivedAt":"2014-09-22T19:15:27Z","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> +static void queue_directory(const unsigned char *sha1,\n> +\t\tconst char *base, int baselen, 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->up\t = c->bottom;\n> +\td->baselen = baselen;\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> +\thashcpy(d->sha1, sha1);\n> +}\n> +\n> +static int write_directory(struct archiver_context *c)\n> +{\n> +\tstruct directory *d = c->bottom;\n> +\tint ret;\n> +\n> +\tif (!d)\n> +\t\treturn 0;\n> +\tc->bottom = d->up;\n> +\td->path[d->len - 1] = '\\0'; /* no trailing slash */\n> +\tret =\n> +\t\twrite_directory(c) ||\n\nBy recursing into itself using c->bottom set to one level higher, we\nfirst flush all the leading directories.\n\n> +\t\twrite_archive_entry(d->sha1, d->path, d->baselen,\n> +\t\t\t\t d->path + d->baselen, d->mode,\n> +\t\t\t\t d->stage, c) != READ_TREE_RECURSIVE;\n\nAnd after the entries for the leading directories, we write an entry\nfor this directory.\n\nWhich makes sense; presumably the caller will then write an entry\nthat hangs this directory after calling this.\n\nWhen we have a/b/c and a/d/e to be written, the first round would\nwrite a/ and then a/b/ with the above, and presumably elsewhere\nsomebody will write a/b/c; next time around we do need to write a/d/\nbut we wouldn't want to write a/ itself. How is this code\npreventing the recursion going all the way up every time to avoid\nrepeating a/?\n\nPuzzled...\n\n> +\tfree(d);\n> +\treturn ret ? -1 : 0;\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\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\nIs this comparison, which does not see if the substring matches from\nthe beginning but without making sure that the tail of the shorter\nsubstring coincides with the directory boundary in the longer\nstring, correctly done? Or does it consider abcd/ef is inside abc/?\n\nAh, we should be OK as I think is passed with a\ntrailing slash appended in read_tree_1() when it recurses, so this\ncomparison is correct.\n"},{"id":"249762","messageId":"CACsJy8CwwzOuiL1GMcK9OWZSdSqCpLzVJPyNkHocH10_1VmXSA@mail.gmail.com","threadId":"37475","inReplyTo":"xmqq4mvzlb5c.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] archive: support filtering paths with glob","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-09-22T23:04:31Z","receivedAt":"2014-09-22T23:04:31Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Sep 23, 2014 at 2:15 AM, Junio C Hamano wrote:\n> When we have a/b/c and a/d/e to be written, the first round would\n> write a/ and then a/b/ with the above, and presumably elsewhere\n> somebody will write a/b/c; next time around we do need to write a/d/\n> but we wouldn't want to write a/ itself. How is this code\n> preventing the recursion going all the way up every time to avoid\n> repeating a/?\n>\n> Puzzled...\n\nWe never traverse 'a' (or any directory) twice and we only push a\ndirectory to the stack when we examine it. After a/b and a are written\ndown and we examine 'd', 'a/d' is pushed to the stack. When we hit\n'a/d/e', we only have 'a/d' in the stack, not 'a'.\n-- \nDuy\n"},{"id":"249799","messageId":"xmqqfvfijmv4.fsf@gitster.dls.corp.google.com","threadId":"37475","inReplyTo":"CACsJy8CwwzOuiL1GMcK9OWZSdSqCpLzVJPyNkHocH10_1VmXSA@mail.gmail.com","subject":"Re: [PATCH v2] archive: support filtering paths with glob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-23T16:57:35Z","receivedAt":"2014-09-23T16:57:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Tue, Sep 23, 2014 at 2:15 AM, Junio C Hamano wrote:\n>> When we have a/b/c and a/d/e to be written, the first round would\n>> write a/ and then a/b/ with the above, and presumably elsewhere\n>> somebody will write a/b/c; next time around we do need to write a/d/\n>> but we wouldn't want to write a/ itself. How is this code\n>> preventing the recursion going all the way up every time to avoid\n>> repeating a/?\n>>\n>> Puzzled...\n>\n> We never traverse 'a' (or any directory) twice and we only push a\n> directory to the stack when we examine it. After a/b and a are written\n> down and we examine 'd', 'a/d' is pushed to the stack. When we hit\n> 'a/d/e', we only have 'a/d' in the stack, not 'a'.\n\nSo the traverser will feed you\n\n\ta/\n a/b/\n a/b/c\n a/d/\n a/d/e\n\nin that order, and you keep a/ and a/b/ until you see a/b/c at which\ntime you flush the two queued ones and likewise for a/d/ and a/d/e.\nWhen you queue a/d/ you do not decompose that to a/ and a/d/ because\nyou know that the caller of the callback would have made sure all\nthe leading path components have been given you by the time it gave\nyou a/d/, and it all works out fine.\n\nThanks for clarifying.\n"}]}