[PATCH] config: suggest the correct form when key contains "="

[PATCH] config: suggest the correct form when key contains "="

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

When a user types "git config foo.bar=baz", git_config_parse_key()
rejects the key with "error: invalid key: foo.bar=baz" but gives no
indication of what the user should have written.  The mistake is a
common one for users who reach for INI-file syntax or for the
"--flag=value" convention used by other command-line tools.

Since "=" is never a valid character in a config key, treat its
presence as a strong signal of this specific mistake and follow the
error with a one-line suggestion in the "(did you mean ...)" style
used elsewhere in git, e.g.:

    $ git config pull.rebase=false
    error: invalid key: pull.rebase=false
      (did you mean "git config set pull.rebase false"?)

The hint is emitted only when the offending character is "="; other
invalid characters (newlines, "@", etc.) keep their existing error
unchanged.

Signed-off-by: Harald Nordgren <harald.nordgren@kostdoktorn.se>
---
    config: suggest the correct form when key contains "="

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v1
Pull-Request: https://github.com/git/git/pull/2302

 config.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/config.c b/config.c
index a1b92fe083..6e658d71d1 100644
--- a/config.c
+++ b/config.c
@@ -580,6 +580,10 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
 			if (!iskeychar(c) ||
 			    (i == baselen + 1 && !isalpha(c))) {
 				error(_("invalid key: %s"), key);
+				if (c == '=')
+					fprintf_ln(stderr,
+						   _("  (did you mean \"git config set %.*s %s\"?)"),
+						   (int)i, key, key + i + 1);
 				goto out_free_ret_1;
 			}
 			c = tolower(c);

base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
-- 
gitgitgadget

Re: [PATCH] config: suggest the correct form when key contains "="

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> When a user types "git config foo.bar=baz", git_config_parse_key()
> rejects the key with "error: invalid key: foo.bar=baz" but gives no
> indication of what the user should have written.  The mistake is a
> common one for users who reach for INI-file syntax or for the
> "--flag=value" convention used by other command-line tools.
>
> Since "=" is never a valid character in a config key, treat its
> presence as a strong signal of this specific mistake and follow the
> error with a one-line suggestion in the "(did you mean ...)" style
> used elsewhere in git, e.g.:
>
>     $ git config pull.rebase=false
>     error: invalid key: pull.rebase=false
>       (did you mean "git config set pull.rebase false"?)

If the command line were

	git config get foo.bar=baz
	git config set foo.bar=baz nitfol

we shouldn't give an extra "did you mean?" at all.

The only cases you may want to do the "did you mean?" I think are

	git config foo.bar=baz
	git config set foo.bar=baz

And I think git_config_parse_key() is at a way too low level to tell
in what context we are seeing this faulty key to guess end-user's
intention to limit our "did you mean?"

I also wonder if, given that "=" in anywhere other than three-level
names, is invalid, we should just start accept

	git config foo.bar=baz
	git config set foo.bar=baz

and interpret them as

	git config set foo.bar baz

We of course need to be careful about non-invalid keys, i.e.

	git config foo.bar=baz.boo

is a request to read the value of that named variable, i.e.

	[foo "bar=baz"]
		boo = its value

so either you start offering unsolicited "did you mean?" or accepting
tokens with '=' in them as new style "set", you need to be extra
careful not to trigger a false positive.

[PATCH] fetch: add fetch.pruneLocalBranches config

[Harald Nordgren]

> I also wonder if, given that "=" in anywhere other than three-level
> names, is invalid, we should just start accept
> 
> 	git config foo.bar=baz
> 	git config set foo.bar=baz
> 
> and interpret them as
> 
> 	git config set foo.bar baz

That sounds good too! Probably even better.


Harald

Re: [PATCH] fetch: add fetch.pruneLocalBranches config

[Junio C Hamano]

Harald Nordgren <haraldnordgren@gmail.com> writes:

>> I also wonder if, given that "=" in anywhere other than three-level
>> names, is invalid, we should just start accept
>> 
>> 	git config foo.bar=baz
>> 	git config set foo.bar=baz
>> 
>> and interpret them as
>> 
>> 	git config set foo.bar baz
>
> That sounds good too! Probably even better.
>
>
> Harald

Why do I get the above, which apparently is a response to my review
for

    [PATCH] config: suggest the correct form when key contains "="

under this thread?  Am I dealing with some sort of mechanical slop?

Email issues

[Harald Nordgren]

> Why do I get the above, which apparently is a response to my review
> for
> 
>     [PATCH] config: suggest the correct form when key contains "="
> 
> under this thread?  Am I dealing with some sort of mechanical slop?

I think the problem here is my email sending process is not good. I edit
all the emails in Sublime text, where I keep the same file for all
different threads.

I have the subject line as the first line of the file and like you notice I
forget to change it sometimes.

I keep each of the topics bookmarked like this, 
https://lore.kernel.org/git/xmqqecjdea13.fsf@gitster.g/, and then utilize
that like to send the email

```
  git send-email \
    --in-reply-to=xmqqecjdea13.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=haraldnordgren@gmail.com \
    /path/to/YOUR_REPLY
```

I tried playing with neomutt and and email client replacement, but that
adds the complexity of downloading a new mbox file for each reply, it
didn't seem easier, but maybe it is.

How do you handle emails?


Harald

Re: [PATCH] fetch: add fetch.pruneLocalBranches config

[Harald Nordgren]

> Why do I get the above, which apparently is a response to my review
> for
>
>     [PATCH] config: suggest the correct form when key contains "="
>
> under this thread?  Am I dealing with some sort of mechanical slop?

