git-format-patch with -o ../ in subdir of working copy writes output in wrong place

PATCH In review Cesar Eduardo Barros 2 participants 5 messages Jun 15, 2016

Share & tools

Cesar Eduardo Barros

author

2016-06-15 22:45 UTC #0 lore raw

If you are in a subdirectory of your working copy (for instance, linux-2.6/drivers/net) and use git-format-patch with -o to a sequence of ../ (for instance, -o ../../../) to write to the working copy's parent directory, it instead interprets the directory passed to -o as relative to the root of the working copy, instead of the expected current directory.

Testcase:
mkdir a
cd a
git init
mkdir b
touch b/c
git add b/c
git commit -m 'test'
cd b
echo 'test' > c
git commit -a -m 'test'
git format-patch -o ../ HEAD^..HEAD

Expected result: put the patch within the "a" directory Result with v1.6.1: put the patch within the parent of the "a" directory

(This testcase uses ../ instead of ../../ to avoid putting the patch file in an unexpected place, like in your home directory, which is what would happen in practice.)

Signature · 3 lines

Cesar Eduardo Barros
[email protected]
[email protected]

Junio C Hamano

2016-06-15 22:45 UTC #1 lore raw

PATCH 1/1 Teach format-patch to handle output directory relative to cwd

Without any explicit -o parameter, we correctly avoided putting the resulting patch output to the toplevel. We should do the same when the user gave a relative pathname to be consistent with this case and also with how other commands handle relative pathnames.

Noticed by Cesar Eduardo Barros.

Signed-off-by Junio C Hamano <[email protected]>

---

Quoted 6 lines · Cesar Eduardo Barros

If you are in a subdirectory of your working copy (for instance, linux-2.6/drivers/net) and use git-format-patch with -o to a sequence of ../ (for instance, -o ../../../) to write to the working copy's parent directory, it instead interprets the directory passed to -o as relative to the root of the working copy, instead of the expected current directory.

 builtin-log.c           |   13 ++++++++++-
 t/t4014-format-patch.sh |   52 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 3 deletions(-)

builtin-log.c +11 -2

index 4a02ee9..3e404ae 100644
@@ -731,6 +731,15 @@ static const char *clean_message_id(const char *msg_id)
 	return xmemdupz(a, z - a);
 }
 
+static const char *set_outdir(const char *prefix, const char *output_directory)
+{
+	if (!output_directory)
+		output_directory = ".";
+	if (prefix)
+		output_directory = xstrdup(prefix_filename(prefix, strlen(prefix), output_directory));
+	return output_directory;
+}
+
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
 	struct commit *commit;

@@ -908,8 +917,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (!DIFF_OPT_TST(&rev.diffopt, TEXT) && !no_binary_diff)
 		DIFF_OPT_SET(&rev.diffopt, BINARY);
 
