[PATCH v2 0/2] format-patch: warn if commit msg contains a patch delimiter

From: Matheus Tavares <matheus.bernardino@usp.br>
To: git@vger.kernel.org
Cc: gitster@pobox.com, avarab@gmail.com, l.s.r@web.de
Subject: [PATCH v2 0/2] format-patch: warn if commit msg contains a patch delimiter
Date: Wed,  7 Sep 2022 11:44:55 -0300	[thread overview]
Message-ID: <cover.1662559356.git.matheus.bernardino@usp.br> (raw)
In-Reply-To: <220905.864jxmme0a.gmgdl@evledraar.gmail.com>

This makes format-patch warn on strings like "---" and "-- >8 --", which
can make a later git am fail to properly apply the generated patch.

Changes in v2:
- Use heredoc in tests.
- Add internationalization _()
- Incorporate René changes to use a buf/size pair.

René, I added your changes from [1] as a preparatory patch. Please let
me know if you are OK with that so that I can add your SoB in the next
re-roll.

[1]: https://lore.kernel.org/git/904b784d-a328-011f-c71a-c2092534e0f7@web.de/

Matheus Tavares (1):
  format-patch: warn if commit msg contains a patch delimiter

René Scharfe (1):
  patchbreak(), is_scissors_line(): work with a buf/len pair

 builtin/log.c           |  1 +
 log-tree.c              |  1 +
 mailinfo.c              | 37 +++++++++++++++++++------------------
 mailinfo.h              |  3 +++
 pretty.c                | 16 +++++++++++++++-
 pretty.h                |  3 ++-
 revision.h              |  3 ++-
 t/t4014-format-patch.sh | 26 ++++++++++++++++++++++++++
 8 files changed, 69 insertions(+), 21 deletions(-)

Range-diff against v1:
-:  ---------- > 1:  99012733e4 patchbreak(), is_scissors_line(): work with a buf/len pair
1:  059811c85f ! 2:  a2c4514aa0 format-patch: warn if commit msg contains a patch delimiter
    @@ mailinfo.c: static void decode_transfer_encoding(struct mailinfo *mi, struct str
      	free(ret);
      }

    --static inline int patchbreak(const struct strbuf *line)
    -+int patchbreak(const struct strbuf *line)
    +-static inline int patchbreak(const char *buf, size_t len)
    ++int patchbreak(const char *buf, size_t len)
      {
    - 	size_t i;
    - 
    -@@ mailinfo.c: static inline int patchbreak(const struct strbuf *line)
    + 	/* Beginning of a "diff -" header? */
    + 	if (skip_prefix_mem(buf, len, "diff -", &buf, &len))
    +@@ mailinfo.c: static inline int patchbreak(const char *buf, size_t len)
      	return 0;
      }

    --static int is_scissors_line(const char *line)
    -+int is_scissors_line(const char *line)
    +-static int is_scissors_line(const char *line, size_t len)
    ++int is_scissors_line(const char *line, size_t len)
      {
      	const char *c;
      	int scissors = 0, gap = 0;
    @@ mailinfo.h: void setup_mailinfo(struct mailinfo *);
      int mailinfo(struct mailinfo *, const char *msg, const char *patch);
      void clear_mailinfo(struct mailinfo *);

    -+int patchbreak(const struct strbuf *line);
    -+int is_scissors_line(const char *line);
    ++int patchbreak(const char *line, size_t len);
    ++int is_scissors_line(const char *line, size_t len);
     +
      #endif /* MAILINFO_H */

    @@ pretty.c: void pp_remainder(struct pretty_print_context *pp,
      	struct grep_opt *opt = pp->rev ? &pp->rev->grep_filter : NULL;
     -	int first = 1;
     +	int first = 1, found_delimiter = 0;
    -+	struct strbuf linebuf = STRBUF_INIT;

      	for (;;) {
      		const char *line = *msg_p;
    @@ pretty.c: void pp_remainder(struct pretty_print_context *pp,
      		if (!linelen)
      			break;

    -+		if (pp->check_in_body_patch_breaks) {
    -+			strbuf_reset(&linebuf);
    -+			strbuf_add(&linebuf, line, linelen);
    -+			if (patchbreak(&linebuf) || is_scissors_line(linebuf.buf)) {
    -+				strbuf_strip_suffix(&linebuf, "\n");
    -+				warning("commit message has a patch delimiter: '%s'",
    -+					linebuf.buf);
    -+				found_delimiter = 1;
    -+			}
    ++		if (pp->check_in_body_patch_breaks &&
    ++		    (patchbreak(line, linelen) || is_scissors_line(line, linelen))) {
    ++			warning(_("commit message has a patch delimiter: '%.*s'"),
    ++				line[linelen - 1] == '\n' ? linelen - 1 : linelen,
    ++				line);
    ++			found_delimiter = 1;
     +		}
     +
      		if (is_blank_line(line, &linelen)) {
    @@ pretty.c: void pp_remainder(struct pretty_print_context *pp,
      		strbuf_addch(sb, '\n');
      	}
     +
    -+	if (found_delimiter)
    -+		warning("git am might fail to apply this patch. "
    -+			"Consider indenting the offending lines.");
    -+
    -+	strbuf_release(&linebuf);
    ++	if (found_delimiter) {
    ++		warning(_("git am might fail to apply this patch. "
    ++			  "Consider indenting the offending lines."));
    ++	}
      }

      void pretty_print_commit(struct pretty_print_context *pp,
    @@ t/t4014-format-patch.sh: test_expect_success 'interdiff: solo-patch' '
     +test_expect_success 'warn if commit message contains patch delimiter' '
     +	>delim &&
     +	git add delim &&
    -+	GIT_EDITOR="printf \"title\n\n---\" >" git commit &&
    ++	cat >msg <<-\EOF &&
    ++	title
    ++
    ++	---
    ++	EOF
    ++	git commit -F msg &&
     +	git format-patch -1 2>stderr &&
     +	grep "warning: commit message has a patch delimiter" stderr
     +'
    @@ t/t4014-format-patch.sh: test_expect_success 'interdiff: solo-patch' '
     +test_expect_success 'warn if commit message contains scissors' '
     +	>scissors &&
     +	git add scissors &&
    -+	GIT_EDITOR="printf \"title\n\n-- >8 --\" >" git commit &&
    ++	cat >msg <<-\EOF &&
    ++	title
    ++
    ++	-- >8 --
    ++	EOF
    ++	git commit -F msg &&
     +	git format-patch -1 2>stderr &&
     +	grep "warning: commit message has a patch delimiter" stderr
     +'
-- 
2.37.2