diff mbox series

[2/3] format-patch: teach `--header-cmd`

Message ID f405a0140b5655bc66a0a7a603517a421d7669cf.1709841147.git.code@khaugsbakk.name (mailing list archive)
State Superseded
Headers show
Series format-patch: teach `--header-cmd` | expand

Commit Message

Kristoffer Haugsbakk March 7, 2024, 7:59 p.m. UTC
Teach git-format-patch(1) `--header-cmd` (with negation) and the
accompanying config variable `format.headerCmd` which allows the user to
add extra headers per-patch.

format-patch knows `--add-header`. However, that seems most useful for
series-wide headers; you cannot really control what the header is like
per patch or specifically for the cover letter. To that end, teach
format-patch a new option which runs a command that has access to the
hash of the current commit (if it is a code patch) and the patch count
which is used for the patch files that this command outputs. Also
include an environment variable which tells the version of this API so
that the command can detect and error out in case the API changes.

This is inspired by `--header-cmd` of git-send-email(1).

§ Discussion

The command can use the provided commit hash to provide relevant
information in the header. For example, the command could output a
header for the current commit as well as the previously-published
commits:

    X-Commit-Hash: 97b53c04894578b23d0c650f69885f734699afc7
    X-Previous-Commits:
        4ad5d4190649dcb5f26c73a6f15ab731891b9dfd
        d275d1d179b90592ddd7b5da2ae4573b3f7a37b7
        402b7937951073466bf4527caffd38175391c7da

Now interested parties can use this information to track where the
patches come from.

This information could of course be given between the
three-dash/three-hyphen line and the patch proper. However, the project
might prefer to use this part for extra patch information written by the
author and leave the above information for tooling; this way the extra
information does not need to disturb the reader.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---

Notes (series):
    Documentation/config/format.txt:
    • I get the impression that `_` is the convention for placeholders now:
      `_<cmd>_`

 Documentation/config/format.txt    |  5 ++++
 Documentation/git-format-patch.txt | 26 ++++++++++++++++++
 builtin/log.c                      | 43 ++++++++++++++++++++++++++++++
 log-tree.c                         | 16 ++++++++++-
 revision.h                         |  2 ++
 t/t4014-format-patch.sh            | 42 +++++++++++++++++++++++++++++
 6 files changed, 133 insertions(+), 1 deletion(-)

Comments

Kristoffer Haugsbakk March 8, 2024, 6:30 p.m. UTC | #1
On Thu, Mar 7, 2024, at 20:59, Kristoffer Haugsbakk wrote:
> +test_expect_success '--header-cmd with no output works' '
> +	write_script cmd <<-\EOF &&
> +	exit 0
> +	EOF
> +	git format-patch --header-cmd=./cmd --stdout main..side
> +'

This can be simplified to `--header-cmd=true`.
Jean-Noël AVILA March 11, 2024, 9:29 p.m. UTC | #2
Le 07/03/2024 à 20:59, Kristoffer Haugsbakk a écrit :
>  
> +format.headerCmd::
> +	Command to run for each patch that should output RFC 2822 email
> +	headers. Has access to some information per patch via
> +	environment variables. See linkgit:git-format-patch[1].
> +
>  format.to::
>  format.cc::
>  	Additional recipients to include in a patch to be submitted
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index 728bb3821c1..41c344902e9 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -303,6 +303,32 @@ feeding the result to `git send-email`.
>  	`Cc:`, and custom) headers added so far from config or command
>  	line.
>  
> +--[no-]header-cmd=<cmd>::
> +	Run _<cmd>_ for each patch. _<cmd>_ should output valid RFC 2822
> +	email headers. This can also be configured with
> +	the configuration variable `format.headerCmd`. Can be turned off
> +	with `--no-header-cmd`. This works independently of
> +	`--[no-]add-header`.
> ++
> +_<cmd>_ has access to these environment variables:
> ++
> +	GIT_FP_HEADER_CMD_VERSION

Better use a nested description list like this:

GIT_FP_HEADER_CMD_VERSION;;
  The version of this API. Currently `1`. _<cmd>_ may return exit code
  `2` in order to signal that it does not support the given version.

> ++
> +The version of this API. Currently `1`. _<cmd>_ may return exit code
> +`2` in order to signal that it does not support the given version.
> ++
> +	GIT_FP_HEADER_CMD_HASH
> ++
> +The hash of the commit corresponding to the current patch. Not set if
> +the current patch is the cover letter.
> ++
> +	GIT_FP_HEADER_CMD_COUNT
> ++
> +The current patch count. Increments for each patch.
> ++
> +`git format-patch` will error out if _<cmd>_ returns a non-zero exit
> +code.
> +
>  --[no-]cover-letter::
>  	In addition to the patches, generate a cover letter file
>  	containing the branch description, shortlog and the overall diffstat.  You can


Overall, thank you for correctly marking up placeholders and options.
Kristoffer Haugsbakk March 12, 2024, 8:13 a.m. UTC | #3
On Mon, Mar 11, 2024, at 22:29, Jean-Noël Avila wrote:
>> +--[no-]header-cmd=<cmd>::
>> +	Run _<cmd>_ for each patch. _<cmd>_ should output valid RFC 2822
>> +	email headers. This can also be configured with
>> +	the configuration variable `format.headerCmd`. Can be turned off
>> +	with `--no-header-cmd`. This works independently of
>> +	`--[no-]add-header`.
>> ++
>> +_<cmd>_ has access to these environment variables:
>> ++
>> +	GIT_FP_HEADER_CMD_VERSION
>
> Better use a nested description list like this:
>
> GIT_FP_HEADER_CMD_VERSION;;
>   The version of this API. Currently `1`. _<cmd>_ may return exit code
>   `2` in order to signal that it does not support the given version.
>

Thanks, I’ll do that in the next version.

>> ++
>> +The version of this API. Currently `1`. _<cmd>_ may return exit code
>> +`2` in order to signal that it does not support the given version.
>> ++
>> +	GIT_FP_HEADER_CMD_HASH
>> ++
>> +The hash of the commit corresponding to the current patch. Not set if
>> +the current patch is the cover letter.
>> ++
>> +	GIT_FP_HEADER_CMD_COUNT
>> ++
>> +The current patch count. Increments for each patch.
>> ++
>> +`git format-patch` will error out if _<cmd>_ returns a non-zero exit
>> +code.
>> +
>>  --[no-]cover-letter::
>>  	In addition to the patches, generate a cover letter file
>>  	containing the branch description, shortlog and the overall diffstat.  You can
>
>
> Overall, thank you for correctly marking up placeholders and options.

Thanks for reviewing!

--
Kristoffer Haugsbakk
diff mbox series

Patch

diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index 7410e930e53..c184b865824 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -31,6 +31,11 @@  format.headers::
 	Additional email headers to include in a patch to be submitted
 	by mail.  See linkgit:git-format-patch[1].
 
+format.headerCmd::
+	Command to run for each patch that should output RFC 2822 email
+	headers. Has access to some information per patch via
+	environment variables. See linkgit:git-format-patch[1].
+
 format.to::
 format.cc::
 	Additional recipients to include in a patch to be submitted
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 728bb3821c1..41c344902e9 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -303,6 +303,32 @@  feeding the result to `git send-email`.
 	`Cc:`, and custom) headers added so far from config or command
 	line.
 
+--[no-]header-cmd=<cmd>::
+	Run _<cmd>_ for each patch. _<cmd>_ should output valid RFC 2822
+	email headers. This can also be configured with
+	the configuration variable `format.headerCmd`. Can be turned off
+	with `--no-header-cmd`. This works independently of
+	`--[no-]add-header`.
++
+_<cmd>_ has access to these environment variables:
++
+	GIT_FP_HEADER_CMD_VERSION
++
+The version of this API. Currently `1`. _<cmd>_ may return exit code
+`2` in order to signal that it does not support the given version.
++
+	GIT_FP_HEADER_CMD_HASH
++
+The hash of the commit corresponding to the current patch. Not set if
+the current patch is the cover letter.
++
+	GIT_FP_HEADER_CMD_COUNT
++
+The current patch count. Increments for each patch.
++
+`git format-patch` will error out if _<cmd>_ returns a non-zero exit
+code.
+
 --[no-]cover-letter::
 	In addition to the patches, generate a cover letter file
 	containing the branch description, shortlog and the overall diffstat.  You can
diff --git a/builtin/log.c b/builtin/log.c
index db1808d7c13..eecbcdf1d6d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -43,10 +43,13 @@ 
 #include "tmp-objdir.h"
 #include "tree.h"
 #include "write-or-die.h"
+#include "run-command.h"
 
 #define MAIL_DEFAULT_WRAP 72
 #define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100
 #define FORMAT_PATCH_NAME_MAX_DEFAULT 64
+#define HC_VERSION "1"
+#define HC_NOT_SUPPORTED 2
 
 /* Set a default date-time format for git log ("log.date" config variable) */
 static const char *default_date_mode = NULL;
