[PATCH v2] send-pack: avoid redundant "pack-objects died with strange error"

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 16, 2010, 06:04 UTC
Message-ID: <20101016060423.GA26538@burratino>
In-Reply-To: <20101012201946.GA15080@burratino>

Saying "pack-objects died with strange error" after "pack-objects died of signal 13" seems kind of redundant. The latter was introduced when send-pack switched to the run-command API, which reports abnormal exits on behalf of the caller.

Normal exits with nonzero status are not reported by run-command, though. Be more helpful in reporting them by including the exit status while at it (and be sure to continue to return a value less than zero from pack_objects() for that case).

The result should look something like this:

	$ git push sf master
	Counting objects: 21542, done.
	Compressing objects: 100% (4179/4179), done.
	fatal: Unable to create temporary file: Permission denied
	error: pack-objects died of signal 13
	error: failed to push some refs to 'ssh://sf.net/gitroot/project/project'
	$

Or in the "controlled exit" case:

	[...]
	error: pack-objects died with status 128
	error: failed to push some refs to 'ssh://example.com/foo/bar'
	$

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jonathan Nieder wrote:

Show 11 quoted lines

> +++ b/builtin/send-pack.c
> @@ -100,9 +100,10 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
>  		po.out = -1;
>  	}
>  
> -	if (finish_command(&po))
> -		return error("pack-objects died with strange error");
> +	status = finish_command(&po);
> +	if (status > 0)
> +		error("pack-objects died with status %d", status);
> +	return status;

Caller:

	if (pack_objects(...) < 0)

So making pack_objects return >0 where it used to return <0 would be a regression. :( An updated patch with the following minimal fix squashed in follows.

	status = finish_command(&po);
	if (status > 0)
 -		error("pack-objects died with status %d", status);
 +		return error("pack-objects died with status %d", status);
	return status;
  }

 builtin/send-pack.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 481602d..8854748 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -50,7 +50,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 		NULL,
 	};
 	struct child_process po;
-	int i;
+	int i, status;
 
 	i = 4;
 	if (args->use_thin_pack)
@@ -100,9 +100,10 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 		po.out = -1;
 	}
 
-	if (finish_command(&po))
-		return error("pack-objects died with strange error");
-	return 0;
+	status = finish_command(&po);
+	if (status > 0)
+		return error("pack-objects died with status %d", status);
+	return status;
 }
 
 static int receive_status(int in, struct ref *refs)

-- 
1.7.2.3