* [PATCH v2 0/2] Small updates to SubmittingPatches
From: Junio C Hamano @ 2026-06-02 14:43 UTC (permalink / raw)
To: git
In-Reply-To: <20260602090808.87837-1-gitster@pobox.com>
Recently I gave some advice on how a cover letter should
try to sell the idea to widest possible audience, and then
I realized that we do not seem to teach how in our guides.
Here is a small series to do so.
In this round, a few typos have been corrected, and improvements are
made thanks to help from Christian, Stolee, and Patrick.
1/2: SubmittingPatches: separate typofixes section
2/2: SubmittingPatches: describe cover letter
Documentation/SubmittingPatches | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
--
2.54.0-591-g9032776dcc
^ permalink raw reply
* Re: [PATCH 1/2] SubmittingPatches: separate typofixes section
From: Junio C Hamano @ 2026-06-02 14:28 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <CAP8UFD0ij4BTVTie1dXwTC8M_9gAvroXebFLmQuY7eUCgHrJhA@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> On Tue, Jun 2, 2026 at 11:13 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> The existing text said something about tests (with [[tests]] to make
>> it easier to refer to it from elsewhere) and then flowed into a
>> different topic of typofixes, but it was unclear where the latter
>> started. Add a similar [[typofies]] marker to the document.
>
> s/typofies/typofixes/
>
> Thanks.
Thanks. It is amusing to see I cannot say typofixes when I talk
about them ;-)
^ permalink raw reply
* Re: [PATCH 1/2] SubmittingPatches: separate typofixes section
From: Christian Couder @ 2026-06-02 14:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20260602090808.87837-2-gitster@pobox.com>
On Tue, Jun 2, 2026 at 11:13 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> The existing text said something about tests (with [[tests]] to make
> it easier to refer to it from elsewhere) and then flowed into a
> different topic of typofixes, but it was unclear where the latter
> started. Add a similar [[typofies]] marker to the document.
s/typofies/typofixes/
Thanks.
^ permalink raw reply
* Re: [PATCH v5 2/2] config: improve diagnostic for "set" with missing value
From: Junio C Hamano @ 2026-06-02 14:18 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Harald Nordgren
In-Reply-To: <e5a2070ee1598bc345556b4afd01ae6d40fab633.1780407557.git.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index 11fc976f3a..ed122d1100 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -469,6 +469,61 @@ test_expect_success 'invalid key' '
> test_must_fail git config inval.2key blabla
> '
>
> +test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
> + test_must_fail git config set pull.rebase=false 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
This is a syntax error of the command line, but the lhs of '=' makes
us suspect that the user may have meant to assign to that variable.
Makes perfect sense.
> +test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
> + test_must_fail git config pull.rebase=false 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
Ditto, the syntax may be an implicit "get" with bogus variable name,
or an implicit "set" with variable name and its value concatenated
into one argument with '='. The message seems to be assuming the
latter, which is OK to me.
> +test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
> + test_must_fail git config set foo=bar 2>err &&
> + test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
> + test_grep ! "did you mean" err
> +'
OK.
> +test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
> + test_must_fail git config set foo.1bar=baz 2>err &&
> + test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
> + test_grep ! "did you mean" err
> +'
OK. The above two should always give us the same error (except for
the actual bogus names given by the user to the command).
> +test_expect_success 'set with 1 arg: digit-led section name is valid' '
> + test_must_fail git config set 1foo.bar=baz 2>err &&
> + test_grep "missing value to set to the variable .1foo\\.bar=baz." err &&
> + test_grep "did you mean .git config set 1foo\\.bar baz." err
> +'
OK.
> +test_expect_success 'set with 1 arg: subsection plus invalid variable name' '
> + test_must_fail git config set foo.some.b_r=baz 2>err &&
> + test_grep "missing value to set to a variable with an invalid name .foo\\.some\\.b_r=baz." err &&
> + test_grep ! "did you mean" err
> +'
This is the third one that should be identical to earlier two that
gave a bogus variable name.
> +test_expect_success 'set with 1 arg of valid key reports missing value' '
> + test_must_fail git config set pull.rebase 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase." err &&
> + test_grep ! "did you mean" err
> +'
Did we see this already? No, this is different from the earlier one
that had "=false". This is a bog standard "you said set but did not
say what value to set to". Good.
> +test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
> + test_must_fail git config set pull.rebase=false true 2>err &&
> + test_grep ! "did you mean" err
> +'
OK. Do we want to see that the bogus variable name reported?
> +test_expect_success '"=" inside subsection is valid' '
> + test_when_finished "rm -f subsection.cfg" &&
> + git config set -f subsection.cfg foo.bar=baz.boo qux &&
> + echo qux >expect &&
> + git config get -f subsection.cfg foo.bar=baz.boo >actual &&
> + test_cmp expect actual
> +'
Excellent.
^ permalink raw reply
* Re: [PATCH v5 1/2] config: let git_config_parse_key() validate quietly
From: Junio C Hamano @ 2026-06-02 14:08 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Harald Nordgren
In-Reply-To: <d938ebf95a817c00a415670c08b839747d711d29.1780407557.git.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> Add a "quiet" parameter that suppresses the error() calls, and let
> store_key be NULL to skip the canonical-copy allocation. Existing
> callers pass 0 for quiet.
Hmph.
The way this patch did this may have been easier to implement, but
is a bit different from what I had in mind when I suggested to
"refactor" the existing logic.
Perhaps the updated "git_config_parse_key()" in this patch should be
renamed to be a file-scape static internal helper, and the existing
"git_config_parse_key()" should become a thin wrapper around that
new helper function, retaining the current external interface,
requiring no changes to existing callers.
Then in the next step, config.[ch] can add a new entry point that
serves the purpose of is_valid_key() in the previous iteration,
perhaps call it is_valid_git_config_key() or something like that
(Patrick or others may want to suggest a better word order in its
name). That way, we do not have to sprinkle many calls into
this (rather ugly) version of git_config_parse_key() with overly
wide interface that repeats meaningless NULL/0/1 parameters that no
callers want to use (other than for the purpose of differenciating
the real git_config_parse_key() calls from the new calls made to the
same function to ask "is this a valid key or not, yes/no?".
Thanks.
^ permalink raw reply
* Re: [PATCH v3] doc: fix typos via codespell
From: Junio C Hamano @ 2026-06-02 13:57 UTC (permalink / raw)
To: Andrew Kreimer; +Cc: git
In-Reply-To: <20260602111552.6084-1-algonell@gmail.com>
Andrew Kreimer <algonell@gmail.com> writes:
> There are some typos in the documentation, comments, etc.
> Fix them via codespell.
>
> Signed-off-by: Andrew Kreimer <algonell@gmail.com>
> ---
> v3:
> - Address test breaking changes (strings bounded by single quotes).
> - Thank you for your patience (extreme noise/gain ratio).
Thanks, but this is wrong.
>
> t/t1700-split-index.sh | 2 +-
> t/t3909-stash-pathspec-file.sh | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
[v3] should not be "on top of" [v2], but the above shows that
apparently this is vastly different from [v2], which had
Documentation/SubmittingPatches | 2 +-
Documentation/git-sparse-checkout.adoc | 2 +-
Documentation/technical/build-systems.adoc | 6 +++---
builtin/pack-objects.c | 2 +-
commit-graph.h | 2 +-
compat/precompose_utf8.c | 2 +-
hook.h | 2 +-
meson_options.txt | 2 +-
midx-write.c | 2 +-
odb/source.h | 2 +-
packfile.h | 2 +-
path.h | 2 +-
reftable/system.h | 2 +-
t/README | 2 +-
t/chainlint.pl | 2 +-
t/chainlint/chain-break-false.expect | 2 +-
t/chainlint/chain-break-false.test | 2 +-
t/t1700-split-index.sh | 2 +-
t/t3909-stash-pathspec-file.sh | 6 +++---
t/t4052-stat-output.sh | 2 +-
t/t4067-diff-partial-clone.sh | 2 +-
t/t9150/svk-merge.dump | 10 +++++-----
t/t9151/svn-mergeinfo.dump | 18 +++++++++---------
t/unit-tests/clar/README.md | 2 +-
24 files changed, 40 insertions(+), 40 deletions(-)
Until the topic is merged to 'next', a new iteration of patch(es)
should cleanly apply to the base that [v2] was meant to apply, but
should pretend as if [v2] never existed.
> diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
> index 869fb4a14e..887e72a5fa 100755
> --- a/t/t1700-split-index.sh
> +++ b/t/t1700-split-index.sh
> @@ -502,7 +502,7 @@ test_expect_success 'do not refresh null base index' '
> git checkout main &&
> git update-index --split-index &&
> test_commit more &&
> - # must not write a new shareindex, or we won't catch the problem
> + # must not write a new shareindex, or we will not catch the problem
The committed code never had "we won't" (what was in 'seen' does not
count), and this patch clearly shows that this is to fix-up the
breakage the previous round caused. We do not want that.
I'll squash the fix-up I already had into [v2] that I have queued,
which should be sufficient to get to the state this [v3] should have
been, I think.
Thanks.
^ permalink raw reply
* Re: [PATCH 2/2] builtin/init-db: deprecate alias for git-init(1)
From: Junio C Hamano @ 2026-06-02 13:50 UTC (permalink / raw)
To: Phillip Wood; +Cc: Patrick Steinhardt, phillip.wood, Kristoffer Haugsbakk, git
In-Reply-To: <336a4202-a55f-4223-b654-985d47233653@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
>> I was wondering whether we want to call `you_still_use_that()` here. I
>> found it to be a bit heavy-handed as it's so trivial to replace with
>> git-init(1), but on the other hand it's a trivial thing to do.
>
> I agree you_still_use_that() is too heavy handed, I was thinking of
> something like
>
> warning(_("this command is deprecated, please use \"git init\""
> "instead");
>
> but that would mean we need to add a separate cmd_init_db() function
> that prints the warning and then calls cmd_init().
If we do plan to remove it in the future, then something like that
may be needed.
But it is not like having "init-db" hidden but accessible in the
command table is hurting anything. Other than that those who want
to create their own
[alias "init-db"] command = foo
that is, and I'd see it a bit crazy.
The "init-db" form is hidden from "git help" listing, and we know
whenever we suggest to run "git init" we do not say "git init-db",
so if we do not have to remove it in the future, I do not think we
even need such a warning().
^ permalink raw reply
* Re: [PATCH 2/2] SubmittingPatches: describe cover letter
From: Junio C Hamano @ 2026-06-02 13:43 UTC (permalink / raw)
To: Derrick Stolee; +Cc: git
In-Reply-To: <fd588cff-be2b-4422-9c01-cef06b2ea5fd@gmail.com>
Derrick Stolee <stolee@gmail.com> writes:
>> +. Make sure your target audience can understand what the patches are
>> + about and why they are needed without prior context.
>
> The thing that I like to say about the cover letter is that this is
> your opportunity to communicate why the value of your change is worth
> the risk of regressions and the cost of maintenance. Perhaps:
>
> . Every code change comes with risk of regression and maintenance cost.
> The cover letter should clearly communicate why the value of your
> proposed change is worth applying. You can also describe how the risk
> is reduced by the design choices you made while writing the patches.
>
> Or something similar may be helpful? I may just be over explaining.
Yeah, it may be a bit on the heavy side, but complements what I
wanted to achieve with this update very well. I wanted to encourage
writing for wider audience, without leaving those "not in the know"
behind. What you wrote above is more about what to write, which is
very much appreciated. I think it fits well as the 0th item before
the three-bullet list.
>> +. For a second or subsequent iteration of the same topic, make sure
>> + people who missed the earlier discussion can still understand what
>> + the patches are about, so they can judge if the topic is worth their
>> + time to read and comment on.
>> +
>> +. To help those who are familiar with earlier iterations, give a
>> + summary of changes since the previous rounds.
>
> I find these updates to be particularly helpful, even for GitGitGadget
> PRs that include a range-diff automatically. It's good to double-check
> the human description of the update against the computed diff.
Oh, absolutely.
A GitGitGadget generated cover letter that lack any human input but
just range-diff dump is often very hard to read, and the receiving
end is better off pretending there was no useful information in the
cover letter. "git diff @{-1}..." after applying the patches to the
same base is sadly a lot easier to read than "git range-diff @{-1}..."
for many series.
^ permalink raw reply
* Re: [PATCH v11 0/6] branch: prune-merged
From: Harald Nordgren @ 2026-06-02 13:41 UTC (permalink / raw)
To: phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
Johannes Sixt
In-Reply-To: <9b44d867-219a-4ca3-b8ae-67fdac1c72f6@gmail.com>
> Hi Harald
>
> Just a quick note to say I've not forgotten about this, hopefully I
> should have time to review it later in the week now I'm back on the list.
>
> Thanks
>
> Phillip
Great to hear! Thanks!
Harald
^ permalink raw reply
* [PATCH v5 2/2] config: improve diagnostic for "set" with missing value
From: Harald Nordgren via GitGitGadget @ 2026-06-02 13:39 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2302.v5.git.git.1780407557.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
"git config set pull.rebase=false" currently fails with "wrong
number of arguments", and the implicit form "git config
pull.rebase=false" fails with "invalid key". Neither points at
the real problem: the value is missing.
Report that directly, and when the argument has the shape
"<valid-key>=<value>", also suggest the split form:
$ git config set pull.rebase=false
error: missing value to set to the variable 'pull.rebase=false'
hint: did you mean "git config set pull.rebase false"?
When the prefix before "=" is not a valid key, drop the hint:
$ git config set foo=bar
error: missing value to set to a variable with an invalid name 'foo=bar'
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/config.c | 32 ++++++++++++++++++++++++++-
t/t1300-config.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 86 insertions(+), 1 deletion(-)
diff --git a/builtin/config.c b/builtin/config.c
index b3188cd8d4..a2d46d0ce1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
#include "abspath.h"
+#include "advice.h"
#include "config.h"
#include "color.h"
#include "date.h"
@@ -210,6 +211,26 @@ static void check_argc(int argc, int min, int max)
exit(129);
}
+static NORETURN void die_missing_set_value(const char *arg)
+{
+ const char *last_dot = strrchr(arg, '.');
+ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
+ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
+
+ if (prefix && !git_config_parse_key(prefix, NULL, NULL, 1)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ advise(_("did you mean \"git config set %s %s\"?"),
+ prefix, eq + 1);
+ } else if (!git_config_parse_key(arg, NULL, NULL, 1)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ } else {
+ error(_("missing value to set to a variable with an invalid name '%s'"),
+ arg);
+ }
+ free(prefix);
+ exit(129);
+}
+
static void show_config_origin(const struct config_display_options *opts,
const struct key_value_info *kvi,
struct strbuf *buf)
@@ -1133,6 +1154,8 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
+ if (argc == 1)
+ die_missing_set_value(argv[0]);
check_argc(argc, 2, 2);
if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
@@ -1371,6 +1394,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
};
char *value = NULL, *comment = NULL;
int ret = 0;
+ int actions_implicit;
struct key_value_info default_kvi = KVI_INIT;
argc = parse_options(argc, argv, prefix, opts,
@@ -1385,7 +1409,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
exit(129);
}
- if (actions == 0)
+ actions_implicit = (actions == 0);
+ if (actions_implicit)
switch (argc) {
case 1: actions = ACTION_GET; break;
case 2: actions = ACTION_SET; break;
@@ -1394,6 +1419,11 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
error(_("no action specified"));
exit(129);
}
+ if (actions_implicit && argc == 1) {
+ const char *last_dot = strrchr(argv[0], '.');
+ if (last_dot && strchr(last_dot + 1, '='))
+ die_missing_set_value(argv[0]);
+ }
if (display_opts.omit_values &&
!(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
error(_("--name-only is only applicable to --list or --get-regexp"));
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 11fc976f3a..ed122d1100 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -469,6 +469,61 @@ test_expect_success 'invalid key' '
test_must_fail git config inval.2key blabla
'
+test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
+ test_must_fail git config set pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
+ test_must_fail git config pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
+ test_must_fail git config set foo=bar 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
+ test_must_fail git config set foo.1bar=baz 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg: digit-led section name is valid' '
+ test_must_fail git config set 1foo.bar=baz 2>err &&
+ test_grep "missing value to set to the variable .1foo\\.bar=baz." err &&
+ test_grep "did you mean .git config set 1foo\\.bar baz." err
+'
+
+test_expect_success 'set with 1 arg: subsection plus invalid variable name' '
+ test_must_fail git config set foo.some.b_r=baz 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo\\.some\\.b_r=baz." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg of valid key reports missing value' '
+ test_must_fail git config set pull.rebase 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
+ test_must_fail git config set pull.rebase=false true 2>err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success '"=" inside subsection is valid' '
+ test_when_finished "rm -f subsection.cfg" &&
+ git config set -f subsection.cfg foo.bar=baz.boo qux &&
+ echo qux >expect &&
+ git config get -f subsection.cfg foo.bar=baz.boo >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'correct key' '
git config 123456.a123 987
'
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 1/2] config: let git_config_parse_key() validate quietly
From: Harald Nordgren via GitGitGadget @ 2026-06-02 13:39 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2302.v5.git.git.1780407557.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Add a "quiet" parameter that suppresses the error() calls, and let
store_key be NULL to skip the canonical-copy allocation. Existing
callers pass 0 for quiet.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/config.c | 2 +-
config.c | 34 ++++++++++++++++++++++------------
config.h | 2 +-
submodule-config.c | 2 +-
4 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index cf4ba0f7cc..b3188cd8d4 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -555,7 +555,7 @@ static int get_value(const struct config_location_options *opts,
goto free_strings;
}
} else {
- if (git_config_parse_key(key_, &key, NULL)) {
+ if (git_config_parse_key(key_, &key, NULL, 0)) {
ret = CONFIG_INVALID_KEY;
goto free_strings;
}
diff --git a/config.c b/config.c
index a1b92fe083..81b31c5155 100644
--- a/config.c
+++ b/config.c
@@ -536,11 +536,14 @@ static inline int iskeychar(int c)
* -2 if there is no section name in the key.
*
* store_key - pointer to char* which will hold a copy of the key with
- * lowercase section and variable name
+ * lowercase section and variable name, can be NULL to skip
+ * allocation when only validation is needed
* baselen - pointer to size_t which will hold the length of the
* section + subsection part, can be NULL
+ * quiet - when non-zero, suppress error() reports on rejection
*/
-int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
+int git_config_parse_key(const char *key, char **store_key, size_t *baselen_,
+ int quiet)
{
size_t i, baselen;
int dot;
@@ -552,12 +555,14 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
*/
if (last_dot == NULL || last_dot == key) {
- error(_("key does not contain a section: %s"), key);
+ if (!quiet)
+ error(_("key does not contain a section: %s"), key);
return -CONFIG_NO_SECTION_OR_NAME;
}
if (!last_dot[1]) {
- error(_("key does not contain variable name: %s"), key);
+ if (!quiet)
+ error(_("key does not contain variable name: %s"), key);
return -CONFIG_NO_SECTION_OR_NAME;
}
@@ -568,7 +573,8 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
/*
* Validate the key and while at it, lower case it for matching.
*/
- *store_key = xmallocz(strlen(key));
+ if (store_key)
+ *store_key = xmallocz(strlen(key));
dot = 0;
for (i = 0; key[i]; i++) {
@@ -579,21 +585,25 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
if (!dot || i > baselen) {
if (!iskeychar(c) ||
(i == baselen + 1 && !isalpha(c))) {
- error(_("invalid key: %s"), key);
+ if (!quiet)
+ error(_("invalid key: %s"), key);
goto out_free_ret_1;
}
c = tolower(c);
} else if (c == '\n') {
- error(_("invalid key (newline): %s"), key);
+ if (!quiet)
+ error(_("invalid key (newline): %s"), key);
goto out_free_ret_1;
}
- (*store_key)[i] = c;
+ if (store_key)
+ (*store_key)[i] = c;
}
return 0;
out_free_ret_1:
- FREE_AND_NULL(*store_key);
+ if (store_key)
+ FREE_AND_NULL(*store_key);
return -CONFIG_INVALID_KEY;
}
@@ -609,7 +619,7 @@ static int config_parse_pair(const char *key, const char *value,
if (!strlen(key))
return error(_("empty config key"));
- if (git_config_parse_key(key, &canonical_name, NULL))
+ if (git_config_parse_key(key, &canonical_name, NULL, 0))
return -1;
ret = (fn(canonical_name, value, &ctx, data) < 0) ? -1 : 0;
@@ -1708,7 +1718,7 @@ static int configset_find_element(struct config_set *set, const char *key,
* `key` may come from the user, so normalize it before using it
* for querying entries from the hashmap.
*/
- ret = git_config_parse_key(key, &normalized_key, NULL);
+ ret = git_config_parse_key(key, &normalized_key, NULL, 0);
if (ret)
return ret;
@@ -3001,7 +3011,7 @@ int repo_config_set_multivar_in_file_gently(struct repository *r,
validate_comment_string(comment);
/* parse-key returns negative; flip the sign to feed exit(3) */
- ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
+ ret = 0 - git_config_parse_key(key, &store.key, &store.baselen, 0);
if (ret)
goto out_free;
diff --git a/config.h b/config.h
index bf47fb3afc..2c66d334c1 100644
--- a/config.h
+++ b/config.h
@@ -341,7 +341,7 @@ int repo_config_set_worktree_gently(struct repository *, const char *, const cha
*/
void repo_config_set(struct repository *, const char *, const char *);
-int git_config_parse_key(const char *, char **, size_t *);
+int git_config_parse_key(const char *, char **, size_t *, int quiet);
/*
* The following macros specify flag bits that alter the behavior
diff --git a/submodule-config.c b/submodule-config.c
index a81897b4e0..a319956f7a 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -970,7 +970,7 @@ int print_config_from_gitmodules(struct repository *repo, const char *key)
int ret;
char *store_key;
- ret = git_config_parse_key(key, &store_key, NULL);
+ ret = git_config_parse_key(key, &store_key, NULL, 0);
if (ret < 0)
return CONFIG_INVALID_KEY;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 0/2] config: suggest the correct form when key contains "="
From: Harald Nordgren via GitGitGadget @ 2026-06-02 13:39 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren
In-Reply-To: <pull.2302.v4.git.git.1779823288005.gitgitgadget@gmail.com>
* New commit config: let git_config_parse_key() validate quietly adds a
quiet parameter (and an optional store_key) so callers can validate
without writing to stderr.
* Validation in die_missing_set_value() now routes through
git_config_parse_key(key, NULL, NULL, 1) instead of the previous local
helper.
* Added tests for 1foo.bar=baz and foo.some.b_r=baz.
Harald Nordgren (2):
config: let git_config_parse_key() validate quietly
config: improve diagnostic for "set" with missing value
builtin/config.c | 34 ++++++++++++++++++++++++++--
config.c | 34 ++++++++++++++++++----------
config.h | 2 +-
submodule-config.c | 2 +-
t/t1300-config.sh | 55 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 111 insertions(+), 16 deletions(-)
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v5
Pull-Request: https://github.com/git/git/pull/2302
Range-diff vs v4:
-: ---------- > 1: d938ebf95a config: let git_config_parse_key() validate quietly
1: 780b99409c ! 2: e5a2070ee1 config: improve diagnostic for "set" with missing value
@@ builtin/config.c: static void check_argc(int argc, int min, int max)
exit(129);
}
-+static int is_valid_key(const char *key)
-+{
-+ const char *last_dot = strrchr(key, '.');
-+
-+ return last_dot && isalpha(last_dot[1]);
-+}
-+
+static NORETURN void die_missing_set_value(const char *arg)
+{
+ const char *last_dot = strrchr(arg, '.');
+ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
+ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
+
-+ if (prefix && is_valid_key(prefix)) {
++ if (prefix && !git_config_parse_key(prefix, NULL, NULL, 1)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ advise(_("did you mean \"git config set %s %s\"?"),
+ prefix, eq + 1);
-+ } else if (is_valid_key(arg)) {
++ } else if (!git_config_parse_key(arg, NULL, NULL, 1)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ } else {
+ error(_("missing value to set to a variable with an invalid name '%s'"),
@@ t/t1300-config.sh: test_expect_success 'invalid key' '
+ test_grep ! "did you mean" err
+'
+
++test_expect_success 'set with 1 arg: digit-led section name is valid' '
++ test_must_fail git config set 1foo.bar=baz 2>err &&
++ test_grep "missing value to set to the variable .1foo\\.bar=baz." err &&
++ test_grep "did you mean .git config set 1foo\\.bar baz." err
++'
++
++test_expect_success 'set with 1 arg: subsection plus invalid variable name' '
++ test_must_fail git config set foo.some.b_r=baz 2>err &&
++ test_grep "missing value to set to a variable with an invalid name .foo\\.some\\.b_r=baz." err &&
++ test_grep ! "did you mean" err
++'
++
+test_expect_success 'set with 1 arg of valid key reports missing value' '
+ test_must_fail git config set pull.rebase 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase." err &&
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH 2/2] SubmittingPatches: describe cover letter
From: Junio C Hamano @ 2026-06-02 13:36 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <ah7HZuy_WRCD9ZZ-@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> On Tue, Jun 02, 2026 at 06:08:08PM +0900, Junio C Hamano wrote:
>> We talk about how a commit log message should look like, but do not
>> give advice on writing the cover letter to sell a series to widest
>
> s/to widest/to the widest/?
Thanks.
>> +[[cover-letter]]
>> +=== Cover Letter
>> +
>> +The purpose of your cover letter is to sell your changes, explain what
>> +they are about, and get your target audience interested enough to read
>> +the patches.
>> +
>> +. Make sure your target audience can understand what the patches are
>> + about and why they are needed without prior context.
>> +
>> +. For a second or subsequent iteration of the same topic, make sure
>> + people who missed the earlier discussion can still understand what
>> + the patches are about, so they can judge if the topic is worth their
>> + time to read and comment on.
>> +
>> +. To help those who are familiar with earlier iterations, give a
>> + summary of changes since the previous rounds.
>
> We might also recommend to include a range-diff in subsequent
> iterations. That being said though, I just sent a small series to the
> mailing list that recommends using b4, and there it get this for free.
> So no idea whether it's still worth it to then cover this here
> explicitly.
I think these are orthogonal. What b4 helps you with is the shape
of the letter, how it looks like. This update is about the contents
in the letter, what you convey to your readers.
Of course, "format-patch --cover-letter" also lets you do range-diff
or interdiff, so they come for free. But the above description is
not tied to any particular tool to prepare your cover letter.
^ permalink raw reply
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Junio C Hamano @ 2026-06-02 13:32 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <20260602-pks-b4-v1-1-a7ae5a49e9cf@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> We're about to extend our documentation to recommend b4 for sending
> patch series ot 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.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> .b4-config | 3 +++
> .b4-cover-template | 11 +++++++++++
> 2 files changed, 14 insertions(+)
Shipping a sample like ".b4-config.sample" that users who opt-in can
copy-and-edit into the final name ".b4-config" is OK, but I'd rather
not to ship the configuration files that the users would want to edit
(hence making the tree dirty).
^ permalink raw reply
* Re: [PATCH 3/4] t/lib-git-p4: silence output when killing p4d and its watchdog
From: Junio C Hamano @ 2026-06-02 13:16 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <ah6uZ6tdIh38X2uZ@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> On Tue, Jun 02, 2026 at 06:32:55PM +0900, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>>
>> > stop_p4d_and_watchdog () {
>> > kill -9 $p4d_pid $watchdog_pid
>> > + wait $p4d $watchdog_pid 2>/dev/null
>> > }
>>
>> Shoudln't we be waiting on $p4d_pid (not $p4d)...
>>
>> > @@ -175,7 +176,7 @@ retry_until_success () {
>> >
>> > stop_and_cleanup_p4d () {
>> > kill -9 $p4d_pid $watchdog_pid
>> > - wait $p4d_pid
>> > + wait $p4d_pid $watchdog_pid 2>/dev/null
>> > rm -rf "$db" "$cli" "$pidfile"
>> > }
>>
>> ... like we do here?
>
> Oh, good catch. The statement basically doesn't do anything, which isn't
> much of a problem because we really only care about silencing the error
> message when the watchdog is being terminated. Will fix.
Thanks. Another thing I noticed is that they look suspiciously
similar.
^ permalink raw reply
* Re: [PATCH 2/2] builtin/init-db: deprecate alias for git-init(1)
From: Phillip Wood @ 2026-06-02 13:12 UTC (permalink / raw)
To: Patrick Steinhardt, Junio C Hamano
Cc: Kristoffer Haugsbakk, Phillip Wood, git
In-Reply-To: <ah7N5bKAiAORtNkp@pks.im>
On 02/06/2026 13:34, Patrick Steinhardt wrote:
>
> That's entirely fair. My take on this is a bit different, as I think
> it's beneficial to accept a short-term adjustment for core contributors
> in favor of making stuff easier to discover/maintain going forward.
> > A new contributor would probably be quick to learn that every
> `cmd_foo()` entry point is named exactly the same as the subcommand
> name, but they will then eventually trip over the few exceptions like
> `cmd_init_db()` where that assumption doesn't hold.
Yes, those exceptions to the rule are annoying. Though they mostly exist
for a good reason (code sharing between builtin commands), it would be
nice to minimize them where we can.
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH 2/2] builtin/init-db: deprecate alias for git-init(1)
From: Phillip Wood @ 2026-06-02 13:09 UTC (permalink / raw)
To: Patrick Steinhardt, phillip.wood; +Cc: Kristoffer Haugsbakk, git
In-Reply-To: <ah2VL-ftCQelNoOc@pks.im>
Hi Patrick
On 01/06/2026 15:20, Patrick Steinhardt wrote:
> On Mon, Jun 01, 2026 at 02:48:05PM +0100, Phillip Wood wrote:
>>
>>
>> On 01/06/2026 13:10, Patrick Steinhardt wrote:
>>> On Mon, Jun 01, 2026 at 11:31:46AM +0200, Kristoffer Haugsbakk wrote:
>>>> On Mon, Jun 1, 2026, at 09:56, Patrick Steinhardt wrote:
>>>>> diff --git a/git.c b/git.c
>>>>> index a72394b599..6bf6a60360 100644
>>>>> --- a/git.c
>>>>> +++ b/git.c
>>>>> @@ -591,7 +591,9 @@ static struct cmd_struct commands[] = {
>>>>> { "hook", cmd_hook, RUN_SETUP_GENTLY },
>>>>> { "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT },
>>>>> { "init", cmd_init },
>>>>> +#ifndef WITH_BREAKING_CHANGES
>>>>> { "init-db", cmd_init },
>>>>
>>>> This can be marked as deprecated.
>>>>
>>>> { "init-db", cmd_init, DEPRECATED },
>>>
>>> Ah, indeed! Added locally now, thanks.
>>
>> Deprecating this command seems very sensible to me. As well as marking it
>> deprecated, do we want to print a warning when it is run? I imagine anyone
>> who has this command in their muscle memory is unlikely to be reading the
>> man page on a regular basis so wont see the warning there.
>
> I was wondering whether we want to call `you_still_use_that()` here. I
> found it to be a bit heavy-handed as it's so trivial to replace with
> git-init(1), but on the other hand it's a trivial thing to do.
I agree you_still_use_that() is too heavy handed, I was thinking of
something like
warning(_("this command is deprecated, please use \"git init\""
"instead");
but that would mean we need to add a separate cmd_init_db() function
that prints the warning and then calls cmd_init().
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH v11 0/6] branch: prune-merged
From: Phillip Wood @ 2026-06-02 13:05 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Kristoffer Haugsbakk, Johannes Sixt, Harald Nordgren
In-Reply-To: <pull.2285.v11.git.git.1779449498.gitgitgadget@gmail.com>
Hi Harald
Just a quick note to say I've not forgotten about this, hopefully I
should have time to review it later in the week now I'm back on the list.
Thanks
Phillip
On 22/05/2026 12:31, Harald Nordgren via GitGitGadget wrote:
> After releasing v10, I hard-reset back to v9 and reworked the series from
> there.
>
> * The flags now take a branch, not a remote. --forked and --prune-merged
> accept a literal upstream short name like origin/main or a wildmatch
> pattern like origin/*. The old --all-remotes flag is gone, since origin/*
> covers that case.
> * The prune guard now compares @{push} against @{upstream}. A branch is
> spared when these are equal. That is the trunk like case, such as local
> main tracking and pushing to origin/main, where "fully merged to
> upstream" cannot be told apart from "just pulled". Only branches that
> push somewhere other than their upstream, typically fork based topics,
> are candidates. The earlier <remote>/HEAD by name guard that the reviewer
> rejected is gone.
> * New --dry-run for --prune-merged.
>
> Harald Nordgren (6):
> branch: add --forked <branch>
> branch: let delete_branches warn instead of error on bulk refusal
> branch: prepare delete_branches for a bulk caller
> branch: add --prune-merged <branch>
> branch: add branch.<name>.pruneMerged opt-out
> branch: add --dry-run for --prune-merged
>
> Documentation/config/branch.adoc | 7 +
> Documentation/git-branch.adoc | 42 ++++
> builtin/branch.c | 303 +++++++++++++++++++++++++--
> t/t3200-branch.sh | 347 +++++++++++++++++++++++++++++++
> 4 files changed, 682 insertions(+), 17 deletions(-)
>
>
> base-commit: aec3f587505a472db67e9462d0702e7d463a449d
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2285%2FHaraldNordgren%2Ffetch-prune-local-branches-v11
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2285/HaraldNordgren/fetch-prune-local-branches-v11
> Pull-Request: https://github.com/git/git/pull/2285
>
> Range-diff vs v10:
>
> 1: f2df159830 ! 1: b9fddd124a branch: add --forked <branch>
> @@ Metadata
> ## Commit message ##
> branch: add --forked <branch>
>
> - git branch --forked <branch>...
> + List local branches whose configured upstream
> + (branch.<name>.merge resolved against branch.<name>.remote)
> + matches any of the given <branch> arguments.
>
> - lists local branches whose configured upstream matches any
> - of the given <branch> arguments.
> + Each <branch> is interpreted against the local repository, not
> + against any specific remote:
>
> - Each <branch> is resolved to the same kind of ref that
> - branch.<name>.remote and branch.<name>.merge together point at:
> - a remote-tracking branch (e.g. origin/master), or, for branches
> - tracking a local upstream, a local branch (e.g. master).
> - Shell-style globs are also accepted (e.g. 'origin/*'). Multiple
> - arguments are unioned.
> + * a literal upstream short name, e.g. "origin/main" or "master"
> + for a branch whose upstream is local;
> + * a wildmatch pattern, e.g. "origin/*";
> + * a bare configured-remote name, e.g. "origin", which resolves
> + to whatever refs/remotes/origin/HEAD points at, matching how
> + "git checkout -b topic origin" picks a starting point.
>
> - This is the building block for --prune-merged.
> + The literal-vs-wildcard distinction is settled at parse time so
> + the per-branch matching loop calls wildmatch() only for genuine
> + wildcards. Multiple <branch> arguments are unioned. Output is
> + sorted by branch name.
> +
> + This is the building block for --prune-merged, which deletes the
> + listed branches once they have landed on their upstream.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>
> @@ Documentation/git-branch.adoc: This option is only applicable in non-verbose mod
> nothing is printed.
>
> +`--forked`::
> -+ List local branches whose configured upstream matches any
> -+ of the given _<branch>_ arguments. Each argument is either
> -+ a ref (e.g. `origin/master`, `master`) or a shell-style
> -+ glob (e.g. `'origin/*'`). Multiple arguments are unioned.
> ++ List local branches whose configured upstream
> ++ (`branch.<name>.merge` resolved against `branch.<name>.remote`)
> ++ matches any of the given _<branch>_ arguments.
> +++
> ++Each _<branch>_ is interpreted against the local repository: a literal
> ++upstream like `origin/main` or a local branch like `master`, or a
> ++wildmatch pattern like `'origin/*'`. A bare configured-remote name
> ++(e.g. `origin`) resolves to the target of `refs/remotes/<remote>/HEAD`,
> ++to match the way `git checkout -b topic origin` picks a starting
> ++point. Multiple _<branch>_ arguments are unioned.
> +
> `-v`::
> `-vv`::
> @@ builtin/branch.c: static const char * const builtin_branch_usage[] = {
> NULL
> };
>
> -@@ builtin/branch.c: static int branch_merged(int kind, const char *name,
> -
> - static int check_branch_commit(const char *branchname, const char *refname,
> - const struct object_id *oid, struct commit *head_rev,
> -- int kinds, int force)
> -+ int kinds, int force, int warn_only,
> -+ int *n_not_merged)
> - {
> - struct commit *rev = lookup_commit_reference(the_repository, oid);
> - if (!force && !rev) {
> -@@ builtin/branch.c: static int check_branch_commit(const char *branchname, const char *refname,
> - return -1;
> - }
> - if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
> -- error(_("the branch '%s' is not fully merged"), branchname);
> -- advise_if_enabled(ADVICE_FORCE_DELETE_BRANCH,
> -- _("If you are sure you want to delete it, "
> -- "run 'git branch -D %s'"), branchname);
> -+ if (warn_only) {
> -+ warning(_("the branch '%s' is not fully merged"),
> -+ branchname);
> -+ } else {
> -+ error(_("the branch '%s' is not fully merged"),
> -+ branchname);
> -+ advise_if_enabled(ADVICE_FORCE_DELETE_BRANCH,
> -+ _("If you are sure you want to delete it, "
> -+ "run 'git branch -D %s'"), branchname);
> -+ }
> -+ if (n_not_merged)
> -+ (*n_not_merged)++;
> - return -1;
> - }
> - return 0;
> -@@ builtin/branch.c: static void delete_branch_config(const char *branchname)
> - }
> -
> - static int delete_branches(int argc, const char **argv, int force, int kinds,
> -- int quiet)
> -+ int quiet, int warn_only, int *n_not_merged)
> - {
> - struct commit *head_rev = NULL;
> - struct object_id oid;
> -@@ builtin/branch.c: static int delete_branches(int argc, const char **argv, int force, int kinds,
> -
> - if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
> - check_branch_commit(bname.buf, name, &oid, head_rev, kinds,
> -- force)) {
> -- ret = 1;
> -+ force, warn_only, n_not_merged)) {
> -+ if (!warn_only)
> -+ ret = 1;
> - goto next;
> - }
> -
> @@ builtin/branch.c: static void copy_or_rename_branch(const char *oldname, const char *newname, int
> free_worktrees(worktrees);
> }
>
> ++struct upstream_pattern {
> ++ char *name;
> ++ int is_wildcard;
> ++};
> ++
> ++static void upstream_pattern_list_clear(struct upstream_pattern *items,
> ++ size_t nr)
> ++{
> ++ size_t i;
> ++ for (i = 0; i < nr; i++)
> ++ free(items[i].name);
> ++ free(items);
> ++}
> ++
> ++static const char *short_upstream_name(const char *full_ref)
> ++{
> ++ const char *short_name = full_ref;
> ++ (void)(skip_prefix(short_name, "refs/heads/", &short_name) ||
> ++ skip_prefix(short_name, "refs/remotes/", &short_name));
> ++ return short_name;
> ++}
> ++
> ++static int parse_one_forked_arg(const char *arg, struct upstream_pattern *out)
> ++{
> ++ struct ref_store *refs = get_main_ref_store(the_repository);
> ++ struct remote *remote;
> ++ struct object_id oid;
> ++ char *full_ref = NULL;
> ++ struct strbuf head_ref = STRBUF_INIT;
> ++ const char *resolved;
> ++
> ++ if (has_glob_specials(arg)) {
> ++ out->name = xstrdup(arg);
> ++ out->is_wildcard = 1;
> ++ return 0;
> ++ }
> ++
> ++ remote = remote_get(arg);
> ++ if (remote && remote_is_configured(remote, 0)) {
> ++ strbuf_addf(&head_ref, "refs/remotes/%s/HEAD", remote->name);
> ++ resolved = refs_resolve_ref_unsafe(refs, head_ref.buf,
> ++ RESOLVE_REF_NO_RECURSE,
> ++ NULL, NULL);
> ++ if (resolved && starts_with(resolved, "refs/remotes/")) {
> ++ out->name = xstrdup(short_upstream_name(resolved));
> ++ out->is_wildcard = 0;
> ++ strbuf_release(&head_ref);
> ++ return 0;
> ++ }
> ++ strbuf_release(&head_ref);
> ++ }
> ++
> ++ if (repo_dwim_ref(the_repository, arg, strlen(arg), &oid,
> ++ &full_ref, 0) == 1 &&
> ++ (starts_with(full_ref, "refs/heads/") ||
> ++ starts_with(full_ref, "refs/remotes/"))) {
> ++ out->name = xstrdup(short_upstream_name(full_ref));
> ++ out->is_wildcard = 0;
> ++ free(full_ref);
> ++ return 0;
> ++ }
> ++ free(full_ref);
> ++ return -1;
> ++}
> ++
> +static void parse_forked_args(int argc, const char **argv,
> -+ struct string_list *upstream_patterns)
> ++ struct upstream_pattern **patterns_out,
> ++ size_t *nr_out)
> +{
> ++ struct upstream_pattern *patterns;
> + int i;
> +
> ++ ALLOC_ARRAY(patterns, argc);
> + for (i = 0; i < argc; i++) {
> -+ const char *arg = argv[i];
> -+ struct object_id oid;
> -+ char *full_ref = NULL;
> -+ const char *short_ref;
> -+
> -+ if (has_glob_specials(arg)) {
> -+ string_list_insert(upstream_patterns, arg);
> -+ continue;
> ++ if (parse_one_forked_arg(argv[i], &patterns[i]) < 0) {
> ++ upstream_pattern_list_clear(patterns, i);
> ++ die(_("'%s' is not a valid branch or pattern"),
> ++ argv[i]);
> + }
> ++ }
> ++ *patterns_out = patterns;
> ++ *nr_out = argc;
> ++}
> +
> -+ if (repo_dwim_ref(the_repository, arg, strlen(arg), &oid,
> -+ &full_ref, 0) == 1 &&
> -+ (skip_prefix(full_ref, "refs/heads/", &short_ref) ||
> -+ skip_prefix(full_ref, "refs/remotes/", &short_ref))) {
> -+ string_list_insert(upstream_patterns, short_ref);
> -+ free(full_ref);
> -+ continue;
> -+ }
> -+ free(full_ref);
> ++static int upstream_matches(const char *short_upstream,
> ++ const struct upstream_pattern *patterns,
> ++ size_t nr)
> ++{
> ++ size_t i;
> +
> -+ die(_("'%s' is not a valid branch or pattern"), arg);
> ++ for (i = 0; i < nr; i++) {
> ++ const struct upstream_pattern *p = &patterns[i];
> ++ if (p->is_wildcard) {
> ++ if (!wildmatch(p->name, short_upstream, WM_PATHNAME))
> ++ return 1;
> ++ } else if (!strcmp(p->name, short_upstream)) {
> ++ return 1;
> ++ }
> + }
> ++ return 0;
> +}
> +
> +struct forked_cb {
> -+ const struct string_list *upstream_patterns;
> ++ const struct upstream_pattern *patterns;
> ++ size_t nr_patterns;
> + struct string_list *out;
> +};
> +
> @@ builtin/branch.c: static void copy_or_rename_branch(const char *oldname, const c
> +{
> + struct forked_cb *cb = cb_data;
> + struct branch *branch;
> -+ const char *upstream, *short_upstream;
> -+ const struct string_list_item *item;
> ++ const char *upstream;
> +
> + if (ref->flags & REF_ISSYMREF)
> + return 0;
> @@ builtin/branch.c: static void copy_or_rename_branch(const char *oldname, const c
> + upstream = branch_get_upstream(branch, NULL);
> + if (!upstream)
> + return 0;
> -+ short_upstream = upstream;
> -+ (void)(skip_prefix(short_upstream, "refs/heads/", &short_upstream) ||
> -+ skip_prefix(short_upstream, "refs/remotes/", &short_upstream));
> -+
> -+ for_each_string_list_item(item, cb->upstream_patterns)
> -+ if (!wildmatch(item->string, short_upstream, WM_PATHNAME)) {
> -+ string_list_append(cb->out, ref->name)->util =
> -+ xstrdup(upstream);
> -+ return 0;
> -+ }
> ++ if (upstream_matches(short_upstream_name(upstream),
> ++ cb->patterns, cb->nr_patterns))
> ++ string_list_append(cb->out, ref->name);
> + return 0;
> +}
> +
> -+static void collect_forked_set(int argc, const char **argv,
> -+ struct string_list *out)
> -+{
> -+ struct string_list upstream_patterns = STRING_LIST_INIT_DUP;
> -+ struct forked_cb cb = {
> -+ .upstream_patterns = &upstream_patterns,
> -+ .out = out,
> -+ };
> -+
> -+ parse_forked_args(argc, argv, &upstream_patterns);
> -+
> -+ refs_for_each_branch_ref(get_main_ref_store(the_repository),
> -+ collect_forked_branch, &cb);
> -+
> -+ string_list_clear(&upstream_patterns, 0);
> -+}
> -+
> +static int list_forked_branches(int argc, const char **argv)
> +{
> ++ struct upstream_pattern *patterns = NULL;
> ++ size_t nr_patterns = 0;
> + struct string_list out = STRING_LIST_INIT_DUP;
> + struct string_list_item *item;
> ++ struct forked_cb cb;
> +
> + if (!argc)
> + die(_("--forked requires at least one <branch>"));
> +
> -+ collect_forked_set(argc, argv, &out);
> ++ parse_forked_args(argc, argv, &patterns, &nr_patterns);
> ++ cb.patterns = patterns;
> ++ cb.nr_patterns = nr_patterns;
> ++ cb.out = &out;
> ++
> ++ refs_for_each_branch_ref(get_main_ref_store(the_repository),
> ++ collect_forked_branch, &cb);
> ++
> ++ string_list_sort(&out);
> + for_each_string_list_item(item, &out)
> + puts(item->string);
> +
> -+ string_list_clear(&out, 1);
> ++ upstream_pattern_list_clear(patterns, nr_patterns);
> ++ string_list_clear(&out, 0);
> + return 0;
> +}
> +
> @@ builtin/branch.c: int cmd_branch(int argc,
> usage_with_options(builtin_branch_usage, options);
>
> @@ builtin/branch.c: int cmd_branch(int argc,
> - if (delete) {
> - if (!argc)
> die(_("branch name required"));
> -- ret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);
> -+ ret = delete_branches(argc, argv, delete > 1, filter.kind,
> -+ quiet, 0, NULL);
> -+ goto out;
> + ret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);
> + goto out;
> + } else if (forked) {
> + ret = list_forked_branches(argc, argv);
> - goto out;
> ++ goto out;
> } else if (show_current) {
> print_current_branch_name();
> + ret = 0;
>
> ## t/t3200-branch.sh ##
> @@ t/t3200-branch.sh: test_expect_success 'errors if given a bad branch name' '
> @@ t/t3200-branch.sh: test_expect_success 'errors if given a bad branch name' '
> + git clone forked-upstream forked &&
> + git -C forked remote add other ../forked-other &&
> + git -C forked fetch other &&
> ++ git -C forked branch local-base &&
> + git -C forked branch --track local-one origin/one &&
> + git -C forked branch --track local-two origin/two &&
> + git -C forked branch --track local-foreign other/foreign &&
> + git -C forked branch detached &&
> -+ git -C forked branch --track topic-on-main main
> ++ git -C forked branch --track local-trunk local-base
> +'
> +
> -+test_expect_success '--forked <remote-tracking-branch> lists matching branches' '
> ++test_expect_success '--forked <upstream-tracking-branch> lists matching branches' '
> + git -C forked branch --forked origin/one >actual &&
> + echo local-one >expect &&
> + test_cmp expect actual
> +'
> +
> -+test_expect_success '--forked <local-branch> lists branches tracking that local branch' '
> -+ git -C forked branch --forked main >actual &&
> -+ echo topic-on-main >expect &&
> -+ test_cmp expect actual
> -+'
> -+
> -+test_expect_success '--forked <glob> matches every upstream under the pattern' '
> ++test_expect_success '--forked <glob> matches by wildmatch' '
> + git -C forked branch --forked "origin/*" >actual &&
> + cat >expect <<-\EOF &&
> + local-one
> @@ t/t3200-branch.sh: test_expect_success 'errors if given a bad branch name' '
> + test_cmp expect actual
> +'
> +
> ++test_expect_success '--forked <local-branch> matches branches with local upstream' '
> ++ git -C forked branch --forked local-base >actual &&
> ++ echo local-trunk >expect &&
> ++ test_cmp expect actual
> ++'
> ++
> ++test_expect_success '--forked <remote> resolves via refs/remotes/<remote>/HEAD' '
> ++ test_when_finished "git -C forked symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/main" &&
> ++ git -C forked symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/one &&
> ++ git -C forked branch --forked origin >actual &&
> ++ echo local-one >expect &&
> ++ test_cmp expect actual
> ++'
> ++
> +test_expect_success '--forked unions multiple <branch> arguments' '
> + git -C forked branch --forked origin/one other/foreign >actual &&
> + cat >expect <<-\EOF &&
> @@ t/t3200-branch.sh: test_expect_success 'errors if given a bad branch name' '
> +'
> +
> +test_expect_success '--forked combines literal and glob arguments' '
> -+ git -C forked branch --forked main "other/*" >actual &&
> ++ git -C forked branch --forked local-base "other/*" >actual &&
> + cat >expect <<-\EOF &&
> + local-foreign
> -+ topic-on-main
> ++ local-trunk
> + EOF
> + test_cmp expect actual
> +'
> -: ---------- > 2: b666d09bf5 branch: let delete_branches warn instead of error on bulk refusal
> -: ---------- > 3: 6e6580270e branch: prepare delete_branches for a bulk caller
> 2: 718e28c7e0 ! 4: e7e03c1338 branch: add --prune-merged <branch>
> @@ Commit message
>
> git branch --prune-merged <branch>...
>
> - deletes the local branches that --forked <branch> would list,
> - but only those whose tip is reachable from their configured
> - upstream: the work has already landed on the upstream the
> - branch tracks, so the local copy is no longer needed.
> + deletes the local branches that "--forked <branch>" would list,
> + restricted to those whose tip is reachable from their configured
> + upstream: the work has already landed on the upstream they track,
> + so the local copy is no longer needed.
>
> - The following branches are always preserved:
> + Reachability is read from the local refs only -- nothing is
> + fetched. Users who want fresh upstream refs run "git fetch" first;
> + the deletion path stays a separate, idempotent step that also
> + works offline.
>
> - * the currently checked-out branch in any worktree;
> - * any local branch whose name matches the default branch of
> - any configured remote (the target of
> - refs/remotes/<remote>/HEAD) -- typically 'main' or
> - 'master';
> - * any branch whose upstream no longer resolves locally.
> + Three classes of branches are spared:
>
> - Reachability is read from whatever branch.<name>.merge
> - resolves to locally, which is usually a remote-tracking ref
> - but may also be a local branch. When the upstream is a
> - remote-tracking ref, the natural workflow is
> + * any branch checked out in any worktree;
> + * any branch whose upstream no longer resolves locally (its
> + disappearance is not, on its own, evidence of integration);
> + * any branch whose push destination equals its upstream
> + (<branch>@{push} == <branch>@{upstream}). Such a branch
> + cannot be distinguished from a freshly pulled trunk that
> + just looks "fully merged" -- e.g. local "main" tracking and
> + pushing to "origin/main" right after a pull. Only branches
> + that push somewhere other than their upstream (typically
> + topics in a fork-based workflow) are treated as candidates.
>
> - git fetch <remote>
> - git branch --prune-merged <upstream-pattern>
> -
> - so the upstream reflects the current state before pruning.
> + Deletion goes through the existing delete_branches() in warn-only
> + mode and with the HEAD-fallback disabled: a branch that is not
> + yet fully merged to its upstream is reported as a one-line warning
> + and skipped, so a single un-mergeable topic does not abort the
> + whole sweep, and there is no fallback to "merged into the
> + currently checked out branch" -- we only act on upstream-merged
> + status.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>
> @@ Documentation/git-branch.adoc: git branch (-c|-C) [<old-branch>] <new-branch>
>
> DESCRIPTION
> -----------
> -@@ Documentation/git-branch.adoc: This option is only applicable in non-verbose mode.
> - a ref (e.g. `origin/master`, `master`) or a shell-style
> - glob (e.g. `'origin/*'`). Multiple arguments are unioned.
> +@@ Documentation/git-branch.adoc: wildmatch pattern like `'origin/*'`. A bare configured-remote name
> + to match the way `git checkout -b topic origin` picks a starting
> + point. Multiple _<branch>_ arguments are unioned.
>
> +`--prune-merged`::
> -+ Delete the local branches that `--forked` would list for
> -+ the same _<branch>_ arguments, but only those whose tip is
> -+ reachable from their configured upstream.
> ++ Delete the local branches that `--forked` would list for the
> ++ same _<branch>_ arguments, but only those whose tip is
> ++ reachable from their configured upstream. In other words,
> ++ the work on the branch has already landed on the upstream it
> ++ tracks, so the local copy is no longer needed.
> ++
> -+For arguments that refer to remote-tracking branches, run
> -+`git fetch` first so reachability is checked against the
> -+current upstream state; refs are read locally.
> ++Reachability is checked against whatever the upstream refs say
> ++locally; nothing is fetched. Run `git fetch` first if you want
> ++the upstream refs refreshed.
> ++
> -+The following branches are always preserved:
> ++A branch is left alone if any of the following holds:
> ++its upstream no longer resolves locally; it is checked out in any
> ++worktree; or its push destination (`<branch>@{push}`) equals its
> ++upstream (`<branch>@{upstream}`), so it cannot be distinguished
> ++from a freshly pulled trunk that just looks "fully merged".
> ++
> -+--
> -+* the currently checked-out branch in any worktree;
> -+* any local branch whose name matches the default branch of
> -+ any configured remote (the target of
> -+ `refs/remotes/<remote>/HEAD`) -- typically `main` or
> -+ `master`;
> -+* any branch whose upstream no longer resolves locally.
> -+--
> ++Branches refused by the "fully merged" safety check are listed as
> ++warnings and skipped; pass them to `git branch -D` explicitly if
> ++you want them gone.
> +
> `-v`::
> `-vv`::
> `--verbose`::
>
> ## builtin/branch.c ##
> -@@
> - #include "branch.h"
> - #include "path.h"
> - #include "string-list.h"
> -+#include "strvec.h"
> - #include "column.h"
> - #include "utf8.h"
> - #include "ref-filter.h"
> -@@ builtin/branch.c: static const char * const builtin_branch_usage[] = {
> - N_("git branch [<options>] [-r | -a] [--points-at]"),
> - N_("git branch [<options>] [-r | -a] [--format]"),
> - N_("git branch [<options>] --forked <branch>..."),
> -+ N_("git branch [<options>] --prune-merged <branch>..."),
> - NULL
> - };
> -
> -@@ builtin/branch.c: static int branch_merged(int kind, const char *name,
> - * any of the following code, but during the transition period,
> - * a gentle reminder is in order.
> - */
> -- if (head_rev != reference_rev) {
> -- int expect = head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0;
> -+ if (head_rev && head_rev != reference_rev) {
> -+ int expect = repo_in_merge_bases(the_repository, rev, head_rev);
> - if (expect < 0)
> - exit(128);
> - if (expect == merged)
> @@ builtin/branch.c: static int collect_forked_branch(const struct reference *ref, void *cb_data)
> return 0;
> }
>
> -+static int collect_default_branch_name(struct remote *remote, void *cb_data)
> -+{
> -+ struct string_list *protected = cb_data;
> -+ struct ref_store *refs = get_main_ref_store(the_repository);
> -+ struct strbuf head = STRBUF_INIT;
> -+ const char *target;
> -+
> -+ strbuf_addf(&head, "refs/remotes/%s/HEAD", remote->name);
> -+ target = refs_resolve_ref_unsafe(refs, head.buf,
> -+ RESOLVE_REF_NO_RECURSE, NULL, NULL);
> -+ if (target) {
> -+ const char *leaf = strrchr(target, '/');
> -+ if (leaf)
> -+ string_list_insert(protected, leaf + 1);
> -+ }
> -+ strbuf_release(&head);
> -+ return 0;
> +-static int list_forked_branches(int argc, const char **argv)
> ++static void collect_forked_set(int argc, const char **argv,
> ++ struct string_list *out)
> + {
> + struct upstream_pattern *patterns = NULL;
> + size_t nr_patterns = 0;
> +- struct string_list out = STRING_LIST_INIT_DUP;
> +- struct string_list_item *item;
> + struct forked_cb cb;
> +
> +- if (!argc)
> +- die(_("--forked requires at least one <branch>"));
> +-
> + parse_forked_args(argc, argv, &patterns, &nr_patterns);
> + cb.patterns = patterns;
> + cb.nr_patterns = nr_patterns;
> +- cb.out = &out;
> ++ cb.out = out;
> +
> + refs_for_each_branch_ref(get_main_ref_store(the_repository),
> + collect_forked_branch, &cb);
> +
> +- string_list_sort(&out);
> ++ string_list_sort(out);
> ++
> ++ upstream_pattern_list_clear(patterns, nr_patterns);
> +}
> +
> - static void collect_forked_set(int argc, const char **argv,
> - struct string_list *out)
> - {
> -@@ builtin/branch.c: static int list_forked_branches(int argc, const char **argv)
> ++static int list_forked_branches(int argc, const char **argv)
> ++{
> ++ struct string_list out = STRING_LIST_INIT_DUP;
> ++ struct string_list_item *item;
> ++
> ++ if (!argc)
> ++ die(_("--forked requires at least one <branch>"));
> ++
> ++ collect_forked_set(argc, argv, &out);
> + for_each_string_list_item(item, &out)
> + puts(item->string);
> +
> +- upstream_pattern_list_clear(patterns, nr_patterns);
> + string_list_clear(&out, 0);
> return 0;
> }
>
> @@ builtin/branch.c: static int list_forked_branches(int argc, const char **argv)
> +{
> + struct ref_store *refs = get_main_ref_store(the_repository);
> + struct string_list candidates = STRING_LIST_INIT_DUP;
> -+ struct string_list protected_default_names = STRING_LIST_INIT_DUP;
> + struct strvec deletable = STRVEC_INIT;
> -+ struct strbuf buf = STRBUF_INIT;
> + struct string_list_item *item;
> -+ int n_not_merged = 0;
> + int ret = 0;
> +
> + if (!argc)
> + die(_("--prune-merged requires at least one <branch>"));
> +
> + collect_forked_set(argc, argv, &candidates);
> -+ for_each_remote(collect_default_branch_name, &protected_default_names);
> +
> + for_each_string_list_item(item, &candidates) {
> + const char *short_name = item->string;
> -+ const char *upstream = item->util;
> -+
> -+ strbuf_reset(&buf);
> -+ strbuf_addf(&buf, "refs/heads/%s", short_name);
> -+ if (branch_checked_out(buf.buf))
> ++ struct branch *branch = branch_get(short_name);
> ++ const char *upstream, *push;
> ++ struct strbuf full = STRBUF_INIT;
> ++ int skip;
> ++
> ++ strbuf_addf(&full, "refs/heads/%s", short_name);
> ++ skip = !!branch_checked_out(full.buf);
> ++ strbuf_release(&full);
> ++ if (skip)
> + continue;
> +
> -+ if (string_list_has_string(&protected_default_names,
> -+ short_name))
> ++ upstream = branch ? branch_get_upstream(branch, NULL) : NULL;
> ++ if (!upstream || !refs_ref_exists(refs, upstream))
> + continue;
> -+
> -+ if (!refs_ref_exists(refs, upstream))
> ++ push = branch ? branch_get_push(branch, NULL) : NULL;
> ++ if (!push || !strcmp(push, upstream))
> + continue;
> +
> + strvec_push(&deletable, short_name);
> + }
> -+ strbuf_release(&buf);
> +
> + if (deletable.nr)
> + ret = delete_branches(deletable.nr, deletable.v,
> -+ 0, FILTER_REFS_BRANCHES, quiet,
> -+ 1, &n_not_merged);
> -+
> -+ if (n_not_merged && !quiet)
> -+ fprintf(stderr,
> -+ Q_("Skipped %d branch that is not fully merged; "
> -+ "delete it with 'git branch -D' if you are sure.\n",
> -+ "Skipped %d branches that are not fully merged; "
> -+ "delete them with 'git branch -D' if you are sure.\n",
> -+ n_not_merged),
> -+ n_not_merged);
> ++ 0, /* force */
> ++ FILTER_REFS_BRANCHES,
> ++ quiet,
> ++ 1, /* warn_only */
> ++ 1, /* no_head_fallback */
> ++ 0 /* dry_run */);
> +
> + strvec_clear(&deletable);
> -+ string_list_clear(&candidates, 1);
> -+ string_list_clear(&protected_default_names, 0);
> ++ string_list_clear(&candidates, 0);
> + return ret;
> +}
> +
> @@ builtin/branch.c: int cmd_branch(int argc,
> OPT_BOOL(0, "forked", &forked,
> N_("list local branches whose upstream matches the given <branch>...")),
> + OPT_BOOL(0, "prune-merged", &prune_merged,
> -+ N_("delete local branches whose upstream matches the given <branch>... and that are merged into it")),
> ++ N_("delete local branches whose upstream matches the given <branch>... and is merged")),
> OPT__FORCE(&force, N_("force creation, move/rename, deletion"), PARSE_OPT_NOCOMPLETE),
> OPT_MERGED(&filter, N_("print only branches that are merged")),
> OPT_NO_MERGED(&filter, N_("print only branches that are not merged")),
> @@ t/t3200-branch.sh: test_expect_success '--forked requires at least one <branch>'
> + git -C pm-upstream branch one HEAD~ &&
> + git -C pm-upstream branch two HEAD &&
> + git -C pm-upstream branch wip main &&
> -+ git -C pm-upstream checkout main
> ++ git -C pm-upstream checkout main &&
> ++ test_create_repo pm-fork
> +'
> +
> +test_expect_success '--prune-merged deletes branches integrated into upstream' '
> + test_when_finished "rm -rf pm-merged" &&
> + git clone pm-upstream pm-merged &&
> ++ git -C pm-merged remote add fork ../pm-fork &&
> ++ test_config -C pm-merged remote.pushDefault fork &&
> ++ test_config -C pm-merged push.default current &&
> + git -C pm-merged branch one one-commit &&
> + git -C pm-merged branch --set-upstream-to=origin/next one &&
> + git -C pm-merged branch two two-commit &&
> @@ t/t3200-branch.sh: test_expect_success '--forked requires at least one <branch>'
> + test_must_fail git -C pm-merged rev-parse --verify refs/heads/two
> +'
> +
> -+test_expect_success '--prune-merged with a literal upstream argument' '
> ++test_expect_success '--prune-merged accepts a literal upstream' '
> + test_when_finished "rm -rf pm-literal" &&
> + git clone pm-upstream pm-literal &&
> ++ git -C pm-literal remote add fork ../pm-fork &&
> ++ test_config -C pm-literal remote.pushDefault fork &&
> ++ test_config -C pm-literal push.default current &&
> + git -C pm-literal branch one one-commit &&
> + git -C pm-literal branch --set-upstream-to=origin/next one &&
> -+ git -C pm-literal branch keepme one-commit &&
> -+ git -C pm-literal branch --set-upstream-to=origin/main keepme &&
> +
> + git -C pm-literal branch --prune-merged origin/next &&
> +
> -+ test_must_fail git -C pm-literal rev-parse --verify refs/heads/one &&
> -+ git -C pm-literal rev-parse --verify refs/heads/keepme
> ++ test_must_fail git -C pm-literal rev-parse --verify refs/heads/one
> +'
> +
> +test_expect_success '--prune-merged unions multiple <branch> arguments' '
> + test_when_finished "rm -rf pm-union" &&
> + git clone pm-upstream pm-union &&
> ++ git -C pm-union remote add fork ../pm-fork &&
> ++ test_config -C pm-union remote.pushDefault fork &&
> ++ test_config -C pm-union push.default current &&
> + git -C pm-union branch one one-commit &&
> + git -C pm-union branch --set-upstream-to=origin/next one &&
> + git -C pm-union branch two base &&
> + git -C pm-union branch --set-upstream-to=origin/main two &&
> ++ git -C pm-union checkout --detach &&
> +
> + git -C pm-union branch --prune-merged origin/next origin/main &&
> +
> @@ t/t3200-branch.sh: test_expect_success '--forked requires at least one <branch>'
> + test_must_fail git -C pm-union rev-parse --verify refs/heads/two
> +'
> +
> -+test_expect_success '--prune-merged with a local-branch argument' '
> -+ test_create_repo pm-local &&
> ++test_expect_success '--prune-merged accepts a local upstream' '
> + test_when_finished "rm -rf pm-local" &&
> -+ test_commit -C pm-local base &&
> -+ git -C pm-local branch topic base &&
> -+ git -C pm-local config branch.topic.remote . &&
> -+ git -C pm-local config branch.topic.merge refs/heads/main &&
> -+ git -C pm-local checkout --detach &&
> -+
> -+ git -C pm-local branch --prune-merged main &&
> -+
> -+ test_must_fail git -C pm-local rev-parse --verify refs/heads/topic &&
> -+ git -C pm-local rev-parse --verify refs/heads/main
> ++ git clone pm-upstream pm-local &&
> ++ git -C pm-local remote add fork ../pm-fork &&
> ++ test_config -C pm-local remote.pushDefault fork &&
> ++ test_config -C pm-local push.default current &&
> ++ git -C pm-local checkout -b trunk &&
> ++ git -C pm-local branch one one-commit &&
> ++ git -C pm-local branch --set-upstream-to=trunk one &&
> ++ git -C pm-local merge --ff-only one-commit &&
> ++
> ++ git -C pm-local branch --prune-merged trunk &&
> ++
> ++ test_must_fail git -C pm-local rev-parse --verify refs/heads/one
> +'
> +
> -+test_expect_success '--prune-merged spares branches with un-integrated commits' '
> ++test_expect_success '--prune-merged warns instead of erroring on un-integrated commits' '
> + test_when_finished "rm -rf pm-unmerged" &&
> + git clone pm-upstream pm-unmerged &&
> ++ git -C pm-unmerged remote add fork ../pm-fork &&
> ++ test_config -C pm-unmerged remote.pushDefault fork &&
> ++ test_config -C pm-unmerged push.default current &&
> + git -C pm-unmerged checkout -b wip origin/wip &&
> + git -C pm-unmerged branch --set-upstream-to=origin/next wip &&
> + test_commit -C pm-unmerged local-only &&
> @@ t/t3200-branch.sh: test_expect_success '--forked requires at least one <branch>'
> +
> + git -C pm-unmerged branch --prune-merged "origin/*" 2>err &&
> + test_grep "not fully merged" err &&
> -+ test_grep "Skipped 1 branch" err &&
> -+ test_grep "git branch -D" err &&
> + test_grep ! "If you are sure you want to delete it" err &&
> + git -C pm-unmerged rev-parse --verify refs/heads/wip
> +'
> +
> ++test_expect_success '--prune-merged is silent about not-merged-to-HEAD' '
> ++ test_when_finished "rm -rf pm-nohead" &&
> ++ git clone pm-upstream pm-nohead &&
> ++ git -C pm-nohead remote add fork ../pm-fork &&
> ++ test_config -C pm-nohead remote.pushDefault fork &&
> ++ test_config -C pm-nohead push.default current &&
> ++ git -C pm-nohead branch topic one-commit &&
> ++ git -C pm-nohead branch --set-upstream-to=origin/next topic &&
> ++
> ++ git -C pm-nohead branch --prune-merged "origin/*" 2>err &&
> ++
> ++ test_grep ! "not yet merged to HEAD" err &&
> ++ test_must_fail git -C pm-nohead rev-parse --verify refs/heads/topic
> ++'
> ++
> +test_expect_success '--prune-merged skips branches whose upstream is gone' '
> + test_when_finished "rm -rf pm-upstream-gone" &&
> + git clone pm-upstream pm-upstream-gone &&
> ++ git -C pm-upstream-gone remote add fork ../pm-fork &&
> ++ test_config -C pm-upstream-gone remote.pushDefault fork &&
> ++ test_config -C pm-upstream-gone push.default current &&
> + git -C pm-upstream-gone branch one one-commit &&
> + git -C pm-upstream-gone branch --set-upstream-to=origin/next one &&
> +
> @@ t/t3200-branch.sh: test_expect_success '--forked requires at least one <branch>'
> +test_expect_success '--prune-merged never deletes the checked-out branch' '
> + test_when_finished "rm -rf pm-head" &&
> + git clone pm-upstream pm-head &&
> ++ git -C pm-head remote add fork ../pm-fork &&
> ++ test_config -C pm-head remote.pushDefault fork &&
> ++ test_config -C pm-head push.default current &&
> + git -C pm-head checkout -b one one-commit &&
> + git -C pm-head branch --set-upstream-to=origin/next one &&
> +
> @@ t/t3200-branch.sh: test_expect_success '--forked requires at least one <branch>'
> + git -C pm-head rev-parse --verify refs/heads/one
> +'
> +
> -+test_expect_success '--prune-merged spares the local default branch' '
> -+ test_when_finished "rm -rf pm-default" &&
> -+ git clone pm-upstream pm-default &&
> -+ git -C pm-default checkout --detach &&
> -+ git -C pm-default branch --prune-merged "origin/*" &&
> -+ git -C pm-default rev-parse --verify refs/heads/main
> ++test_expect_success '--prune-merged spares branches that push back to their upstream' '
> ++ test_when_finished "rm -rf pm-push-eq" &&
> ++ git clone pm-upstream pm-push-eq &&
> ++ git -C pm-push-eq checkout --detach &&
> ++
> ++ git -C pm-push-eq branch --prune-merged "origin/*" &&
> ++
> ++ git -C pm-push-eq rev-parse --verify refs/heads/main
> +'
> +
> -+test_expect_success '--prune-merged protects the default branch by name only' '
> -+ test_when_finished "rm -rf pm-default-alias" &&
> -+ git clone pm-upstream pm-default-alias &&
> -+ git -C pm-default-alias branch --track trunk origin/main &&
> -+ git -C pm-default-alias checkout --detach &&
> -+ git -C pm-default-alias branch --prune-merged "origin/*" &&
> -+ git -C pm-default-alias rev-parse --verify refs/heads/main &&
> -+ test_must_fail git -C pm-default-alias rev-parse --verify refs/heads/trunk
> ++test_expect_success '--prune-merged spares a per-branch pushRemote==upstream remote' '
> ++ test_when_finished "rm -rf pm-push-branch" &&
> ++ git clone pm-upstream pm-push-branch &&
> ++ git -C pm-push-branch remote add fork ../pm-fork &&
> ++ test_config -C pm-push-branch remote.pushDefault fork &&
> ++ test_config -C pm-push-branch push.default current &&
> ++ test_config -C pm-push-branch branch.main.pushRemote origin &&
> ++ git -C pm-push-branch checkout --detach &&
> ++
> ++ git -C pm-push-branch branch --prune-merged "origin/*" &&
> ++
> ++ git -C pm-push-branch rev-parse --verify refs/heads/main
> +'
> +
> -+test_expect_success '--prune-merged with literal arg also protects default-name' '
> -+ test_when_finished "rm -rf pm-literal-default" &&
> -+ git clone pm-upstream pm-literal-default &&
> -+ git -C pm-literal-default checkout --detach &&
> -+ git -C pm-literal-default branch --prune-merged origin/main &&
> -+ git -C pm-literal-default rev-parse --verify refs/heads/main
> ++test_expect_success '--prune-merged prunes when @{push} differs from @{upstream}' '
> ++ test_when_finished "rm -rf pm-push-diff" &&
> ++ git clone pm-upstream pm-push-diff &&
> ++ git -C pm-push-diff remote add fork ../pm-fork &&
> ++ test_config -C pm-push-diff remote.pushDefault fork &&
> ++ test_config -C pm-push-diff push.default current &&
> ++ git -C pm-push-diff branch topic one-commit &&
> ++ git -C pm-push-diff branch --set-upstream-to=origin/next topic &&
> ++ git -C pm-push-diff checkout --detach &&
> ++
> ++ git -C pm-push-diff branch --prune-merged "origin/*" &&
> ++
> ++ test_must_fail git -C pm-push-diff rev-parse --verify refs/heads/topic
> +'
> +
> +test_expect_success '--prune-merged requires at least one <branch>' '
> -+ test_must_fail git -C pm-upstream branch --prune-merged 2>err &&
> ++ test_must_fail git -C forked branch --prune-merged 2>err &&
> + test_grep "at least one <branch>" err
> +'
> +
> 3: 6e38d7af3a ! 5: 75b6d2366a branch: add branch.<name>.pruneMerged opt-out
> @@ Metadata
> ## Commit message ##
> branch: add branch.<name>.pruneMerged opt-out
>
> - Setting branch.<name>.pruneMerged=false exempts that branch
> - from --prune-merged. Useful for topic branches you intend to
> - develop further after an initial round has been merged
> + Setting branch.<name>.pruneMerged=false exempts that branch from
> + "git branch --prune-merged". Useful for a topic branch you want
> + to develop further after an initial round has been merged
> upstream.
>
> - Explicit deletion via 'git branch -d' is unaffected.
> + Unless --quiet is given, the skip is reported per branch so the
> + user knows why their topic was preserved.
> +
> + Explicit deletion via "git branch -d" continues to consult the
> + normal merge check and is not affected by this setting.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>
> @@ Documentation/config/branch.adoc: for details).
> +
> +`branch.<name>.pruneMerged`::
> + If set to `false`, branch _<name>_ is exempt from
> -+ `git branch --prune-merged`. Defaults to true. Explicit
> -+ deletion via `git branch -d` is unaffected.
> ++ `git branch --prune-merged`. Useful for a topic branch you
> ++ intend to develop further after an initial round has been
> ++ merged upstream. Defaults to true. Explicit deletion via
> ++ `git branch -d` is unaffected.
>
> ## Documentation/git-branch.adoc ##
> -@@ Documentation/git-branch.adoc: The following branches are always preserved:
> - any configured remote (the target of
> - `refs/remotes/<remote>/HEAD`) -- typically `main` or
> - `master`;
> -+* any branch with `branch.<name>.pruneMerged` set to `false`;
> - * any branch whose upstream no longer resolves locally.
> - --
> -
> +@@ Documentation/git-branch.adoc: the upstream refs refreshed.
> + +
> + A branch is left alone if any of the following holds:
> + its upstream no longer resolves locally; it is checked out in any
> +-worktree; or its push destination (`<branch>@{push}`) equals its
> ++worktree; its push destination (`<branch>@{push}`) equals its
> + upstream (`<branch>@{upstream}`), so it cannot be distinguished
> +-from a freshly pulled trunk that just looks "fully merged".
> ++from a freshly pulled trunk that just looks "fully merged"; or
> ++`branch.<name>.pruneMerged` is set to `false`.
> + +
> + Branches refused by the "fully merged" safety check are listed as
> + warnings and skipped; pass them to `git branch -D` explicitly if
>
> ## builtin/branch.c ##
> @@ builtin/branch.c: static int prune_merged_branches(int argc, const char **argv, int quiet)
> - for_each_string_list_item(item, &candidates) {
> - const char *short_name = item->string;
> - const char *upstream = item->util;
> -+ int prune_allowed = 1;
> + struct branch *branch = branch_get(short_name);
> + const char *upstream, *push;
> + struct strbuf full = STRBUF_INIT;
> ++ struct strbuf key = STRBUF_INIT;
> + int skip;
> ++ int opt_out;
>
> - strbuf_reset(&buf);
> - strbuf_addf(&buf, "refs/heads/%s", short_name);
> + strbuf_addf(&full, "refs/heads/%s", short_name);
> + skip = !!branch_checked_out(full.buf);
> @@ builtin/branch.c: static int prune_merged_branches(int argc, const char **argv, int quiet)
> - if (!refs_ref_exists(refs, upstream))
> + if (!push || !strcmp(push, upstream))
> continue;
>
> -+ strbuf_reset(&buf);
> -+ strbuf_addf(&buf, "branch.%s.prunemerged", short_name);
> -+ if (!repo_config_get_bool(the_repository, buf.buf,
> -+ &prune_allowed) &&
> -+ !prune_allowed) {
> ++ strbuf_addf(&key, "branch.%s.prunemerged", short_name);
> ++ if (!repo_config_get_bool(the_repository, key.buf, &opt_out) &&
> ++ !opt_out) {
> + if (!quiet)
> -+ fprintf(stderr, _("Skipping '%s' "
> -+ "(branch.%s.pruneMerged is false)\n"),
> ++ fprintf(stderr,
> ++ _("Skipping '%s' (branch.%s.pruneMerged is false)\n"),
> + short_name, short_name);
> ++ strbuf_release(&key);
> + continue;
> + }
> ++ strbuf_release(&key);
> +
> strvec_push(&deletable, short_name);
> }
> - strbuf_release(&buf);
> +
>
> ## t/t3200-branch.sh ##
> @@ t/t3200-branch.sh: test_expect_success '--prune-merged requires at least one <branch>' '
> @@ t/t3200-branch.sh: test_expect_success '--prune-merged requires at least one <br
> +test_expect_success '--prune-merged honours branch.<name>.pruneMerged=false' '
> + test_when_finished "rm -rf pm-optout" &&
> + git clone pm-upstream pm-optout &&
> ++ git -C pm-optout remote add fork ../pm-fork &&
> ++ test_config -C pm-optout remote.pushDefault fork &&
> ++ test_config -C pm-optout push.default current &&
> + git -C pm-optout branch one one-commit &&
> + git -C pm-optout branch --set-upstream-to=origin/next one &&
> + git -C pm-optout branch two two-commit &&
> + git -C pm-optout branch --set-upstream-to=origin/next two &&
> -+ git -C pm-optout config branch.one.pruneMerged false &&
> ++ test_config -C pm-optout branch.one.pruneMerged false &&
> +
> + git -C pm-optout branch --prune-merged "origin/*" 2>err &&
> +
> @@ t/t3200-branch.sh: test_expect_success '--prune-merged requires at least one <br
> + git clone pm-upstream pm-optout-d &&
> + git -C pm-optout-d branch one one-commit &&
> + git -C pm-optout-d branch --set-upstream-to=origin/next one &&
> -+ git -C pm-optout-d config branch.one.pruneMerged false &&
> ++ test_config -C pm-optout-d branch.one.pruneMerged false &&
> +
> + git -C pm-optout-d branch -d one &&
> + test_must_fail git -C pm-optout-d rev-parse --verify refs/heads/one
> 4: c68d162e22 ! 6: a1a42a6b19 branch: add --dry-run for --prune-merged
> @@ Metadata
> ## Commit message ##
> branch: add --dry-run for --prune-merged
>
> - With --dry-run, --prune-merged prints the branches it would
> - delete and exits without touching any ref. Useful for
> - sanity-checking a glob like 'origin/*' before letting it run.
> + With --dry-run, --prune-merged prints the local branches it would
> + delete -- one "Would delete branch <name>" line per candidate --
> + and exits without touching any ref.
> +
> + This is the natural sanity check before letting a broad pattern
> + like 'origin/*' run for real: the @{push}-vs-@{upstream} and
> + unmerged filtering still applies, so the dry-run output is
> + exactly the set that the live run would delete.
> +
> + --dry-run is only meaningful in combination with --prune-merged
> + and is rejected otherwise.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>
> @@ Documentation/git-branch.adoc: git branch (-c|-C) [<old-branch>] <new-branch>
>
> DESCRIPTION
> -----------
> -@@ Documentation/git-branch.adoc: The following branches are always preserved:
> - * any branch whose upstream no longer resolves locally.
> - --
> +@@ Documentation/git-branch.adoc: Branches refused by the "fully merged" safety check are listed as
> + warnings and skipped; pass them to `git branch -D` explicitly if
> + you want them gone.
>
> +`--dry-run`::
> -+ With `--prune-merged`, print the branches that would be
> -+ deleted instead of deleting them.
> ++ With `--prune-merged`, print which branches would be
> ++ deleted and exit without touching any ref. Useful for
> ++ sanity-checking a wide pattern like `'origin/*'` before
> ++ committing to the deletion.
> +
> `-v`::
> `-vv`::
> `--verbose`::
>
> ## builtin/branch.c ##
> -@@ builtin/branch.c: static const char * const builtin_branch_usage[] = {
> - N_("git branch [<options>] [-r | -a] [--points-at]"),
> - N_("git branch [<options>] [-r | -a] [--format]"),
> - N_("git branch [<options>] --forked <branch>..."),
> -- N_("git branch [<options>] --prune-merged <branch>..."),
> -+ N_("git branch [<options>] --prune-merged [--dry-run] <branch>..."),
> - NULL
> - };
> -
> -@@ builtin/branch.c: static void delete_branch_config(const char *branchname)
> - }
> -
> - static int delete_branches(int argc, const char **argv, int force, int kinds,
> -- int quiet, int warn_only, int *n_not_merged)
> -+ int quiet, int warn_only, int dry_run,
> -+ int *n_not_merged)
> - {
> - struct commit *head_rev = NULL;
> - struct object_id oid;
> -@@ builtin/branch.c: static int delete_branches(int argc, const char **argv, int force, int kinds,
> - goto next;
> - }
> -
> -+ if (dry_run) {
> -+ printf(_("Would delete branch '%s'\n"),
> -+ name + branch_name_pos);
> -+ goto next;
> -+ }
> -+
> - item = string_list_append(&refs_to_delete, name);
> - item->util = xstrdup((flags & REF_ISBROKEN) ? "broken"
> - : (flags & REF_ISSYMREF) ? target
> @@ builtin/branch.c: static int list_forked_branches(int argc, const char **argv)
> return 0;
> }
>
> -static int prune_merged_branches(int argc, const char **argv, int quiet)
> -+static int prune_merged_branches(int argc, const char **argv,
> -+ int dry_run, int quiet)
> ++static int prune_merged_branches(int argc, const char **argv, int quiet,
> ++ int dry_run)
> {
> struct ref_store *refs = get_main_ref_store(the_repository);
> struct string_list candidates = STRING_LIST_INIT_DUP;
> @@ builtin/branch.c: static int prune_merged_branches(int argc, const char **argv, int quiet)
> - if (deletable.nr)
> - ret = delete_branches(deletable.nr, deletable.v,
> - 0, FILTER_REFS_BRANCHES, quiet,
> -- 1, &n_not_merged);
> -+ 1, dry_run, &n_not_merged);
> + quiet,
> + 1, /* warn_only */
> + 1, /* no_head_fallback */
> +- 0 /* dry_run */);
> ++ dry_run);
>
> - if (n_not_merged && !quiet)
> - fprintf(stderr,
> + strvec_clear(&deletable);
> + string_list_clear(&candidates, 0);
> @@ builtin/branch.c: int cmd_branch(int argc,
> unset_upstream = 0, show_current = 0, edit_description = 0;
> int forked = 0;
> @@ builtin/branch.c: int cmd_branch(int argc,
> @@ builtin/branch.c: int cmd_branch(int argc,
> N_("list local branches whose upstream matches the given <branch>...")),
> OPT_BOOL(0, "prune-merged", &prune_merged,
> - N_("delete local branches whose upstream matches the given <branch>... and that are merged into it")),
> + N_("delete local branches whose upstream matches the given <branch>... and is merged")),
> + OPT_BOOL(0, "dry-run", &dry_run,
> -+ N_("with --prune-merged, only print what would be deleted")),
> ++ N_("with --prune-merged, only print which branches would be deleted")),
> OPT__FORCE(&force, N_("force creation, move/rename, deletion"), PARSE_OPT_NOCOMPLETE),
> OPT_MERGED(&filter, N_("print only branches that are merged")),
> OPT_NO_MERGED(&filter, N_("print only branches that are not merged")),
> @@ builtin/branch.c: int cmd_branch(int argc,
> - argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
> - 0);
> + if (noncreate_actions > 1)
> + usage_with_options(builtin_branch_usage, options);
>
> + if (dry_run && !prune_merged)
> + die(_("--dry-run requires --prune-merged"));
> +
> - if (!delete && !rename && !copy && !edit_description && !new_upstream &&
> - !show_current && !unset_upstream && !forked && !prune_merged &&
> - argc == 0)
> + if (recurse_submodules_explicit) {
> + if (!submodule_propagate_branches)
> + die(_("branch with --recurse-submodules can only be used if submodule.propagateBranches is enabled"));
> @@ builtin/branch.c: int cmd_branch(int argc,
> - if (!argc)
> - die(_("branch name required"));
> - ret = delete_branches(argc, argv, delete > 1, filter.kind,
> -- quiet, 0, NULL);
> -+ quiet, 0, 0, NULL);
> - goto out;
> - } else if (forked) {
> ret = list_forked_branches(argc, argv);
> goto out;
> } else if (prune_merged) {
> - ret = prune_merged_branches(argc, argv, quiet);
> -+ ret = prune_merged_branches(argc, argv, dry_run, quiet);
> ++ ret = prune_merged_branches(argc, argv, quiet, dry_run);
> goto out;
> } else if (show_current) {
> print_current_branch_name();
> @@ t/t3200-branch.sh: test_expect_success 'branch -d still deletes a pruneMerged=fa
> test_must_fail git -C pm-optout-d rev-parse --verify refs/heads/one
> '
>
> -+test_expect_success '--prune-merged --dry-run prints but does not delete' '
> -+ test_when_finished "rm -rf pm-dryrun" &&
> -+ git clone pm-upstream pm-dryrun &&
> -+ git -C pm-dryrun branch one one-commit &&
> -+ git -C pm-dryrun branch --set-upstream-to=origin/next one &&
> ++test_expect_success '--prune-merged --dry-run lists but does not delete' '
> ++ test_when_finished "rm -rf pm-dry" &&
> ++ git clone pm-upstream pm-dry &&
> ++ git -C pm-dry remote add fork ../pm-fork &&
> ++ test_config -C pm-dry remote.pushDefault fork &&
> ++ test_config -C pm-dry push.default current &&
> ++ git -C pm-dry branch one one-commit &&
> ++ git -C pm-dry branch --set-upstream-to=origin/next one &&
> ++ git -C pm-dry branch two two-commit &&
> ++ git -C pm-dry branch --set-upstream-to=origin/next two &&
> ++
> ++ git -C pm-dry branch --prune-merged --dry-run "origin/*" >actual &&
> ++ test_grep "Would delete branch one " actual &&
> ++ test_grep "Would delete branch two " actual &&
> +
> -+ git -C pm-dryrun branch --prune-merged --dry-run "origin/*" >out &&
> -+ test_grep "Would delete branch .one." out &&
> -+ git -C pm-dryrun rev-parse --verify refs/heads/one
> ++ git -C pm-dry rev-parse --verify refs/heads/one &&
> ++ git -C pm-dry rev-parse --verify refs/heads/two
> +'
> +
> -+test_expect_success '--prune-merged --dry-run skips un-integrated branches' '
> -+ test_when_finished "rm -rf pm-dryrun-unmerged" &&
> -+ git clone pm-upstream pm-dryrun-unmerged &&
> -+ git -C pm-dryrun-unmerged checkout -b wip origin/next &&
> -+ git -C pm-dryrun-unmerged branch --set-upstream-to=origin/next wip &&
> -+ test_commit -C pm-dryrun-unmerged local-only &&
> -+ git -C pm-dryrun-unmerged checkout - &&
> -+ git -C pm-dryrun-unmerged branch merged one-commit &&
> -+ git -C pm-dryrun-unmerged branch --set-upstream-to=origin/next merged &&
> ++test_expect_success '--prune-merged --dry-run only lists branches the live run would delete' '
> ++ test_when_finished "rm -rf pm-dry-mixed" &&
> ++ git clone pm-upstream pm-dry-mixed &&
> ++ git -C pm-dry-mixed remote add fork ../pm-fork &&
> ++ test_config -C pm-dry-mixed remote.pushDefault fork &&
> ++ test_config -C pm-dry-mixed push.default current &&
> ++ git -C pm-dry-mixed checkout -b wip origin/next &&
> ++ git -C pm-dry-mixed branch --set-upstream-to=origin/next wip &&
> ++ test_commit -C pm-dry-mixed local-only &&
> ++ git -C pm-dry-mixed checkout - &&
> ++ git -C pm-dry-mixed branch merged one-commit &&
> ++ git -C pm-dry-mixed branch --set-upstream-to=origin/next merged &&
> +
> -+ git -C pm-dryrun-unmerged branch --prune-merged --dry-run "origin/*" \
> -+ >out 2>err &&
> -+ test_grep "Would delete branch .merged." out &&
> -+ test_grep ! "Would delete branch .wip." out &&
> -+ test_grep "not fully merged" err &&
> -+ git -C pm-dryrun-unmerged rev-parse --verify refs/heads/wip &&
> -+ git -C pm-dryrun-unmerged rev-parse --verify refs/heads/merged
> ++ git -C pm-dry-mixed branch --prune-merged --dry-run "origin/*" >out &&
> ++ test_grep "Would delete branch merged" out &&
> ++ test_grep ! "Would delete branch wip" out &&
> ++ git -C pm-dry-mixed rev-parse --verify refs/heads/wip &&
> ++ git -C pm-dry-mixed rev-parse --verify refs/heads/merged
> +'
> +
> -+test_expect_success '--dry-run requires --prune-merged' '
> -+ test_must_fail git -C pm-upstream branch --dry-run 2>err &&
> ++test_expect_success '--dry-run without --prune-merged is rejected' '
> ++ test_must_fail git -C forked branch --dry-run 2>err &&
> + test_grep "requires --prune-merged" err
> +'
> +
>
^ permalink raw reply
* Re: [GSoC][PATCH 0/4] teach git repo info to handle path keys
From: Phillip Wood @ 2026-06-02 13:03 UTC (permalink / raw)
To: K Jayatheerth, git
Cc: jltobler, lucasseikioshiro, gitster, phillip.wood, sandals,
kumarayushjha123, a3205153416
In-Reply-To: <20260601151950.30686-1-jayatheerthkulkarni2005@gmail.com>
On 01/06/2026 16:19, K Jayatheerth wrote:
>
> So in patches 3 and 4, we add both `path.<field>.absolute` and
> `path.<field>.relative` for `gitdir` and `commondir`. Initially,
> it was proposed by Ayush to use `path.absolute.<field>`, but
> this would break the lexicographical order of the internal field
> array. I tweaked it to place the variant at the end as a suffix instead.
I don't understand the comment about breaking the lexicographical order,
surely it only breaks if the new items are added out of order? Why can't
we have
path.absolute.commondir
path.absolute.gitdir
path.relative.commondir
path.relative.gitdir
?
Thanks
Phillip
> There are still a few open questions that should be addressed
> by the community. I am tagging members who were involved in the
> previous discussions:
>
> Justin Tobler, Lucas Seiki Oshiro, Junio, Phillip Wood,
> brian m. carlson, and Ayush Jha.
>
> Apologies if I missed anyone; I included everyone who reviewed
> or participated in the discussions of Eslam's and Lucas's
> patches.
>
> Questions:
>
> 1. Should there still be a --path-format flag?
> 2. Should we consider a default option?
> Currently we have path.gitdir.absolute; should we consider
> an option where a plain path.gitdir returns some default?
> If yes:
> 2.1 Should we keep the default the same as rev-parse? Or
> should either relative or absolute be the default?
> 2.2 When printing using --all, should the default be
> printed, or should we print both absolute and
> relative?
> 3. Is printing both absolute and relative in a single call
> using --all acceptable?
> If no:
> 3.1 What's a better approach?
>
> I have discussed these changes with both Justin and Lucas
> internally. This series is presented to gather opinions from the
> wider community before moving forward.
>
> K Jayatheerth (4):
> path: add strbuf_add_path for formatting paths
> rev-parse: use strbuf_add_path for path formatting
> repo: add path.gitdir with absolute and relative suffix formatting
> repo: add path.commondir with absolute and relative suffix formatting
>
> Documentation/git-repo.adoc | 15 ++++++
> builtin/repo.c | 50 ++++++++++++++++++
> builtin/rev-parse.c | 100 ++++++++----------------------------
> path.c | 58 +++++++++++++++++++++
> path.h | 16 ++++++
> t/t1900-repo-info.sh | 32 ++++++++++++
> 6 files changed, 192 insertions(+), 79 deletions(-)
>
^ permalink raw reply
* Re: [GSoC][PATCH 1/4] path: add strbuf_add_path for formatting paths
From: Phillip Wood @ 2026-06-02 13:00 UTC (permalink / raw)
To: K Jayatheerth, git
Cc: jltobler, lucasseikioshiro, gitster, phillip.wood, sandals,
kumarayushjha123, a3205153416
In-Reply-To: <20260601151950.30686-2-jayatheerthkulkarni2005@gmail.com>
On 01/06/2026 16:19, K Jayatheerth wrote:
>
> diff --git a/path.h b/path.h
> index 0434ba5e07..b9b626ce4a 100644
> --- a/path.h
> +++ b/path.h
> @@ -262,6 +262,22 @@ enum scld_error safe_create_leading_directories_no_share(char *path);
> int safe_create_file_with_leading_directories(struct repository *repo,
> const char *path);
>
> +enum path_format_type {
> + PATH_FORMAT_DEFAULT,
> + PATH_FORMAT_RELATIVE,
> + PATH_FORMAT_CANONICAL
> +};
> +
> +enum path_default_type {
> + PATH_DEFAULT_RELATIVE,
> + PATH_DEFAULT_RELATIVE_IF_SHARED,
> + PATH_DEFAULT_CANONICAL,
> + PATH_DEFAULT_UNMODIFIED
> +};
> +
> +void strbuf_add_path(struct strbuf *buf, const char *path, const char *prefix,
> + enum path_format_type format, enum path_default_type def);
This API is very specific to rev-parse and to me at least it is hard to
understand. I think it would be clearer if we had a single enum
describing the desired format and let the rev-parse code worry about
passing the appropriate value based on the options the user passed.
enum path_format {
PATH_FORMAT_ABSOLUTE,
PATH_FORMAT_CANONICAL,
PATH_FORMAT_RELATIVE,
PATH_FORMAT_RELATIVE_IF_SHARED PATH_FORMAT_UNMODIFIED,
};
void format_path(struct strbuf *buf, const char *path,
const char *prefix, enum path_format format);
We tend to avoid adding "strbuf_" to the beginning of functions these
days when they're adding things to a strbuf. This function also needs
some documentation explaining what the arguments are.
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH 2/2] builtin/init-db: deprecate alias for git-init(1)
From: Patrick Steinhardt @ 2026-06-02 12:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristoffer Haugsbakk, Phillip Wood, git
In-Reply-To: <xmqqv7c1xs76.fsf@gitster.g>
On Tue, Jun 02, 2026 at 05:27:41PM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > I wouldn't mind that outcome much, either. What triggered this series is
> > that I'm always annoyed that it's "builtin/init-db.c" instead of
> > "builtin/init.c", and the same for `cmd_init_db()`. But I intentionally
> > constructed the series in a way that the first commit can be picked
> > as-is, so that we can adjust our code to the modern world while not
> > doing the deprecation dance.
> >
> > So I'd be equally happy if we just drop the second commit in this
> > series.
>
> I'd actually find myself annoyed by such a rename when looking for
> builtin/init-db.c only to find it gone---much like how a previous
> rename made ll-merge difficult to locate.
>
> My point is that while static names may annoy some, renaming them
> does not resolve the annoyance; it merely shifts it to someone else.
>
> So, if the primary motivation is just the first patch, I would be
> less inclined to support this series.
That's entirely fair. My take on this is a bit different, as I think
it's beneficial to accept a short-term adjustment for core contributors
in favor of making stuff easier to discover/maintain going forward.
A new contributor would probably be quick to learn that every
`cmd_foo()` entry point is named exactly the same as the subcommand
name, but they will then eventually trip over the few exceptions like
`cmd_init_db()` where that assumption doesn't hold.
But I can see that this is not always clear-cut.
Patrick
^ permalink raw reply
* Re: [PATCH 2/2] builtin/init-db: deprecate alias for git-init(1)
From: Patrick Steinhardt @ 2026-06-02 12:34 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: Junio C Hamano, Phillip Wood, git
In-Reply-To: <455fc75a-444f-4760-a22f-54a2ec29618b@app.fastmail.com>
On Tue, Jun 02, 2026 at 09:54:02AM +0200, Kristoffer Haugsbakk wrote:
> On Tue, Jun 2, 2026, at 08:45, Patrick Steinhardt wrote:
> > On Tue, Jun 02, 2026 at 07:22:50AM +0900, Junio C Hamano wrote:
> >> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
> >>>[snip]
> >> Or just leave it without deprecation. It does not cost much to keep
> >> "init-db", and because we expanded what "git database" means in
> >> later versions of Git since its invention, the name still makes
> >> sense. Thank Linus for not naming it "init-odb"---that might have
> >> been a valid excuse to rename it because it does not cover the ref
> >> database and config database and others.
> >
> > I wouldn't mind that outcome much, either. What triggered this series is
> > that I'm always annoyed that it's "builtin/init-db.c" instead of
> > "builtin/init.c", and the same for `cmd_init_db()`. But I intentionally
> > constructed the series in a way that the first commit can be picked
> > as-is, so that we can adjust our code to the modern world while not
> > doing the deprecation dance.
> >
> > So I'd be equally happy if we just drop the second commit in this
> > series.
>
> Could it be worthwhile to mark it as soft deprecated? In the sense that
> it is a legacy alias that is not planned for removal?
The question is how such a soft deprecation would look like. Would it be
a warning, only, but other than that it behaves just as before? Should
we mark it as `DEPRECATED` in "git.c"? Both of those?
Patrick
^ permalink raw reply
* Re: [PATCH 2/2] SubmittingPatches: describe cover letter
From: Derrick Stolee @ 2026-06-02 12:29 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <20260602090808.87837-3-gitster@pobox.com>
On 6/2/2026 5:08 AM, Junio C Hamano wrote:
> We talk about how a commit log message should look like, but do not
> give advice on writing the cover letter to sell a series to widest
> possible audience.
This is a good thing to boost in the documentation.
> +[[cover-letter]]
> +=== Cover Letter
> +
> +The purpose of your cover letter is to sell your changes, explain what
> +they are about, and get your target audience interested enough to read
> +the patches.
> +
> +. Make sure your target audience can understand what the patches are
> + about and why they are needed without prior context.
The thing that I like to say about the cover letter is that this is
your opportunity to communicate why the value of your change is worth
the risk of regressions and the cost of maintenance. Perhaps:
. Every code change comes with risk of regression and maintenance cost.
The cover letter should clearly communicate why the value of your
proposed change is worth applying. You can also describe how the risk
is reduced by the design choices you made while writing the patches.
Or something similar may be helpful? I may just be over explaining.
> +. For a second or subsequent iteration of the same topic, make sure
> + people who missed the earlier discussion can still understand what
> + the patches are about, so they can judge if the topic is worth their
> + time to read and comment on.
> +
> +. To help those who are familiar with earlier iterations, give a
> + summary of changes since the previous rounds.
I find these updates to be particularly helpful, even for GitGitGadget
PRs that include a range-diff automatically. It's good to double-check
the human description of the update against the computed diff.
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH 2/2] SubmittingPatches: describe cover letter
From: Patrick Steinhardt @ 2026-06-02 12:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20260602090808.87837-3-gitster@pobox.com>
On Tue, Jun 02, 2026 at 06:08:08PM +0900, Junio C Hamano wrote:
> We talk about how a commit log message should look like, but do not
> give advice on writing the cover letter to sell a series to widest
s/to widest/to the widest/?
> possible audience.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Documentation/SubmittingPatches | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index dec8aea4cb..8ff1792b9b 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -472,6 +472,25 @@ highlighted above.
> Only capitalize the very first letter of the trailer, i.e. favor
> "Signed-off-by" over "Signed-Off-By" and "Acked-by:" over "Acked-By".
>
> +[[cover-letter]]
> +=== Cover Letter
> +
> +The purpose of your cover letter is to sell your changes, explain what
> +they are about, and get your target audience interested enough to read
> +the patches.
> +
> +. Make sure your target audience can understand what the patches are
> + about and why they are needed without prior context.
> +
> +. For a second or subsequent iteration of the same topic, make sure
> + people who missed the earlier discussion can still understand what
> + the patches are about, so they can judge if the topic is worth their
> + time to read and comment on.
> +
> +. To help those who are familiar with earlier iterations, give a
> + summary of changes since the previous rounds.
We might also recommend to include a range-diff in subsequent
iterations. That being said though, I just sent a small series to the
mailing list that recommends using b4, and there it get this for free.
So no idea whether it's still worth it to then cover this here
explicitly.
Patrick
^ permalink raw reply
* [PATCH 2/2] Documentation/MyFirstContribution: recommend the use of b4
From: Patrick Steinhardt @ 2026-06-02 11:59 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20260602-pks-b4-v1-0-a7ae5a49e9cf@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 | 81 ++++++++++++++++++++++++++++++++--
Documentation/SubmittingPatches | 6 ++-
2 files changed, 82 insertions(+), 5 deletions(-)
diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index b9fdefce02..2e50111d89 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,76 @@ 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.
+
[[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.1064.gd145956f57.dirty
^ permalink raw reply related
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