@@ -902,6 +905,7 @@  static int auto_number = 1;
 
 static char *default_attach = NULL;
 
+static const char *header_cmd = NULL;
 static struct string_list extra_hdr = STRING_LIST_INIT_NODUP;
 static struct string_list extra_to = STRING_LIST_INIT_NODUP;
 static struct string_list extra_cc = STRING_LIST_INIT_NODUP;
@@ -1100,6 +1104,8 @@  static int git_format_config(const char *var, const char *value,
 		format_no_prefix = 1;
 		return 0;
 	}
+	if (!strcmp(var, "format.headercmd"))
+		return git_config_string(&header_cmd, var, value);
 
 	/*
 	 * ignore some porcelain config which would otherwise be parsed by
@@ -1419,6 +1425,7 @@  static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 		show_range_diff(rev->rdiff1, rev->rdiff2, &range_diff_opts);
 		strvec_clear(&other_arg);
 	}
+	free((char *)pp.after_subject);
 }
 
 static char *clean_message_id(const char *msg_id)
@@ -1865,6 +1872,35 @@  static void infer_range_diff_ranges(struct strbuf *r1,
 	}
 }
 
+/* Returns an owned pointer */
+static char *header_cmd_output(struct rev_info *rev, const struct commit *cmit)
+{
+	struct child_process header_cmd_proc = CHILD_PROCESS_INIT;
+	struct strbuf output = STRBUF_INIT;
+	int res;
+
+	strvec_pushl(&header_cmd_proc.args, header_cmd, NULL);
+	if (cmit)
+		strvec_pushf(&header_cmd_proc.env, "GIT_FP_HEADER_CMD_HASH=%s",
+			     oid_to_hex(&cmit->object.oid));
+	strvec_pushl(&header_cmd_proc.env,
+		     "GIT_FP_HEADER_CMD_VERSION=" HC_VERSION, NULL);
+	strvec_pushf(&header_cmd_proc.env, "GIT_FP_HEADER_CMD_COUNT=%" PRIuMAX,
+		     (uintmax_t)rev->nr);
+	res = capture_command(&header_cmd_proc, &output, 0);
+	if (res) {
+		if (res == HC_NOT_SUPPORTED)
+			die(_("header-cmd %s: returned exit "
+			      "code %d; the command does not support "
+			      "version " HC_VERSION),
+			    header_cmd, HC_NOT_SUPPORTED);
+		else
+			die(_("header-cmd %s: failed with exit code %d"),
+			    header_cmd, res);
+	}
+	return strbuf_detach(&output, NULL);
+}
+
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
 	struct commit *commit;
@@ -1955,6 +1991,7 @@  int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(N_("Messaging")),
 		OPT_CALLBACK(0, "add-header", NULL, N_("header"),
 			    N_("add email header"), header_callback),
+		OPT_STRING(0, "header-cmd", &header_cmd, N_("email"), N_("command that will be run to generate headers")),
 		OPT_STRING_LIST(0, "to", &extra_to, N_("email"), N_("add To: header")),
 		OPT_STRING_LIST(0, "cc", &extra_cc, N_("email"), N_("add Cc: header")),
 		OPT_CALLBACK_F(0, "from", &from, N_("ident"),
@@ -2321,6 +2358,8 @@  int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (cover_letter) {
 		if (thread)
 			gen_message_id(&rev, "cover");
+		if (header_cmd)
+			rev.pe_headers = header_cmd_output(&rev, NULL);
 		make_cover_letter(&rev, !!output_directory,
 				  origin, nr, list, description_file, branch_name, quiet);
 		print_bases(&bases, rev.diffopt.file);
@@ -2330,6 +2369,7 @@  int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		/* interdiff/range-diff in cover-letter; omit from patches */
 		rev.idiff_oid1 = NULL;
 		rev.rdiff1 = NULL;
+		free((char *)rev.pe_headers);
 	}
 	rev.add_signoff = do_signoff;
 
@@ -2376,6 +2416,8 @@  int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			gen_message_id(&rev, oid_to_hex(&commit->object.oid));
 		}
 
+		if (header_cmd)
+			rev.pe_headers = header_cmd_output(&rev, commit);
 		if (output_directory &&
 		    open_next_file(rev.numbered_files ? NULL : commit, NULL, &rev, quiet))
 			die(_("failed to create output files"));
@@ -2402,6 +2444,7 @@  int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		}
 		if (output_directory)
 			fclose(rev.diffopt.file);
