threads / patch / 4409

patchgit-format-patch: add --output-directory long option again

Subject: [PATCH] git-format-patch: add --output-directory long option again

## tl;dr

4 messages between Jun 6, 2006 and Jun 6, 2006. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Dennis Stosberg· Jun 6, 2006, 14:19 UTC · lore

Additionally this fixes two minor issues:

(1) git-format-patch left an empty directory behind if an invalid
    option was given on the command line.

(2) mkdir() was called with a NULL argument (argv[argc]), if -o was
    the last option on the command line.

Signed-off-by: Dennis Stosberg <dennis@stosberg.net>
---
 builtin-log.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

Show changes to builtin-log.c +11 −7

diff --git a/builtin-log.c b/builtin-log.c
index 6612f4c..0018d13 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -206,14 +206,12 @@ int cmd_format_patch(int argc, const cha
 			keep_subject = 1;
 			rev.total = -1;
 		}
-		else if (!strcmp(argv[i], "-o")) {
-			if (argc < 3)
-				die ("Which directory?");
-			if (mkdir(argv[i + 1], 0777) < 0 && errno != EEXIST)
-				die("Could not create directory %s",
-						argv[i + 1]);
-			output_directory = strdup(argv[i + 1]);
+		else if (!strcmp(argv[i], "-o") ||
+				!strcmp(argv[i], "--output-directory")) {
 			i++;
+			if (i == argc)
+				die ("Which directory?");
+			output_directory = strdup(argv[i]);
 		}
 		else if (!strcmp(argv[i], "--signoff") ||
 			 !strcmp(argv[i], "-s")) {
@@ -243,6 +241,12 @@ int cmd_format_patch(int argc, const cha
 	if (argc > 1)
 		die ("unrecognized argument: %s", argv[1]);
 
+	if (output_directory && !stdout) {
+		if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
+			die("Could not create directory %s",
+			    output_directory);
+	}
+
 	if (rev.pending_objects && rev.pending_objects->next == NULL) {
 		rev.pending_objects->item->flags |= UNINTERESTING;
 		add_head(&rev);

-- 
1.3.3+git20060602-dest0

Johannes Schindelin· Jun 6, 2006, 14:53 UTC · re: Dennis Stosberg · lore

Re: [PATCH] git-format-patch: add --output-directory long option again

Hi,

what idiot wrote that part of the code originally? ;-)

On Tue, 6 Jun 2006, Dennis Stosberg wrote:

Show 13 quoted lines

> @@ -243,6 +241,12 @@ int cmd_format_patch(int argc, const cha
>  	if (argc > 1)
>  		die ("unrecognized argument: %s", argv[1]);
>  
> +	if (output_directory && !stdout) {
> +		if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
> +			die("Could not create directory %s",
> +			    output_directory);
> +	}
> +
>  	if (rev.pending_objects && rev.pending_objects->next == NULL) {
>  		rev.pending_objects->item->flags |= UNINTERESTING;
>  		add_head(&rev);

Would it not be better to

	if (output_directory && stdout)
		die("What do you want: stdout or a directory?");

Ciao, Dscho

Junio C Hamano· Jun 6, 2006, 15:46 UTC · re: Dennis Stosberg · lore

Re: [PATCH] git-format-patch: add --output-directory long option again

Dennis Stosberg <dennis@stosberg.net> writes:

> Additionally this fixes two minor issues:

Thanks.

> -		else if (!strcmp(argv[i], "-o")) {
> -			if (argc < 3)
> -				die ("Which directory?");

Oops.

Show 6 quoted lines

> +	if (output_directory && !stdout) {
> +		if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
> +			die("Could not create directory %s",
> +			    output_directory);
> +	}
> +

This code couldn't have been tested -- you meant to say "use_stdout" here, not "stdout".

How about this instead?

-- >8 -- [PATCH] git-format-patch: add --output-directory long option again

Additionally notices and complains to an -o option without directory or a duplicated -o option, -o and --stdout given together. Also delays the creation of directory until all arguments are parsed, so that the command does not leave an empty directory behind when it exits after seeing an unrelated invalid option.

[Originally from Dennis Stosberg but with minor fixes.]

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-log.c |   26 ++++++++++++++++----------
 1 files changed, 16 insertions(+), 10 deletions(-)

Show changes to builtin-log.c +16 −10

diff --git a/builtin-log.c b/builtin-log.c
index 6612f4c..29a8851 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -103,7 +103,7 @@ static int git_format_config(const char 
 
 
 static FILE *realstdout = NULL;
-static char *output_directory = NULL;
+static const char *output_directory = NULL;
 
 static void reopen_stdout(struct commit *commit, int nr, int keep_subject)
 {
@@ -206,14 +206,14 @@ int cmd_format_patch(int argc, const cha
 			keep_subject = 1;
 			rev.total = -1;
 		}
-		else if (!strcmp(argv[i], "-o")) {
-			if (argc < 3)
-				die ("Which directory?");
-			if (mkdir(argv[i + 1], 0777) < 0 && errno != EEXIST)
-				die("Could not create directory %s",
-						argv[i + 1]);
-			output_directory = strdup(argv[i + 1]);
+		else if (!strcmp(argv[i], "--output-directory") ||
+			 !strcmp(argv[i], "-o")) {
 			i++;
+			if (argc <= i)
+				die("Which directory?");
+			if (output_directory)
+				die("Two output directories?");
+			output_directory = argv[i];
 		}
 		else if (!strcmp(argv[i], "--signoff") ||
 			 !strcmp(argv[i], "-s")) {
@@ -243,6 +243,14 @@ int cmd_format_patch(int argc, const cha
 	if (argc > 1)
 		die ("unrecognized argument: %s", argv[1]);
 
+	if (output_directory) {
+		if (use_stdout)
+			die("standard output, or directory, which one?");
+		if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
+			die("Could not create directory %s",
+			    output_directory);
+	}
+
 	if (rev.pending_objects && rev.pending_objects->next == NULL) {
 		rev.pending_objects->item->flags |= UNINTERESTING;
 		add_head(&rev);
@@ -293,8 +301,6 @@ int cmd_format_patch(int argc, const cha
 		if (!use_stdout)
 			fclose(stdout);
 	}
-	if (output_directory)
-		free(output_directory);
 	free(list);
 	return 0;
 }

-- 
1.4.0.rc1-dirty

Dennis Stosberg· Jun 6, 2006, 18:22 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-format-patch: add --output-directory long option again

Junio C Hamano wrote:

Show 9 quoted lines

> > +	if (output_directory && !stdout) {
> > +		if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
> > +			die("Could not create directory %s",
> > +			    output_directory);
> > +	}
> > +
> 
> This code couldn't have been tested -- you meant to say
> "use_stdout" here, not "stdout".

Sorry.  My version compiled and ran cleanly, but I was blinded by my
goal: I wanted the directory to be _not_ created and checked that,
but I didn't make sure, that the directory really existed in the
other case, since Git didn't produce an error.

Show 5 quoted lines

> How about this instead?
> [...]
> +	if (output_directory) {
> +		if (use_stdout)
> +			die("standard output, or directory, which one?");

Looks good. This piece changes behaviour; it no longer matches the docs.

Regards, Dennis

---
 Documentation/git-format-patch.txt |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

Show changes to Documentation/git-format-patch.txt +1 −2

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 493cac2..4ca0014 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -40,8 +40,7 @@ OPTIONS
 -------
 -o|--output-directory <dir>::
 	Use <dir> to store the resulting files, instead of the
-	current working directory. This option is ignored if
-	--stdout is specified.
+	current working directory.
 
 -n|--numbered::
 	Name output in '[PATCH n/m]' format.