On Thursday 04 September 2014 20:37:52 Nguyễn Thái Ngọc Duy wrote:
> This patch fixes two problems with using :(glob) (or even "*.c"
> without ":(glob)").
>
> The first one is we forgot to turn on the 'recursive' flag in struct
> pathspec. Without that, tree_entry_interesting() will not mark
> potential directories "interesting" so that it can confirm whether
> those directories have anything matching the pathspec.
>
> The marking directories interesting has a side effect that we need to
> walk inside a directory to realize that there's nothing interested in
> there. By that time, 'archive' code has already written the (empty)
> directory down. That means lots of empty directories in the result
> archive.
>
> This problem is fixed by lazily writing directories down when we know
> they are actually needed. There is a theoretical bug in this
> implementation: we can't write empty trees/directories that match that
> pathspec.
>
> Noticed-by: Peter Wu <peter@xxxxxxxxxxxxx>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
Ah, ignore my last mail, I just noticed this one. This patch works fine. By the
way, the glob pattern is treated as a 'nullglob'. If you specify a glob pattern
that matches nothing, an empty archive is created. Perhaps an error message
should be raised for that as it is unlikely that a user wants that?
Tested-by: Peter Wu <peter@xxxxxxxxxxxxx>
> ---
> Similar approach could probably be used for teaching ls-tree about pathspec..
>
> archive.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> t/t5000-tar-tree.sh | 10 +++++++
> 2 files changed, 91 insertions(+), 1 deletion(-)
>
> diff --git a/archive.c b/archive.c
> index 3fc0fb2..9e62bf4 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -98,9 +98,19 @@ static void setup_archive_check(struct git_attr_check *check)
> check[1].attr = attr_export_subst;
> }
>
> +struct directory {
> + struct directory *up;
> + unsigned char sha1[20];
> + int baselen, len;
> + unsigned mode;
> + int stage;
> + char path[FLEX_ARRAY];
> +};
> +
> struct archiver_context {
> struct archiver_args *args;
> write_archive_entry_fn_t write_entry;
> + struct directory *bottom;
> };
>
> static int write_archive_entry(const unsigned char *sha1, const char *base,
> @@ -146,6 +156,65 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
> return write_entry(args, sha1, path.buf, path.len, mode);
> }
>
> +static void queue_directory(const unsigned char *sha1,
> + const char *base, int baselen, const char *filename,
> + unsigned mode, int stage, struct archiver_context *c)
> +{
> + struct directory *d;
> + d = xmallocz(sizeof(*d) + baselen + 1 + strlen(filename));
> + d->up = c->bottom;
> + d->baselen = baselen;
> + d->mode = mode;
> + d->stage = stage;
> + c->bottom = d;
> + d->len = sprintf(d->path, "%.*s%s/", baselen, base, filename);
> + hashcpy(d->sha1, sha1);
> +}
> +
> +static int write_directory(struct archiver_context *c)
> +{
> + struct directory *d = c->bottom;
> + int ret;
> +
> + if (!d)
> + return 0;
> + c->bottom = d->up;
> + d->path[d->len - 1] = '\0'; /* no trailing slash */
> + ret =
> + write_directory(c) ||
> + write_archive_entry(d->sha1, d->path, d->baselen,
> + d->path + d->baselen, d->mode,
> + d->stage, c) != READ_TREE_RECURSIVE;
> + free(d);
> + return ret ? -1 : 0;
> +}
> +
> +static int queue_or_write_archive_entry(const unsigned char *sha1,
> + const char *base, int baselen, 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))) {
> + struct directory *next = c->bottom->up;
> + free(c->bottom);
> + c->bottom = next;
> + }
> +
> + if (S_ISDIR(mode)) {
> + queue_directory(sha1, base, baselen, filename,
> + mode, stage, c);
> + return READ_TREE_RECURSIVE;
> + }
> +
> + if (write_directory(c))
> + return -1;
> + return write_archive_entry(sha1, base, baselen, filename, mode,
> + stage, context);
> +}
> +
> int write_archive_entries(struct archiver_args *args,
> write_archive_entry_fn_t write_entry)
> {
> @@ -167,6 +236,7 @@ int write_archive_entries(struct archiver_args *args,
> return err;
> }
>
> + memset(&context, 0, sizeof(context));
> context.args = args;
> context.write_entry = write_entry;
>
> @@ -187,9 +257,17 @@ int write_archive_entries(struct archiver_args *args,
> }
>
> err = read_tree_recursive(args->tree, "", 0, 0, &args->pathspec,
> - write_archive_entry, &context);
> + args->pathspec.has_wildcard ?
> + queue_or_write_archive_entry :
> + write_archive_entry,
> + &context);
> if (err == READ_TREE_RECURSIVE)
> err = 0;
> + while (context.bottom) {
> + struct directory *next = context.bottom->up;
> + free(context.bottom);
> + context.bottom = next;
> + }
> return err;
> }
>
> @@ -221,6 +299,7 @@ static int path_exists(struct tree *tree, const char *path)
> int ret;
>
> parse_pathspec(&pathspec, 0, 0, "", paths);
> + pathspec.recursive = 1;
> ret = read_tree_recursive(tree, "", 0, 0, &pathspec, reject_entry, NULL);
> free_pathspec(&pathspec);
> return ret != 0;
> @@ -237,6 +316,7 @@ static void parse_pathspec_arg(const char **pathspec,
> parse_pathspec(&ar_args->pathspec, 0,
> PATHSPEC_PREFER_FULL,
> "", pathspec);
> + ar_args->pathspec.recursive = 1;
> if (pathspec) {
> while (*pathspec) {
> if (**pathspec && !path_exists(ar_args->tree, *pathspec))
> diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
> index 7b8babd..ade7a7f 100755
> --- a/t/t5000-tar-tree.sh
> +++ b/t/t5000-tar-tree.sh
> @@ -305,4 +305,14 @@ test_expect_success GZIP 'remote tar.gz can be disabled' '
> >remote.tar.gz
> '
>
> +test_expect_success 'archive and :(glob)' '
> + git archive -v HEAD -- ":(glob)**/sh" >/dev/null 2>actual &&
> + cat >expect <<EOF &&
> +a/
> +a/bin/
> +a/bin/sh
> +EOF
> + test_cmp expect actual
> +'
> +
> test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html