{"thread":{"id":"62078","subject":"Thoughts on the \"branch is not fully merged\" error of \"git-branch -d\"","startedAt":"2024-09-07T16:37:52Z","lastAt":"2024-09-08T05:33:29Z","messageCount":6,"participants":["Stefan Haller","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"502391","messageId":"bf6308ce-3914-4b85-a04b-4a9716bac538@haller-berlin.de","threadId":"62078","inReplyTo":null,"subject":"Thoughts on the \"branch is not fully merged\" error of \"git-branch -d\"","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2024-09-07T16:28:38Z","receivedAt":"2024-09-07T16:37:52Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"I frequently get this error when trying to delete a branch that was\nmerged on github, and the remote branch was deleted through github's UI too.\n\nWhen I then fetch and see that \"git branch -v\" shows it as \"[gone]\"), I\nwill want to delete it, but at that time it is pretty random which\nbranch I happen to have checked out. If it's some other unrelated branch\nthat I didn't rebase onto origin/main yet, or if it is main but I didn't\npull yet, then I get the error; but if I'm on main and I have pulled, or\nI'm on an unrelated branch and I have rebased onto origin/main, then I\ndon't.\n\nThis feels arbitrary to me. It would seem more useful to me if the error\nonly appeared if the branch is not contained in any of my local or\nremote branches, because only then do I lose commits. Any thoughts on that?\n\n-Stefan\n"},{"id":"502393","messageId":"xmqqy143wgao.fsf@gitster.g","threadId":"62078","inReplyTo":"bf6308ce-3914-4b85-a04b-4a9716bac538@haller-berlin.de","subject":"Re: Thoughts on the \"branch is not fully merged\" error of \"git-branch -d\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-07T16:59:43Z","receivedAt":"2024-09-07T16:59:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Haller writes:\n\n> I frequently get this error when trying to delete a branch that was\n> merged on github, and the remote branch was deleted through github's UI too.\n>\n> When I then fetch and see that \"git branch -v\" shows it as \"[gone]\"), I\n> will want to delete it, but at that time it is pretty random which\n> branch I happen to have checked out. If it's some other unrelated branch\n> that I didn't rebase onto origin/main yet, or if it is main but I didn't\n> pull yet, then I get the error; but if I'm on main and I have pulled, or\n> I'm on an unrelated branch and I have rebased onto origin/main, then I\n> don't.\n>\n> This feels arbitrary to me. It would seem more useful to me if the error\n> only appeared if the branch is not contained in any of my local or\n> remote branches, because only then do I lose commits. Any thoughts on that?\n\nI think we check against the @{upstream} as well as HEAD these days.\nThe very original design was geared towards folks who do\n\n $ git checkout integration-branch\n $ git merge topic\n $ git branch -d topic\n\nand that was why HEAD is a sensible thing to use as a reference\npoint. What makes the choice _appear_ arbitrary is your being on a\nrandom unrelated branch when you think of using \"branch -d\" ;-)\n\n\"merged to any other branch\" is an absolute no-no. Imagine that I\nhave a branch A, and then tentatively build a wip branch B that I am\nless sure about than branch A on top:\n\n ---o---o---a---a A\n \\\n b---b---b B\n\nThe reason why I said \"tentatively\" is because I fully intend to\nrebase B (these three 'b' commits) on top of an updated A after I\npolish branch A.\n\n b'--b'--b' B\n /\n a---a A\n /\n ---o---o---a---a (old)A\n \\\n b---b---b (old)B\n\nThe tip of branch A deserves the same protection as the tip of\nbranch B from \"git branch -d\", until the whole thing is integrated.\nGranted, you may find A's tip from \"git log B\", but that should not\nbe a reason to allow a mistaken \"git branch -d A\" merely because I\nhappen to have started exploring another idea that may not work at\nall on branch B.\n\nHaving said all that, I do not mind if somebody wanted to further\nextend builtin/branch.c:branch_merged() so that users can explicitly\nconfigure a set of reference branches. \"The 'master' and 'maint'\nare the integration branches that are used in this repository.\nUnless the history of a local branch is fully merged to one of\nthese, 'git branch -d' of such a local branch will stop.\" may be a\nreasonable thing to do.\n\nThanks.\n"},{"id":"502394","messageId":"xmqqtterwfwb.fsf@gitster.g","threadId":"62078","inReplyTo":"xmqqy143wgao.fsf@gitster.g","subject":"Re: Thoughts on the \"branch is not fully merged\" error of \"git-branch -d\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-07T17:08:20Z","receivedAt":"2024-09-07T17:08:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Having said all that, I do not mind if somebody wanted to further\n> extend builtin/branch.c:branch_merged() so that users can explicitly\n> configure a set of reference branches. \"The 'master' and 'maint'\n> are the integration branches that are used in this repository.\n> Unless the history of a local branch is fully merged to one of\n> these, 'git branch -d' of such a local branch will stop.\" may be a\n> reasonable thing to do.\n\nIf anybody is interested in doing this, I think the design should\nalso make sure these branches that are designated as reference\nbranches are protected from 'git branch -d'.\n\n"},{"id":"502395","messageId":"d97a69bc-85f0-46e3-8c99-0e5556ffdc9a@haller-berlin.de","threadId":"62078","inReplyTo":"xmqqy143wgao.fsf@gitster.g","subject":"Re: Thoughts on the \"branch is not fully merged\" error of \"git-branch -d\"","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2024-09-07T18:51:42Z","receivedAt":"2024-09-07T18:51:45Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"On 07.09.24 18:59, Junio C Hamano wrote:\n> Stefan Haller writes:\n> \n>> I frequently get this error when trying to delete a branch that was\n>> merged on github, and the remote branch was deleted through github's UI too.\n>>\n>> When I then fetch and see that \"git branch -v\" shows it as \"[gone]\"), I\n>> will want to delete it, but at that time it is pretty random which\n>> branch I happen to have checked out. If it's some other unrelated branch\n>> that I didn't rebase onto origin/main yet, or if it is main but I didn't\n>> pull yet, then I get the error; but if I'm on main and I have pulled, or\n>> I'm on an unrelated branch and I have rebased onto origin/main, then I\n>> don't.\n>>\n>> This feels arbitrary to me. It would seem more useful to me if the error\n>> only appeared if the branch is not contained in any of my local or\n>> remote branches, because only then do I lose commits. Any thoughts on that?\n> \n> I think we check against the @{upstream} as well as HEAD these days.\n\nYes I know, but in the example I gave, upstream is already gone (maybe I\nshould have added that I have fetch.prune set to true, that's why).\n\n> Having said all that, I do not mind if somebody wanted to further\n> extend builtin/branch.c:branch_merged() so that users can explicitly\n> configure a set of reference branches. \"The 'master' and 'maint'\n> are the integration branches that are used in this repository.\n> Unless the history of a local branch is fully merged to one of\n> these, 'git branch -d' of such a local branch will stop.\" may be a\n> reasonable thing to do.\n\nThis makes sense to me (if you include the upstreams of master and maint\nin that logic, because the local ones might not be up to date).\n\nI should have added that I'm looking at all this from the perspective of\nlazygit, again. Currently, lazygit tries \"git branch -d\" first, and if\nthat errors (yes, it parses the error output, gasp), it prompts the user\nand then uses \"git branch -D\" if they said yes. Since lazygit already\nhas a config for what the main branches are, I'll probably change this\nlogic to do the check on our side, prompt the user if necessary, and use\n-D right away.\n\nThanks for the input!\n\n-Stefan\n"},{"id":"502397","messageId":"xmqqmskjw7wr.fsf@gitster.g","threadId":"62078","inReplyTo":"d97a69bc-85f0-46e3-8c99-0e5556ffdc9a@haller-berlin.de","subject":"Re: Thoughts on the \"branch is not fully merged\" error of \"git-branch -d\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-07T20:00:52Z","receivedAt":"2024-09-07T20:00:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Haller writes:\n\n>> Having said all that, I do not mind if somebody wanted to further\n>> extend builtin/branch.c:branch_merged() so that users can explicitly\n>> configure a set of reference branches. \"The 'master' and 'maint'\n>> are the integration branches that are used in this repository.\n>> Unless the history of a local branch is fully merged to one of\n>> these, 'git branch -d' of such a local branch will stop.\" may be a\n>> reasonable thing to do.\n>\n> This makes sense to me (if you include the upstreams of master and maint\n> in that logic, because the local ones might not be up to date).\n\nI get the idea behind that statement, but I do not think it is\nnecessary to make Git second guess the end user is warranted in this\ncase.\n\nIf refs/heads/master builds on top of refs/remotes/origin/master,\nand if the user is worried about the former being not up to date\nrelative to the latter, then the user can say \"'branch -d' is safe\nif the commit is merged in refs/remotes/origin/master\", instead of\ntelling the command to check with 'refs/heads/master'.\n\n"},{"id":"502405","messageId":"a2bdec84-541a-490b-9456-e99adf400b1c@haller-berlin.de","threadId":"62078","inReplyTo":"xmqqmskjw7wr.fsf@gitster.g","subject":"Re: Thoughts on the \"branch is not fully merged\" error of \"git-branch -d\"","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2024-09-08T05:33:26Z","receivedAt":"2024-09-08T05:33:29Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"On 07.09.24 22:00, Junio C Hamano wrote:\n> Stefan Haller writes:\n> \n>>> Having said all that, I do not mind if somebody wanted to further\n>>> extend builtin/branch.c:branch_merged() so that users can explicitly\n>>> configure a set of reference branches. \"The 'master' and 'maint'\n>>> are the integration branches that are used in this repository.\n>>> Unless the history of a local branch is fully merged to one of\n>>> these, 'git branch -d' of such a local branch will stop.\" may be a\n>>> reasonable thing to do.\n>>\n>> This makes sense to me (if you include the upstreams of master and maint\n>> in that logic, because the local ones might not be up to date).\n> \n> I get the idea behind that statement, but I do not think it is\n> necessary to make Git second guess the end user is warranted in this\n> case.\n> \n> If refs/heads/master builds on top of refs/remotes/origin/master,\n> and if the user is worried about the former being not up to date\n> relative to the latter, then the user can say \"'branch -d' is safe\n> if the commit is merged in refs/remotes/origin/master\", instead of\n> telling the command to check with 'refs/heads/master'.\n\nAh sure, if that configuration takes full refs, then yes, let the user\nconfigure exactly what they want. I thought it would take just bare\nbranch names. (That's what we do in lazygit, we find this more\nconvenient. It does take some heuristics to see which of these actually\nexist, and whether to use the upstream or local one, but it makes it\neasier for users. I'm not proposing to go this way with git.)\n\n-Stefan\n"}]}