-	if (!output_directory && !use_stdout)
-		output_directory = prefix;
+	if (!use_stdout)
+		output_directory = set_outdir(prefix, output_directory);
 
 	if (output_directory) {
 		if (use_stdout)

t/t4014-format-patch.sh +51 -1

62 lines not on this page · raw message

Cesar Eduardo Barros

author

2016-06-15 22:45 UTC #2 lore raw

Re: [PATCH] Teach format-patch to handle output directory relative to cwd

Junio C Hamano escreveu:

Quoted 4 lines

Without any explicit -o parameter, we correctly avoided putting the resulting patch output to the toplevel. We should do the same when the user gave a relative pathname to be consistent with this case and also with how other commands handle relative pathnames.

Works great, only the resulting output to the screen is a bit ugly/confusing:

drivers/net/../../../0001-sc92031-more-useful-banner-in-kernel-log.patch drivers/net/../../../0002-sc92031-remove-meaningless-version-string.patch drivers/net/../../../0003-sc92031-inline-SC92031_DESCRIPTION.patch drivers/net/../../../0004-sc92031-use-device-id-directly-instead-of-made-up-n.patch drivers/net/../../../0005-sc92031-add-a-link-to-the-datasheet.patch

I would expect:

../../../0001-sc92031-more-useful-banner-in-kernel-log.patch ../../../0002-sc92031-remove-meaningless-version-string.patch ../../../0003-sc92031-inline-SC92031_DESCRIPTION.patch ../../../0004-sc92031-use-device-id-directly-instead-of-made-up-n.patch ../../../0005-sc92031-add-a-link-to-the-datasheet.patch

(after all, I am still inside the drivers/net directory)

Signature · 3 lines

Cesar Eduardo Barros
[email protected]
[email protected]

Junio C Hamano

2016-06-15 22:45 UTC #3 lore raw

Re: [PATCH] Teach format-patch to handle output directory relative to cwd

Quoted 6 lines · Cesar Eduardo Barros

Works great, only the resulting output to the screen is a bit ugly/confusing:
drivers/net/../../../0001-sc92031-more-useful-banner-in-kernel-log.patch
...
(after all, I am still inside the drivers/net directory)

Fair enough. Here is a replacement diff.

 builtin-log.c           |   28 ++++++++++++++++++++++--
 t/t4014-format-patch.sh |   52 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 76 insertions(+), 4 deletions(-)

builtin-log.c +25 -3

index db71e0d..16a0f11 100644
@@ -568,6 +568,7 @@ static const char *get_oneline_for_filename(struct commit *commit,
 
 static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
+static int outdir_offset;
 
 static int reopen_stdout(const char *oneline, int nr, int total)
 {

@@ -594,7 +595,7 @@ static int reopen_stdout(const char *oneline, int nr, int total)
 		strcpy(filename + len, fmt_patch_suffix);
 	}
 
-	fprintf(realstdout, "%s\n", filename);
+	fprintf(realstdout, "%s\n", filename + outdir_offset);
 	if (freopen(filename, "w", stdout) == NULL)
 		return error("Cannot open patch file %s",filename);

@@ -757,6 +758,27 @@ static const char *clean_message_id(const char *msg_id)
 	return xmemdupz(a, z - a);
 }
 
+static const char *set_outdir(const char *prefix, const char *output_directory)
+{
+	if (output_directory && is_absolute_path(output_directory))
+		return output_directory;
+
+	if (!prefix || !*prefix) {
+		if (output_directory)
+			return output_directory;
+		/* The user did not explicitly ask for "./" */
+		outdir_offset = 2;
+		return "./";
+	}
+
+	outdir_offset = strlen(prefix);
+	if (!output_directory)
+		return prefix;
+
+	return xstrdup(prefix_filename(prefix, outdir_offset,
+				       output_directory));
+}
+
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
 	struct commit *commit;

@@ -935,8 +957,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (!DIFF_OPT_TST(&rev.diffopt, TEXT) && !no_binary_diff)
 		DIFF_OPT_SET(&rev.diffopt, BINARY);
 
-	if (!output_directory && !use_stdout)
-		output_directory = prefix;
+	if (!use_stdout)
+		output_directory = set_outdir(prefix, output_directory);
 
 	if (output_directory) {
 		if (use_stdout)

t/t4014-format-patch.sh +51 -1

62 lines not on this page · raw message

Cesar Eduardo Barros

author

2016-06-15 22:45 UTC #4 lore raw

Re: [PATCH] Teach format-patch to handle output directory relative to cwd

Junio C Hamano escreveu:

Quoted 10 lines

Quoted 6 lines · Cesar Eduardo Barros
Works great, only the resulting output to the screen is a bit ugly/confusing:
drivers/net/../../../0001-sc92031-more-useful-banner-in-kernel-log.patch
...
(after all, I am still inside the drivers/net directory)
Fair enough. Here is a replacement diff.

Works fine, thanks.

Signature · 3 lines

Cesar Eduardo Barros
[email protected]
[email protected]