(Testing plain text email sending via Gmail for a less error-prone
workflow, does it still add the CC's correctly?)


Harald

Re: [PATCH v8 0/5] branch: prune-merged

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

>      +    Delete the local branches that --forked <remote> would list, but
>      +    only those whose tip is reachable from their configured upstream
>      +    remote-tracking branch (branch.<name>.merge): the work has already
>      +    landed on the upstream it tracks, so the local copy is no longer
>      +    needed.
>      +
>      +    A branch whose upstream no longer resolves locally is left alone --
>      +    its disappearance is not, on its own, evidence that the work was
>      +    integrated.

That matches my understanding of the original motivation of this
topic a lot better than the previous round.

>      +    integrated. With --force, skip the reachability check and delete
>      +    every branch in the candidate set.

I am not sure if this is a good idea at all.  The option is called
prune-MERGED and with or without --force, mergedness should be what
determines if a branch is deleted.

To perform an equivalent of

	$ git branch -D $(git branch --forked <remote>)

it would be better not to (ab)use the more commonly useful and much
safer "--prune-merged", and let's not add "--prune-forked" either as
a short-cut.  A nuclear option should be made harder to trigger, ot
easier to trigger by confusion between "--prune-{merged,forked}".

[PATCH] fetch: add fetch.pruneLocalBranches config

[Harald Nordgren]

> I am not sure if this is a good idea at all.  The option is called
> prune-MERGED and with or without --force, mergedness should be what
> determines if a branch is deleted.

Well, when I started writing the feature it was "prune local branches",
and it evolved from there to prune merged.

But you're probably right. I did wipe up some branches with real work on my
side using this (I restored them), so it seems to be more of a foot-gun
than I first imagined.

Seems reasonable to remove the '--force' functionality.


Harald

Re: [PATCH v7 3/5] branch: add --prune-merged <remote>

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +`--prune-merged`::
> +	Delete the local branches that `--forked` would list for
> +	the same _<remote>_ arguments, but only when the branch's
> +	push destination remote-tracking branch (the branch `git push`
> +	would update; see `branch_get_push` semantics) no longer
> +	resolves locally.

I thought the thing you were aiming for is this scenario:

    git fetch origin
    git checkout -t -b hn/topic origin/next
    ... work work work ...
    git push origin
    ... the above pushes the current hn/topic to update their next
    git checkout master ;# or anywhere other than hn/topic
    git fetch origin
    git branch --prune-merged origin

The last step notices that our local hn/topic has been merged at the
remote to the target branch 'next', and the second-to-last fetch
makes us notice that origin/next now has our hn/topic merged, so we
no longer have a reason to keep hn/topic around.

But the above description uses quite different condition.  You want
to notice that _they_ removed 'next' (for that, the second-to-last
fetch may need to be run with --prune) and then remove our local
hn/topic, but to me, that sounds nonsense for two reasons.

 (1) Their 'next' is something contributors may fork from to work
     on.  You exactly did that with hn/topic branch of your own.
     Why would we even expect that to go away?

 (2) When disappearance of their 'next' is fetched to our
     remote-tracking namespace, we would not even know if hn/topic
     that used to fork from has been already integrated and stashed
     safely on some other branch on the remote.  It sounds very
     unsafe to remove it based on disappearance of origin/next
     remote-tracking branch.

> In other words: the branch was pushed
> +	under some name on _<remote>_, and that name has since
> +	been pruned upstream.
> ++
> +As a safety check, branches with commits not yet integrated into
> +their upstream remote-tracking branch are refused; if the upstream
> +itself is gone, the remote's default branch is consulted instead.

Again, this is as nonsense as our example in an earlier iteration of
having a topic forked from my 'todo' branch while the HEAD is
pointing at the default branch that is 'master'.  If the upstream
itself is gone, removing anything based on some other criteria
cannot by definition a "safety check".  I'd suggest rethinking the
logic.

> @@ -171,7 +197,7 @@ 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) {
> +	if (!no_head_fallback && head_rev != reference_rev) {

I somehow thought that the necessary check at the lowest level can
reuse most of the "branch -d" protection logic, except that it needs
to pass NULL for head_rev from check_branch_commit() down to
branch_merged() when doing "branch --prune-merged".  Do we really
need an extra no_head_fallback parameter?

[PATCH] fetch: add fetch.pruneLocalBranches config

[Harald Nordgren]

> But the above description uses quite different condition.  You want
> to notice that _they_ removed 'next' (for that, the second-to-last
> fetch may need to be run with --prune) and then remove our local
> hn/topic, but to me, that sounds nonsense for two reasons.

I wanted to be able to be aggresice with deleting, but maybe this went a
bit overboard. Would still be nice to have a nuclear option.


Harald

Re: [PATCH v6 0/5] branch: prune-merged

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  * --prune-merged now measures merged-ness against the remote's default
>    branch instead of the candidate's upstream — so the decision no longer
>    depends on which branch happens to be checked out locally.

I may be misreading the above and misunderstood you, but if you mean
that the feature now checks with remote/origin/master when I have a
local branch that were forked from remote/origin/todo and set to
merge new changes from there, I do not think it is a good change.

The two remote-tracking branches may not even share any commit.

The "what it tracks or HEAD" logic I raised as questionable is in
the function builtin/branch.c:branch_merged() that is called from
the function check_branch_commit() you updated and used in the
implementation of prune-merged.  It does branch_get_upstream() to
find the tip of remote-tracking branch that the target branch builds
upon, and performs comparison (which is very sensible).  I do not
think you want to change it to check with remotes/<remote>/HEAD
instead, as the upstream of the local branch may not be building on
their HEAD at all.  But when the upstream is not found, the code
makes the reference_rev variable fall back to head_rev, and checks
if the commit at the tip of the target branch is already merged
there.

It is still not clear to me if we want to optionally disable this
fallback to HEAD, but a quick scan of branch_merged() tells me that
it is prepared to see NULL in head_rev.

[PATCH] fetch: add fetch.pruneLocalBranches config

[Harald Nordgren]

> I may be misreading the above and misunderstood you, but if you mean
> that the feature now checks with remote/origin/master when I have a
> local branch that were forked from remote/origin/todo and set to
> merge new changes from there, I do not think it is a good change.

I think you are right. My latest code assumes that everyone works toward
the default branch, which is what I do 99% of the time, but yeah, it should
be more agnostic from different workflow.

I'll take another look.


Harald

Re: [PATCH v5 2/5] branch: let delete_branches warn instead of error on bulk refusal

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> Add two new parameters to delete_branches() and the helper
> check_branch_commit():
>
> * warn_only switches the per-branch refusal from a hard error
>   ("error: the branch 'X' is not fully merged" plus a four-line
>   hint about 'git branch -D X') to a one-line warning, and
>   causes the function to skip those branches without setting its
>   exit code. Each refused branch is still skipped from deletion.
> * n_not_merged, when non-NULL, is incremented for each branch
>   refused on the not-merged path, so a bulk caller can summarize
>   rather than print per-branch advice.
>
> All existing call sites pass 0 / NULL and so are unaffected. Both
> parameters are wired up so a bulk-deletion caller can suppress
> the noise normally appropriate for a one-shot 'git branch -d'.

Existing call sites are about "branch -d <other>" that allows the
other branch to be deleted if it is part of HEAD or if it is part of
its tracking branch, but should "branch --prune-merged" pay
attention to what branch happens to be checked out the same way (not
a rherotical question to hint that I do not think it should---I do
not have a strong opinion on this either way)?

> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>  builtin/branch.c | 29 ++++++++++++++++++++---------
>  1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index b3289a8875..1941f8a9ad 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -192,7 +192,8 @@ 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) {
> @@ -200,10 +201,18 @@ 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;
> @@ -219,7 +228,7 @@ 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;
> @@ -309,8 +318,9 @@ 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;
>  		}
>  
> @@ -961,7 +971,8 @@ 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;
>  	} else if (forked) {
>  		ret = list_forked_branches(argc, argv);

[PATCH] fetch: add fetch.pruneLocalBranches config

[Harald Nordgren]

> Existing call sites are about "branch -d <other>" that allows the
> other branch to be deleted if it is part of HEAD or if it is part of
> its tracking branch, but should "branch --prune-merged" pay
> attention to what branch happens to be checked out the same way (not
> a rherotical question to hint that I do not think it should---I do
> not have a strong opinion on this either way)?

This is a very good question! My opion is that it should work the same way
regardless of which branch you are on, it should always compare against the
remote's default branch.

I this explains some weirdness I saw today when running it from non-main
and prune didn't get triggered.

I will look into making that change.


Harald

Re: [PATCH v4 4/6] fetch: add --prune-merged

[Johannes Sixt]

Am 05.05.26 um 21:23 schrieb Harald Nordgren via GitGitGadget:
> After a successful fetch from a configured remote, run
> 'git branch --prune-merged <remote>' to delete local branches
> whose push destination ref has just been pruned.

I have some sympathy for the desire to clean up unnecessary local
branches, but I don't like the concept that `git fetch` modifies local
branches, not even as an opt-in. Deleting local branches should be `git
branch`'s task exclusively (at the porcelain level).

-- Hannes

[PATCH] fetch: add fetch.pruneLocalBranches config

[Harald Nordgren]

> I have some sympathy for the desire to clean up unnecessary local
> branches, but I don't like the concept that `git fetch` modifies local
> branches, not even as an opt-in. Deleting local branches should be `git
> branch`'s task exclusively (at the porcelain level).

Yeah, maybe that's a good point.


Harald

Re: [PATCH] fetch: add fetch.pruneLocalBranches config

[Junio C Hamano]

Harald Nordgren <haraldnordgren@gmail.com> writes:

>> I have some sympathy for the desire to clean up unnecessary local
>> branches, but I don't like the concept that `git fetch` modifies local
>> branches, not even as an opt-in. Deleting local branches should be `git
>> branch`'s task exclusively (at the porcelain level).
>
> Yeah, maybe that's a good point.

I think the latest iteration was sent after the above exchange, yet
it seems to have changes to builtin/fetch.c to cause `git fetch` to
modify local branches still.  Will we have another update that is
hopefully final to excise that part, or are we OK to allow `fetch`
to modify the local state as an opt-in now?

Thanks.

[PATCH] fetch: add fetch.pruneLocalBranches config

[Harald Nordgren]

>>> I have some sympathy for the desire to clean up unnecessary local
>>> branches, but I don't like the concept that `git fetch` modifies local
>>> branches, not even as an opt-in. Deleting local branches should be `git
>>> branch`'s task exclusively (at the porcelain level).
>>
>> Yeah, maybe that's a good point.
>
> I think the latest iteration was sent after the above exchange, yet
> it seems to have changes to builtin/fetch.c to cause `git fetch` to
> modify local branches still.  Will we have another update that is
> hopefully final to excise that part, or are we OK to allow `fetch`
> to modify the local state as an opt-in now?

Done! (I didn't know if we wanted to do this yet, or we still just
discussion it, but now I deleted it.)


Harald

[PATCH] fetch: add fetch.pruneLocalBranches config

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

Introduce a tri-state config option that, when --prune (or
fetch.prune / remote.<name>.prune) removes a remote-tracking
ref, also deletes local branches whose configured upstream is
that ref.

Values:
- false (default): no change in behavior.
- safe: delete only if the local tip is reachable from the
  upstream tip, preserving any unpushed work.
- force: delete unconditionally; recoverable only via reflog.

The currently checked-out branch is always preserved.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
    fetch: add fetch.pruneBranches config

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2285%2FHaraldNordgren%2Ffetch-prune-local-branches-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2285/HaraldNordgren/fetch-prune-local-branches-v1
Pull-Request: https://github.com/git/git/pull/2285

 Documentation/config/fetch.adoc  |  39 +++++++
 Documentation/config/remote.adoc |   7 ++
 Documentation/fetch-options.adoc |   9 ++
 Documentation/git-fetch.adoc     |   6 ++
 builtin/fetch.c                  | 172 ++++++++++++++++++++++++++++++-
 remote.c                         |  16 +++
 remote.h                         |  10 ++
 t/t5510-fetch.sh                 |  84 +++++++++++++++
 8 files changed, 339 insertions(+), 4 deletions(-)

diff --git a/Documentation/config/fetch.adoc b/Documentation/config/fetch.adoc
index cd40db0cad..5a60507a84 100644
--- a/Documentation/config/fetch.adoc
+++ b/Documentation/config/fetch.adoc
@@ -50,6 +50,45 @@
 	refs. See also `remote.<name>.pruneTags` and the PRUNING
 	section of linkgit:git-fetch[1].
 
+`fetch.pruneBranches`::
+	When set in addition to `fetch.prune` (or `--prune`), also
+	delete local branches whose configured upstream
+	(`branch.<name>.merge`) is one of the remote-tracking refs
+	just removed by pruning. This is useful for cleaning up topic
+	branches whose upstream counterpart has been merged and then
+	removed. The same effect can be requested per-invocation with
+	`--prune-branches[=<mode>]`, or per-remote with
+	`remote.<name>.pruneBranches`.
++
+The currently checked-out branch (in any worktree) is never
+deleted. The value is one of:
++
+--
+`false` (the default);;
+	Do not delete any local branches. Equivalent to leaving
+	the option unset.
+`safe`;;
+	Delete a local branch only if its tip is an ancestor of
+	the upstream remote-tracking ref's last-known position.
+	In other words, only delete the branch if it contains no
+	commits that the upstream did not also have at the moment
+	it was deleted. This catches the common case of a branch
+	that was pushed and then squash- or rebase-merged
+	upstream (the local branch has no extra commits beyond
+	what was pushed), but preserves any branch with unpushed
+	local work.
+`force`;;
+	Delete the local branch unconditionally, even if it
+	contains unpushed commits. Use with care: if a remote
+	branch is deleted for any reason other than that its
+	contents were merged, the corresponding local commits
+	will only be retrievable through the reflog.
+--
++
+This option has no effect unless pruning is also enabled, since
+local branches are only considered for deletion when their
+upstream remote-tracking ref is being pruned in the same fetch.
+
 `fetch.all`::
 	If true, fetch will attempt to update all available remotes.
 	This behavior can be overridden by passing `--no-all` or by
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 91e46f66f5..60fd5841c6 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -87,6 +87,13 @@ remote.<name>.pruneTags::
 See also `remote.<name>.prune` and the PRUNING section of
 linkgit:git-fetch[1].
 
+remote.<name>.pruneBranches::
+	When pruning is active for this remote and this is set to `safe`
+	or `force`, also delete local branches whose upstream
+	remote-tracking ref is being pruned. Overrides
+	`fetch.pruneBranches` settings, if any. See `fetch.pruneBranches`
+	for the meaning of the values.
+
 remote.<name>.promisor::
 	When set to true, this remote will be used to fetch promisor
 	objects.
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index 81a9d7f9bb..0764f67cc3 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -185,6 +185,15 @@ See the PRUNING section below for more details.
 +
 See the PRUNING section below for more details.
 
+`--prune-branches[=(safe|force)]`::
+	When pruning, also delete local branches whose configured
+	upstream (`branch.<name>.merge`) is one of the remote-tracking
+	refs being pruned. With no value or `safe`, refuse to delete a
+	branch with unpushed commits; with `force`, delete it
+	regardless. The currently checked-out branch is never
+	deleted. See `fetch.pruneBranches` in linkgit:git-config[1] for
+	details.
+
 endif::git-pull[]
 
 ifndef::git-pull[]
diff --git a/Documentation/git-fetch.adoc b/Documentation/git-fetch.adoc
index db03541915..a50b9672a1 100644
--- a/Documentation/git-fetch.adoc
+++ b/Documentation/git-fetch.adoc
@@ -179,6 +179,12 @@ It's reasonable to e.g. configure `fetch.pruneTags=true` in
 run, without making every invocation of `git fetch` without `--prune`
 an error.
 
+Local branches whose upstream remote-tracking ref is being pruned can
+also be deleted automatically with `--prune-branches[=<mode>]` (or its
+config equivalents `fetch.pruneBranches` and `remote.<name>.pruneBranches`).
+See linkgit:git-config[1] for the data-loss tradeoff between the
+`safe` and `force` modes.
+
 Pruning tags with `--prune-tags` also works when fetching a URL
 instead of a named remote. These will all prune tags not found on
 origin:
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a22c319467..c6c2f00be0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -82,6 +82,21 @@ static int prune = -1; /* unspecified */
 static int prune_tags = -1; /* unspecified */
 #define PRUNE_TAGS_BY_DEFAULT 0 /* do we prune tags by default? */
 
+static int prune_branches = PRUNE_BRANCHES_UNSPECIFIED;
+
+static int parse_prune_branches_opt(const struct option *opt,
+				    const char *arg, int unset)
+{
+	int *v = opt->value;
+	if (unset)
+		*v = PRUNE_BRANCHES_OFF;
+	else if (arg)
+		*v = parse_prune_branches_value(opt->long_name, arg);
+	else
+		*v = PRUNE_BRANCHES_SAFE;
+	return 0;
+}
+
 static int append, dry_run, force, keep, update_head_ok;
 static int write_fetch_head = 1;
 static int verbosity, deepen_relative, set_upstream, refetch;
@@ -105,6 +120,7 @@ struct fetch_config {
 	int all;
 	int prune;
 	int prune_tags;
+	enum prune_branches_mode prune_branches;
 	int show_forced_updates;
 	int recurse_submodules;
 	int parallel;
@@ -131,6 +147,11 @@ static int git_fetch_config(const char *k, const char *v,
 		return 0;
 	}
 
+	if (!strcmp(k, "fetch.prunebranches")) {
+		fetch_config->prune_branches = parse_prune_branches_value(k, v);
+		return 0;
+	}
+
 	if (!strcmp(k, "fetch.showforcedupdates")) {
 		fetch_config->show_forced_updates = git_config_bool(k, v);
 		return 0;
@@ -1445,7 +1466,8 @@ out:
 static int prune_refs(struct display_state *display_state,
 		      struct refspec *rs,
 		      struct ref_transaction *transaction,
-		      struct ref *ref_map)
+		      struct ref *ref_map,
+		      struct ref **stale_refs_out)
 {
 	int result = 0;
 	struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map);
@@ -1487,7 +1509,126 @@ static int prune_refs(struct display_state *display_state,
 cleanup:
 	string_list_clear(&refnames, 0);
 	strbuf_release(&err);
-	free_refs(stale_refs);
+	if (!result && stale_refs_out)
+		*stale_refs_out = stale_refs;
+	else
+		free_refs(stale_refs);
+	return result;
+}
+
+struct prune_branches_cb {
+	struct string_list *pruned_refs;
+	struct string_list *to_delete;
+	struct string_list *skipped_unmerged;
+	enum prune_branches_mode mode;
+};
+
+static int collect_branches_to_prune(const struct reference *ref, void *cb_data)
+{
+	struct prune_branches_cb *cb = cb_data;
+	const char *short_name = ref->name;
+	char *full_ref = xstrfmt("refs/heads/%s", short_name);
+	const char *upstream;
+	struct string_list_item *pruned;
+	int result = 0;
+
+	if (ref->flags & REF_ISSYMREF)
+		goto out;
+	if (branch_checked_out(full_ref))
+		goto out;
+
+	upstream = branch_get_upstream(branch_get(short_name), NULL);
+	if (!upstream)
+		goto out;
+
+	pruned = string_list_lookup(cb->pruned_refs, upstream);
+	if (!pruned)
+		goto out;
+
+	if (cb->mode == PRUNE_BRANCHES_SAFE) {
+		struct commit *local = lookup_commit_reference(the_repository,
+							       ref->oid);
+		struct commit *up = lookup_commit_reference(the_repository,
+							    pruned->util);
+		int reachable = local && up &&
+			repo_in_merge_bases(the_repository, local, up);
+
+		if (reachable < 0) {
+			result = -1;
+			goto out;
+		}
+		if (!reachable) {
+			string_list_append(cb->skipped_unmerged, short_name);
+			goto out;
+		}
+	}
+
+	string_list_append(cb->to_delete, full_ref);
+
+out:
+	free(full_ref);
+	return result;
+}
+
+static int do_prune_branches(struct display_state *display_state,
+			     struct ref *stale_refs,
+			     enum prune_branches_mode mode)
+{
+	struct string_list pruned_refs = STRING_LIST_INIT_NODUP;
+	struct string_list to_delete = STRING_LIST_INIT_DUP;
+	struct string_list skipped_unmerged = STRING_LIST_INIT_DUP;
+	struct prune_branches_cb cb = {
+		.pruned_refs = &pruned_refs,
+		.to_delete = &to_delete,
+		.skipped_unmerged = &skipped_unmerged,
+		.mode = mode,
+	};
+	struct ref *ref;
+	struct string_list_item *item;
+	int result = 0;
+
+	if (!stale_refs)
+		return 0;
+
+	for (ref = stale_refs; ref; ref = ref->next)
+		string_list_append(&pruned_refs, ref->name)->util = &ref->new_oid;
+	string_list_sort(&pruned_refs);
+
+	if (refs_for_each_branch_ref(get_main_ref_store(the_repository),
+				     collect_branches_to_prune, &cb)) {
+		result = -1;
+		goto cleanup;
+	}
+
+	if (!dry_run && to_delete.nr)
+		result = refs_delete_refs(get_main_ref_store(the_repository),
+					  "fetch: prune branches",
+					  &to_delete, REF_NO_DEREF);
+
+	if (verbosity >= 0) {
+		const struct object_id *zero = null_oid(the_repository->hash_algo);
+		for_each_string_list_item(item, &to_delete) {
+			const char *short_name;
+			if (skip_prefix(item->string, "refs/heads/", &short_name))
+				display_ref_update(display_state, '-',
+						   _("[deleted local]"), NULL,
+						   _("(none)"), short_name,
+						   zero, zero,
+						   transport_summary_width(NULL));
+		}
+	}
+	for_each_string_list_item(item, &skipped_unmerged)
+		warning(_("not deleting local branch '%s' that is not "
+			  "fully merged into its upstream;\n"
+			  "         set fetch.pruneBranches=force to "
+			  "delete anyway, or delete manually with "
+			  "'git branch -D %s'"),
+			item->string, item->string);
+
+cleanup:
+	string_list_clear(&pruned_refs, 0);
+	string_list_clear(&to_delete, 0);
+	string_list_clear(&skipped_unmerged, 0);
 	return result;
 }
 
@@ -1945,19 +2086,28 @@ static int do_fetch(struct transport *transport,
 	if (tags == TAGS_DEFAULT && autotags)
 		transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
 	if (prune) {
+		struct ref *stale_refs = NULL;
+		struct ref **stale_refs_out = prune_branches != PRUNE_BRANCHES_OFF
+			? &stale_refs : NULL;
 		/*
 		 * We only prune based on refspecs specified
 		 * explicitly (via command line or configuration); we
 		 * don't care whether --tags was specified.
 		 */
 		if (rs->nr) {
-			retcode = prune_refs(&display_state, rs, transaction, ref_map);
+			retcode = prune_refs(&display_state, rs, transaction,
+					     ref_map, stale_refs_out);
 		} else {
 			retcode = prune_refs(&display_state, &transport->remote->fetch,
-					     transaction, ref_map);
+					     transaction, ref_map, stale_refs_out);
 		}
 		if (retcode != 0)
 			retcode = 1;
+		else if (stale_refs &&
+			 do_prune_branches(&display_state, stale_refs,
+					   prune_branches))
+			retcode = 1;
+		free_refs(stale_refs);
 	}
 
 	/*
@@ -2419,6 +2569,16 @@ static int fetch_one(struct remote *remote, int argc, const char **argv,
 			prune_tags = PRUNE_TAGS_BY_DEFAULT;
 	}
 
+	if (prune_branches == PRUNE_BRANCHES_UNSPECIFIED) {
+		/* no command line request */
+		if (remote->prune_branches >= 0)
+			prune_branches = remote->prune_branches;
+		else if (config->prune_branches >= 0)
+			prune_branches = config->prune_branches;
+		else
+			prune_branches = PRUNE_BRANCHES_OFF;
+	}
+
 	maybe_prune_tags = prune_tags_ok && prune_tags;
 	if (maybe_prune_tags && remote_via_config)
 		refspec_append(&remote->fetch, TAG_REFSPEC);
@@ -2469,6 +2629,7 @@ int cmd_fetch(int argc,
 		.display_format = DISPLAY_FORMAT_FULL,
 		.prune = -1,
 		.prune_tags = -1,
+		.prune_branches = PRUNE_BRANCHES_UNSPECIFIED,
 		.show_forced_updates = 1,
 		.recurse_submodules = RECURSE_SUBMODULES_DEFAULT,
 		.parallel = 1,
@@ -2520,6 +2681,9 @@ int cmd_fetch(int argc,
 			 N_("prune remote-tracking branches no longer on remote")),
 		OPT_BOOL('P', "prune-tags", &prune_tags,
 			 N_("prune local tags no longer on remote and clobber changed tags")),
+		OPT_CALLBACK_F(0, "prune-branches", &prune_branches, N_("mode"),
+			       N_("delete local branches whose upstream was pruned ('safe' or 'force')"),
+			       PARSE_OPT_OPTARG, parse_prune_branches_opt),
 		OPT_CALLBACK_F(0, "recurse-submodules", &recurse_submodules_cli, N_("on-demand"),
 			    N_("control recursive fetching of submodules"),
 			    PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
diff --git a/remote.c b/remote.c
index a664cd166a..1e2b4803e7 100644
--- a/remote.c
+++ b/remote.c
@@ -148,6 +148,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
 	CALLOC_ARRAY(ret, 1);
 	ret->prune = -1;  /* unspecified */
 	ret->prune_tags = -1;  /* unspecified */
+	ret->prune_branches = -1;  /* unspecified */
 	ret->name = xstrndup(name, len);
 	refspec_init_push(&ret->push);
 	refspec_init_fetch(&ret->fetch);
@@ -423,6 +424,19 @@ out:
 }
 #endif /* WITH_BREAKING_CHANGES */
 
+int parse_prune_branches_value(const char *k, const char *v)
+{
+	if (v) {
+		if (!strcasecmp(v, "safe"))
+			return PRUNE_BRANCHES_SAFE;
+		if (!strcasecmp(v, "force"))
+			return PRUNE_BRANCHES_FORCE;
+	}
+	if (git_parse_maybe_bool(v) == 0)
+		return PRUNE_BRANCHES_OFF;
+	die(_("invalid value for '%s': '%s'"), k, v);
+}
+
 static int handle_config(const char *key, const char *value,
 			 const struct config_context *ctx, void *cb)
 {
@@ -507,6 +521,8 @@ static int handle_config(const char *key, const char *value,
 		remote->prune = git_config_bool(key, value);
 	else if (!strcmp(subkey, "prunetags"))
 		remote->prune_tags = git_config_bool(key, value);
+	else if (!strcmp(subkey, "prunebranches"))
+		remote->prune_branches = parse_prune_branches_value(key, value);
 	else if (!strcmp(subkey, "url")) {
 		if (!value)
 			return config_error_nonbool(key);
diff --git a/remote.h b/remote.h
index fc052945ee..5b750c8229 100644
--- a/remote.h
+++ b/remote.h
@@ -28,6 +28,15 @@ enum {
 #endif /* WITH_BREAKING_CHANGES */
 };
 
+enum prune_branches_mode {
+	PRUNE_BRANCHES_UNSPECIFIED = -1,
+	PRUNE_BRANCHES_OFF = 0,
+	PRUNE_BRANCHES_SAFE,
+	PRUNE_BRANCHES_FORCE,
+};
+
+int parse_prune_branches_value(const char *k, const char *v);
+
 struct rewrite {
 	const char *base;
 	size_t baselen;
@@ -102,6 +111,7 @@ struct remote {
 	int mirror;
 	int prune;
 	int prune_tags;
+	int prune_branches;
 
 	/**
 	 * The configured helper programs to run on the remote side, for
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 6fe21e2b3a..5a2ff40132 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -386,6 +386,90 @@ test_expect_success REFFILES 'fetch --prune fails to delete branches' '
 	)
 '
 
+test_expect_success 'fetch.pruneBranches: setup parent' '
+	git init -b main prune-branches-parent &&
+	test_commit -C prune-branches-parent base
+'
+
+test_expect_success 'fetch.pruneBranches=safe deletes merged local branch' '
+	git -C prune-branches-parent branch doomed base &&
+	git clone prune-branches-parent prune-branches-safe &&
+	git -C prune-branches-safe checkout -b doomed --track origin/doomed &&
+	git -C prune-branches-safe checkout -b stay &&
+	git -C prune-branches-parent branch -D doomed &&
+	git -C prune-branches-safe -c fetch.pruneBranches=safe fetch --prune origin &&
+	test_must_fail git -C prune-branches-safe rev-parse refs/remotes/origin/doomed &&
+	test_must_fail git -C prune-branches-safe rev-parse refs/heads/doomed
+'
+
+test_expect_success 'fetch.pruneBranches=safe keeps unmerged local branch' '
+	git -C prune-branches-parent branch doomed base &&
+	git clone prune-branches-parent prune-branches-safe-unmerged &&
+	git -C prune-branches-safe-unmerged checkout -b doomed --track origin/doomed &&
+	test_commit -C prune-branches-safe-unmerged local-only &&
+	git -C prune-branches-safe-unmerged checkout -b stay &&
+	git -C prune-branches-parent branch -D doomed &&
+	git -C prune-branches-safe-unmerged -c fetch.pruneBranches=safe fetch --prune origin 2>err &&
+	test_must_fail git -C prune-branches-safe-unmerged rev-parse refs/remotes/origin/doomed &&
+	git -C prune-branches-safe-unmerged rev-parse refs/heads/doomed &&
+	test_grep "not fully merged" err
+'
+
+test_expect_success 'fetch.pruneBranches=force deletes unmerged local branch' '
+	git -C prune-branches-parent branch doomed base &&
+	git clone prune-branches-parent prune-branches-force &&
+	git -C prune-branches-force checkout -b doomed --track origin/doomed &&
+	test_commit -C prune-branches-force local-only-force &&
+	git -C prune-branches-force checkout -b stay &&
+	git -C prune-branches-parent branch -D doomed &&
+	git -C prune-branches-force -c fetch.pruneBranches=force fetch --prune origin &&
+	test_must_fail git -C prune-branches-force rev-parse refs/remotes/origin/doomed &&
+	test_must_fail git -C prune-branches-force rev-parse refs/heads/doomed
+'
+
+test_expect_success 'fetch.pruneBranches=force never deletes checked-out branch' '
+	git -C prune-branches-parent branch doomed base &&
+	git clone prune-branches-parent prune-branches-checked-out &&
+	git -C prune-branches-checked-out checkout -b doomed --track origin/doomed &&
+	git -C prune-branches-parent branch -D doomed &&
+	git -C prune-branches-checked-out -c fetch.pruneBranches=force fetch --prune origin &&
+	test_must_fail git -C prune-branches-checked-out rev-parse refs/remotes/origin/doomed &&
+	git -C prune-branches-checked-out rev-parse refs/heads/doomed
+'
+
+test_expect_success '--prune-branches deletes merged local branch' '
+	git -C prune-branches-parent branch doomed base &&
+	git clone prune-branches-parent prune-branches-cli &&
+	git -C prune-branches-cli checkout -b doomed --track origin/doomed &&
+	git -C prune-branches-cli checkout -b stay &&
+	git -C prune-branches-parent branch -D doomed &&
+	git -C prune-branches-cli fetch --prune --prune-branches origin &&
+	test_must_fail git -C prune-branches-cli rev-parse refs/heads/doomed
+'
+
+test_expect_success '--no-prune-branches overrides fetch.pruneBranches' '
+	git -C prune-branches-parent branch doomed base &&
+	git clone prune-branches-parent prune-branches-no-cli &&
+	git -C prune-branches-no-cli checkout -b doomed --track origin/doomed &&
+	git -C prune-branches-no-cli checkout -b stay &&
+	git -C prune-branches-no-cli config fetch.pruneBranches force &&
+	git -C prune-branches-parent branch -D doomed &&
+	git -C prune-branches-no-cli fetch --prune --no-prune-branches origin &&
+	git -C prune-branches-no-cli rev-parse refs/heads/doomed
+'
+
+test_expect_success 'remote.<name>.pruneBranches overrides fetch.pruneBranches' '
+	git -C prune-branches-parent branch doomed base &&
+	git clone prune-branches-parent prune-branches-per-remote &&
+	git -C prune-branches-per-remote checkout -b doomed --track origin/doomed &&
+	git -C prune-branches-per-remote checkout -b stay &&
+	git -C prune-branches-per-remote config fetch.pruneBranches force &&
+	git -C prune-branches-per-remote config remote.origin.pruneBranches false &&
+	git -C prune-branches-parent branch -D doomed &&
+	git -C prune-branches-per-remote fetch --prune origin &&
+	git -C prune-branches-per-remote rev-parse refs/heads/doomed
+'
+
 test_expect_success 'fetch --atomic works with a single branch' '
 	test_when_finished "rm -rf atomic" &&
 

base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
-- 
gitgitgadget

Re: [PATCH] fetch: add fetch.pruneLocalBranches config

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Introduce a tri-state config option that, when --prune (or
> fetch.prune / remote.<name>.prune) removes a remote-tracking
> ref, also deletes local branches whose configured upstream is
> that ref.
>
> Values:
> - false (default): no change in behavior.
> - safe: delete only if the local tip is reachable from the
>   upstream tip, preserving any unpushed work.
> - force: delete unconditionally; recoverable only via reflog.
>
> The currently checked-out branch is always preserved.

I do like the feature that allows you to identify which local
branches are already merged and prune them.  It will help users keep
their local branch namespace clean.

I however do not like to see the feature tied to "fetch".  By this,
I do not mean I do not want an option to trigger the feature when
"git fetch" is run.  What I mean is that users should have an option
to prune merged branches without having to fetch first.  And you can
then optionally trigger that machinery from "git fetch".

Of course they aleady can do something silly like

    $ git branch -d $(git branch --list | sed -e 's/^..//')

and remove all the merged branches, but compared to what is
presented here, one thing missing is that you allow pruning the
local branches that are merged only to remote-tracking branches from
a single remote.

To break the feature down to make it easier to use by our users with
various needs and workflows, we would benefit from having a
collection of smaller features that can be composed, like these:

 * "git branch --forked <remote>" lists local branches that build on
   something taken from <remote>s.  The option can be given multiple
   times to make a union of the results from individual "--forked
   <remote>".

   - <remote> may be a name of a remote, e.g., "origin" to mean all
     the remote-tracking branches "refs/remotes/origin/*", 

   - <remote> may be "origin/master" to name a specific
     remote-tracking branch.

   - There may be other handy things to cover with <remote>, like
     "--all" that may act as if you listed all the available
     <remote> on the command line.

 * "git branch --prune-merged <remote>..." is a short-hand for "git
   branch -d $(git branch --forked <remote>...".

 * "git fetch/pull --prune-merged <remote>" can trigger "git branch
   --prune-merged <remote>" after "git fetch" successfully updates
   the remote-tracking branches, which should be equivalent to what
   you have here..

Some local branches that fork from remote and have their initial
round already merged may not want to be pruned, however.  You may
have multi-stage development plans for that topic, and you know
already the second phase would want to build on top of the initial
round, not a random version of the mainline with many topics from
other folks merged in.  So you'd rather want to keep the topic
branch around after your initial round has been merged to the
upstream before you start the second phase.  This is especially true
if your topic is designed to apply to an existing release (in other
words, a bugfix) and you want to keep the second and subsequent
rounds of the topic to be applicable to the same target version
without contaminating the topic with irrelevant features from others
that happened to have been developed and merged upstream around the
same time.

And we'd need to cater to their needs.  By this, I do not mean "they
do not have to use --prune-merged", but by giving them a way to say
"this branch should not be auto-pruned with --prune-merged".

Re: [PATCH] fetch: add fetch.pruneLocalBranches config

[Johannes Sixt]

Am 04.05.26 um 00:39 schrieb Junio C Hamano:
> To break the feature down to make it easier to use by our users with
> various needs and workflows, we would benefit from having a
> collection of smaller features that can be composed, like these:
> 
>  * "git branch --forked <remote>" lists local branches that build on
>    something taken from <remote>s.  The option can be given multiple
>    times to make a union of the results from individual "--forked
>    <remote>".

Clearly, this version of --forked does something very different from the
option `--merged some_branch` that we already have.

> 
>    - <remote> may be a name of a remote, e.g., "origin" to mean all
>      the remote-tracking branches "refs/remotes/origin/*", 
> 
>    - <remote> may be "origin/master" to name a specific
>      remote-tracking branch.
> 
>    - There may be other handy things to cover with <remote>, like
>      "--all" that may act as if you listed all the available
>      <remote> on the command line.
> >  * "git branch --prune-merged <remote>..." is a short-hand for "git
>    branch -d $(git branch --forked <remote>...".

I don't understand this. The option includes the word "merged". Then I
interpret the command to prune only branches that have already been
merged into something (BTW, merged into what?), but as described, the
command removes all local branches that have been forked from some
(remote) branch.

> 
>  * "git fetch/pull --prune-merged <remote>" can trigger "git branch
>    --prune-merged <remote>" after "git fetch" successfully updates
>    the remote-tracking branches, which should be equivalent to what
>    you have here..

I think that the intended behavior is to call the equivalent of `git
branch --merged X | xargs git branch -d` for a suitable set of 'X' to be
determined by `git fetch`.

-- Hannes