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

[Junio C Hamano <gitster@pobox.com>]

"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?