Re: Antw: Re: bug deleting "unmerged" branch (2.12.3)
- From
Philip Oakley <philipoakley@iee.org>
- Date
- Dec 2, 2017, 20:52 UTC
- Message-ID
- <C75A5523697E4772A984765E8535FDE2@PhilipOakley>
- In-Reply-To
- <alpine.DEB.2.21.1.1711291302370.6482@virtualbox>
Hi Ulrich
From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de> To: "Ulrich Windl" <Ulrich.Windl@rz.uni-regensburg.de> Cc: <git@vger.kernel.org> Sent: Wednesday, November 29, 2017 12:27 PM Subject: Re: Antw: Re: bug deleting "unmerged" branch (2.12.3)
Show 94 quoted lines
> Hi Ulrich, > > On Wed, 29 Nov 2017, Ulrich Windl wrote: > >> > On Tue, 28 Nov 2017, Ulrich Windl wrote: >> > >> >> During a rebase that turned out to be heavier than expected 8-( I >> >> decided to keep the old branch by creating a temporary branch name to >> >> the commit of the branch to rebase (which was still the old commit ID >> >> at that time). >> >> >> >> When done rebasing, I attached a new name to the new (rebased) >> >> branch, deleted the old name (pointing at the same rebase commit), >> >> then recreated the old branch from the temporary branch name (created >> >> to remember the commit id). >> >> >> >> When I wanted to delete the temporary branch (which is of no use >> >> now), I got a message that the branch is unmerged. >> > >> > This is actually as designed, at least for performance reasons (it is >> > not exactly cheap to figure out whether a given commit is contained in >> > any other branch). >> > >> >> I think if more than one branches are pointing to the same commit, >> >> one should be allowed to delete all but the last one without warning. >> >> Do you agree? >> > >> > No, respectfully disagree, because I have found myself with branches >> > pointing to the same commit, even if the branches served different >> > purposes. I really like the current behavior where you can delete a >> > branch with `git branch -d` as long as it is contained in its upstream >> > branch. >> >> I'm not talking about the intention of a branch, but of the state of a >> branch: If multiple branches point (not "contain") the same commit, they >> are equivalent (besides the name) at that moment. > > I did a poor job of explaining myself, please let me try again. I'll give > you one concrete example: > > Recently, while working on some topic, I stumbled over a bug and committed > a bug fix, then committed that and branched off a new branch to remind > myself to rebase the bug fix and contribute it. > > At that point, those branches were at the same revision, but distinctly > not equivalent (except in just one, very narrow sense of the word, which I > would argue is the wrong interpretation in this context). > > Sadly, I was called away at that moment to take care of something > completely different. Even if I had not been, the worktree with the first > branch would still have been at that revision for a longer time, as I had > to try out a couple of changes before I could commit. > > This is just one example where the idea backfires that you can safely > delete one of two branches that happen to point at the same commit at the > same time. > > I am sure that you possess vivid enough of an imagination to come up with > plenty more examples where that is the case. > >> As no program can predict the future or the intentions of the user, it >> should be safe to delete the branch, because it can easily be recreated >> (from the remaining branches pointing to the same commit). > > Yes, no program can predict the future (at least *accurately*). > > No, it is not safe to delete that branch. Especially if you take the > current paradigm of "it is safe to delete a branch if it is up-to-date > with, or at least fast-forwardable to, its upstream branch" into account. > > And no, a branch cannot easily be recreated from the remaining branches in > the future, as branches can have different reflogs (and they are lost when > deleting the branch). > >> It shouldn't need a lot of computational power to find out when multiple >> branches point to the same commit. > > Sure, that test can even be scripted easily by using the `git for-each-ref > --points-at=<revision>` command. > > By the way, if you are still convinced that my argument is flawed and that > it should be considered safe to delete a branch if any other branch points > to the same revision, I encourage you to work on a patch to make it so. > > For maximum chance of getting included, you would want to guard this > behind a new config setting, say, branch.deleteRedundantIsSafe, parse it > here: > > https://github.com/git/git/blob/v2.15.1/config.c#L1260-L1288 > > or here: > > https://github.com/git/git/blob/v2.15.1/builtin/branch.c#L78-L97 >
I'd agree that it is easy to misinterpret the message. After close reading of the thread, Junio put his finger on the scenario with:
- "branch -d" protects branches that are yet to be merged to the **current** branch. (my emphasis)
Maybe the error message could say that (what exactly was the error message?), or the documenation be improved to clarify.
Show 21 quoted lines
> document it here: > > https://github.com/git/git/blob/v2.15.1/Documentation/git-branch.txt > > and here: > > https://github.com/git/git/blob/v2.15.1/Documentation/config.txt#L969 > > and handle it here: > > https://github.com/git/git/blob/v2.15.1/builtin/branch.c#L185-L288 > > (look for the places where `force` is used, likely just before the call to > `check_branch_commit()`). > > The way you'd want it to handle is most lilkely by imitating the > `--points-at` code here: > https://github.com/git/git/blob/v2.15.1/builtin/for-each-ref.c#L42 > > Ciao, > Johannes