{"thread":{"id":"34958","subject":"[PATCH] fetch-pack.c: show correct command name that fails","startedAt":"2013-09-18T13:41:18Z","lastAt":"2013-09-18T18:20:53Z","messageCount":3,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"227836","messageId":"1379511678-11611-1-git-send-email-pclouds@gmail.com","threadId":"34958","inReplyTo":null,"subject":"[PATCH] fetch-pack.c: show correct command name that fails","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-09-18T13:41:18Z","receivedAt":"2013-09-18T13:41:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When --shallow-file is added to the command line, it has to be\nbefore the subcommand name, the first argument won't be the command\nname any more. Stop assuming that and keep track of the command name\nexplicitly.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n fetch-pack.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 6684348..b259c51 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -688,7 +688,7 @@ static int get_pack(struct fetch_pack_args *args,\n \tconst char *argv[22];\n \tchar keep_arg[256];\n \tchar hdr_arg[256];\n-\tconst char **av;\n+\tconst char **av, *cmd_name;\n \tint do_keep = args->keep_pack;\n \tstruct child_process cmd;\n \tint ret;\n@@ -735,7 +735,7 @@ static int get_pack(struct fetch_pack_args *args,\n \tif (do_keep) {\n \t\tif (pack_lockfile)\n \t\t\tcmd.out = -1;\n-\t\t*av++ = \"index-pack\";\n+\t\t*av++ = cmd_name = \"index-pack\";\n \t\t*av++ = \"--stdin\";\n \t\tif (!args->quiet && !args->no_progress)\n \t\t\t*av++ = \"-v\";\n@@ -752,7 +752,7 @@ static int get_pack(struct fetch_pack_args *args,\n \t\t\t*av++ = \"--check-self-contained-and-connected\";\n \t}\n \telse {\n-\t\t*av++ = \"unpack-objects\";\n+\t\t*av++ = cmd_name = \"unpack-objects\";\n \t\tif (args->quiet || args->no_progress)\n \t\t\t*av++ = \"-q\";\n \t\targs->check_self_contained_and_connected = 0;\n@@ -770,7 +770,7 @@ static int get_pack(struct fetch_pack_args *args,\n \tcmd.in = demux.out;\n \tcmd.git_cmd = 1;\n \tif (start_command(&cmd))\n-\t\tdie(\"fetch-pack: unable to fork off %s\", argv[0]);\n+\t\tdie(\"fetch-pack: unable to fork off %s\", cmd_name);\n \tif (do_keep && pack_lockfile) {\n \t\t*pack_lockfile = index_pack_lockfile(cmd.out);\n \t\tclose(cmd.out);\n@@ -782,7 +782,7 @@ static int get_pack(struct fetch_pack_args *args,\n \t\t\targs->check_self_contained_and_connected &&\n \t\t\tret == 0;\n \telse\n-\t\tdie(\"%s failed\", argv[0]);\n+\t\tdie(\"%s failed\", cmd_name);\n \tif (use_sideband && finish_async(&demux))\n \t\tdie(\"error in sideband demultiplexer\");\n \treturn 0;\n-- \n1.8.2.83.gc99314b\n"},{"id":"227849","messageId":"xmqqvc1yuhao.fsf@gitster.dls.corp.google.com","threadId":"34958","inReplyTo":"1379511678-11611-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] fetch-pack.c: show correct command name that fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T18:16:15Z","receivedAt":"2013-09-18T18:16:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> When --shallow-file is added to the command line, it has to be\n> before the subcommand name, the first argument won't be the command\n> name any more. Stop assuming that and keep track of the command name\n> explicitly.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n\nThanks; this dates back to v1.8.3.2 but it is at the end of an error\npath and only for reporting, so it may not be worth merging it down\nto the 1.8.3.x maintenance track.\n\nI wondered if this is something we can have a test for easily.\nUnlike the \"--upload-pack\" option, there is no way for the receiving\nend to choose which index-pack (or unpack-objects) to run, but we\nmay force a failure by temporarily futzing with PATH to make a dummy\ngit-index-pack that always fails to be found or something silly like\nthat.\n\n>  fetch-pack.c | 10 +++++-----\n>  1 file changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index 6684348..b259c51 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -688,7 +688,7 @@ static int get_pack(struct fetch_pack_args *args,\n>  \tconst char *argv[22];\n>  \tchar keep_arg[256];\n>  \tchar hdr_arg[256];\n> -\tconst char **av;\n> +\tconst char **av, *cmd_name;\n>  \tint do_keep = args->keep_pack;\n>  \tstruct child_process cmd;\n>  \tint ret;\n> @@ -735,7 +735,7 @@ static int get_pack(struct fetch_pack_args *args,\n>  \tif (do_keep) {\n>  \t\tif (pack_lockfile)\n>  \t\t\tcmd.out = -1;\n> -\t\t*av++ = \"index-pack\";\n> +\t\t*av++ = cmd_name = \"index-pack\";\n>  \t\t*av++ = \"--stdin\";\n>  \t\tif (!args->quiet && !args->no_progress)\n>  \t\t\t*av++ = \"-v\";\n> @@ -752,7 +752,7 @@ static int get_pack(struct fetch_pack_args *args,\n>  \t\t\t*av++ = \"--check-self-contained-and-connected\";\n>  \t}\n>  \telse {\n> -\t\t*av++ = \"unpack-objects\";\n> +\t\t*av++ = cmd_name = \"unpack-objects\";\n>  \t\tif (args->quiet || args->no_progress)\n>  \t\t\t*av++ = \"-q\";\n>  \t\targs->check_self_contained_and_connected = 0;\n> @@ -770,7 +770,7 @@ static int get_pack(struct fetch_pack_args *args,\n>  \tcmd.in = demux.out;\n>  \tcmd.git_cmd = 1;\n>  \tif (start_command(&cmd))\n> -\t\tdie(\"fetch-pack: unable to fork off %s\", argv[0]);\n> +\t\tdie(\"fetch-pack: unable to fork off %s\", cmd_name);\n>  \tif (do_keep && pack_lockfile) {\n>  \t\t*pack_lockfile = index_pack_lockfile(cmd.out);\n>  \t\tclose(cmd.out);\n> @@ -782,7 +782,7 @@ static int get_pack(struct fetch_pack_args *args,\n>  \t\t\targs->check_self_contained_and_connected &&\n>  \t\t\tret == 0;\n>  \telse\n> -\t\tdie(\"%s failed\", argv[0]);\n> +\t\tdie(\"%s failed\", cmd_name);\n>  \tif (use_sideband && finish_async(&demux))\n>  \t\tdie(\"error in sideband demultiplexer\");\n>  \treturn 0;\n"},{"id":"227850","messageId":"20130918182052.GB18821@sigill.intra.peff.net","threadId":"34958","inReplyTo":"xmqqvc1yuhao.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] fetch-pack.c: show correct command name that fails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-18T18:20:53Z","receivedAt":"2013-09-18T18:20:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 18, 2013 at 11:16:15AM -0700, Junio C Hamano wrote:\n\n> I wondered if this is something we can have a test for easily.\n> Unlike the \"--upload-pack\" option, there is no way for the receiving\n> end to choose which index-pack (or unpack-objects) to run, but we\n> may force a failure by temporarily futzing with PATH to make a dummy\n> git-index-pack that always fails to be found or something silly like\n> that.\n\nYou can feed a bogus pack, which will cause either to fail reliably. I'm\nnot sure if that counts as \"easy\" though. :) Munging PATH is probably\nsimpler.\n\n> >  fetch-pack.c | 10 +++++-----\n> >  1 file changed, 5 insertions(+), 5 deletions(-)\n\nPatch looks good to me, too.\n\n-Peff\n"}]}