All of lore.kernel.org
 help / color / mirror / Atom feed
From: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
To: Junio C Hamano <gitster@pobox.com>
Cc: Miklos Vajna <vmiklos@frugalware.org>,
	Ilari Liusvaara <ilari.liusvaara@elisanet.fi>,
	git@vger.kernel.org
Subject: Re: [PATCH] Fix archive format with -- on the command line
Date: Sat, 12 Dec 2009 16:00:41 +0100	[thread overview]
Message-ID: <4B23B019.40106@lsrfire.ath.cx> (raw)
In-Reply-To: <7vd42m8kyr.fsf@alter.siamese.dyndns.org>

Am 11.12.2009 00:31, schrieb Junio C Hamano:
> Junio C Hamano <gitster@pobox.com> 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 <rene.scharfe@lsrfire.ath.cx>
---
 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,

  reply	other threads:[~2009-12-13  2:16 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-10 21:26 [BUG] git archive formats and dashdash Ilari Liusvaara
2009-12-10 22:05 ` Junio C Hamano
2009-12-10 22:22   ` Miklos Vajna
2009-12-10 22:42     ` Junio C Hamano
2009-12-10 23:27       ` [PATCH] Fix archive format with -- on the command line Junio C Hamano
2009-12-10 23:31         ` Junio C Hamano
2009-12-12 15:00           ` René Scharfe [this message]
2009-12-30  3:13             ` Nanako Shiraishi
2009-12-30  8:11               ` Junio C Hamano
2009-12-10 22:25   ` [BUG] git archive formats and dashdash Ilari Liusvaara
2009-12-10 22:20 ` [PATCH] builtin-archive: insert --format before double dash if necessary Miklos Vajna

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B23B019.40106@lsrfire.ath.cx \
    --to=rene.scharfe@lsrfire.ath.cx \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ilari.liusvaara@elisanet.fi \
    --cc=vmiklos@frugalware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.