[PATCH] fetch-pack.c: show correct command name that fails

[PATCH] fetch-pack.c: show correct command name that fails

[Nguyễn Thái Ngọc Duy]

When --shallow-file is added to the command line, it has to be
before the subcommand name, the first argument won't be the command
name any more. Stop assuming that and keep track of the command name
explicitly.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 fetch-pack.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 6684348..b259c51 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -688,7 +688,7 @@ static int get_pack(struct fetch_pack_args *args,
 	const char *argv[22];
 	char keep_arg[256];
 	char hdr_arg[256];
-	const char **av;
+	const char **av, *cmd_name;
 	int do_keep = args->keep_pack;
 	struct child_process cmd;
 	int ret;
@@ -735,7 +735,7 @@ static int get_pack(struct fetch_pack_args *args,
 	if (do_keep) {
 		if (pack_lockfile)
 			cmd.out = -1;
-		*av++ = "index-pack";
+		*av++ = cmd_name = "index-pack";
 		*av++ = "--stdin";
 		if (!args->quiet && !args->no_progress)
 			*av++ = "-v";
@@ -752,7 +752,7 @@ static int get_pack(struct fetch_pack_args *args,
 			*av++ = "--check-self-contained-and-connected";
 	}
 	else {
-		*av++ = "unpack-objects";
+		*av++ = cmd_name = "unpack-objects";
 		if (args->quiet || args->no_progress)
 			*av++ = "-q";
 		args->check_self_contained_and_connected = 0;
@@ -770,7 +770,7 @@ static int get_pack(struct fetch_pack_args *args,
 	cmd.in = demux.out;
 	cmd.git_cmd = 1;
 	if (start_command(&cmd))
-		die("fetch-pack: unable to fork off %s", argv[0]);
+		die("fetch-pack: unable to fork off %s", cmd_name);
 	if (do_keep && pack_lockfile) {
 		*pack_lockfile = index_pack_lockfile(cmd.out);
 		close(cmd.out);
@@ -782,7 +782,7 @@ static int get_pack(struct fetch_pack_args *args,
 			args->check_self_contained_and_connected &&
 			ret == 0;
 	else
-		die("%s failed", argv[0]);
+		die("%s failed", cmd_name);
 	if (use_sideband && finish_async(&demux))
 		die("error in sideband demultiplexer");
 	return 0;
-- 
1.8.2.83.gc99314b

Re: [PATCH] fetch-pack.c: show correct command name that fails

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> When --shallow-file is added to the command line, it has to be
> before the subcommand name, the first argument won't be the command
> name any more. Stop assuming that and keep track of the command name
> explicitly.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

Thanks; this dates back to v1.8.3.2 but it is at the end of an error
path and only for reporting, so it may not be worth merging it down
to the 1.8.3.x maintenance track.

I wondered if this is something we can have a test for easily.
Unlike the "--upload-pack" option, there is no way for the receiving
end to choose which index-pack (or unpack-objects) to run, but we
may force a failure by temporarily futzing with PATH to make a dummy
git-index-pack that always fails to be found or something silly like
that.

>  fetch-pack.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 6684348..b259c51 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -688,7 +688,7 @@ static int get_pack(struct fetch_pack_args *args,
>  	const char *argv[22];
>  	char keep_arg[256];
>  	char hdr_arg[256];
> -	const char **av;
> +	const char **av, *cmd_name;
>  	int do_keep = args->keep_pack;
>  	struct child_process cmd;
>  	int ret;
> @@ -735,7 +735,7 @@ static int get_pack(struct fetch_pack_args *args,
>  	if (do_keep) {
>  		if (pack_lockfile)
>  			cmd.out = -1;
> -		*av++ = "index-pack";
> +		*av++ = cmd_name = "index-pack";
>  		*av++ = "--stdin";
>  		if (!args->quiet && !args->no_progress)
>  			*av++ = "-v";
> @@ -752,7 +752,7 @@ static int get_pack(struct fetch_pack_args *args,
>  			*av++ = "--check-self-contained-and-connected";
>  	}
>  	else {
> -		*av++ = "unpack-objects";
> +		*av++ = cmd_name = "unpack-objects";
>  		if (args->quiet || args->no_progress)
>  			*av++ = "-q";
>  		args->check_self_contained_and_connected = 0;
> @@ -770,7 +770,7 @@ static int get_pack(struct fetch_pack_args *args,
>  	cmd.in = demux.out;
>  	cmd.git_cmd = 1;
>  	if (start_command(&cmd))
> -		die("fetch-pack: unable to fork off %s", argv[0]);
> +		die("fetch-pack: unable to fork off %s", cmd_name);
>  	if (do_keep && pack_lockfile) {
>  		*pack_lockfile = index_pack_lockfile(cmd.out);
>  		close(cmd.out);
> @@ -782,7 +782,7 @@ static int get_pack(struct fetch_pack_args *args,
>  			args->check_self_contained_and_connected &&
>  			ret == 0;
>  	else
> -		die("%s failed", argv[0]);
> +		die("%s failed", cmd_name);
>  	if (use_sideband && finish_async(&demux))
>  		die("error in sideband demultiplexer");
>  	return 0;

Re: [PATCH] fetch-pack.c: show correct command name that fails

[Jeff King]

On Wed, Sep 18, 2013 at 11:16:15AM -0700, Junio C Hamano wrote:

> I wondered if this is something we can have a test for easily.
> Unlike the "--upload-pack" option, there is no way for the receiving
> end to choose which index-pack (or unpack-objects) to run, but we
> may force a failure by temporarily futzing with PATH to make a dummy
> git-index-pack that always fails to be found or something silly like
> that.

You can feed a bogus pack, which will cause either to fail reliably. I'm
not sure if that counts as "easy" though. :) Munging PATH is probably
simpler.

> >  fetch-pack.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)

Patch looks good to me, too.

-Peff