* [PATCH GSoC RFC v12 05/12] fetch-pack: move function to connect.c
From: Pablo Sabater @ 2026-06-08 10:14 UTC (permalink / raw)
To: eric.peijian
Cc: calvinwan, chriscool, git, jltobler, jonathantanmy, karthik.188,
toon, chandrapratap3519, Pablo Sabater
In-Reply-To: <20260608-ps-eric-work-rebase-v12-0-5338b766e658@gmail.com>
write_fetch_command_and_capabilities will be refactored in a subsequent
commit where it will become a more general-purpose function, making it
more accessible to additional commands in the future.
To move `write_fetch_command_and_capabilities()` to `connect.c`, we need
to adjust how `advertise_sid` is managed. Previously in `fetch_pack.c`,
`advertise_sid` was a static variable, modified using
`repo_config_get_bool()`.
In `connect.c`, we now initialize `advertise_sid` at the begining by
directly using `repo_config_get_bool()`. This change is safe because:
In the original `fetch-pack.c` code, there are only two places that write
`advertise_sid`:
1. In function `do_fetch_pack()`:
if (!sever_supports("session_id"))
advertise_sid = 0;
2. In function `fetch_pack_config()`:
repo_config_get_bool("transfer.advertisesid", &advertise_sid);
About 1, since `do_fetch_pack()` is only relevant for protocol v1, this
assignment can be ignored, as `write_fetch_command_and_capabilities()`
is only used in v2.
About 2, `repo_config_get_bool()` is from `config.h` and it's an out-of-box
dependency of `connect.c`, so we can reuse it directly.
Move `write_fetch_command_and_capabilities()` to `connect.c`
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Eric Ju <eric.peijian@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
connect.c | 34 ++++++++++++++++++++++++++++++++++
connect.h | 4 ++++
fetch-pack.c | 31 -------------------------------
3 files changed, 38 insertions(+), 31 deletions(-)
diff --git a/connect.c b/connect.c
index 47e39d2a73..1dced8e632 100644
--- a/connect.c
+++ b/connect.c
@@ -700,6 +700,40 @@ int server_supports(const char *feature)
return !!server_feature_value(feature, NULL);
}
+void write_fetch_command_and_capabilities(struct strbuf *req_buf,
+ const struct string_list *server_options)
+{
+ const char *hash_name;
+ int advertise_sid;
+
+ repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
+
+ ensure_server_supports_v2("fetch");
+ packet_buf_write(req_buf, "command=fetch");
+ if (server_supports_v2("agent"))
+ packet_buf_write(req_buf, "agent=%s", git_user_agent_sanitized());
+ if (advertise_sid && server_supports_v2("session-id"))
+ packet_buf_write(req_buf, "session-id=%s", trace2_session_id());
+ if (server_options && server_options->nr) {
+ ensure_server_supports_v2("server-option");
+ for (size_t i = 0; i < server_options->nr; i++)
+ packet_buf_write(req_buf, "server-option=%s",
+ server_options->items[i].string);
+ }
+
+ if (server_feature_v2("object-format", &hash_name)) {
+ const unsigned int hash_algo = hash_algo_by_name(hash_name);
+ if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
+ die(_("mismatched algorithms: client %s; server %s"),
+ the_hash_algo->name, hash_name);
+ packet_buf_write(req_buf, "object-format=%s", the_hash_algo->name);
+ } else if (hash_algo_by_ptr(the_hash_algo) != GIT_HASH_SHA1_LEGACY) {
+ die(_("the server does not support algorithm '%s'"),
+ the_hash_algo->name);
+ }
+ packet_buf_delim(req_buf);
+}
+
static const char *url_scheme_name(enum url_scheme scheme)
{
switch (scheme) {
diff --git a/connect.h b/connect.h
index aa482a37fb..c4f6ea4b0a 100644
--- a/connect.h
+++ b/connect.h
@@ -34,4 +34,8 @@ void check_stateless_delimiter(int stateless_rpc,
struct packet_reader *reader,
const char *error);
+struct string_list;
+void write_fetch_command_and_capabilities(struct strbuf *req_buf,
+ const struct string_list *server_options);
+
#endif
diff --git a/fetch-pack.c b/fetch-pack.c
index f13951d154..4a8a70b5f3 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1376,37 +1376,6 @@ static int add_haves(struct fetch_negotiator *negotiator,
return haves_added;
}
-static void write_fetch_command_and_capabilities(struct strbuf *req_buf,
- const struct string_list *server_options)
-{
- const char *hash_name;
-
- ensure_server_supports_v2("fetch");
- packet_buf_write(req_buf, "command=fetch");
- if (server_supports_v2("agent"))
- packet_buf_write(req_buf, "agent=%s", git_user_agent_sanitized());
- if (advertise_sid && server_supports_v2("session-id"))
- packet_buf_write(req_buf, "session-id=%s", trace2_session_id());
- if (server_options && server_options->nr) {
- ensure_server_supports_v2("server-option");
- for (size_t i = 0; i < server_options->nr; i++)
- packet_buf_write(req_buf, "server-option=%s",
- server_options->items[i].string);
- }
-
- if (server_feature_v2("object-format", &hash_name)) {
- int hash_algo = hash_algo_by_name(hash_name);
- if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
- die(_("mismatched algorithms: client %s; server %s"),
- the_hash_algo->name, hash_name);
- packet_buf_write(req_buf, "object-format=%s", the_hash_algo->name);
- } else if (hash_algo_by_ptr(the_hash_algo) != GIT_HASH_SHA1_LEGACY) {
- die(_("the server does not support algorithm '%s'"),
- the_hash_algo->name);
- }
- packet_buf_delim(req_buf);
-}
-
static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
struct fetch_pack_args *args,
const struct ref *wants, struct oidset *common,
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC RFC v12 04/12] t1006: split test utility functions into new "lib-cat-file.sh"
From: Pablo Sabater @ 2026-06-08 10:14 UTC (permalink / raw)
To: eric.peijian
Cc: calvinwan, chriscool, git, jltobler, jonathantanmy, karthik.188,
toon, chandrapratap3519, Pablo Sabater
In-Reply-To: <20260608-ps-eric-work-rebase-v12-0-5338b766e658@gmail.com>
From: Eric Ju <eric.peijian@gmail.com>
This refactor extracts utility functions from the cat-file's test
script "t1006-cat-file.sh" into a new "lib-cat-file.sh" dedicated
library file. The goal is to improve code reuse and readability,
enabling future tests to leverage these utilities without duplicating
code.
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
t/lib-cat-file.sh | 16 ++++++++++++++++
t/t1006-cat-file.sh | 13 +------------
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/t/lib-cat-file.sh b/t/lib-cat-file.sh
new file mode 100644
index 0000000000..44af232d74
--- /dev/null
+++ b/t/lib-cat-file.sh
@@ -0,0 +1,16 @@
+# Library of git-cat-file related test functions.
+
+# Print a string without a trailing newline.
+echo_without_newline () {
+ printf '%s' "$*"
+}
+
+# Print a string without newlines and replace them with a NULL character (\0).
+echo_without_newline_nul () {
+ echo_without_newline "$@" | tr '\n' '\0'
+}
+
+# Calculate the length of a string.
+strlen () {
+ echo_without_newline "$1" | wc -c | sed -e 's/^ *//'
+}
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 8e2c52652c..8360f3bbd9 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -4,6 +4,7 @@ test_description='git cat-file'
. ./test-lib.sh
. "$TEST_DIRECTORY/lib-loose.sh"
+. "$TEST_DIRECTORY"/lib-cat-file.sh
test_cmdmode_usage () {
test_expect_code 129 "$@" 2>err &&
@@ -99,18 +100,6 @@ do
'
done
-echo_without_newline () {
- printf '%s' "$*"
-}
-
-echo_without_newline_nul () {
- echo_without_newline "$@" | tr '\n' '\0'
-}
-
-strlen () {
- echo_without_newline "$1" | wc -c | sed -e 's/^ *//'
-}
-
run_tests () {
type=$1
object_name="$2"
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC RFC v12 03/12] cat-file: add declaration of variable i inside its for loop
From: Pablo Sabater @ 2026-06-08 10:14 UTC (permalink / raw)
To: eric.peijian
Cc: calvinwan, chriscool, git, jltobler, jonathantanmy, karthik.188,
toon, chandrapratap3519, Pablo Sabater
In-Reply-To: <20260608-ps-eric-work-rebase-v12-0-5338b766e658@gmail.com>
From: Eric Ju <eric.peijian@gmail.com>
Some code used in this series declares variable i and only uses it
in a for loop, not in any other logic outside the loop.
Change the declaration of i to be inside the for loop for readability.
While at it, we also change its type from "int" to "size_t" where the latter makes more sense.
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Eric Ju <eric.peijian@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
builtin/cat-file.c | 11 +++--------
fetch-pack.c | 3 +--
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index fa45f774d7..c060fd4800 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -726,12 +726,10 @@ static void dispatch_calls(struct batch_options *opt,
struct queued_cmd *cmd,
int nr)
{
- int i;
-
if (!opt->buffer_output)
die(_("flush is only for --buffer mode"));
- for (i = 0; i < nr; i++)
+ for (size_t i = 0; i < nr; i++)
cmd[i].fn(opt, cmd[i].line, output, data);
fflush(stdout);
@@ -739,9 +737,7 @@ static void dispatch_calls(struct batch_options *opt,
static void free_cmds(struct queued_cmd *cmd, size_t *nr)
{
- size_t i;
-
- for (i = 0; i < *nr; i++)
+ for (size_t i = 0; i < *nr; i++)
FREE_AND_NULL(cmd[i].line);
*nr = 0;
@@ -768,7 +764,6 @@ static void batch_objects_command(struct batch_options *opt,
size_t alloc = 0, nr = 0;
while (strbuf_getdelim_strip_crlf(&input, stdin, opt->input_delim) != EOF) {
- int i;
const struct parse_cmd *cmd = NULL;
const char *p = NULL, *cmd_end;
struct queued_cmd call = {0};
@@ -778,7 +773,7 @@ static void batch_objects_command(struct batch_options *opt,
if (isspace(*input.buf))
die(_("whitespace before command: '%s'"), input.buf);
- for (i = 0; i < ARRAY_SIZE(commands); i++) {
+ for (size_t i = 0; i < ARRAY_SIZE(commands); i++) {
if (!skip_prefix(input.buf, commands[i].name, &cmd_end))
continue;
diff --git a/fetch-pack.c b/fetch-pack.c
index 120e01f3cf..f13951d154 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1388,9 +1388,8 @@ static void write_fetch_command_and_capabilities(struct strbuf *req_buf,
if (advertise_sid && server_supports_v2("session-id"))
packet_buf_write(req_buf, "session-id=%s", trace2_session_id());
if (server_options && server_options->nr) {
- int i;
ensure_server_supports_v2("server-option");
- for (i = 0; i < server_options->nr; i++)
+ for (size_t i = 0; i < server_options->nr; i++)
packet_buf_write(req_buf, "server-option=%s",
server_options->items[i].string);
}
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC RFC v12 02/12] git-compat-util: add strtoul_ul() with error handling
From: Pablo Sabater @ 2026-06-08 10:14 UTC (permalink / raw)
To: eric.peijian
Cc: calvinwan, chriscool, git, jltobler, jonathantanmy, karthik.188,
toon, chandrapratap3519, Pablo Sabater
In-Reply-To: <20260608-ps-eric-work-rebase-v12-0-5338b766e658@gmail.com>
From: Eric Ju <eric.peijian@gmail.com>
We already have strtoul_ui() and similar functions that provide proper
error handling using strtoul from the standard library. However,
there isn't currently a variant that returns an unsigned long.
This variant is needed in a subsequent commit.
Add strtoul_ul() to address this gap, enabling the
return of an unsigned long with proper error handling.
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
git-compat-util.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 8809776407..4bf569f35c 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -975,6 +975,26 @@ static inline int strtoul_ui(char const *s, int base, unsigned int *result)
return 0;
}
+/*
+ * Convert a string to an unsigned long using the standard library's strtoul,
+ * with additional error handling to ensure robustness.
+ */
+static inline int strtoul_ul(char const *s, int base, unsigned long *result)
+{
+ unsigned long ul;
+ char *p;
+
+ errno = 0;
+ /* negative values would be accepted by strtoul */
+ if (strchr(s, '-'))
+ return -1;
+ ul = strtoul(s, &p, base);
+ if (errno || *p || p == s)
+ return -1;
+ *result = ul;
+ return 0;
+}
+
static inline int strtol_i(char const *s, int base, int *result)
{
long ul;
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC RFC v12 01/12] transport-helper: fix memory leak of helper on disconnect
From: Pablo Sabater @ 2026-06-08 10:14 UTC (permalink / raw)
To: eric.peijian
Cc: calvinwan, chriscool, git, jltobler, jonathantanmy, karthik.188,
toon, chandrapratap3519, Pablo Sabater
In-Reply-To: <20260608-ps-eric-work-rebase-v12-0-5338b766e658@gmail.com>
disconnect_helper() only frees data inside of the if(data->helper)
block [1]. When the transport is disconnected without the helper
being fully started, data->name allocated in transport_helper_init()
is never freed.
Move FREE_AND_NULL(data->name) outside the conditional block so it's
always freed on disconnect.
[1]: https://lore.kernel.org/git/05fbadbae2184479c87c37675dde7bd79b3e32ab.1716465556.git.ps@pks.im/
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
transport-helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/transport-helper.c b/transport-helper.c
index 04d55572a9..20a6ea8f81 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -266,9 +266,9 @@ static int disconnect_helper(struct transport *transport)
close(data->helper->out);
fclose(data->out);
res = finish_command(data->helper);
- FREE_AND_NULL(data->name);
FREE_AND_NULL(data->helper);
}
+ FREE_AND_NULL(data->name);
return res;
}
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC RFC v12 00/12] cat-file: add remote-object-info to batch-command
From: Pablo Sabater @ 2026-06-08 10:14 UTC (permalink / raw)
To: eric.peijian
Cc: calvinwan, chriscool, git, jltobler, jonathantanmy, karthik.188,
toon, chandrapratap3519, Pablo Sabater
In-Reply-To: <20250221190451.12536-1-eric.peijian@gmail.com>
This path series is a continuation of Eric Ju's (eric.peijian@gmail.com) and
Calvin Wan's (calvinwan@google.com) patch series [1] and [2] respectively.
Sometimes it is beneficial to retrieve information about an object without
having to download it completely. The server logic for retrieving size has
already been implemented and merged in "a2ba162cda (object-info: support for
retrieving object info, 2021-04-20)"[3]. This patch series implement the client
option for it.
Eric's series adds the `remote-object-info` command to
`cat-file --batch-command`. This command allows the client to make an
object-info command request to a server that supports protocol v2.
If the server uses protocol v2 but does not support the object-info capability,
`cat-file --batch-command` will die.
If a user attempts to use `remote-object-info` with protocol v1,
`cat-file --batch-command` will die.
Currently, only the size (%(objectsize)) is supported end to end in this
implementation. The type (%(objecttype)) is known by the client's allow-list
and request path but is not supported on the server side nor the response
parsing. A follow up series will add full end-to-end support for %(objecttype).
The default format for remote-object-info is set to %(objectname) %(objectsize).
Once %(objecttype) is supported, the default format will be unified accordingly.
If the batch command format includes unsupported fields such as %(objecttype),
%(objectsize:disk), or %(deltabase), the command will return empty strings for
each unsupported field.
This series completes Eric's work mainly with the refactor of the validation
of the placeholder with an allow-list that filters what the client asks with
what the server is capable of provide following Jeff King's idea [4].
I have a question for the design:
1. If the format includes unsupported fields such as %(objecttype) or
%(deltabase) it currently returns an empty string for each unsupported
field, this follows what for-each-ref does with known but inapplicable
atoms. However future placeholders that will be implemented: %(rest),
%(objectmode) can return empty strings. How should we differentiate
"unsupported" vs "no data".
Eric proposed to use a placeholder like "???" [5].
[1]: https://lore.kernel.org/git/20250221190451.12536-1-eric.peijian@gmail.com/
[2]: https://lore.kernel.org/git/20220728230210.2952731-1-calvinwan@google.com/#t
[3]: https://git.kernel.org/pub/scm/git/git.git/commit/?id=a2ba162cda2acc171c3e36acbbc854792b093cb7
[4]: https://lore.kernel.org/git/20250313060250.GH94015@coredump.intra.peff.net/
[5]: https://lore.kernel.org/git/CAN2LT1D3d=yMYVhBjpj5PvyjfTVjwqcFPNViuCJ=f49YbCZuJg@mail.gmail.com/
Changes since v11:
- Rebased onto current master.
- Added fetch-object-info.c to meson.build.
- Added t1017-cat-file-remote-object-info.sh to t/meson.build.
- Fixed style feedback from v10.
- Replaced strstr() validation with a dynamic allow-list in
expand_atom() based on server-advertised capabilities, filtering
unsupported options in fetch_object_info() before sending.
- Unsupported placeholders return empty strings instead of die().
- Initialize data->type to OBJ_BAD with NULL check on type_name().
- Moved global variables to local scope in parse_cmd_remote_object_info.
- Decoupled batch_object_write() from sizep so output is always produced.
- Added remote_atom_map[] for protocol-to-atom name mapping.
- Included "type" in the client request path for forward-compatibility.
- Fixed memory leak of data->name in transport-helper disconnect.
- Changed error message "not our ref" to "server does not recognize object".
- Split fetch-pack refactoring into two commits (move + refactor).
- Updated caveats documentation and tests.
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
Calvin Wan (3):
fetch-pack: move fetch initialization
serve: advertise object-info feature
transport: add client support for object-info
Eric Ju (4):
git-compat-util: add strtoul_ul() with error handling
cat-file: add declaration of variable i inside its for loop
t1006: split test utility functions into new "lib-cat-file.sh"
cat-file: add remote-object-info to batch-command
Pablo Sabater (5):
transport-helper: fix memory leak of helper on disconnect
fetch-pack: move function to connect.c
connect: refactor packet writing
cat-file: validate remote atoms with allow_list
cat-file: make remote-object-info allow-list dynamic
Documentation/git-cat-file.adoc | 25 +-
Makefile | 1 +
builtin/cat-file.c | 191 ++++++++-
connect.c | 34 ++
connect.h | 8 +
fetch-object-info.c | 93 +++++
fetch-object-info.h | 22 ++
fetch-pack.c | 51 +--
fetch-pack.h | 2 +
git-compat-util.h | 20 +
meson.build | 1 +
object-file.c | 10 +
odb.h | 3 +
serve.c | 5 +-
t/lib-cat-file.sh | 16 +
t/meson.build | 1 +
t/t1006-cat-file.sh | 13 +-
t/t1017-cat-file-remote-object-info.sh | 684 +++++++++++++++++++++++++++++++++
transport-helper.c | 13 +-
transport.c | 28 +-
transport.h | 11 +
21 files changed, 1158 insertions(+), 74 deletions(-)
---
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
change-id: 20260608-ps-eric-work-rebase-b73ae84ba671
Best regards,
--
Pablo Sabater <pabloosabaterr@gmail.com>
^ permalink raw reply
* Re: [PATCH RFC 2/2] builtin/history: print feedback after successful reword
From: Patrick Steinhardt @ 2026-06-08 9:30 UTC (permalink / raw)
To: Pablo Sabater; +Cc: git, Kaartic Sivaraam
In-Reply-To: <20260607-ps-history-reword-v1-2-ba43a3cbb81b@gmail.com>
On Sun, Jun 07, 2026 at 10:07:21PM +0200, Pablo Sabater wrote:
> Unlike `git commit --amend` and `git rebase -i`, `git history reword`
> doesn't print anything, this makes it feel empty for a porcelain command
> and hard to tell if the command did anything without using other
> commands like `git log <commit>` to check if the reword was done.
>
> Print a message on successful rewords so the user has feedback about it.
I dunno about this one. My take here is that a command should be silent
unless it has something to say, for example when it couldn't honor the
user's request [1].
> diff --git a/builtin/history.c b/builtin/history.c
> index 51a22a9a1c..0f1ba3b531 100644
> --- a/builtin/history.c
> +++ b/builtin/history.c
> @@ -739,6 +739,10 @@ static int cmd_history_reword(int argc,
> goto out;
> }
>
> + fprintf(stderr, _("Successfully reworded commit %s to %s\n"),
> + repo_find_unique_abbrev(repo, &original->object.oid, DEFAULT_ABBREV),
> + repo_find_unique_abbrev(repo, &rewritten->object.oid, DEFAULT_ABBREV));
> +
Seeing the implementation also raises a couple of questions:
- Why do we mention the rewritten commit, only? Shouldn't we also
print the changed HEAD?
- Why don't we print any of the other rewritten branches?
- What makes "git history reword" so special as compared to for
example "git history fixup" or "git history split" so that it needs
a message while the others don't?
It might make sense to maybe introduce a verbose mode where we do print
such information. But if so, we should have good answers to the above
questions and implement this in a way that makes sense for the other
subcommands, too, so that we can apply the same principle to all of
them.
Thanks!
Patrick
[1]: https://www.linfo.org/rule_of_silence.html
^ permalink raw reply
* Re: [PATCH RFC 1/2] builtin/history: abort reword on unchanged message
From: Patrick Steinhardt @ 2026-06-08 9:30 UTC (permalink / raw)
To: Pablo Sabater; +Cc: git, Kaartic Sivaraam
In-Reply-To: <20260607-ps-history-reword-v1-1-ba43a3cbb81b@gmail.com>
On Sun, Jun 07, 2026 at 10:07:20PM +0200, Pablo Sabater wrote:
> When using `git history reword` if the new message is the same as the
> original it continues anyway creating a new commit with the same
> message and updates its descendants, modifying the history after this
> 'reworded' commit even though there was no actual change.
>
> `git commit --amend` and `git rebase -i` + reword share this behavior,
> however `git history reword` is different:
> 1. Works in-memory without touching the index or the worktree [1], so
> there are no side effects like staged files that could justify
> rewriting the history when the commit message is the same.
> 2. `git history` by default updates all the branches [2] that contain the
> original commit making it more costly than `git rebase -i` that only
> updates the current branch.
>
> Add a check if the original commit message is the same as the new one
> and abort if so.
>
> [1]: https://lore.kernel.org/git/20260113-b4-pks-history-builtin-v11-8-e74ebfa2652d@pks.im/
> [2]: https://git-scm.com/docs/git-history#_description
Nit: I feel like both of the links don't really add much value.
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> builtin/history.c | 10 ++++++++++
> t/t3451-history-reword.sh | 20 ++++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/builtin/history.c b/builtin/history.c
> index 0fc06fb204..51a22a9a1c 100644
> --- a/builtin/history.c
> +++ b/builtin/history.c
> @@ -135,6 +135,13 @@ static int commit_tree_ext(struct repository *repo,
> original_body, action, &commit_message);
> if (ret < 0)
> goto out;
> +
> + if (!strcmp(original_body, commit_message.buf)) {
> + fprintf(stderr, _("Message unchanged,"
> + " aborting reword.\n"));
> + ret = 1;
> + goto out;
> + }
> } else {
> strbuf_addstr(&commit_message, original_body);
> }
We also execute this logic via "git history fixup --reedit-message", and
here it wouldn't make sense to abort the commit in case the message is
unchanged.
Patrick
^ permalink raw reply
* Re: [PATCH v2 5/9] reset: introduce ability to skip reference updates
From: Patrick Steinhardt @ 2026-06-08 9:18 UTC (permalink / raw)
To: phillip.wood; +Cc: git, Pablo Sabater, Junio C Hamano
In-Reply-To: <aiaH3ZmFZfmWYwr7@pks.im>
On Mon, Jun 08, 2026 at 11:14:08AM +0200, Patrick Steinhardt wrote:
> On Fri, Jun 05, 2026 at 04:12:42PM +0100, Phillip Wood wrote:
> > Hi Patrick
> >
> > On 03/06/2026 17:14, Patrick Steinhardt wrote:
> > > In a subsequent commit we'll introduce a new caller to `reset_head()`
> > > that really only wants to update the index and working tree, without
> > > updating any references. Introduce a new flag that lets the caller
> > > perform this operation.
> >
> > We already have a flag to update ORIG_HEAD so would it make more sense to
> > have a flag to update HEAD, rather than adding a flag to disable the
> > updates? It would mean updating the existing callers but I think it is a
> > clearer api and it avoids the pitfall of
> >
> > RESET_HEAD_ORIG_HEAD | RESET_HEAD_SKIP_REF_UPDATES
>
> Hm. The question is whether it's sensible to have
> `!RESET_HEAD_UPDATE_HEAD && RESET_HEAD_UPDATE_ORIG_HEAD`. That feels
> like a somewhat weird request, too, and we'd have to introduce extra
> logic to make that combination work.
>
> > I wonder about the function name as well if we make updating HEAD optional
> > then what does reset_head() mean? Maybe we should rename it something along
> > the lines of reset_worktree() or update_working_copy()? I'm not really sure
> > what a good name would be.
>
> That's a good point, the name does get somewhat awkward. I think we
> should keep "reset" in there, but `reset_worktree()` to me reads as it
> if was rather related to git-worktree(1) than anything else. Maybe
> `reset_working_tree()`?
I think I'll skip these changes for the next iteration for now. The
patch series has already exploded quite a bit in its scope due to the
refactorings of `reset_head()`, so I'd prefer to maybe do such changes
as a follow up.
Let me know in case you feel strongly about this though. Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH v2 3/9] reset: modernize flags passed to `reset_head()`
From: Patrick Steinhardt @ 2026-06-08 9:14 UTC (permalink / raw)
To: phillip.wood; +Cc: git, Pablo Sabater, Junio C Hamano
In-Reply-To: <9e2cb34b-97f6-44f4-be44-60f44760e601@gmail.com>
On Fri, Jun 05, 2026 at 04:08:57PM +0100, Phillip Wood wrote:
> Hi Patrick
>
> On 03/06/2026 17:14, Patrick Steinhardt wrote:
>
> > -/* Update ORIG_HEAD as well as HEAD */
> > -#define RESET_ORIG_HEAD (1<<4)
> > [...]> + /* Update ORIG_HEAD as well as HEAD */
> > + RESET_HEAD_ORIG_HEAD = (1 << 4),
>
> I'm having a hard time parsing this new name, if we must have a
> "RESET_HEAD_" prefix can we call it something like
> RESET_HEAD_UPDATE_ORIG_HEAD?
Yeah, that reads better indeed. Amended locally, thanks!
Patrick
^ permalink raw reply
* Re: [PATCH v2 5/9] reset: introduce ability to skip reference updates
From: Patrick Steinhardt @ 2026-06-08 9:14 UTC (permalink / raw)
To: phillip.wood; +Cc: git, Pablo Sabater, Junio C Hamano
In-Reply-To: <0fdaeec8-99cd-4dc9-9549-8a08133deebf@gmail.com>
On Fri, Jun 05, 2026 at 04:12:42PM +0100, Phillip Wood wrote:
> Hi Patrick
>
> On 03/06/2026 17:14, Patrick Steinhardt wrote:
> > In a subsequent commit we'll introduce a new caller to `reset_head()`
> > that really only wants to update the index and working tree, without
> > updating any references. Introduce a new flag that lets the caller
> > perform this operation.
>
> We already have a flag to update ORIG_HEAD so would it make more sense to
> have a flag to update HEAD, rather than adding a flag to disable the
> updates? It would mean updating the existing callers but I think it is a
> clearer api and it avoids the pitfall of
>
> RESET_HEAD_ORIG_HEAD | RESET_HEAD_SKIP_REF_UPDATES
Hm. The question is whether it's sensible to have
`!RESET_HEAD_UPDATE_HEAD && RESET_HEAD_UPDATE_ORIG_HEAD`. That feels
like a somewhat weird request, too, and we'd have to introduce extra
logic to make that combination work.
> I wonder about the function name as well if we make updating HEAD optional
> then what does reset_head() mean? Maybe we should rename it something along
> the lines of reset_worktree() or update_working_copy()? I'm not really sure
> what a good name would be.
That's a good point, the name does get somewhat awkward. I think we
should keep "reset" in there, but `reset_worktree()` to me reads as it
if was rather related to git-worktree(1) than anything else. Maybe
`reset_working_tree()`?
Patrick
^ permalink raw reply
* Re: [PATCH v2 1/3] Documentation/MyFirstContribution: recommend shallow threading
From: Weijie Yuan @ 2026-06-08 8:31 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Tuomas Ahola, Ramsay Jones
In-Reply-To: <aiZlu36Fh020L1Ip@pks.im>
On Mon, Jun 08, 2026 at 08:48:27AM +0200, Patrick Steinhardt wrote:
> On Wed, Jun 03, 2026 at 06:29:32PM +0800, Weijie Yuan wrote:
> > I'm afraid there will be some chaos.
>
> I think "chaos" is a bit exaggerated.
Oops, sorry for my poor English, I guess "messy" would be a more proper
choice here? ;-)
> > As mentioned earlier, GitGitGadget now supports deep nesting of
> > iterations, if b4 changes while GitGitGadget doesn't, it would be
> > inconsistent in the archive. So, negotiation is necessary here.
>
> That's a good point though -- if we change the recommendation, we should
> aim to change it consistently. I'll talk with Dscho (maintainer of GGG)
> today.
Thanks for your effort on this! Let´s see if the community can get on
the same page about this.
Thanks!
^ permalink raw reply
* Re: [PATCH] doc: fix typo in GIT_ALTERNATE_OBJECT_DIRECTORIES
From: Patrick Steinhardt @ 2026-06-08 7:02 UTC (permalink / raw)
To: Alexander Monakov; +Cc: git
In-Reply-To: <20260605172643.8796-1-amonakov@ispras.ru>
On Fri, Jun 05, 2026 at 08:26:43PM +0300, Alexander Monakov wrote:
> One file accidentally spelled GIT_ALTERNATE_OBJECT_DIRECTORIES with
> REPOSITORIES instead of DIRECTORIES. Fix the typo.
>
> Signed-off-by: Alexander Monakov <amonakov@ispras.ru>
> ---
> Documentation/technical/hash-function-transition.adoc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/technical/hash-function-transition.adoc b/Documentation/technical/hash-function-transition.adoc
> index 2359d7d106..241d2f763d 100644
> --- a/Documentation/technical/hash-function-transition.adoc
> +++ b/Documentation/technical/hash-function-transition.adoc
> @@ -545,7 +545,7 @@ Alternates
> ~~~~~~~~~~
> For the same reason, a SHA-256 repository cannot borrow objects from a
> SHA-1 repository using objects/info/alternates or
> -$GIT_ALTERNATE_OBJECT_REPOSITORIES.
> +$GIT_ALTERNATE_OBJECT_DIRECTORIES.
Yup, this change looks obviously good to me. Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH] describe: limit default ref iteration to tags
From: Patrick Steinhardt @ 2026-06-08 6:59 UTC (permalink / raw)
To: Tamir Duberstein; +Cc: git, Shawn O. Pearce, Junio C Hamano, Jeff King
In-Reply-To: <20260607-describe-tag-ref-scope-v1-1-653d232b86b5@gmail.com>
On Sun, Jun 07, 2026 at 04:51:53PM -0400, Tamir Duberstein wrote:
> Unless --all is given, get_name() rejects every ref outside refs/tags/.
> The rejection happens only after the ref backend has enumerated the ref,
> so repositories with many other refs spend most of a simple describe
> invocation visiting refs which cannot affect its result.
Right. The relevant block is this one:
if (skip_prefix(ref->name, "refs/tags/", &path_to_match)) {
is_tag = 1;
} else if (all) {
if ((exclude_patterns.nr || patterns.nr) &&
!skip_prefix(ref->name, "refs/heads/", &path_to_match) &&
!skip_prefix(ref->name, "refs/remotes/", &path_to_match)) {
/* Only accept reference of known type if there are match/exclude patterns */
return 0;
}
} else {
/* Reject anything outside refs/tags/ unless --all */
return 0;
}
So we really only use tags unless "--all" is given.
> Commit 8a5a1884e9 (Avoid accessing non-tag refs in git-describe unless
> --all is requested, 2008-02-24) moved this rejection before object
> lookup, but left iteration unscoped. Pass the existing refs/tags/
> restriction to the iterator unless --all is given so the backend can
> avoid unrelated refs.
>
> On a checkout with 124,357 refs, of which 330 were tags, I ran the
> following command with the parent and patched binaries:
>
> hyperfine --warmup 3 --runs 15 \
> 'git describe --always --long --abbrev=40 HEAD'
>
> The results were:
>
> parent this commit
> elapsed 196.2 ms 63.3 ms
> user 69.5 ms 48.0 ms
> system 123.0 ms 12.0 ms
It's a bit curious that you don't post the hyperfine(1) results as-is
here.
> The wall-time standard deviations were 13.2 ms and 2.6 ms, respectively,
> for a 3.10x speedup.
Makes sense that this would result in a sizeable speedup, depending of
course on the shape of the existing refs in the repository.
> diff --git a/builtin/describe.c b/builtin/describe.c
> index 1c47d7c0b7..3532c8ff22 100644
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -740,6 +740,9 @@ int cmd_describe(int argc,
> return ret;
> }
>
> + if (!all)
> + for_each_ref_opts.prefix = "refs/tags/";
> +
> hashmap_init(&names, commit_name_neq, NULL, 0);
> refs_for_each_ref_ext(get_main_ref_store(the_repository),
> get_name, NULL, &for_each_ref_opts);
Another performance optimization that we could do here is to wire up the
exclude patterns via `for_each_ref_opts.exclude_patterns`. But that's
outside the scope of this patch series, and also much less likely to
help many use cases out there.
> diff --git a/t/perf/p6100-describe.sh b/t/perf/p6100-describe.sh
> index 069f91ce49..dfcaf59e90 100755
> --- a/t/perf/p6100-describe.sh
> +++ b/t/perf/p6100-describe.sh
> @@ -5,6 +5,12 @@ test_description='performance of git-describe'
>
> test_perf_default_repo
>
> +test_lazy_prereq PERF_REFFILES '
> + test "$(git rev-parse --show-ref-format)" = files
> +'
> +
> +ref_count=10000
Let's not declare this variable outside of tests.
> @@ -27,4 +33,18 @@ test_perf 'describe HEAD with one tag' '
> git describe --match=new HEAD
> '
>
> +test_expect_success PERF_REFFILES 'set up many unrelated refs' '
> + git tag -m tip tip HEAD &&
> + for i in $(test_seq $ref_count)
> + do
> + printf "create refs/heads/describe-perf/%05d HEAD\n" $i ||
> + return 1
> + done >instructions &&
> + git update-ref --stdin <instructions
> +'
Why is this limited to the "files" backend, only? The logic should work
for both backends as-is.
Thanks!
Patrick
^ permalink raw reply
* [PATCH v3 3/3] b4: introduce configuration for the Git project
From: Patrick Steinhardt @ 2026-06-08 6:49 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones,
SZEDER Gábor, Kristoffer Haugsbakk, Toon Claes
In-Reply-To: <20260608-pks-b4-v3-0-f5e497d10c56@pks.im>
We're about to extend our documentation to recommend b4 for sending
patch series to the mailing list. Prepare for this by introducing a b4
configuration so that the tool knows to honor our preferences. For now,
this configuration does two things:
- It configures "send-same-thread = shallow", which tells b4 to always
send subsequent versions of the same patch series as a reply to the
cover letter of the first version.
- It configures "prep-cover-template", which tells b4 to use a custom
template for the cover letter. The most important change compared to
the default template is that our custom template also includes a
range-diff.
There's potentially more things that we may want to configure going
forward, like for example auto-configuration of folks to Cc on certain
patches. But these two tweaks feel like a good place to start.
Note that these values only serve as defaults, and users may want to
tweak those defaults based on their own preference. Luckily, users can
do that without having to touch `.b4-config` at all, as b4 allows them
to override values via Git configuration:
```
$ git config set b4.prep-cover-template /does/not/exist
$ b4 send --dry-run
ERROR: prep-cover-template says to use x, but it does not exist
```
So this gives users an easy way to override our defaults without having
to touch ".b4-config", which would dirty the tree.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
.b4-config | 6 ++++++
.b4-cover-template | 11 +++++++++++
2 files changed, 17 insertions(+)
diff --git a/.b4-config b/.b4-config
new file mode 100644
index 0000000000..fd4fb56b6d
--- /dev/null
+++ b/.b4-config
@@ -0,0 +1,6 @@
+# Note that these are default values that you can tweak via the typical
+# git-config(1) machinery. You thus shouldn't ever have to change this file.
+# See also https://b4.docs.kernel.org/en/latest/config.html.
+[b4]
+send-same-thread = shallow
+prep-cover-template = ./.b4-cover-template
diff --git a/.b4-cover-template b/.b4-cover-template
new file mode 100644
index 0000000000..ab864933b5
--- /dev/null
+++ b/.b4-cover-template
@@ -0,0 +1,11 @@
+${cover}
+
+---
+${shortlog}
+
+${diffstat}
+
+${range_diff}
+---
+base-commit: ${base_commit}
+${prerequisites}
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v3 2/3] MyFirstContribution: recommend the use of b4
From: Patrick Steinhardt @ 2026-06-08 6:49 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones,
SZEDER Gábor, Kristoffer Haugsbakk, Toon Claes
In-Reply-To: <20260608-pks-b4-v3-0-f5e497d10c56@pks.im>
The b4 tool originates from the Linux kernel community and is intended
to help mailing-list based workflows. It automates a lot of the annoying
bookkeeping tasks that contributors typically need to do: tracking the
list of recipients, Message-IDs, range-diffs and the like. In addition
to that, b4 also has many other subcommands that help the maintainer and
reviewers.
The Git project uses the same infrastructure as the kernel, so this tool
is also a very good fit for us. Adapt "MyFirstContribution" to
explicitly recommend its use.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/MyFirstContribution.adoc | 92 ++++++++++++++++++++++++++++++++--
Documentation/SubmittingPatches | 6 ++-
2 files changed, 93 insertions(+), 5 deletions(-)
diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index 984b7f5aa8..607876f3d8 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -833,7 +833,7 @@ This patchset is part of the MyFirstContribution tutorial and should not
be merged.
----
-At this point the tutorial diverges, in order to demonstrate two
+At this point the tutorial diverges, in order to demonstrate three
different methods of formatting your patchset and getting it reviewed.
The first method to be covered is GitGitGadget, which is useful for those
@@ -845,9 +845,14 @@ more fine-grained control over the emails to be sent. This method requires some
setup which can change depending on your system and will not be covered in this
tutorial.
+The third method to be covered is `b4`, which builds on top of `git
+format-patch` and `git send-email`. This method is the recommended way to
+submit patches via mail as it automates a lot of the bookkeeping required by
+`git send-email`.
+
Regardless of which method you choose, your engagement with reviewers will be
-the same; the review process will be covered after the sections on GitGitGadget
-and `git send-email`.
+the same; the review process will be covered after the sections on GitGitGadget,
+`git send-email` and `b4`.
[[howto-ggg]]
== Sending Patches via GitGitGadget
@@ -1296,6 +1301,87 @@ index 88f126184c..38da593a60 100644
2.21.0.392.gf8f6787159e-goog
----
+[[howto-b4]]
+== Sending Patches with `b4`
+
+`b4` is a tool that builds on top of `git format-patch` and `git send-email`.
+It automates much of the bookkeeping involved in sending a patch series to a
+mailing-list-based project.
+
+Refer to the https://b4.docs.kernel.org/[b4 documentation] for a full reference.
+
+[[prep-b4]]
+=== Preparing a Patch Series
+
+`b4` tracks your patch series as a branch. To start tracking the `psuh` branch
+you have been working on, run:
+
+----
+$ b4 prep --enroll master
+----
+
+This enrolls the current branch, using `master` as the base of the topic. `b4`
+manages the cover letter as part of the branch, so you can edit it at any time
+with:
+
+----
+$ b4 prep --edit-cover
+----
+
+The cover letter not only tracks the content of the top-level mail, but also
+the set of recipients. You can add recipients by adding `To:` and `Cc:`
+trailer lines.
+
+[[send-b4]]
+=== Sending the Patches
+
+Before sending the series out for real, you can inspect what `b4` would send by
+passing `--dry-run`:
+
+----
+$ b4 send --dry-run
+----
+
+Once you are happy with the result, send the series with:
+
+----
+$ b4 send
+----
+
+[[v2-b4]]
+=== Sending v2
+
+When you are ready to send a new iteration of your series, refine your
+patches as usual using linkgit:git-rebase[1]. Note that you typically want to
+rebase on top of the cover letter. You can configure an alias to enable easy
+rebases going forward:
+
+---
+$ git config set alias.b4-rebase 'rebase "HEAD^{/--- b4-submit-tracking ---}"'
+$ git b4-rebase -i
+---
+
+Before sending out the new version you should also update the cover letter with
+`b4 prep --edit-cover` to note the relevant changes compared to the previous
+version. You can inspect the changes between the two versions with `b4 prep
+--compare-to=v1`.
+
+Same as with the first version, you can use `b4 send` to send out the second
+version. `b4` automatically bumps the version to `v2`, generates the range-diff
+against the previous iteration, and threads the new series as a reply to the
+cover letter of the first version.
+
+[[configure-b4]]
+=== Configure b4
+
+`b4` can be configured via linkgit:git-config[1]. In addition to that, projects
+can have their own set of defaults in `.b4-config` in the root tree, which also
+uses Git's config format. The user's configuration always takes precedence over
+the per-project defaults.
+
+Refer to the https://b4.docs.kernel.org/en/latest/config.html[b4 config documentation]
+for more information on the available options.
+
[[now-what]]
== My Patch Got Emailed - Now What?
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index d570184ec8..99427e1ee1 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -573,8 +573,10 @@ your existing e-mail client (often optimized for "multipart/*" MIME
type e-mails) might render your patches unusable.
NOTE: Here we outline the procedure using `format-patch` and
-`send-email`, but you can instead use GitGitGadget to send in your
-patches (see link:MyFirstContribution.html[MyFirstContribution]).
+`send-email`, but you can instead use GitGitGadget or `b4` to send in
+your patches (see link:MyFirstContribution.html[MyFirstContribution]).
+Contributors are encouraged to use `b4`, which automates much of the
+bookkeeping that is otherwise done by hand.
People on the Git mailing list need to be able to read and
comment on the changes you are submitting. It is important for
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v3 1/3] MyFirstContribution: recommend shallow threading of cover letters
From: Patrick Steinhardt @ 2026-06-08 6:49 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones,
SZEDER Gábor, Kristoffer Haugsbakk, Toon Claes
In-Reply-To: <20260608-pks-b4-v3-0-f5e497d10c56@pks.im>
The "MyFirstContribution" document recommends the use of deep threading
of cover letters: every cover letter of subsequent iterations shall be
linked to the cover letter of the preceding version. The result of this
is that eventually, threads with many versions are getting nested so
deep that it becomes hard to follow.
Adapt the recommendation to instead propose shallow threading of cover
letters: instead of linking the cover letter to the previous cover
letter, the user is supposed to always link it to the first cover
letter. This still makes it easy to follow the iterations, but has the
benefit of nesting to a much shallower level.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/MyFirstContribution.adoc | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index b9fdefce02..984b7f5aa8 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -790,7 +790,7 @@ We can note a few things:
v3", etc. in place of "PATCH". For example, "[PATCH v2 1/3]" would be the first of
three patches in the second iteration. Each iteration is sent with a new cover
letter (like "[PATCH v2 0/3]" above), itself a reply to the cover letter of the
- previous iteration (more on that below).
+ first iteration (more on that below).
NOTE: A single-patch topic is sent with "[PATCH]", "[PATCH v2]", etc. without
_i_/_n_ numbering (in the above thread overview, no single-patch topic appears,
@@ -1214,7 +1214,7 @@ between your last version and now, if it's something significant. You do not
need the exact same body in your second cover letter; focus on explaining to
reviewers the changes you've made that may not be as visible.
-You will also need to go and find the Message-ID of your previous cover letter.
+You will also need to go and find the Message-ID of your first cover letter.
You can either note it when you send the first series, from the output of `git
send-email`, or you can look it up on the
https://lore.kernel.org/git[mailing list]. Find your cover letter in the
@@ -1227,8 +1227,8 @@ Message-ID: <foo.12345.author@example.com>
Your Message-ID is `<foo.12345.author@example.com>`. This example will be used
below as well; make sure to replace it with the correct Message-ID for your
-**previous cover letter** - that is, if you're sending v2, use the Message-ID
-from v1; if you're sending v3, use the Message-ID from v2.
+**first cover letter** - that is, for any subsequent version that you send,
+always use the Message-ID from v1.
While you're looking at the email, you should also note who is CC'd, as it's
common practice in the mailing list to keep all CCs on a thread. You can add
--
2.54.0.1136.gdb2ca164c4.dirty
^ permalink raw reply related
* [PATCH v3 0/3] Documentation: recommend the use of b4
From: Patrick Steinhardt @ 2026-06-08 6:49 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones,
SZEDER Gábor, Kristoffer Haugsbakk, Toon Claes
In-Reply-To: <20260602-pks-b4-v1-0-a7ae5a49e9cf@pks.im>
Hi,
this small patch series wires up b4 in Git and recommends the use
thereof via "MyFirstContribution", as discussed in [1].
Changes in v3:
- I wasn't really able to judge consensus one way or the other
regarding the deep vs shallow nesting of cover letters, so I still
have the change to shallow nesting of cover letters part of this
series. If we continue to be split on this one (or if we favor the
current status quo) I'm happy to drop the first patch and adapt the
last patch to use deep nesting of cover letters instead.
- Hopefully fix some confusion by saying "shallow/deep threading of
cover letters".
- Fix some more instances where we recommend deep threading of cover
letters.
- Link to v2: https://patch.msgid.link/20260603-pks-b4-v2-0-a8aea0aa2c23@pks.im
Changes in v2:
- Reorder commits so that the b4 docs are added first.
- Add a section that highlights how to configure b4, and that points
out that the per-project defaults can be overridden via Git
configuration.
- Add a patch to MyFirstContribution that recommends shallow
threading. I mostly intend this to be a discussion starter so that
the `.b4-config` file matches our preferred threading style.
- Fix a typo.
- Link to v1: https://patch.msgid.link/20260602-pks-b4-v1-0-a7ae5a49e9cf@pks.im
Thanks!
Patrick
[1]: <xmqqik81xpqx.fsf@gitster.g>
---
Patrick Steinhardt (3):
MyFirstContribution: recommend shallow threading of cover letters
MyFirstContribution: recommend the use of b4
b4: introduce configuration for the Git project
.b4-config | 6 ++
.b4-cover-template | 11 ++++
Documentation/MyFirstContribution.adoc | 100 ++++++++++++++++++++++++++++++---
Documentation/SubmittingPatches | 6 +-
4 files changed, 114 insertions(+), 9 deletions(-)
Range-diff versus v2:
1: f7784c8f7f ! 1: 4b0c4f9aca Documentation/MyFirstContribution: recommend shallow threading
@@ Metadata
Author: Patrick Steinhardt <ps@pks.im>
## Commit message ##
- Documentation/MyFirstContribution: recommend shallow threading
+ MyFirstContribution: recommend shallow threading of cover letters
- The "MyFirstContribution" document recommends the use of deep threading:
- every cover letter of subsequent iterations shall be linked to the cover
- letter of the preceding version. The result of this is that eventually,
- threads with many versions are getting nested so deep that it becomes
- hard to follow.
+ The "MyFirstContribution" document recommends the use of deep threading
+ of cover letters: every cover letter of subsequent iterations shall be
+ linked to the cover letter of the preceding version. The result of this
+ is that eventually, threads with many versions are getting nested so
+ deep that it becomes hard to follow.
- Adapt the recommendation to instead propose shallow threading: instead
- of linking the cover letter to the previous cover letter, the user is
- supposed to always link it to the first cover letter. This still makes
- it easy to follow the iterations, but has the benefit of nesting to a
- much shallower level.
+ Adapt the recommendation to instead propose shallow threading of cover
+ letters: instead of linking the cover letter to the previous cover
+ letter, the user is supposed to always link it to the first cover
+ letter. This still makes it easy to follow the iterations, but has the
+ benefit of nesting to a much shallower level.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
## Documentation/MyFirstContribution.adoc ##
+@@ Documentation/MyFirstContribution.adoc: We can note a few things:
+ v3", etc. in place of "PATCH". For example, "[PATCH v2 1/3]" would be the first of
+ three patches in the second iteration. Each iteration is sent with a new cover
+ letter (like "[PATCH v2 0/3]" above), itself a reply to the cover letter of the
+- previous iteration (more on that below).
++ first iteration (more on that below).
+
+ NOTE: A single-patch topic is sent with "[PATCH]", "[PATCH v2]", etc. without
+ _i_/_n_ numbering (in the above thread overview, no single-patch topic appears,
+@@ Documentation/MyFirstContribution.adoc: between your last version and now, if it's something significant. You do not
+ need the exact same body in your second cover letter; focus on explaining to
+ reviewers the changes you've made that may not be as visible.
+
+-You will also need to go and find the Message-ID of your previous cover letter.
++You will also need to go and find the Message-ID of your first cover letter.
+ You can either note it when you send the first series, from the output of `git
+ send-email`, or you can look it up on the
+ https://lore.kernel.org/git[mailing list]. Find your cover letter in the
@@ Documentation/MyFirstContribution.adoc: Message-ID: <foo.12345.author@example.com>
Your Message-ID is `<foo.12345.author@example.com>`. This example will be used
2: e8f3caf73a ! 2: 625de75a33 Documentation/MyFirstContribution: recommend the use of b4
@@ Metadata
Author: Patrick Steinhardt <ps@pks.im>
## Commit message ##
- Documentation/MyFirstContribution: recommend the use of b4
+ MyFirstContribution: recommend the use of b4
The b4 tool originates from the Linux kernel community and is intended
to help mailing-list based workflows. It automates a lot of the annoying
3: 35591c55c8 = 3: a95973cfb6 b4: introduce configuration for the Git project
---
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
change-id: 20260602-pks-b4-31cc20d7f84b
^ permalink raw reply
* Re: [PATCH v2 3/3] b4: introduce configuration for the Git project
From: Patrick Steinhardt @ 2026-06-08 6:48 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones
In-Reply-To: <87qzmn20a9.fsf@emacs.iotcl.com>
On Wed, Jun 03, 2026 at 03:58:38PM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/.b4-config b/.b4-config
> > new file mode 100644
> > index 0000000000..fd4fb56b6d
> > --- /dev/null
> > +++ b/.b4-config
> > @@ -0,0 +1,6 @@
> > +# Note that these are default values that you can tweak via the typical
> > +# git-config(1) machinery. You thus shouldn't ever have to change this file.
> > +# See also https://b4.docs.kernel.org/en/latest/config.html.
> > +[b4]
> > +send-same-thread = shallow
>
> Is it worth to note this requires v0.15 or higher?
>
> That version was released only 2 months ago, I can imagine many distros
> still ship an older version, what happens if a version doesn't support
> this setting yet?
That's fair. In case it's not supported we fall back to the default,
which is to not use threading at all.
Might be another indicator that we should just stick with the current
threading style.
Patrick
^ permalink raw reply
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Patrick Steinhardt @ 2026-06-08 6:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: SZEDER Gábor, Weijie Yuan, Tuomas Ahola, git
In-Reply-To: <xmqqmrxbp0s6.fsf@gitster.g>
On Thu, Jun 04, 2026 at 10:11:37AM +0900, Junio C Hamano wrote:
> Having said that, I've seen a cover letter of iteration N (for any
> value of N > 1) that respondes to the cover letter of the initial
> iteration. While it seems not to break "br" and the lore archive
> does not seem unhappy about it, I am not sure if tooling used by
> other people are also happy with it.
Yeah, I always use that style myself. I mostly prefer it because the
nesting for long-running patch series with many versions is eventually
getting out of hand and hard to navigate, and I haven't seen any tooling
breaking as a result of that style.
If I'm the only one who thinks that style to be preferable I am happy to
adapt. I'm not really sure yet what the consensus is -- I'll send one
more version that includes the changes, but if we continue to be split
or in favor of the current status quo I'll drop those in the next
version.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Patrick Steinhardt @ 2026-06-08 6:48 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Weijie Yuan, Tuomas Ahola, git, Junio C Hamano
In-Reply-To: <aiAK9eLvew+mgWt+@szeder.dev>
On Wed, Jun 03, 2026 at 01:07:33PM +0200, SZEDER Gábor wrote:
> On Wed, Jun 03, 2026 at 08:55:04AM +0200, Patrick Steinhardt wrote:
> > On Wed, Jun 03, 2026 at 10:12:22AM +0800, Weijie Yuan wrote:
> > > On Tue, Jun 02, 2026 at 08:09:55PM +0300, Tuomas Ahola wrote:
> > > > Huh? Doesn't MyFirstContribution speak *against* shallow threading?
> > > >
> > > > [...] make sure to replace it with the correct Message-ID for your
> > > > **previous cover letter** - that is, if you're sending v2, use the Message-ID
> > > > from v1; if you're sending v3, use the Message-ID from v2.
> > >
> > > I don't get it. Doesn't shallow threading means every following patches
> > > are replying to the cover letter? Replying to the previous one is
> > > --chain-reply-to, if I'm not mistaken.
> >
> > Shallow threading basically means that all patches are sent as a
> > response to the current cover letter, and the current cover letter is
> > always attached to the cover letter of the _first_ version.
>
> No, in Git shallow threading means that all patches are sent as a
> respose to the current cover letter, period. It has nothing to do
> with whether the current cover letter is sent as a reply to the cover
> letter of the first or the previous version.
>
> > So this quote is definitely at odds with the configuration I have
> > proposed. It's actually quite surprising to me that we recommend deep
> > threading -- I personally find it extremely hard to navigate as the
> > nesting eventually gets way too deep.
>
> Deep threading means that every mail is a reply to the previous one.
> Again, it has nothing to do with the relation of the current cover
> letter and the previous cover letters.
>
> Therefore, we do not recommend deep threading.
Oh, you're right of course. I totally forgot that we even had this
style.
Patrick
^ permalink raw reply
* Re: [PATCH v2 2/3] Documentation/MyFirstContribution: recommend the use of b4
From: Patrick Steinhardt @ 2026-06-08 6:48 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones
In-Reply-To: <87mrxa27xq.fsf@emacs.iotcl.com>
On Thu, Jun 04, 2026 at 07:25:37AM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> > index 069020196c..fc0b06ae67 100644
> > --- a/Documentation/MyFirstContribution.adoc
> > +++ b/Documentation/MyFirstContribution.adoc
> > @@ -833,7 +833,7 @@ This patchset is part of the MyFirstContribution tutorial and should not
> > be merged.
> > ----
> >
> > -At this point the tutorial diverges, in order to demonstrate two
> > +At this point the tutorial diverges, in order to demonstrate three
> > different methods of formatting your patchset and getting it reviewed.
> >
> > The first method to be covered is GitGitGadget, which is useful for those
> > @@ -845,9 +845,14 @@ more fine-grained control over the emails to be sent. This method requires some
> > setup which can change depending on your system and will not be covered in this
> > tutorial.
> >
> > +The third method to be covered is `b4`, which builds on top of `git
> > +format-patch` and `git send-email`. This method is the recommended way to
> > +submit patches via mail as it automates a lot of the bookkeeping required by
> > +`git send-email`.
>
> The GitGitGadget method includes Running CI, maybe that's worth
> mentioning the user is responsible themselves to run the whole test
> suite? Or is this outside the scope of this series, since `git
> send-email` doesn't include that too.
I'd say it's out-of-scope for this patch series.
That being said, I have been wondering last week whether we can automate
running CI in some fashion to shorten feedback cycles, bridge the gap
between the mailing list and CI and ultimately help both reviewers and
Junio. Some subsystems in the Linux kernel for example have tooling that
picks up patch series from the mailing list, runs it through CI and then
reports results to the mailing list (for example [1]).
Having something like that might be valuable for us, too.
Patrick
[1]: https://github.com/linux-netdev/nipa
^ permalink raw reply
* Re: [PATCH v2 1/3] Documentation/MyFirstContribution: recommend shallow threading
From: Patrick Steinhardt @ 2026-06-08 6:48 UTC (permalink / raw)
To: Kristoffer Haugsbakk
Cc: git, Junio C Hamano, Tuomas Ahola, Weijie Yuan, Ramsay Jones
In-Reply-To: <f1dbb848-2d9b-488a-835b-2d23006b5fa6@app.fastmail.com>
On Wed, Jun 03, 2026 at 10:09:39PM +0200, Kristoffer Haugsbakk wrote:
> On Wed, Jun 3, 2026, at 08:58, Patrick Steinhardt wrote:
> > The "MyFirstContribution" document recommends the use of deep threading:
> > every cover letter of subsequent iterations shall be linked to the cover
> > letter of the preceding version. The result of this is that eventually,
> > threads with many versions are getting nested so deep that it becomes
> > hard to follow.
> >
> > Adapt the recommendation to instead propose shallow threading: instead
> > of linking the cover letter to the previous cover letter, the user is
> > supposed to always link it to the first cover letter. This still makes
> > it easy to follow the iterations, but has the benefit of nesting to a
> > much shallower level.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > Documentation/MyFirstContribution.adoc | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >[snip]
>
> Only today did I notice that your eleven-version git-history(1) series
> uses this style. (Or: today I noticed that it’a thing)
>
> https://lore.kernel.org/git/20250819-b4-pks-history-builtin-v1-0-9b77c32688fe@pks.im/
>
> That would have had a bad rightward drift with the usual reply to
> previous version style.
>
> I’ve been reading Lore on Safari on mobile and some threads go so deep
> that the replies just become unclickable backticks. *Huh?* Well I can
> use the Next/Previous buttons and maybe there is a way to make it work,
> but I’ve just given up on those right-going subthreads. ;)
>
> ... and I also don’t see any drawbacks to that threading, using that
> series as an example. It looks just as comprehensible as the usual
> style.
Yeah. It doesn't matter much for patch series that don't require many
iterations. But eventually I feel like it gets out-of-hand to have the
deep nesting.
Patrick
^ permalink raw reply
* Re: [PATCH v2 1/3] Documentation/MyFirstContribution: recommend shallow threading
From: Patrick Steinhardt @ 2026-06-08 6:48 UTC (permalink / raw)
To: Weijie Yuan; +Cc: git, Junio C Hamano, Tuomas Ahola, Ramsay Jones
In-Reply-To: <aiACDLOtd_0_CCD7@wyuan.org>
On Wed, Jun 03, 2026 at 06:29:32PM +0800, Weijie Yuan wrote:
> I'm afraid there will be some chaos.
I think "chaos" is a bit exaggerated. We already have both styles on the
mailing list, and I think in general people are able to cope with that
just fine. :)
> As mentioned earlier, GitGitGadget now supports deep nesting of
> iterations, if b4 changes while GitGitGadget doesn't, it would be
> inconsistent in the archive. So, negotiation is necessary here.
That's a good point though -- if we change the recommendation, we should
aim to change it consistently. I'll talk with Dscho (maintainer of GGG)
today.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH v2 1/3] Documentation/MyFirstContribution: recommend shallow threading
From: Patrick Steinhardt @ 2026-06-08 6:48 UTC (permalink / raw)
To: Tuomas Ahola; +Cc: git, Junio C Hamano, Weijie Yuan, Ramsay Jones
In-Reply-To: <20260603100145.7iym5%taahol@utu.fi>
On Wed, Jun 03, 2026 at 01:01:45PM +0300, Tuomas Ahola wrote:
> Patrick Steinhardt <ps@pks.im> wrote:
>
> > The "MyFirstContribution" document recommends the use of deep threading:
> > every cover letter of subsequent iterations shall be linked to the cover
> > letter of the preceding version. The result of this is that eventually,
> > threads with many versions are getting nested so deep that it becomes
> > hard to follow.
> >
> > Adapt the recommendation to instead propose shallow threading: instead
> > of linking the cover letter to the previous cover letter, the user is
> > supposed to always link it to the first cover letter. This still makes
> > it easy to follow the iterations, but has the benefit of nesting to a
> > much shallower level.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > Documentation/MyFirstContribution.adoc | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> > index b9fdefce02..069020196c 100644
> > --- a/Documentation/MyFirstContribution.adoc
> > +++ b/Documentation/MyFirstContribution.adoc
> > @@ -1227,8 +1227,8 @@ Message-ID: <foo.12345.author@example.com>
> >
> > Your Message-ID is `<foo.12345.author@example.com>`. This example will be used
> > below as well; make sure to replace it with the correct Message-ID for your
> > -**previous cover letter** - that is, if you're sending v2, use the Message-ID
> > -from v1; if you're sending v3, use the Message-ID from v2.
> > +**first cover letter** - that is, for any subsequent version that you send,
> > +always use the Message-ID from v1.
> >
> > While you're looking at the email, you should also note who is CC'd, as it's
> > common practice in the mailing list to keep all CCs on a thread. You can add
> >
> > --
> > 2.54.0.1064.gd145956f57.dirty
>
> If we adapt this change to the guidance, let's fix also other places of the
> document that talk about replying to the previous cover letter.
Good catch, thanks!
Patrick
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox