Subject: Re: [PATCH] Fix archive format with -- on the command line

From: René Scharfe <[email-masked]>
Date: Sat, 12 Dec 2009 16:00:41 +0100

Am 11.12.2009 00:31, schrieb Junio C Hamano:
> Junio C Hamano <[email-masked]> writes:
> 
>> Giving --format from the command line, or using output file extention to
>> DWIM the output format, with a pathspec that is disambiguated with an
>> explicit double-dash on the command line, e.g.
>>
>>     git archive -o file --format=zip HEAD -- path
>>     git archive -o file.zip HEAD -- path
>>
>> didn't work correctly.
>>
>> This was because the code reordered (when one was given) or added (when
>> the former was inferred) a --format argument at the end, effectively
>> making it to "archive HEAD -- path --format=zip", i.e. an extra pathspec
>> that is unlikely to match anything.
> 
> A side note to this issue is that
> 
>     $ git add non-existing-path
> 
> complains but
> 
>     $ git archive HEAD non-existing-path
> 
> doesn't.  Is this something we should consider a bug, or a feature?

I wouldn't go so far as to call it a bug, but it's certainly a missing
feature.  If the user asks for something we can't give him or her, it's
better to report that fact.  We didn't do that because it doesn't fall
out naturally from the archive streaming loop.  ls-tree doesn't do it,
either, by the way.

Something like this?

-- >8 --
Subject: archive: complain about path specs that don't match anything

Verify that all path specs match at least one path in the specified
tree and reject those that don't.

This would have made the bug fixed by 782a0005 easier to find.

This implementation is simple to the point of being stupid.  It walks
the full tree for each path spec until it matches something.  It's short
and seems to be fast enough, though.

Signed-off-by: Rene Scharfe <[email-masked]>
---
 archive.c |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/archive.c b/archive.c
index 55b2732..5b88507 100644
--- a/archive.c
+++ b/archive.c
@@ -211,10 +211,33 @@ 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,
+			int stage, void *context)
+{
+	return -1;
+}
+
+static int path_exists(struct tree *tree, const char *path)
+{
+	const char *pathspec[] = { path, NULL };
+
+	if (read_tree_recursive(tree, "", 0, 0, pathspec, reject_entry, NULL))
+		return 1;
+	return 0;
+}
+
 static void parse_pathspec_arg(const char **pathspec,
 		struct archiver_args *ar_args)
 {
-	ar_args->pathspec = get_pathspec("", pathspec);
+	ar_args->pathspec = pathspec = get_pathspec("", pathspec);
+	if (pathspec) {
+		while (*pathspec) {
+			if (!path_exists(ar_args->tree, *pathspec))
+				die("path not found: %s", *pathspec);
+			pathspec++;
+		}
+	}
 }
 
 static void parse_treeish_arg(const char **argv,