{"thread":{"id":"4409","subject":"[PATCH] git-format-patch: add --output-directory long option again","startedAt":"2006-06-06T14:19:46Z","lastAt":"2006-06-06T18:22:07Z","messageCount":4,"participants":["Dennis Stosberg","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"21318","messageId":"20060606141946.G59b8a1fd@leonov.stosberg.net","threadId":"4409","inReplyTo":null,"subject":"[PATCH] git-format-patch: add --output-directory long option again","fromName":"Dennis Stosberg","fromEmail":"dennis@stosberg.net","sentAt":"2006-06-06T14:19:46Z","receivedAt":"2006-06-06T14:19:46Z","isPatch":true,"sender":{"key":"dennis@stosberg.net","avatar":null},"body":"Additionally this fixes two minor issues:\n\n(1) git-format-patch left an empty directory behind if an invalid\n    option was given on the command line.\n\n(2) mkdir() was called with a NULL argument (argv[argc]), if -o was\n    the last option on the command line.\n\nSigned-off-by: Dennis Stosberg <dennis@stosberg.net>\n---\n builtin-log.c |   18 +++++++++++-------\n 1 files changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 6612f4c..0018d13 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -206,14 +206,12 @@ int cmd_format_patch(int argc, const cha\n \t\t\tkeep_subject = 1;\n \t\t\trev.total = -1;\n \t\t}\n-\t\telse if (!strcmp(argv[i], \"-o\")) {\n-\t\t\tif (argc < 3)\n-\t\t\t\tdie (\"Which directory?\");\n-\t\t\tif (mkdir(argv[i + 1], 0777) < 0 && errno != EEXIST)\n-\t\t\t\tdie(\"Could not create directory %s\",\n-\t\t\t\t\t\targv[i + 1]);\n-\t\t\toutput_directory = strdup(argv[i + 1]);\n+\t\telse if (!strcmp(argv[i], \"-o\") ||\n+\t\t\t\t!strcmp(argv[i], \"--output-directory\")) {\n \t\t\ti++;\n+\t\t\tif (i == argc)\n+\t\t\t\tdie (\"Which directory?\");\n+\t\t\toutput_directory = strdup(argv[i]);\n \t\t}\n \t\telse if (!strcmp(argv[i], \"--signoff\") ||\n \t\t\t !strcmp(argv[i], \"-s\")) {\n@@ -243,6 +241,12 @@ int cmd_format_patch(int argc, const cha\n \tif (argc > 1)\n \t\tdie (\"unrecognized argument: %s\", argv[1]);\n \n+\tif (output_directory && !stdout) {\n+\t\tif (mkdir(output_directory, 0777) < 0 && errno != EEXIST)\n+\t\t\tdie(\"Could not create directory %s\",\n+\t\t\t    output_directory);\n+\t}\n+\n \tif (rev.pending_objects && rev.pending_objects->next == NULL) {\n \t\trev.pending_objects->item->flags |= UNINTERESTING;\n \t\tadd_head(&rev);\n-- \n1.3.3+git20060602-dest0\n"},{"id":"21319","messageId":"Pine.LNX.4.63.0606061650360.2063@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4409","inReplyTo":"20060606141946.G59b8a1fd@leonov.stosberg.net","subject":"Re: [PATCH] git-format-patch: add --output-directory long option again","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-06T14:53:25Z","receivedAt":"2006-06-06T14:53:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nwhat idiot wrote that part of the code originally? ;-)\n\nOn Tue, 6 Jun 2006, Dennis Stosberg wrote:\n\n> @@ -243,6 +241,12 @@ int cmd_format_patch(int argc, const cha\n>  \tif (argc > 1)\n>  \t\tdie (\"unrecognized argument: %s\", argv[1]);\n>  \n> +\tif (output_directory && !stdout) {\n> +\t\tif (mkdir(output_directory, 0777) < 0 && errno != EEXIST)\n> +\t\t\tdie(\"Could not create directory %s\",\n> +\t\t\t    output_directory);\n> +\t}\n> +\n>  \tif (rev.pending_objects && rev.pending_objects->next == NULL) {\n>  \t\trev.pending_objects->item->flags |= UNINTERESTING;\n>  \t\tadd_head(&rev);\n\nWould it not be better to\n\n\tif (output_directory && stdout)\n\t\tdie(\"What do you want: stdout or a directory?\");\n\nCiao,\nDscho\n"},{"id":"21323","messageId":"7vwtbuthgw.fsf@assigned-by-dhcp.cox.net","threadId":"4409","inReplyTo":"20060606141946.G59b8a1fd@leonov.stosberg.net","subject":"Re: [PATCH] git-format-patch: add --output-directory long option again","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-06T15:46:23Z","receivedAt":"2006-06-06T15:46:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dennis Stosberg <dennis@stosberg.net> writes:\n\n> Additionally this fixes two minor issues:\n\nThanks.\n\n> -\t\telse if (!strcmp(argv[i], \"-o\")) {\n> -\t\t\tif (argc < 3)\n> -\t\t\t\tdie (\"Which directory?\");\n\nOops.\n\n> +\tif (output_directory && !stdout) {\n> +\t\tif (mkdir(output_directory, 0777) < 0 && errno != EEXIST)\n> +\t\t\tdie(\"Could not create directory %s\",\n> +\t\t\t    output_directory);\n> +\t}\n> +\n\nThis code couldn't have been tested -- you meant to say\n\"use_stdout\" here, not \"stdout\".\n\nHow about this instead?\n\n-- >8 --\n[PATCH] git-format-patch: add --output-directory long option again\n\nAdditionally notices and complains to an -o option without\ndirectory or a duplicated -o option, -o and --stdout given\ntogether.  Also delays the creation of directory until all\narguments are parsed, so that the command does not leave an\nempty directory behind when it exits after seeing an unrelated\ninvalid option.\n\n[Originally from Dennis Stosberg but with minor fixes.]\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n builtin-log.c |   26 ++++++++++++++++----------\n 1 files changed, 16 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 6612f4c..29a8851 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -103,7 +103,7 @@ static int git_format_config(const char \n \n \n static FILE *realstdout = NULL;\n-static char *output_directory = NULL;\n+static const char *output_directory = NULL;\n \n static void reopen_stdout(struct commit *commit, int nr, int keep_subject)\n {\n@@ -206,14 +206,14 @@ int cmd_format_patch(int argc, const cha\n \t\t\tkeep_subject = 1;\n \t\t\trev.total = -1;\n \t\t}\n-\t\telse if (!strcmp(argv[i], \"-o\")) {\n-\t\t\tif (argc < 3)\n-\t\t\t\tdie (\"Which directory?\");\n-\t\t\tif (mkdir(argv[i + 1], 0777) < 0 && errno != EEXIST)\n-\t\t\t\tdie(\"Could not create directory %s\",\n-\t\t\t\t\t\targv[i + 1]);\n-\t\t\toutput_directory = strdup(argv[i + 1]);\n+\t\telse if (!strcmp(argv[i], \"--output-directory\") ||\n+\t\t\t !strcmp(argv[i], \"-o\")) {\n \t\t\ti++;\n+\t\t\tif (argc <= i)\n+\t\t\t\tdie(\"Which directory?\");\n+\t\t\tif (output_directory)\n+\t\t\t\tdie(\"Two output directories?\");\n+\t\t\toutput_directory = argv[i];\n \t\t}\n \t\telse if (!strcmp(argv[i], \"--signoff\") ||\n \t\t\t !strcmp(argv[i], \"-s\")) {\n@@ -243,6 +243,14 @@ int cmd_format_patch(int argc, const cha\n \tif (argc > 1)\n \t\tdie (\"unrecognized argument: %s\", argv[1]);\n \n+\tif (output_directory) {\n+\t\tif (use_stdout)\n+\t\t\tdie(\"standard output, or directory, which one?\");\n+\t\tif (mkdir(output_directory, 0777) < 0 && errno != EEXIST)\n+\t\t\tdie(\"Could not create directory %s\",\n+\t\t\t    output_directory);\n+\t}\n+\n \tif (rev.pending_objects && rev.pending_objects->next == NULL) {\n \t\trev.pending_objects->item->flags |= UNINTERESTING;\n \t\tadd_head(&rev);\n@@ -293,8 +301,6 @@ int cmd_format_patch(int argc, const cha\n \t\tif (!use_stdout)\n \t\t\tfclose(stdout);\n \t}\n-\tif (output_directory)\n-\t\tfree(output_directory);\n \tfree(list);\n \treturn 0;\n }\n-- \n1.4.0.rc1-dirty\n"},{"id":"21333","messageId":"20060606182206.G36caaa7f@leonov.stosberg.net","threadId":"4409","inReplyTo":"7vwtbuthgw.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-format-patch: add --output-directory long option again","fromName":"Dennis Stosberg","fromEmail":"dennis@stosberg.net","sentAt":"2006-06-06T18:22:07Z","receivedAt":"2006-06-06T18:22:07Z","isPatch":true,"sender":{"key":"dennis@stosberg.net","avatar":null},"body":"Junio C Hamano wrote:\n\n> > +\tif (output_directory && !stdout) {\n> > +\t\tif (mkdir(output_directory, 0777) < 0 && errno != EEXIST)\n> > +\t\t\tdie(\"Could not create directory %s\",\n> > +\t\t\t    output_directory);\n> > +\t}\n> > +\n> \n> This code couldn't have been tested -- you meant to say\n> \"use_stdout\" here, not \"stdout\".\n\nSorry.  My version compiled and ran cleanly, but I was blinded by my\ngoal: I wanted the directory to be _not_ created and checked that,\nbut I didn't make sure, that the directory really existed in the\nother case, since Git didn't produce an error.\n \n> How about this instead?\n> [...]\n> +\tif (output_directory) {\n> +\t\tif (use_stdout)\n> +\t\t\tdie(\"standard output, or directory, which one?\");\n\nLooks good.  This piece changes behaviour; it no longer matches the\ndocs.\n\nRegards,\nDennis\n\n---\n Documentation/git-format-patch.txt |    3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 493cac2..4ca0014 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -40,8 +40,7 @@ OPTIONS\n -------\n -o|--output-directory <dir>::\n \tUse <dir> to store the resulting files, instead of the\n-\tcurrent working directory. This option is ignored if\n-\t--stdout is specified.\n+\tcurrent working directory.\n \n -n|--numbered::\n \tName output in '[PATCH n/m]' format.\n"}]}