[PATCH v2 0/3] format-patch: teach `--header-cmd`
- From
Kristoffer Haugsbakk <code@khaugsbakk.name>
- Date
- Mar 19, 2024, 18:35 UTC
- Message-ID
- <cover.1710873210.git.code@khaugsbakk.name>
- In-Reply-To
- <cover.1709841147.git.code@khaugsbakk.name>
(most of this is from the main commit/patch with some elaboration in parts)
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.
§ Motivation
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).[1]
🔗 1: https://lore.kernel.org/git/20230423122744.4865-1-maxim.cournoyer@gmail.com/
§ Discussion
One can already get the commit hash from the first line of the patch file:
From 04967d53399ed2db09d224008f557ec1a5847cc1 Mon Sep 17 00:00:00 2001
However, the point of this option is to be able to calculate header output based primarily on the commit hash without having to go back and forth between commands (i.e. running format-patch and then parsing the patch files).
As an example, the command could output a header for the current commit as well as another header for the previously-published commits:
X-Commit-Hash: 97b53c04894578b23d0c650f69885f734699afc7
X-Previous-Commits:
4ad5d4190649dcb5f26c73a6f15ab731891b9dfd
d275d1d179b90592ddd7b5da2ae4573b3f7a37b7
402b7937951073466bf4527caffd38175391c7daOne can imagine that (1) these previous commit hashes were stored on every commit rewrite and (2) the commits that had been published previously were also stored. Then the command just needed the current commit hash in order to look up this information.
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 hypothetical project in question 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.
§ Demonstration
The above current/previous hash example is taken from:
https://lore.kernel.org/git/97b53c04894578b23d0c650f69885f734699afc7.1709670287.git.code@khaugsbakk.name/
§ Changes in v2
Changes based on feedback from Jean-Noël and Peff.
Peff suggested changes to the strbuf handling in `log_tree`, removing constness in order to facilitate freeing without casting, and fleshing out the preliminary patch.
In detail:
• revision: add a per-email field to rev-info
• Replaces (subject) “log-tree: take ownership of pointer”
• Separate out changes for `rev.info.pe_headers`, changes to
ownership, const, and strbuf handling in `log_tree`
• Link: https://lore.kernel.org/git/20240313065454.GB125150@coredump.intra.peff.net/
• format-patch: teach `--header-cmd`
• Simplify tests: use `true` and `false` as commands
• Fix indentation in code
• Use AsciiDoc definition list
• Link: https://lore.kernel.org/git/53ea3745-205b-40c0-a4c5-9be26d9b88bf@gmail.com/
• format-patch: check if header output looks valid
• (no changes)§ CC
Cc: Jeff King <peff@peff.net> Cc: Maxim Cournoyer <maxim.cournoyer@gmail.com> Cc: "Jean-Noël Avila" <avila.jn@gmail.com>
§ CI
https://github.com/LemmingAvalanche/git/actions/runs/8332742200/job/22802597793
Fails but looks similar to the failures for `master` at 3bd955d2691 (The ninth batch, 2024-03-18):
https://github.com/git/git/actions/runs/8333455189/job/22804953254
Kristoffer Haugsbakk (3): revision: add a per-email field to rev-info format-patch: teach `--header-cmd` format-patch: check if header output looks valid
Documentation/config/format.txt | 5 ++ Documentation/git-format-patch.txt | 23 +++++++++ builtin/log.c | 76 ++++++++++++++++++++++++++++++ log-tree.c | 21 +++++---- log-tree.h | 2 +- pretty.h | 2 +- revision.h | 4 +- t/t4014-format-patch.sh | 49 +++++++++++++++++++ 8 files changed, 169 insertions(+), 13 deletions(-)
Range-diff against v1:
1: 3b12a8cf393 < -: ----------- log-tree: take ownership of pointer
-: ----------- > 1: 9a7102b708e revision: add a per-email field to rev-info
2: f405a0140b5 ! 2: 8c511797a47 format-patch: teach `--header-cmd`
@@ Commit message
## Notes (series) ##
- Documentation/config/format.txt:
- • I get the impression that `_` is the convention for placeholders now:
- `_<cmd>_`
+ v2:
+ • Simplify tests: use `true` and `false` as commands
+ • Fix indentation
+ • Don’t use `const` for owned pointer (avoid cast when freeing)
+ • Link: https://lore.kernel.org/git/cover.1709841147.git.code@khaugsbakk.name/T/#m12d104a5a769c7f6e02b1d0a75855142004e9206
+ • Use AsciiDoc definition list
+ • Link: https://lore.kernel.org/git/53ea3745-205b-40c0-a4c5-9be26d9b88bf@gmail.com/
## Documentation/config/format.txt ##
@@ Documentation/config/format.txt: format.headers::
@@ Documentation/git-format-patch.txt: feeding the result to `git send-email`.
++
+_<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_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.
@@ builtin/log.c: static void make_cover_letter(struct rev_info *rev, int use_separ
show_range_diff(rev->rdiff1, rev->rdiff2, &range_diff_opts);
strvec_clear(&other_arg);
}
-+ free((char *)pp.after_subject);
++ free(pp.after_subject);
}
static char *clean_message_id(const char *msg_id)
@@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
/* interdiff/range-diff in cover-letter; omit from patches */
rev.idiff_oid1 = NULL;
rev.rdiff1 = NULL;
-+ free((char *)rev.pe_headers);
++ free(rev.pe_headers);
}
rev.add_signoff = do_signoff;
@@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
}
if (output_directory)
fclose(rev.diffopt.file);
-+ free((char *)rev.pe_headers);
++ free(rev.pe_headers);
}
stop_progress(&progress);
free(list);
- ## log-tree.c ##
-@@ log-tree.c: 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);
-
-@@ log-tree.c: 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)
-@@ log-tree.c: 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;
-
- ## revision.h ##
-@@ revision.h: 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;
-
## t/t4014-format-patch.sh ##
@@ t/t4014-format-patch.sh: test_expect_failure 'configuration To: header (rfc2047)' '
grep "^To: =?UTF-8?q?R=20=C3=84=20Cipient?= <rcipient@example.com>\$" hdrs9
@@ t/t4014-format-patch.sh: test_expect_failure 'configuration To: header (rfc2047)
+'
+
+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
++ git format-patch --header-cmd=true --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
++ cat > expect <<-\EOF &&
++ fatal: header-cmd false: failed with exit code 1
+ EOF
-+ test_must_fail git format-patch --header-cmd=./cmd --stdout main..side >actual 2>&1 &&
++ test_must_fail git format-patch --header-cmd=false --stdout main..side >actual 2>&1 &&
+ test_cmp expect actual
+'
+
3: 0e8409227e4 ! 3: c570467c8db format-patch: check if header output looks valid
@@ builtin/log.c: static char *header_cmd_output(struct rev_info *rev, const struct
## t/t4014-format-patch.sh ##
@@ t/t4014-format-patch.sh: test_expect_success '--header-cmd with no output works' '
- git format-patch --header-cmd=./cmd --stdout main..side
+ git format-patch --header-cmd=true --stdout main..side
'
+test_expect_success '--header-cmd without headers-like output fails' '
@@ t/t4014-format-patch.sh: test_expect_success '--header-cmd with no output works'
+'
+
test_expect_success '--header-cmd reports failed command' '
- write_script cmd <<-\EOF &&
- exit 1
+ cat > expect <<-\EOF &&
+ fatal: header-cmd false: failed with exit code 1-- 2.44.0.144.g29ae9861142