git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: "Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Phillip Wood <phillip.wood@dunelm.org.uk>,
	Daniel Li <dan@danielyli.com>,
	git@vger.kernel.org
Subject: [PATCH] 2.36 format-patch regression fix
Date: Sat, 30 Apr 2022 12:32:44 +0200	[thread overview]
Message-ID: <c36896a1-6247-123b-4fa3-b7eb24af1897@web.de> (raw)
In-Reply-To: <xmqqo80j87g0.fsf_-_@gitster.g>

e900d494dc (diff: add an API for deferred freeing, 2021-02-11) added a
way to allow reusing diffopts: the no_free bit.  244c27242f (diff.[ch]:
have diff_free() call clear_pathspec(opts.pathspec), 2022-02-16) made
that mechanism mandatory.

git format-patch only sets no_free when --output is given, causing it to
forget pathspecs after the first commit.  Set no_free unconditionally
instead.

The existing test was unable to detect this breakage because it checks
stderr for the absence of a certain string, but format-patch writes to
stdout.  Also the test was not checking the case of one commit modifying
multiple files and a pathspec limiting the diff.  Replace it with a more
thorough one.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/log.c           |  9 ++-------
 t/t4014-format-patch.sh | 33 +++++++++++++++++++++++++++++++--
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index c211d66d1d..9acc130594 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1883,6 +1883,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.diff = 1;
 	rev.max_parents = 1;
 	rev.diffopt.flags.recursive = 1;
+	rev.diffopt.no_free = 1;
 	rev.subject_prefix = fmt_patch_subject_prefix;
 	memset(&s_r_opt, 0, sizeof(s_r_opt));
 	s_r_opt.def = "HEAD";
@@ -2008,13 +2009,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)

 	if (use_stdout) {
 		setup_pager();
-	} else if (rev.diffopt.close_file) {
-		/*
-		 * The diff code parsed --output; it has already opened the
-		 * file, but we must instruct it not to close after each diff.
-		 */
-		rev.diffopt.no_free = 1;
-	} else {
+	} else if (!rev.diffopt.close_file) {
 		int saved;

 		if (!output_directory)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 7dc5a5c736..fbec8ad2ef 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -926,11 +926,40 @@ test_expect_success 'format-patch --numstat should produce a patch' '
 '

 test_expect_success 'format-patch -- <path>' '
-	git format-patch main..side -- file 2>error &&
-	! grep "Use .--" error
+	rm -f *.patch &&
+	git checkout -b pathspec main &&
+
+	echo file_a 1 >file_a &&
+	echo file_b 1 >file_b &&
+	git add file_a file_b &&
+	git commit -m pathspec_initial &&
+
+	echo file_a 2 >>file_a &&
+	git add file_a &&
+	git commit -m pathspec_a &&
+
+	echo file_b 2 >>file_b &&
+	git add file_b &&
+	git commit -m pathspec_b &&
+
+	echo file_a 3 >>file_a &&
+	echo file_b 3 >>file_b &&
+	git add file_a file_b &&
+	git commit -m pathspec_ab &&
+
+	cat >expect <<-\EOF &&
+	0001-pathspec_initial.patch
+	0002-pathspec_a.patch
+	0003-pathspec_ab.patch
+	EOF
+
+	git format-patch main..pathspec -- file_a >output &&
+	test_cmp expect output &&
+	! grep file_b *.patch
 '

 test_expect_success 'format-patch --ignore-if-in-upstream HEAD' '
+	git checkout side &&
 	git format-patch --ignore-if-in-upstream HEAD
 '

--
2.35.3


  reply	other threads:[~2022-04-30 10:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-30  2:22 Bug: `git show` honors path filters only for the first commit Daniel Li
2022-04-30  4:59 ` Junio C Hamano
2022-04-30  5:29   ` [PATCH] 2.36 show regression fix Junio C Hamano
2022-04-30 10:32     ` René Scharfe [this message]
2022-04-30 16:32       ` [PATCH] 2.36 format-patch " Carlo Marcelo Arenas Belón
2022-05-01  9:35         ` René Scharfe
2022-05-20 15:23           ` the state of diff_free() and release_revisions() (was: [PATCH] 2.36 format-patch regression fix) Ævar Arnfjörð Bjarmason
2022-05-20 17:23             ` the state of diff_free() and release_revisions() Junio C Hamano
2022-04-30 14:31     ` [PATCH] 2.36 fast-export regression fix René Scharfe
2022-04-30 20:50       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c36896a1-6247-123b-4fa3-b7eb24af1897@web.de \
    --to=l.s.r@web.de \
    --cc=avarab@gmail.com \
    --cc=dan@danielyli.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).