+		free((char *)rev.pe_headers);
 	}
 	stop_progress(&progress);
 	free(list);
diff --git a/log-tree.c b/log-tree.c
index 2eabd19962b..3ca383d099f 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -469,12 +469,24 @@  void fmt_output_email_subject(struct strbuf *sb, struct rev_info *opt)
 	}
 }
 
+static char *extra_and_pe_headers(const char *extra_headers, const char *pe_headers) {
+	struct strbuf all_headers = STRBUF_INIT;
+
+	if (extra_headers)
+		strbuf_addstr(&all_headers, extra_headers);
+	if (pe_headers) {
+		strbuf_addstr(&all_headers, pe_headers);
+	}
+	return strbuf_detach(&all_headers, NULL);
+}
+
 void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			     const char **extra_headers_p,
 			     int *need_8bit_cte_p,
 			     int maybe_multipart)
 {
-	const char *extra_headers = opt->extra_headers;
+	const char *extra_headers =
+		extra_and_pe_headers(opt->extra_headers, opt->pe_headers);
 	const char *name = oid_to_hex(opt->zero_commit ?
 				      null_oid() : &commit->object.oid);
 
@@ -519,6 +531,7 @@  void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			 extra_headers ? extra_headers : "",
 			 mime_boundary_leader, opt->mime_boundary,
 			 mime_boundary_leader, opt->mime_boundary);
+		free((char *)extra_headers);
 		extra_headers = strbuf_detach(&subject_buffer, NULL);
 
 		if (opt->numbered_files)
@@ -857,6 +870,7 @@  void show_log(struct rev_info *opt)
 
 	strbuf_release(&msgbuf);
 	free(ctx.notes_message);
+	free((char *)ctx.after_subject);
 
 	if (cmit_fmt_is_mail(ctx.fmt) && opt->idiff_oid1) {
 		struct diff_queue_struct dq;
diff --git a/revision.h b/revision.h
index 94c43138bc3..eb36bdea36e 100644
--- a/revision.h
+++ b/revision.h
@@ -291,6 +291,8 @@  struct rev_info {
 	struct string_list *ref_message_ids;
 	int		add_signoff;
 	const char	*extra_headers;
+	/* per-email headers */
+	const char	*pe_headers;
 	const char	*log_reencode;
 	const char	*subject_prefix;
 	int		patch_name_max;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index e37a1411ee2..dfda21d4b2b 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -238,6 +238,48 @@  test_expect_failure 'configuration To: header (rfc2047)' '
 	grep "^To: =?UTF-8?q?R=20=C3=84=20Cipient?= <rcipient@example.com>\$" hdrs9
 '
 
+test_expect_success '--header-cmd' '
+	write_script cmd <<-\EOF &&
+	printf "X-S: $GIT_FP_HEADER_CMD_HASH\n"
+	printf "X-V: $GIT_FP_HEADER_CMD_VERSION\n"
+	printf "X-C: $GIT_FP_HEADER_CMD_COUNT\n"
+	EOF
+	expect_sha1=$(git rev-parse side) &&
+	git format-patch --header-cmd=./cmd --stdout main..side >patch &&
+	grep "^X-S: $expect_sha1" patch &&
+	grep "^X-V: 1" patch &&
+	grep "^X-C: 3" patch
+'
+
+test_expect_success '--header-cmd with no output works' '
+	write_script cmd <<-\EOF &&
+	exit 0
+	EOF
+	git format-patch --header-cmd=./cmd --stdout main..side
+'
+
+test_expect_success '--header-cmd reports failed command' '
+	write_script cmd <<-\EOF &&
+	exit 1
+	EOF
+		cat > expect <<-\EOF &&
+	fatal: header-cmd ./cmd: failed with exit code 1
+	EOF
+	test_must_fail git format-patch --header-cmd=./cmd --stdout main..side >actual 2>&1 &&
+	test_cmp expect actual
+'
+
+test_expect_success '--header-cmd reports exit code 2' '
+	write_script cmd <<-\EOF &&
+	exit 2
+	EOF
+	cat > expect <<-\EOF &&
+	fatal: header-cmd ./cmd: returned exit code 2; the command does not support version 1
+	EOF
+	test_must_fail git format-patch --header-cmd=./cmd --stdout main..side >actual 2>&1 &&
+	test_cmp expect actual
+'
+
 # check_patch <patch>: Verify that <patch> looks like a half-sane
 # patch email to avoid a false positive with !grep
 check_patch () {