git-upload-archive help was not shown correctly

git-upload-archive help was not shown correctly

[devendra]

Hi git folks,

the command git-upload-archive is not properly showing usage info when
ran barely with out any args.

it shows some kind of unwanted garbage instead of showing a nice help
message.

The output is pasted here.

root@devendra-Linux:/home/devendra/git/Documentation#
git-upload-archive 
0008ACK
00000026 usage: git upload-archive <repo>
0031 git upload-archive: archiver died with errorfatal: sent error to
the client: git upload-archive: archiver died with error

my git latest version shows commit sha1 number
6db5c6e43dccb380ca6e9947777985eb11248c31.

Devendra.

Re: git-upload-archive help was not shown correctly

[Carlos Martín Nieto]

[-- Attachment #1: Type: text/plain, Size: 525 bytes --]

On Sat, Jan 14, 2012 at 07:10:16PM +0530, devendra wrote:
> Hi git folks,
> 
> the command git-upload-archive is not properly showing usage info when
> ran barely with out any args.

git-upload-package is not for human use. It's what gets run on the
remote end when you run e.g. 'git archive --remote=origin HEAD'

> 
> it shows some kind of unwanted garbage instead of showing a nice help
> message.

It's trying to talk to git. What you see is the "Git Smart
Protocol". What were you trying to do?

   cmn

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: git-upload-archive help was not shown correctly

[Jeff King]

On Sat, Jan 14, 2012 at 04:46:33PM +0100, Carlos Martín Nieto wrote:

> On Sat, Jan 14, 2012 at 07:10:16PM +0530, devendra wrote:
> > Hi git folks,
> > 
> > the command git-upload-archive is not properly showing usage info when
> > ran barely with out any args.
> 
> git-upload-package is not for human use. It's what gets run on the
> remote end when you run e.g. 'git archive --remote=origin HEAD'

Yeah, but notice that it does detect the error condition; it just wraps
it in a bunch of cruft. More explanation (and a patch) are below, though
I'm lukewarm on the patch.

> > it shows some kind of unwanted garbage instead of showing a nice help
> > message.
> 
> It's trying to talk to git. What you see is the "Git Smart
> Protocol". What were you trying to do?

Yeah, this is the more important question. You shouldn't generally ever
be running git-upload-* yourself.

-- >8 --
Subject: upload-archive: detect bad command-line arguments early

The upload-archive command spawns a helper process to
actually generate the archive (passing along all of its
command-line arguments), and the parent process then
multiplexes the stdout and stderr of the helper back to the
client.

This means an incorrect invocation (which in this case means
failing to provide a single <repo> argument) will cause the
helper to complain, and the error will be sent to the client
over the multiplexed channel.

This is not ideal for two reasons:

  1. An invocation error in upload-pack is almost certainly
     not something the client can do anything about. It
     generally implies a bug in either git-daemon or
     git-archive. The one exception is something like:

       git archive --remote=ssh_host:repo.git \
                   --exec="git upload-archive bogus" \

  2. For local users who are experimenting with git
     commands, running "upload-archive" appears to generate
     a bunch of barely-readable garbage (which is in fact
     the git protocol intermingled with stderr). It's nicer
     if we provide them with a readable error message.

Instead, we can detect a bad command-line before we spawn
the helper and give a more human-readable complaint (i.e.,
helping reason (2) above). This doesn't hurt case (1) as
much as you might think, because in the ssh case, the user
should be receiving stderr from the parent process anyway.

So before they saw:

  $ git archive --remote=... --exec='git upload-archive bogus'
  fatal: sent error to the client: git upload-archive: archiver died with error
  remote: usage: git upload-archive <repo>
  remote: git upload-archive: archiver died with error

and now they see:

  usage: git upload-archive <repo>
  fatal: The remote end hung up unexpectedly

Signed-off-by: Jeff King <peff@peff.net>
---
I'm lukewarm on this patch. I actually think the original output is
slightly nicer, so you are hurting case (1) for people in case (2). But
case (2), running random programs to see what happens, is not something
we probably need to be encouraging.

 builtin/upload-archive.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index b928beb..0733775 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -10,6 +10,8 @@
 
 static const char upload_archive_usage[] =
 	"git upload-archive <repo>";
+static const char upload_archive_writer_usage[] =
+	"git upload-archive--writer <repo>";
 
 static const char deadchild[] =
 "git upload-archive: archiver died with error";
@@ -25,7 +27,7 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)
 	int len;
 
 	if (argc != 2)
-		usage(upload_archive_usage);
+		usage(upload_archive_writer_usage);
 
 	if (strlen(argv[1]) + 1 > sizeof(buf))
 		die("insanely long repository name");
@@ -96,6 +98,9 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 {
 	struct child_process writer = { argv };
 
+	if (argc != 2)
+		usage(upload_archive_usage);
+
 	/*
 	 * Set up sideband subprocess.
 	 *
-- 
1.7.9.rc0.33.gd3c17