{"thread":{"id":"48135","subject":"[PATCH] branch -l: print useful info whilst rebasing a non-local branch","startedAt":"2018-03-24T18:39:13Z","lastAt":"2018-04-04T08:09:14Z","messageCount":27,"participants":["Kaartic Sivaraam","Eric Sunshine","Jeff King","Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"342837","messageId":"20180324183844.4565-1-kaartic.sivaraam@gmail.com","threadId":"48135","inReplyTo":null,"subject":"[PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2018-03-24T18:38:44Z","receivedAt":"2018-03-24T18:39:13Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"When rebasing interacitvely (rebase -i), \"git branch -l\" prints a line\nindicating the current branch being rebased. This works well when the\ninteractive rebase was intiated when a local branch is checked out.\n\nThis doesn't play well when the rebase was initiated on a remote\nbranch or an arbitrary commit that is not pointed to by a local\nbranch. In this case \"git branch -l\" tries to print the name of a\nbranch using an unintialized variable and thus tries to print a \"null\npointer string\". As a consequence, it does not provide useful\ninformation while also inducing undefined behaviour.\n\nSo, print the commit from which the rebase started when interactive\nrebasing a non-local branch.\n\nSigned-off-by: Kaartic Sivaraam \n---\n ref-filter.c | 10 +++++++++-\n 1 file changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex f9e25aea7..a4c917c96 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1310,8 +1310,16 @@ char *get_head_description(void)\n \twt_status_get_state(&state, 1);\n \tif (state.rebase_in_progress ||\n \t state.rebase_interactive_in_progress)\n+\t{\n+\t\tconst char *rebasing = NULL;\n+\t\tif (state.branch != NULL)\n+\t\t\trebasing = state.branch;\n+\t\telse\n+\t\t\trebasing = state.detached_from;\n+\n \t\tstrbuf_addf(&desc, _(\"(no branch, rebasing %s)\"),\n-\t\t\t state.branch);\n+\t\t\t rebasing);\n+\t}\n \telse if (state.bisect_in_progress)\n \t\tstrbuf_addf(&desc, _(\"(no branch, bisect started on %s)\"),\n \t\t\t state.branch);\n-- \n2.17.0.rc0.231.g781580f06\n\n"},{"id":"342858","messageId":"CAPig+cQ8xw23SGhpx5qtDEyzJGR1v4L2Lm9tEWe56Rh3c8Q3cg@mail.gmail.com","threadId":"48135","inReplyTo":"20180324183844.4565-1-kaartic.sivaraam@gmail.com","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-03-25T01:34:59Z","receivedAt":"2018-03-25T01:35:05Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Mar 24, 2018 at 2:38 PM, Kaartic Sivaraam\n wrote:\n> When rebasing interacitvely (rebase -i), \"git branch -l\" prints a line\n\nThe \"git branch -l\" threw me since \"-l\" is short for --create-reflog.\nI'm guessing you meant \"git branch --list\".\n\n> indicating the current branch being rebased. This works well when the\n> interactive rebase was intiated when a local branch is checked out.\n>\n> This doesn't play well when the rebase was initiated on a remote\n> branch or an arbitrary commit that is not pointed to by a local\n> branch.\n\nA shorter way of saying \"arbitrary commit ... not pointed at by local\nbranch\" would be \"detached HEAD\".\n\n> In this case \"git branch -l\" tries to print the name of a\n> branch using an unintialized variable and thus tries to print a \"null\n> pointer string\". As a consequence, it does not provide useful\n> information while also inducing undefined behaviour.\n>\n> So, print the commit from which the rebase started when interactive\n> rebasing a non-local branch.\n\nMakes sense. The commit message gives enough information for the\nreader to understand the problem easily.\n\n> Signed-off-by: Kaartic Sivaraam \n> ---\n> diff --git a/ref-filter.c b/ref-filter.c\n> @@ -1310,8 +1310,16 @@ char *get_head_description(void)\n> wt_status_get_state(&state, 1);\n> if (state.rebase_in_progress ||\n> state.rebase_interactive_in_progress)\n> + {\n\nStyle: attach '{' to the line above it (don't make it standalone)\n\n> + const char *rebasing = NULL;\n> + if (state.branch != NULL)\n> + rebasing = state.branch;\n> + else\n> + rebasing = state.detached_from;\n> +\n> strbuf_addf(&desc, _(\"(no branch, rebasing %s)\"),\n> - state.branch);\n> + rebasing);\n\nYou could collapse the whole thing back down to:\n\n strbuf_addf(&desc, _(\"(no branch, rebasing %s)\"),\n state.branch ? state.branch : state.detached_from);\n\nwhich means you don't need the 'rebasing' variable or the braces.\n\n> + }\n> else if (state.bisect_in_progress)\n\nStyle: cuddle 'else' with '}': } else\n\n> strbuf_addf(&desc, _(\"(no branch, bisect started on %s)\"),\n> state.branch);\n\nCan we have a couple new tests: one checking \"git branch --list\" for\nthe typical case (when rebasing off a named branch) and one checking\nwhen rebasing from a detached HEAD?\n"},{"id":"342860","messageId":"87ea8cac-c745-b7e6-7804-5116cd94ed48@gmail.com","threadId":"48135","inReplyTo":"CAPig+cQ8xw23SGhpx5qtDEyzJGR1v4L2Lm9tEWe56Rh3c8Q3cg@mail.gmail.com","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2018-03-25T03:41:34Z","receivedAt":"2018-03-25T03:41:48Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Sunday 25 March 2018 07:04 AM, Eric Sunshine wrote:\n> On Sat, Mar 24, 2018 at 2:38 PM, Kaartic Sivaraam\n> wrote:\n>> When rebasing interacitvely (rebase -i), \"git branch -l\" prints a line\n> \n> The \"git branch -l\" threw me since \"-l\" is short for --create-reflog.\n> I'm guessing you meant \"git branch --list\".\n> \n\nThat's surprising, I just tried \"git branch -l\" on a repository and I\ndid get a list of branch names. Is this a consequence of some option\nparsing weirdness ?!\n\nTo be honest, I actually assumed \"-l\" to be a shorthand for \"--list\" and\ndidn't check with it in the documentation; which I should have. Sorry,\nfor that. I still wonder why \"git branch -l\" prints a list of branch\nnames when it is not a shorthand for \"--list\" ? (BTW, I'm also surprised\nby the fact that \"-l\" is not act shorthand for \"--list\"!)\n\nRegardless, I'll update the commit message to use \"--list\" in place of \"-l\".\n\n\n>> indicating the current branch being rebased. This works well when the\n>> interactive rebase was intiated when a local branch is checked out.\n>>\n>> This doesn't play well when the rebase was initiated on a remote\n>> branch or an arbitrary commit that is not pointed to by a local\n>> branch.\n> \n> A shorter way of saying \"arbitrary commit ... not pointed at by local\n> branch\" would be \"detached HEAD\".\n> \n\nThanks. I was actually searching for this word. It didn't strike when I\nwrote the commit message, yesterday.\n\n\n> You could collapse the whole thing back down to:\n> \n> strbuf_addf(&desc, _(\"(no branch, rebasing %s)\"),\n> state.branch ? state.branch : state.detached_from);\n> \n> which means you don't need the 'rebasing' variable or the braces.\n> \n\nNice point.\n\n\n> Can we have a couple new tests: one checking \"git branch --list\" for\n> the typical case (when rebasing off a named branch) and one checking\n> when rebasing from a detached HEAD?\n> \n\nSure, but I guess it would take some time for me to add the tests. I'll\nsend a v2 with the suggested changes.\n\n\n-- \nKaartic\n\n"},{"id":"342861","messageId":"20180325041056.GA22321@sigill.intra.peff.net","threadId":"48135","inReplyTo":"87ea8cac-c745-b7e6-7804-5116cd94ed48@gmail.com","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-25T04:10:57Z","receivedAt":"2018-03-25T04:11:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 25, 2018 at 09:11:34AM +0530, Kaartic Sivaraam wrote:\n\n> >> When rebasing interacitvely (rebase -i), \"git branch -l\" prints a line\n> > \n> > The \"git branch -l\" threw me since \"-l\" is short for --create-reflog.\n> > I'm guessing you meant \"git branch --list\".\n> \n> That's surprising, I just tried \"git branch -l\" on a repository and I\n> did get a list of branch names. Is this a consequence of some option\n> parsing weirdness ?!\n\nSort of. The \"-l\" option causes us to set the \"reflog\" variable to 1.\nAnd then we have no other command-line options, so we default to\n\"--list\" mode. The listing code does not look at the \"reflog\" variable\nat all, so it's just silently ignored.\n\nSo:\n\n git branch -l\n\n_looks_ like it works, but only because list mode is the default. If you\ndid:\n\n git branch -l foo\n\nyou would find that it does list \"foo\" at all, but instead creates a new\nbranch \"foo\" with reflog.\n\n> To be honest, I actually assumed \"-l\" to be a shorthand for \"--list\" and\n> didn't check with it in the documentation; which I should have. Sorry,\n> for that. I still wonder why \"git branch -l\" prints a list of branch\n> names when it is not a shorthand for \"--list\" ? (BTW, I'm also surprised\n> by the fact that \"-l\" is not act shorthand for \"--list\"!)\n\nIt's historical and quite unfortunate. Doubly so since probably nobody\nhas ever actually wanted to use the short \"-l\" to create a reflog, since\nit's typically the default and has been for a decade.\n\nWe've been hesitant to change it due to backwards compatibility. While\n\"branch\" is generally considered porcelain, it probably is the main\nscripting interface for creating branches (the only other option would\nbe using \"update-ref\" manually). So I dunno. Maybe it would be OK to\ntransition.\n\nAlternatively, we could at least detect the situation that confused you:\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 6d0cea9d4b..89e7fdc89c 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -676,6 +676,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tcolopts = 0;\n \t}\n \n+\tif (list && reflog)\n+\t\tdie(_(\"--reflog in list mode does not make sense\"));\n+\n \tif (force) {\n \t\tdelete *= 2;\n \t\trename *= 2;\n\nThat doesn't help somebody mistakenly doing \"git branch -l foo\", but\nmore likely they'd do \"git branch -l jk/*\" if they were trying to list\nbranches (and then \"branch\" would barf with \"that's not a valid branch\nname\", though that may still leave them quite confused).\n\n-Peff\n"},{"id":"342862","messageId":"CAPig+cSSy2AFc22EOFWLOE1MszHdeA3ijDPbFVNGK70AmHUg_w@mail.gmail.com","threadId":"48135","inReplyTo":"20180325041056.GA22321@sigill.intra.peff.net","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-03-25T04:13:56Z","receivedAt":"2018-03-25T04:14:03Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Mar 25, 2018 at 12:10 AM, Jeff King wrote:\n> So:\n>\n> git branch -l\n>\n> _looks_ like it works, but only because list mode is the default. If you\n> did:\n>\n> git branch -l foo\n>\n> you would find that it does list \"foo\" at all, but instead creates a new\n> branch \"foo\" with reflog.\n\ns/does/doesn't/\n"},{"id":"342863","messageId":"CAPig+cRe9AmFv=GCxPOo5vcLGFuT1qdM60M4KV5P6UN+Ai-QoQ@mail.gmail.com","threadId":"48135","inReplyTo":"20180325041056.GA22321@sigill.intra.peff.net","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-03-25T04:28:30Z","receivedAt":"2018-03-25T04:28:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Mar 25, 2018 at 12:10 AM, Jeff King wrote:\n> Alternatively, we could at least detect the situation that confused you:\n>\n> diff --git a/builtin/branch.c b/builtin/branch.c\n> @@ -676,6 +676,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> + if (list && reflog)\n> + die(_(\"--reflog in list mode does not make sense\"));\n> +\n>\n> That doesn't help somebody mistakenly doing \"git branch -l foo\", but\n> more likely they'd do \"git branch -l jk/*\" if they were trying to list\n> branches (and then \"branch\" would barf with \"that's not a valid branch\n> name\", though that may still leave them quite confused).\n\nAssuming that existing clients of \"-l\" (if there are any) only invoke\n\"git branch -l \" to create a new branch, then it would be\npossible to interpret \"-l\" as --list when is an existing\nbranch. That is, the \"-l\" in \"git branch -l\" and \"git branch -l\n...\" is recognized as --list, and (for backward\ncompatibility only) the \"-l\" in \"git branch -l \" is still\nrecognized as --create-reflog.\n\nThis idea falls flat, however, if there are clients out there which\nactually depend upon \"git branch -l \" failing.\n"},{"id":"342864","messageId":"20180325043337.GA32465@sigill.intra.peff.net","threadId":"48135","inReplyTo":"CAPig+cRe9AmFv=GCxPOo5vcLGFuT1qdM60M4KV5P6UN+Ai-QoQ@mail.gmail.com","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-25T04:33:37Z","receivedAt":"2018-03-25T04:33:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 25, 2018 at 12:28:30AM -0400, Eric Sunshine wrote:\n\n> On Sun, Mar 25, 2018 at 12:10 AM, Jeff King wrote:\n> > Alternatively, we could at least detect the situation that confused you:\n> >\n> > diff --git a/builtin/branch.c b/builtin/branch.c\n> > @@ -676,6 +676,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> > + if (list && reflog)\n> > + die(_(\"--reflog in list mode does not make sense\"));\n> > +\n> >\n> > That doesn't help somebody mistakenly doing \"git branch -l foo\", but\n> > more likely they'd do \"git branch -l jk/*\" if they were trying to list\n> > branches (and then \"branch\" would barf with \"that's not a valid branch\n> > name\", though that may still leave them quite confused).\n> \n> Assuming that existing clients of \"-l\" (if there are any) only invoke\n> \"git branch -l \" to create a new branch, then it would be\n> possible to interpret \"-l\" as --list when is an existing\n> branch. That is, the \"-l\" in \"git branch -l\" and \"git branch -l\n> ...\" is recognized as --list, and (for backward\n> compatibility only) the \"-l\" in \"git branch -l \" is still\n> recognized as --create-reflog.\n> \n> This idea falls flat, however, if there are clients out there which\n> actually depend upon \"git branch -l \" failing.\n\nI agree that might work most of the time as a sort of \"do what I mean\",\nbut I'd prefer to avoid those kinds of magic rules if we can. They're\nvery hard to explain to the user, and can be quite baffling when they go\nwrong.\n\nIMHO we should do one of:\n\n 1. Nothing. ;)\n\n 2. Complain about \"-l\" in list mode to help educate users about the\n current craziness.\n\n 3. Drop \"-l\" (probably with a deprecation period); it seems unlikely\n to me that anybody uses it for branch creation, and this would at\n least reduce the confusion (then it would just be \"so why don't we\n have -l\" instead of \"why is -l not what I expect\").\n\n 4. Repurpose \"-l\" as a shortcut for --list (also after a deprecation\n period). This is slightly more dangerous in that it may confuse\n people using multiple versions of Git that cross the deprecation\n line. But that's kind of what the deprecation period is for...\n\n-Peff\n"},{"id":"342865","messageId":"20180325054824.GA56795@flurp.local","threadId":"48135","inReplyTo":"87ea8cac-c745-b7e6-7804-5116cd94ed48@gmail.com","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-03-25T05:48:24Z","receivedAt":"2018-03-25T05:48:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Mar 25, 2018 at 09:11:34AM +0530, Kaartic Sivaraam wrote:\n> On Sunday 25 March 2018 07:04 AM, Eric Sunshine wrote:\n> > Can we have a couple new tests: one checking \"git branch --list\" for\n> > the typical case (when rebasing off a named branch) and one checking\n> > when rebasing from a detached HEAD?\n> \n> Sure, but I guess it would take some time for me to add the tests. I'll\n> send a v2 with the suggested changes.\n\nA couple more comments:\n\n* Please run the commit message through a spell checker; it contains\n several typographical errors.\n\n* I wonder if it makes sense to give slightly different output in the\n detached HEAD case. Normal output is:\n\n (no branch, rebasing )\n\n and, with your change, detached HEAD output is:\n\n (no branch, rebasing d3adb33f)\n\n which is okay, but perhaps it could be better; for instance:\n\n (no branch, rebasing detached HEAD d3adb33f)\n\nAnyhow, I wrote the tests for you. When you re-roll, you can make the\nfollowing patch 2/2 and your fix 1/2. (If you go with the above idea\nof using a slightly different wording for the detached HEAD case, then\nyou'll need to adjust the 'grep' slightly in the second test.)\n\n--- >8 ---\nFrom: Eric Sunshine \nDate: Sun, 25 Mar 2018 01:29:58 -0400\nSubject: [PATCH] t3200: verify \"branch --list\" sanity when rebasing from\n detached HEAD\n\n\"git branch --list\" shows an in-progress rebase as:\n\n * (no branch, rebasing )\n master\n ...\n\nHowever, if the rebase is started from a detached HEAD, then there is no\n, and it would attempt to print a NULL pointer. The previous\ncommit fixed this problem, so add a test to verify that the output is\nsane in this situation.\n\nSigned-off-by: Eric Sunshine \n---\n t/t3200-branch.sh | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 6c0b7ea4ad..d1f80c80ab 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -6,6 +6,7 @@\n test_description='git branch assorted tests'\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n \n test_expect_success 'prepare a trivial repository' '\n \techo Hello >A &&\n@@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with --no-merged' '\n \ttest_must_fail git branch --merged HEAD --no-merged HEAD\n '\n \n+test_expect_success '--list during rebase' '\n+\ttest_when_finished \"reset_rebase\" &&\n+\tgit checkout master &&\n+\tFAKE_LINES=\"1 edit 2\" &&\n+\texport FAKE_LINES &&\n+\tset_fake_editor &&\n+\tgit rebase -i HEAD~2 &&\n+\tgit branch --list >actual &&\n+\tgrep \"rebasing master\" actual\n+'\n+\n+test_expect_success '--list during rebase from detached HEAD' '\n+\ttest_when_finished \"reset_rebase && git checkout master\" &&\n+\tgit checkout HEAD^0 &&\n+\toid=$(git rev-parse --short HEAD) &&\n+\tFAKE_LINES=\"1 edit 2\" &&\n+\texport FAKE_LINES &&\n+\tset_fake_editor &&\n+\tgit rebase -i HEAD~2 &&\n+\tgit branch --list >actual &&\n+\tgrep \"rebasing $oid\" actual\n+'\n+\n test_expect_success 'tracking with unexpected .fetch refspec' '\n \trm -rf a b c d &&\n \tgit init a &&\n-- \n2.17.0.rc1.321.gba9d0f2565\n--- >8 ---\n"},{"id":"342870","messageId":"75722545-fe6e-322e-0485-70ec6b606cbf@gmail.com","threadId":"48135","inReplyTo":"20180325043337.GA32465@sigill.intra.peff.net","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2018-03-25T06:54:19Z","receivedAt":"2018-03-25T06:54:31Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Sunday 25 March 2018 10:03 AM, Jeff King wrote:\n> ...\n> but I'd prefer to avoid those kinds of magic rules if we can. They're\n> very hard to explain to the user, and can be quite baffling when they go\n> wrong.\n>\n\nI fell the same too.\n\n> IMHO we should do one of:\n> \n> 1. Nothing. ;)\n> \n> 2. Complain about \"-l\" in list mode to help educate users about the\n> current craziness.\n> \n> 3. Drop \"-l\" (probably with a deprecation period); it seems unlikely\n> to me that anybody uses it for branch creation, and this would at\n> least reduce the confusion (then it would just be \"so why don't we\n> have -l\" instead of \"why is -l not what I expect\").\n> \n> 4. Repurpose \"-l\" as a shortcut for --list (also after a deprecation\n> period). This is slightly more dangerous in that it may confuse\n> people using multiple versions of Git that cross the deprecation\n> line. But that's kind of what the deprecation period is for...\n> \n\nI think we should do 2 as a short term fix for sure. For the long term,\nI would prefer 4 as I think most users would expect \"-l\" to be a\nshortcut for \"--list\" particularly given the current situation that \"git\nbranch -l\" lists all the branch names.\n\nThat said, I would not mind considering 3 if 4 has more bad consequences\nthan the good it does (but I heavily doubt it ;-) ).\n\nI don't consider 1 to be an option ;-)\n\n\n-- \nKaartic\n\n"},{"id":"342873","messageId":"CA+P7+xr2-OidiX9ve6GwOR4pSOe4Gn=A3Aow5L=oLZgZE+XqMQ@mail.gmail.com","threadId":"48135","inReplyTo":"20180325043337.GA32465@sigill.intra.peff.net","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2018-03-25T07:15:42Z","receivedAt":"2018-03-25T07:16:07Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Sat, Mar 24, 2018 at 9:33 PM, Jeff King wrote:\n> IMHO we should do one of:\n>\n> 1. Nothing. ;)\n>\n> 2. Complain about \"-l\" in list mode to help educate users about the\n> current craziness.\n>\n\nI think we should do this at a minimum. It's easy, and it doesn't\nbreak any scripts who are doing something sane.\n\n> 3. Drop \"-l\" (probably with a deprecation period); it seems unlikely\n> to me that anybody uses it for branch creation, and this would at\n> least reduce the confusion (then it would just be \"so why don't we\n> have -l\" instead of \"why is -l not what I expect\").\n\nPersonally, I'd prefer this, because it's minimal effort on scripts\npart to fix themselves to use the long option name for reflog, and\ndoesn't cause that much heart burn.\n\n>\n> 4. Repurpose \"-l\" as a shortcut for --list (also after a deprecation\n> period). This is slightly more dangerous in that it may confuse\n> people using multiple versions of Git that cross the deprecation\n> line. But that's kind of what the deprecation period is for...\n>\n> -Peff\n\nI don't think this is particularly all that valuable, since we default\nto list mode so it only helps if you want to pass an argument to the\nlist mode (since otherwise we'd create a branch). Maybe it could be\nuseful, but if we did it, I'd do it as a sort of double deprecation\nperiod where we use one period to remove the -l functionality\nentirely, before adding anything back. I think the *gain* of having -l\nis not really worth it though.\n\nRegards,\nJake\n"},{"id":"342874","messageId":"42ca98f1-e916-a159-27fe-02137f73a525@gmail.com","threadId":"48135","inReplyTo":"20180325054824.GA56795@flurp.local","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2018-03-25T07:36:19Z","receivedAt":"2018-03-25T07:39:57Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Sunday 25 March 2018 11:18 AM, Eric Sunshine wrote:\n> On Sun, Mar 25, 2018 at 09:11:34AM +0530, Kaartic Sivaraam wrote:\n>> On Sunday 25 March 2018 07:04 AM, Eric Sunshine wrote:\n>>> Can we have a couple new tests: one checking \"git branch --list\" for\n>>> the typical case (when rebasing off a named branch) and one checking\n>>> when rebasing from a detached HEAD?\n>>\n>> Sure, but I guess it would take some time for me to add the tests. I'll\n>> send a v2 with the suggested changes.\n> \n> A couple more comments:\n> \n> * Please run the commit message through a spell checker; it contains\n> several typographical errors.\n> \n\nThanks for motivating me to search for a spell checker. I have now\ndiscovered the spell check feature (:set spell) in Vim!\n\n\n> * I wonder if it makes sense to give slightly different output in the\n> detached HEAD case. Normal output is:\n> \n> (no branch, rebasing )\n> \n> and, with your change, detached HEAD output is:\n> \n> (no branch, rebasing d3adb33f)\n> \n> which is okay, but perhaps it could be better; for instance:\n> \n> (no branch, rebasing detached HEAD d3adb33f)\n> \n\nI just recently discovered that the variable used to print information\nrelated to detached HEAD (state.detached_from) might also contain remote\nbranch names (origin/master, etc.) other than commit hashes. So, it\nmight make sense to distinguish detached HEAD.\n\n\n> Anyhow, I wrote the tests for you. When you re-roll, you can make the\n> following patch 2/2 and your fix 1/2.\nThanks a lot!\n\n\n> (If you go with the above idea\n> of using a slightly different wording for the detached HEAD case, then\n> you'll need to adjust the 'grep' slightly in the second test.)\n> \n> --- >8 ---\n> From: Eric Sunshine \n> Date: Sun, 25 Mar 2018 01:29:58 -0400\n> Subject: [PATCH] t3200: verify \"branch --list\" sanity when rebasing from\n> detached HEAD\n> \n> \"git branch --list\" shows an in-progress rebase as:\n> \n> * (no branch, rebasing )\n> master\n> ...\n> \n> However, if the rebase is started from a detached HEAD, then there is no\n> , and it would attempt to print a NULL pointer. The previous\n> commit fixed this problem, so add a test to verify that the output is\n> sane in this situation.\n> \n> Signed-off-by: Eric Sunshine \n> ---\n> t/t3200-branch.sh | 24 ++++++++++++++++++++++++\n> 1 file changed, 24 insertions(+)\n> \n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> index 6c0b7ea4ad..d1f80c80ab 100755\n> --- a/t/t3200-branch.sh\n> +++ b/t/t3200-branch.sh\n> @@ -6,6 +6,7 @@\n> test_description='git branch assorted tests'\n> \n> . ./test-lib.sh\n> +. \"$TEST_DIRECTORY\"/lib-rebase.sh\n> \n> test_expect_success 'prepare a trivial repository' '\n> \techo Hello >A &&\n> @@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with --no-merged' '\n> \ttest_must_fail git branch --merged HEAD --no-merged HEAD\n> '\n> \n> +test_expect_success '--list during rebase' '\n> +\ttest_when_finished \"reset_rebase\" &&\n> +\tgit checkout master &&\n> +\tFAKE_LINES=\"1 edit 2\" &&\n> +\texport FAKE_LINES &&\n> +\tset_fake_editor &&\n> +\tgit rebase -i HEAD~2 &&\n> +\tgit branch --list >actual &&\n> +\tgrep \"rebasing master\" actual\n> +'\n> +\n> +test_expect_success '--list during rebase from detached HEAD' '\n> +\ttest_when_finished \"reset_rebase && git checkout master\" &&\n> +\tgit checkout HEAD^0 &&\n> +\toid=$(git rev-parse --short HEAD) &&\n> +\tFAKE_LINES=\"1 edit 2\" &&\n> +\texport FAKE_LINES &&\n> +\tset_fake_editor &&\n> +\tgit rebase -i HEAD~2 &&\n> +\tgit branch --list >actual &&\n> +\tgrep \"rebasing $oid\" actual\n> +'\n> +\n> test_expect_success 'tracking with unexpected .fetch refspec' '\n> \trm -rf a b c d &&\n> \tgit init a &&\n> \n\n\n-- \nKaartic\n\n"},{"id":"342909","messageId":"xmqq7eq0f5ju.fsf@gitster-ct.c.googlers.com","threadId":"48135","inReplyTo":"20180325043337.GA32465@sigill.intra.peff.net","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-25T17:06:45Z","receivedAt":"2018-03-25T17:06:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> IMHO we should do one of:\n>\n> 1. Nothing. ;)\n>\n> 2. Complain about \"-l\" in list mode to help educate users about the\n> current craziness.\n\nNah. We've seen this, perhaps not often but enough times over long\nperiod of time. The above two would not fly as a longer term\nsolution.\n\n>\n> 3. Drop \"-l\" (probably with a deprecation period); it seems unlikely\n> to me that anybody uses it for branch creation, and this would at\n> least reduce the confusion (then it would just be \"so why don't we\n> have -l\" instead of \"why is -l not what I expect\").\n>\n> 4. Repurpose \"-l\" as a shortcut for --list (also after a deprecation\n> period). This is slightly more dangerous in that it may confuse\n> people using multiple versions of Git that cross the deprecation\n> line. But that's kind of what the deprecation period is for...\n\n3. is prerequisite for 4. If we haven't gone through both in 5\nyears we should be ashamed of ourselves ;-) But at least we should\nstart 3. and aim to finish 3. in 2 years if not sooner.\n"},{"id":"342970","messageId":"20180326072505.GA12436@sigill.intra.peff.net","threadId":"48135","inReplyTo":"CA+P7+xr2-OidiX9ve6GwOR4pSOe4Gn=A3Aow5L=oLZgZE+XqMQ@mail.gmail.com","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-26T07:25:05Z","receivedAt":"2018-03-26T07:25:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 25, 2018 at 12:15:42AM -0700, Jacob Keller wrote:\n\n> > 3. Drop \"-l\" (probably with a deprecation period); it seems unlikely\n> > to me that anybody uses it for branch creation, and this would at\n> > least reduce the confusion (then it would just be \"so why don't we\n> > have -l\" instead of \"why is -l not what I expect\").\n> \n> Personally, I'd prefer this, because it's minimal effort on scripts\n> part to fix themselves to use the long option name for reflog, and\n> doesn't cause that much heart burn.\n> \n> >\n> > 4. Repurpose \"-l\" as a shortcut for --list (also after a deprecation\n> > period). This is slightly more dangerous in that it may confuse\n> > people using multiple versions of Git that cross the deprecation\n> > line. But that's kind of what the deprecation period is for...\n> \n> I don't think this is particularly all that valuable, since we default\n> to list mode so it only helps if you want to pass an argument to the\n> list mode (since otherwise we'd create a branch). Maybe it could be\n> useful, but if we did it, I'd do it as a sort of double deprecation\n> period where we use one period to remove the -l functionality\n> entirely, before adding anything back. I think the *gain* of having -l\n> is not really worth it though.\n\nOK, so here's some patches. We could do the first three now, wait a\nwhile before the fourth, and then wait a while (or never) on the fifth.\n\n [1/5]: t3200: unset core.logallrefupdates when testing reflog creation\n [2/5]: t: switch \"branch -l\" to \"branch --create-reflog\"\n [3/5]: branch: deprecate \"-l\" option\n [4/5]: branch: drop deprecated \"-l\" option\n [5/5]: branch: make \"-l\" a synonym for \"--list\"\n\n Documentation/git-branch.txt | 3 ++-\n builtin/branch.c | 4 ++--\n t/t1410-reflog.sh | 4 ++--\n t/t3200-branch.sh | 34 +++++++++++++++++-----------------\n 4 files changed, 23 insertions(+), 22 deletions(-)\n\n-Peff\n"},{"id":"342971","messageId":"20180326072618.GA12530@sigill.intra.peff.net","threadId":"48135","inReplyTo":"20180326072505.GA12436@sigill.intra.peff.net","subject":"[PATCH 1/5] t3200: unset core.logallrefupdates when testing reflog creation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-26T07:26:18Z","receivedAt":"2018-03-26T07:26:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This test checks that the \"-l\" option creates a reflog. But\nin fact we'd create one even without it, since the default\nin a non-bare repository is to do so. Let's unset the config\nso we can be sure our \"-l\" option is kicking in.\n\nNote that we can't do this with test_config, since that\nwould leave the variable unset after our test finishes,\nconfusing downstream tests (the helper is not smart enough\nto restore the previous value, and just always runs\ntest_unconfig).\n\nSigned-off-by: Jeff King \n---\n t/t3200-branch.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 6c0b7ea4ad..e0c316b71a 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -50,7 +50,7 @@ $_z40 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000\tbranch:\n EOF\n test_expect_success 'git branch -l d/e/f should create a branch and a log' '\n \tGIT_COMMITTER_DATE=\"2005-05-26 23:30\" \\\n-\tgit branch -l d/e/f &&\n+\tgit -c core.logallrefupdates=false branch -l d/e/f &&\n \ttest_path_is_file .git/refs/heads/d/e/f &&\n \ttest_path_is_file .git/logs/refs/heads/d/e/f &&\n \ttest_cmp expect .git/logs/refs/heads/d/e/f\n-- \n2.17.0.rc1.509.g060626845b\n\n"},{"id":"342972","messageId":"20180326072649.GB12530@sigill.intra.peff.net","threadId":"48135","inReplyTo":"20180326072505.GA12436@sigill.intra.peff.net","subject":"[PATCH 2/5] t: switch \"branch -l\" to \"branch --create-reflog\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-26T07:26:50Z","receivedAt":"2018-03-26T07:26:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In preparation for deprecating \"-l\", let's make sure we're\nusing the recommended option ourselves.\n\nThis patch just mechanically converts \"branch -l\" to \"branch\n--create-reflog\". Note that with the exception of the\nactual \"--create-reflog\" test, we could actually remove \"-l\"\nentirely from most of these callers. That's because these\ndays core.logallrefupdates defaults to true in a non-bare\nrepository.\n\nI've left them in place, though, since they serve to\ndocument the expectation of the test, even if they are\ntechnically noops.\n\nSigned-off-by: Jeff King \n---\n t/t1410-reflog.sh | 4 ++--\n t/t3200-branch.sh | 34 +++++++++++++++++-----------------\n 2 files changed, 19 insertions(+), 19 deletions(-)\n\ndiff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh\nindex 553e26d9ce..8293131001 100755\n--- a/t/t1410-reflog.sh\n+++ b/t/t1410-reflog.sh\n@@ -339,8 +339,8 @@ test_expect_failure 'reflog with non-commit entries displays all entries' '\n '\n \n test_expect_success 'reflog expire operates on symref not referrent' '\n-\tgit branch -l the_symref &&\n-\tgit branch -l referrent &&\n+\tgit branch --create-reflog the_symref &&\n+\tgit branch --create-reflog referrent &&\n \tgit update-ref referrent HEAD &&\n \tgit symbolic-ref refs/heads/the_symref refs/heads/referrent &&\n \ttest_when_finished \"rm -f .git/refs/heads/referrent.lock\" &&\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex e0c316b71a..da97b8a62b 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -48,9 +48,9 @@ test_expect_success 'git branch HEAD should fail' '\n cat >expect < 1117150200 +0000\tbranch: Created from master\n EOF\n-test_expect_success 'git branch -l d/e/f should create a branch and a log' '\n+test_expect_success 'git branch --create-reflog d/e/f should create a branch and a log' '\n \tGIT_COMMITTER_DATE=\"2005-05-26 23:30\" \\\n-\tgit -c core.logallrefupdates=false branch -l d/e/f &&\n+\tgit -c core.logallrefupdates=false branch --create-reflog d/e/f &&\n \ttest_path_is_file .git/refs/heads/d/e/f &&\n \ttest_path_is_file .git/logs/refs/heads/d/e/f &&\n \ttest_cmp expect .git/logs/refs/heads/d/e/f\n@@ -81,7 +81,7 @@ test_expect_success 'git branch -m dumps usage' '\n \n test_expect_success 'git branch -m m broken_symref should work' '\n \ttest_when_finished \"git branch -D broken_symref\" &&\n-\tgit branch -l m &&\n+\tgit branch --create-reflog m &&\n \tgit symbolic-ref refs/heads/broken_symref refs/heads/i_am_broken &&\n \tgit branch -m m broken_symref &&\n \tgit reflog exists refs/heads/broken_symref &&\n@@ -89,13 +89,13 @@ test_expect_success 'git branch -m m broken_symref should work' '\n '\n \n test_expect_success 'git branch -m m m/m should work' '\n-\tgit branch -l m &&\n+\tgit branch --create-reflog m &&\n \tgit branch -m m m/m &&\n \tgit reflog exists refs/heads/m/m\n '\n \n test_expect_success 'git branch -m n/n n should work' '\n-\tgit branch -l n/n &&\n+\tgit branch --create-reflog n/n &&\n \tgit branch -m n/n n &&\n \tgit reflog exists refs/heads/n\n '\n@@ -377,9 +377,9 @@ mv .git/config-saved .git/config\n git config branch.s/s.dummy Hello\n \n test_expect_success 'git branch -m s/s s should work when s/t is deleted' '\n-\tgit branch -l s/s &&\n+\tgit branch --create-reflog s/s &&\n \tgit reflog exists refs/heads/s/s &&\n-\tgit branch -l s/t &&\n+\tgit branch --create-reflog s/t &&\n \tgit reflog exists refs/heads/s/t &&\n \tgit branch -d s/t &&\n \tgit branch -m s/s s &&\n@@ -443,7 +443,7 @@ test_expect_success 'git branch --copy dumps usage' '\n '\n \n test_expect_success 'git branch -c d e should work' '\n-\tgit branch -l d &&\n+\tgit branch --create-reflog d &&\n \tgit reflog exists refs/heads/d &&\n \tgit config branch.d.dummy Hello &&\n \tgit branch -c d e &&\n@@ -458,7 +458,7 @@ test_expect_success 'git branch -c d e should work' '\n '\n \n test_expect_success 'git branch --copy is a synonym for -c' '\n-\tgit branch -l copy &&\n+\tgit branch --create-reflog copy &&\n \tgit reflog exists refs/heads/copy &&\n \tgit config branch.copy.dummy Hello &&\n \tgit branch --copy copy copy-to &&\n@@ -485,7 +485,7 @@ test_expect_success 'git branch -c ee ef should copy ee to create branch ef' '\n '\n \n test_expect_success 'git branch -c f/f g/g should work' '\n-\tgit branch -l f/f &&\n+\tgit branch --create-reflog f/f &&\n \tgit reflog exists refs/heads/f/f &&\n \tgit config branch.f/f.dummy Hello &&\n \tgit branch -c f/f g/g &&\n@@ -496,7 +496,7 @@ test_expect_success 'git branch -c f/f g/g should work' '\n '\n \n test_expect_success 'git branch -c m2 m2 should work' '\n-\tgit branch -l m2 &&\n+\tgit branch --create-reflog m2 &&\n \tgit reflog exists refs/heads/m2 &&\n \tgit config branch.m2.dummy Hello &&\n \tgit branch -c m2 m2 &&\n@@ -505,18 +505,18 @@ test_expect_success 'git branch -c m2 m2 should work' '\n '\n \n test_expect_success 'git branch -c zz zz/zz should fail' '\n-\tgit branch -l zz &&\n+\tgit branch --create-reflog zz &&\n \tgit reflog exists refs/heads/zz &&\n \ttest_must_fail git branch -c zz zz/zz\n '\n \n test_expect_success 'git branch -c b/b b should fail' '\n-\tgit branch -l b/b &&\n+\tgit branch --create-reflog b/b &&\n \ttest_must_fail git branch -c b/b b\n '\n \n test_expect_success 'git branch -C o/q o/p should work when o/p exists' '\n-\tgit branch -l o/q &&\n+\tgit branch --create-reflog o/q &&\n \tgit reflog exists refs/heads/o/q &&\n \tgit reflog exists refs/heads/o/p &&\n \tgit branch -C o/q o/p\n@@ -569,10 +569,10 @@ test_expect_success 'git branch -C master5 master5 should work when master is ch\n '\n \n test_expect_success 'git branch -C ab cd should overwrite existing config for cd' '\n-\tgit branch -l cd &&\n+\tgit branch --create-reflog cd &&\n \tgit reflog exists refs/heads/cd &&\n \tgit config branch.cd.dummy CD &&\n-\tgit branch -l ab &&\n+\tgit branch --create-reflog ab &&\n \tgit reflog exists refs/heads/ab &&\n \tgit config branch.ab.dummy AB &&\n \tgit branch -C ab cd &&\n@@ -684,7 +684,7 @@ test_expect_success 'renaming a symref is not allowed' '\n '\n \n test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '\n-\tgit branch -l u &&\n+\tgit branch --create-reflog u &&\n \tmv .git/logs/refs/heads/u real-u &&\n \tln -s real-u .git/logs/refs/heads/u &&\n \ttest_must_fail git branch -m u v\n-- \n2.17.0.rc1.509.g060626845b\n\n"},{"id":"342973","messageId":"20180326072839.GC12530@sigill.intra.peff.net","threadId":"48135","inReplyTo":"20180326072505.GA12436@sigill.intra.peff.net","subject":"[PATCH 3/5] branch: deprecate \"-l\" option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-26T07:28:39Z","receivedAt":"2018-03-26T07:28:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The \"-l\" option is short for \"--create-reflog\". This has\ncaused much confusion over the years. Most people expect it\nto work as \"--list\", because that would match the other\n\"mode\" options like -d/--delete and -m/--move, as well as\nthe similar -l/--list option of git-tag.\n\nAdding to the confusion, using \"-l\" _appears_ to work as\n\"--list\" in some cases:\n\n $ git branch -l\n * master\n\nbecause the branch command defaults to listing (so even\ntrying to specify --list in the command above is redundant).\nBut that may bite the user later when they add a pattern,\nlike:\n\n $ git branch -l foo\n\nwhich does not return an empty list, but in fact creates a\nnew branch (with a reflog, naturally) called \"foo\".\n\nIt's also probably quite uncommon for people to actually use\n\"-l\" to create a reflog. Since 0bee591869 (Enable reflogs by\ndefault in any repository with a working directory.,\n2006-12-14), this is the default in non-bare repositories.\nSo it's rather unfortunate that the feature squats on the\nshort-and-sweet \"-l\" (which was only added in 3a4b3f269c\n(Create/delete branch ref logs., 2006-05-19), meaning there\nwere only 7 months where it was actually useful).\n\nLet's deprecate \"-l\" in hopes of eventually dropping it\n(it's a little too soon to repurpose it to \"--list\", but we\nmay even do that eventually).\n\nSigned-off-by: Jeff King \n---\n Documentation/git-branch.txt | 3 ++-\n builtin/branch.c | 17 ++++++++++++++++-\n 2 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt\nindex b3084c99c1..b959df1cbf 100644\n--- a/Documentation/git-branch.txt\n+++ b/Documentation/git-branch.txt\n@@ -91,7 +91,6 @@ OPTIONS\n -D::\n \tShortcut for `--delete --force`.\n \n--l::\n --create-reflog::\n \tCreate the branch's reflog. This activates recording of\n \tall changes made to the branch ref, enabling use of date\n@@ -101,6 +100,8 @@ OPTIONS\n \tThe negated form `--no-create-reflog` only overrides an earlier\n \t`--create-reflog`, but currently does not negate the setting of\n \t`core.logAllRefUpdates`.\n++\n+The `-l` option is a deprecated synonym for `--create-reflog`.\n \n -f::\n --force::\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 6d0cea9d4b..e50a5a1680 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -570,6 +570,15 @@ static int edit_branch_description(const char *branch_name)\n \treturn 0;\n }\n \n+static int deprecated_reflog_option_cb(const struct option *opt,\n+\t\t\t\t const char *arg, int unset)\n+{\n+\twarning(\"the '-l' alias for '--create-reflog' is deprecated;\");\n+\twarning(\"it will be removed in a future version of Git\");\n+\t*(int *)opt->value = !unset;\n+\treturn 0;\n+}\n+\n int cmd_branch(int argc, const char **argv, const char *prefix)\n {\n \tint delete = 0, rename = 0, copy = 0, force = 0, list = 0;\n@@ -612,7 +621,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('c', \"copy\", ©, N_(\"copy a branch and its reflog\"), 1),\n \t\tOPT_BIT('C', NULL, ©, N_(\"copy a branch, even if target exists\"), 2),\n \t\tOPT_BOOL(0, \"list\", &list, N_(\"list branch names\")),\n-\t\tOPT_BOOL('l', \"create-reflog\", &reflog, N_(\"create the branch's reflog\")),\n+\t\tOPT_BOOL(0, \"create-reflog\", &reflog, N_(\"create the branch's reflog\")),\n+\t\t{\n+\t\t\tOPTION_CALLBACK, 'l', NULL, &reflog, NULL,\n+\t\t\tN_(\"deprecated synonym for --create-reflog\"),\n+\t\t\tPARSE_OPT_NOARG | PARSE_OPT_HIDDEN,\n+\t\t\tdeprecated_reflog_option_cb\n+\t\t},\n \t\tOPT_BOOL(0, \"edit-description\", &edit_description,\n \t\t\t N_(\"edit the description for the branch\")),\n \t\tOPT__FORCE(&force, N_(\"force creation, move/rename, deletion\"), PARSE_OPT_NOCOMPLETE),\n-- \n2.17.0.rc1.509.g060626845b\n\n"},{"id":"342974","messageId":"20180326072922.GD12530@sigill.intra.peff.net","threadId":"48135","inReplyTo":"20180326072505.GA12436@sigill.intra.peff.net","subject":"[PATCH 4/5] branch: drop deprecated \"-l\" option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-26T07:29:22Z","receivedAt":"2018-03-26T07:29:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We marked the \"-l\" option as deprecated back in . Now that sufficient time has passed, let's follow\nthrough and get rid of it.\n\nSigned-off-by: Jeff King \n---\nI'll need some help from the maintainer on the commit message. :)\n\n builtin/branch.c | 15 ---------------\n 1 file changed, 15 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex e50a5a1680..f7cd333587 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -570,15 +570,6 @@ static int edit_branch_description(const char *branch_name)\n \treturn 0;\n }\n \n-static int deprecated_reflog_option_cb(const struct option *opt,\n-\t\t\t\t const char *arg, int unset)\n-{\n-\twarning(\"the '-l' alias for '--create-reflog' is deprecated;\");\n-\twarning(\"it will be removed in a future version of Git\");\n-\t*(int *)opt->value = !unset;\n-\treturn 0;\n-}\n-\n int cmd_branch(int argc, const char **argv, const char *prefix)\n {\n \tint delete = 0, rename = 0, copy = 0, force = 0, list = 0;\n@@ -622,12 +613,6 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('C', NULL, ©, N_(\"copy a branch, even if target exists\"), 2),\n \t\tOPT_BOOL(0, \"list\", &list, N_(\"list branch names\")),\n \t\tOPT_BOOL(0, \"create-reflog\", &reflog, N_(\"create the branch's reflog\")),\n-\t\t{\n-\t\t\tOPTION_CALLBACK, 'l', NULL, &reflog, NULL,\n-\t\t\tN_(\"deprecated synonym for --create-reflog\"),\n-\t\t\tPARSE_OPT_NOARG | PARSE_OPT_HIDDEN,\n-\t\t\tdeprecated_reflog_option_cb\n-\t\t},\n \t\tOPT_BOOL(0, \"edit-description\", &edit_description,\n \t\t\t N_(\"edit the description for the branch\")),\n \t\tOPT__FORCE(&force, N_(\"force creation, move/rename, deletion\"), PARSE_OPT_NOCOMPLETE),\n-- \n2.17.0.rc1.509.g060626845b\n\n"},{"id":"342975","messageId":"20180326072947.GE12530@sigill.intra.peff.net","threadId":"48135","inReplyTo":"20180326072505.GA12436@sigill.intra.peff.net","subject":"[PATCH 5/5] branch: make \"-l\" a synonym for \"--list\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-26T07:29:48Z","receivedAt":"2018-03-26T07:29:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The other \"mode\" options of git-branch have short-option\naliases that are easy to type (e.g., \"-d\" and \"-m\"). Let's\ngive \"--list\" the same treatment.\n\nThis also makes it consistent with the similar \"git tag -l\"\noption.\n\nWe didn't do this originally because \"--create-reflog\" was\nsquatting on the \"-l\" option. Now that sufficient time has\npassed with that alias removed, we can finally repurpose it.\n\nSigned-off-by: Jeff King \n---\n builtin/branch.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex f7cd333587..fd55e9720e 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -611,7 +611,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('M', NULL, &rename, N_(\"move/rename a branch, even if target exists\"), 2),\n \t\tOPT_BIT('c', \"copy\", ©, N_(\"copy a branch and its reflog\"), 1),\n \t\tOPT_BIT('C', NULL, ©, N_(\"copy a branch, even if target exists\"), 2),\n-\t\tOPT_BOOL(0, \"list\", &list, N_(\"list branch names\")),\n+\t\tOPT_BOOL('l', \"list\", &list, N_(\"list branch names\")),\n \t\tOPT_BOOL(0, \"create-reflog\", &reflog, N_(\"create the branch's reflog\")),\n \t\tOPT_BOOL(0, \"edit-description\", &edit_description,\n \t\t\t N_(\"edit the description for the branch\")),\n-- \n2.17.0.rc1.509.g060626845b\n"},{"id":"342976","messageId":"CAPig+cTcqSa6AfeMQivnSdL=y2+WWw2MtSavDciMc84RcKURMA@mail.gmail.com","threadId":"48135","inReplyTo":"20180326072505.GA12436@sigill.intra.peff.net","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-03-26T07:44:39Z","receivedAt":"2018-03-26T07:44:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 26, 2018 at 3:25 AM, Jeff King wrote:\n> OK, so here's some patches. We could do the first three now, wait a\n> while before the fourth, and then wait a while (or never) on the fifth.\n>\n> [1/5]: t3200: unset core.logallrefupdates when testing reflog creation\n> [2/5]: t: switch \"branch -l\" to \"branch --create-reflog\"\n> [3/5]: branch: deprecate \"-l\" option\n> [4/5]: branch: drop deprecated \"-l\" option\n> [5/5]: branch: make \"-l\" a synonym for \"--list\"\n\nThe entire series looks good to me. FWIW,\n\nReviewed-by: Eric Sunshine \n"},{"id":"343049","messageId":"CA+P7+xp3QMzpqDaB0O_kza+bBcP1vM6Nm_u0=D0tzDsduhnmEQ@mail.gmail.com","threadId":"48135","inReplyTo":"CAPig+cTcqSa6AfeMQivnSdL=y2+WWw2MtSavDciMc84RcKURMA@mail.gmail.com","subject":"Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2018-03-26T18:38:18Z","receivedAt":"2018-03-26T18:38:45Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Mon, Mar 26, 2018 at 12:44 AM, Eric Sunshine wrote:\n> On Mon, Mar 26, 2018 at 3:25 AM, Jeff King wrote:\n>> OK, so here's some patches. We could do the first three now, wait a\n>> while before the fourth, and then wait a while (or never) on the fifth.\n>>\n>> [1/5]: t3200: unset core.logallrefupdates when testing reflog creation\n>> [2/5]: t: switch \"branch -l\" to \"branch --create-reflog\"\n>> [3/5]: branch: deprecate \"-l\" option\n>> [4/5]: branch: drop deprecated \"-l\" option\n>> [5/5]: branch: make \"-l\" a synonym for \"--list\"\n>\n> The entire series looks good to me. FWIW,\n>\n> Reviewed-by: Eric Sunshine \n\nSame to me.\n\nReviewed-by: Jacob Keller \n\nThanks,\nJake\n"},{"id":"343653","messageId":"20180403043101.4072-1-kaartic.sivaraam@gmail.com","threadId":"48135","inReplyTo":"20180324183844.4565-1-kaartic.sivaraam@gmail.com","subject":"[PATCH v2 1/2] branch --list: print useful info whilst interactive rebasing a detached HEAD","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2018-04-03T04:31:00Z","receivedAt":"2018-04-03T04:31:28Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"When rebasing interactively (rebase -i), \"git branch --list\" prints\na line indicating the current branch being rebased. This works well\nwhen the interactive rebase is initiated when a local branch is\nchecked out.\n\nThis doesn't play well when the rebase is initiated on a detached\nHEAD. When \"git branch --list\" tries to print information related\nto the interactive rebase in this case it tries to print the name\nof a branch using an uninitialized variable and thus tries to\nprint a \"null pointer string\". As a consequence, it does not provide\nuseful information while also inducing undefined behaviour.\n\nSo, print the point from which the rebase was started when interactive\nrebasing a detached HEAD.\n\nSigned-off-by: Kaartic Sivaraam \n---\n ref-filter.c | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex f9e25aea7..db2baedfe 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1309,10 +1309,14 @@ char *get_head_description(void)\n \tmemset(&state, 0, sizeof(state));\n \twt_status_get_state(&state, 1);\n \tif (state.rebase_in_progress ||\n-\t state.rebase_interactive_in_progress)\n-\t\tstrbuf_addf(&desc, _(\"(no branch, rebasing %s)\"),\n-\t\t\t state.branch);\n-\telse if (state.bisect_in_progress)\n+\t state.rebase_interactive_in_progress) {\n+\t\tif (state.branch)\n+\t\t\tstrbuf_addf(&desc, _(\"(no branch, rebasing %s)\"),\n+\t\t\t\t state.branch);\n+\t\telse\n+\t\t\tstrbuf_addf(&desc, _(\"(no branch, rebasing detached HEAD %s)\"),\n+\t\t\t\t state.detached_from);\n+\t} else if (state.bisect_in_progress)\n \t\tstrbuf_addf(&desc, _(\"(no branch, bisect started on %s)\"),\n \t\t\t state.branch);\n \telse if (state.detached_from) {\n-- \n2.17.0.rc0.231.g781580f06\n\n"},{"id":"343654","messageId":"20180403043101.4072-2-kaartic.sivaraam@gmail.com","threadId":"48135","inReplyTo":"20180403043101.4072-1-kaartic.sivaraam@gmail.com","subject":"[PATCH v2 2/2] t3200: verify \"branch --list\" sanity when rebasing from detached HEAD","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2018-04-03T04:31:01Z","receivedAt":"2018-04-03T04:31:41Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"From: Eric Sunshine \n\n\"git branch --list\" shows an in-progress rebase as:\n\n * (no branch, rebasing )\n master\n ...\n\nHowever, if the rebase is started from a detached HEAD, then there is no\n, and it would attempt to print a NULL pointer. The previous\ncommit fixed this problem, so add a test to verify that the output is\nsane in this situation.\n\nNote that the \"detached HEAD\" test case might actually fail in some cases\nas the actual output of \"git branch --list\" might contain remote branch\nnames which is not considered by the test case as it is rare to happen\nin the test environment.\n\nSigned-off-by: Eric Sunshine \nSigned-off-by: Kaartic Sivaraam \n---\n t/t3200-branch.sh | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 503a88d02..738b5eb22 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -6,6 +6,7 @@\n test_description='git branch assorted tests'\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n \n test_expect_success 'prepare a trivial repository' '\n \techo Hello >A &&\n@@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with --no-merged' '\n \ttest_must_fail git branch --merged HEAD --no-merged HEAD\n '\n \n+test_expect_success '--list during rebase' '\n+\ttest_when_finished \"reset_rebase\" &&\n+\tgit checkout master &&\n+\tFAKE_LINES=\"1 edit 2\" &&\n+\texport FAKE_LINES &&\n+\tset_fake_editor &&\n+\tgit rebase -i HEAD~2 &&\n+\tgit branch --list >actual &&\n+\ttest_i18ngrep \"rebasing master\" actual\n+'\n+\n+test_expect_success '--list during rebase from detached HEAD' '\n+\ttest_when_finished \"reset_rebase && git checkout master\" &&\n+\tgit checkout HEAD^0 &&\n+\toid=$(git rev-parse --short HEAD) &&\n+\tFAKE_LINES=\"1 edit 2\" &&\n+\texport FAKE_LINES &&\n+\tset_fake_editor &&\n+\tgit rebase -i HEAD~2 &&\n+\tgit branch --list >actual &&\n+\ttest_i18ngrep \"rebasing detached HEAD $oid\" actual\n+'\n+\n test_expect_success 'tracking with unexpected .fetch refspec' '\n \trm -rf a b c d &&\n \tgit init a &&\n-- \n2.17.0.rc0.231.g781580f06\n\n"},{"id":"343655","messageId":"3566c82c-114a-ec2d-286c-2851e4b2952d@gmail.com","threadId":"48135","inReplyTo":"20180403043101.4072-1-kaartic.sivaraam@gmail.com","subject":"[PATCH v2 0/2] branch --list: print useful info whilst interactive rebasing a detached HEAD","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2018-04-03T05:02:31Z","receivedAt":"2018-04-03T05:02:44Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"Ok, I seem to have forgotten the cover letter. So, here's one.\n\nThe changes from v1 are as follows:\n\n* Changes to the commit message of 1/2 to fix some errors\n\n* Code changes to 1/2 to address the comments from v1\n\n* Patch 2/2 is new. It's adds tests for the issue that 1/2 tries to fix.\n It's written by Eric with a little fix by me to make it work with\n GETTEXT_POISON.\n\nAn interdiff for 1/2:\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex a4c917c96..db2baedfe 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1309,18 +1309,14 @@ char *get_head_description(void)\n memset(&state, 0, sizeof(state));\n wt_status_get_state(&state, 1);\n if (state.rebase_in_progress ||\n- state.rebase_interactive_in_progress)\n- {\n- const char *rebasing = NULL;\n- if (state.branch != NULL)\n- rebasing = state.branch;\n+ state.rebase_interactive_in_progress) {\n+ if (state.branch)\n+ strbuf_addf(&desc, _(\"(no branch, rebasing %s)\"),\n+ state.branch);\n else\n- rebasing = state.detached_from;\n-\n- strbuf_addf(&desc, _(\"(no branch, rebasing %s)\"),\n- rebasing);\n- }\n- else if (state.bisect_in_progress)\n+ strbuf_addf(&desc, _(\"(no branch, rebasing\ndetached HEAD %s)\"),\n+ state.detached_from);\n+ } else if (state.bisect_in_progress)\n strbuf_addf(&desc, _(\"(no branch, bisect started on %s)\"),\n state.branch);\n else if (state.detached_from) {\n\n\n\nEric Sunshine (1):\n t3200: verify \"branch --list\" sanity when rebasing from detached HEAD\n\nKaartic Sivaraam (1):\n branch --list: print useful info whilst interactive rebasing a\n detached HEAD\n\n ref-filter.c | 12 ++++++++----\n t/t3200-branch.sh | 24 ++++++++++++++++++++++++\n 2 files changed, 32 insertions(+), 4 deletions(-)\n\n"},{"id":"343658","messageId":"CAPig+cSrAN2LgL1dAEUoR4PJk-rUzHdqTusXm8MYUn7p6G4puQ@mail.gmail.com","threadId":"48135","inReplyTo":"20180403043101.4072-2-kaartic.sivaraam@gmail.com","subject":"Re: [PATCH v2 2/2] t3200: verify \"branch --list\" sanity when rebasing from detached HEAD","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-03T08:00:36Z","receivedAt":"2018-04-03T08:00:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Apr 3, 2018 at 12:31 AM, Kaartic Sivaraam\n wrote:\n> From: Eric Sunshine \n>\n> \"git branch --list\" shows an in-progress rebase as:\n>\n> * (no branch, rebasing )\n> master\n> ...\n>\n> However, if the rebase is started from a detached HEAD, then there is no\n> , and it would attempt to print a NULL pointer. The previous\n> commit fixed this problem, so add a test to verify that the output is\n> sane in this situation.\n>\n> Note that the \"detached HEAD\" test case might actually fail in some cases\n> as the actual output of \"git branch --list\" might contain remote branch\n> names which is not considered by the test case as it is rare to happen\n> in the test environment.\n\nThis paragraph was not in the original patch[1]. I _think_ what you\nare saying (which took a while to decipher) is that if a command such\nas \"git checkout origin/next\" ever gets inserted into the script\nbefore the test, the test will be fooled since \"git branch --list\"\nwill show \"detached HEAD origin/next\" rather than \"detached HEAD\nd3adb33f\", the latter of which is what the test is expecting.\n\nUnfortunately, this paragraph makes it sound as if the test can fail\nrandomly (which, I believe, is not the case), and nobody would want a\ntest added which is unreliable, thus this paragraph is not helping to\nsell this patch (in fact, it's actively hurting it). Ideally, the test\nshould be entirely deterministic so that it can't be fooled like this.\nRather than including this (harmful) paragraph in the commit message,\nlet's ensure that the test is deterministic (see below).\n\n[1]: https://public-inbox.org/git/20180325054824.GA56795@flurp.local/\n\n> Signed-off-by: Eric Sunshine \n> Signed-off-by: Kaartic Sivaraam \n> ---\n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> @@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with --no-merged' '\n> +test_expect_success '--list during rebase from detached HEAD' '\n> + test_when_finished \"reset_rebase && git checkout master\" &&\n> + git checkout HEAD^0 &&\n\nThis is the line which I think is causing concern for you. If someone\ninserted a new test before this one which invoked \"git checkout\norigin/next\" (or something), then even after \"git checkout HEAD^0\",\n\"git branch --list\" would still report the unexpected \"detached HEAD\norigin/next\". Let's fix this, and make the test deterministic, by\ndoing this instead:\n\n git checkout master^0 &&\n\nThanks.\n\n> + oid=$(git rev-parse --short HEAD) &&\n> + FAKE_LINES=\"1 edit 2\" &&\n> + export FAKE_LINES &&\n> + set_fake_editor &&\n> + git rebase -i HEAD~2 &&\n> + git branch --list >actual &&\n> + test_i18ngrep \"rebasing detached HEAD $oid\" actual\n> +'\n"},{"id":"343686","messageId":"7f07a76a-c467-11b9-1d93-233c0892077d@gmail.com","threadId":"48135","inReplyTo":"CAPig+cSrAN2LgL1dAEUoR4PJk-rUzHdqTusXm8MYUn7p6G4puQ@mail.gmail.com","subject":"Re: [PATCH v2 2/2] t3200: verify \"branch --list\" sanity when rebasing from detached HEAD","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2018-04-03T12:58:43Z","receivedAt":"2018-04-03T12:59:00Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Tuesday 03 April 2018 01:30 PM, Eric Sunshine wrote:\n>> Note that the \"detached HEAD\" test case might actually fail in some cases\n>> as the actual output of \"git branch --list\" might contain remote branch\n>> names which is not considered by the test case as it is rare to happen\n>> in the test environment.\n> \n> This paragraph was not in the original patch[1]. I _think_ what you\n> are saying (which took a while to decipher) is that if a command such\n> as \"git checkout origin/next\" ever gets inserted into the script\n> before the test, the test will be fooled since \"git branch --list\"\n> will show \"detached HEAD origin/next\" rather than \"detached HEAD\n> d3adb33f\", the latter of which is what the test is expecting.\n> \n\nYeah, you're right. To know the reason for the unclear paragraph, see below.\n\n\n> Unfortunately, this paragraph makes it sound as if the test can fail\n> randomly (which, I believe, is not the case), and nobody would want a\n> test added which is unreliable, thus this paragraph is not helping to\n> sell this patch (in fact, it's actively hurting it). Ideally, the test\n> should be entirely deterministic so that it can't be fooled like this.\n> Rather than including this (harmful) paragraph in the commit message,\n> let's ensure that the test is deterministic (see below).\n> \n\nSorry for the harmful and not so clear paragraph! I actually kept that\nparagraph there to **remind me** that I have to fix the issue which it\ndescribes before sending out the patch but I somehow forgot about it\nafter I added it initially :-(\n\n\n>> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n>> @@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with --no-merged' '\n>> +test_expect_success '--list during rebase from detached HEAD' '\n>> + test_when_finished \"reset_rebase && git checkout master\" &&\n>> + git checkout HEAD^0 &&\n> \n> This is the line which I think is causing concern for you. If someone\n> inserted a new test before this one which invoked \"git checkout\n> origin/next\" (or something), then even after \"git checkout HEAD^0\",\n> \"git branch --list\" would still report the unexpected \"detached HEAD\n> origin/next\". Let's fix this, and make the test deterministic, by\n> doing this instead:\n> \n> git checkout master^0 &&\n>\n\nNice idea, will re-send a v3 with this fix and the harmful paragraph\nremoved.\n\n\nThanks,\nKaartic\n\n"},{"id":"343698","messageId":"20180403144715.11174-1-kaartic.sivaraam@gmail.com","threadId":"48135","inReplyTo":"CAPig+cSrAN2LgL1dAEUoR4PJk-rUzHdqTusXm8MYUn7p6G4puQ@mail.gmail.com","subject":"[PATCH v3 2/2] t3200: verify \"branch --list\" sanity when rebasing from detached HEAD","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2018-04-03T14:47:15Z","receivedAt":"2018-04-03T14:47:58Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"From: Eric Sunshine \n\n\"git branch --list\" shows an in-progress rebase as:\n\n * (no branch, rebasing )\n master\n ...\n\nHowever, if the rebase is started from a detached HEAD, then there is no\n, and it would attempt to print a NULL pointer. The previous\ncommit fixed this problem, so add a test to verify that the output is\nsane in this situation.\n\nSigned-off-by: Eric Sunshine \nSigned-off-by: Kaartic Sivaraam \n---\n t/t3200-branch.sh | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 503a88d02..89fff3fa9 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -6,6 +6,7 @@\n test_description='git branch assorted tests'\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n \n test_expect_success 'prepare a trivial repository' '\n \techo Hello >A &&\n@@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with --no-merged' '\n \ttest_must_fail git branch --merged HEAD --no-merged HEAD\n '\n \n+test_expect_success '--list during rebase' '\n+\ttest_when_finished \"reset_rebase\" &&\n+\tgit checkout master &&\n+\tFAKE_LINES=\"1 edit 2\" &&\n+\texport FAKE_LINES &&\n+\tset_fake_editor &&\n+\tgit rebase -i HEAD~2 &&\n+\tgit branch --list >actual &&\n+\ttest_i18ngrep \"rebasing master\" actual\n+'\n+\n+test_expect_success '--list during rebase from detached HEAD' '\n+\ttest_when_finished \"reset_rebase && git checkout master\" &&\n+\tgit checkout master^0 &&\n+\toid=$(git rev-parse --short HEAD) &&\n+\tFAKE_LINES=\"1 edit 2\" &&\n+\texport FAKE_LINES &&\n+\tset_fake_editor &&\n+\tgit rebase -i HEAD~2 &&\n+\tgit branch --list >actual &&\n+\ttest_i18ngrep \"rebasing detached HEAD $oid\" actual\n+'\n+\n test_expect_success 'tracking with unexpected .fetch refspec' '\n \trm -rf a b c d &&\n \tgit init a &&\n-- \n2.17.0.484.g0c8726318\n\n"},{"id":"343784","messageId":"CAPig+cSGZunGDU5yOngwsDfH9w=TNGP1fUq94j1qfb_iaC2oZQ@mail.gmail.com","threadId":"48135","inReplyTo":"20180403144715.11174-1-kaartic.sivaraam@gmail.com","subject":"Re: [PATCH v3 2/2] t3200: verify \"branch --list\" sanity when rebasing from detached HEAD","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-04T08:09:07Z","receivedAt":"2018-04-04T08:09:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Apr 3, 2018 at 10:47 AM, Kaartic Sivaraam\n wrote:\n> From: Eric Sunshine \n>\n> \"git branch --list\" shows an in-progress rebase as:\n>\n> * (no branch, rebasing )\n> master\n> ...\n>\n> However, if the rebase is started from a detached HEAD, then there is no\n> , and it would attempt to print a NULL pointer. The previous\n> commit fixed this problem, so add a test to verify that the output is\n> sane in this situation.\n>\n> Signed-off-by: Eric Sunshine \n> Signed-off-by: Kaartic Sivaraam \n\nThanks. This re-roll looks fine.\n\n> ---\n> t/t3200-branch.sh | 24 ++++++++++++++++++++++++\n> 1 file changed, 24 insertions(+)\n>\n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> index 503a88d02..89fff3fa9 100755\n> --- a/t/t3200-branch.sh\n> +++ b/t/t3200-branch.sh\n> @@ -6,6 +6,7 @@\n> test_description='git branch assorted tests'\n>\n> . ./test-lib.sh\n> +. \"$TEST_DIRECTORY\"/lib-rebase.sh\n>\n> test_expect_success 'prepare a trivial repository' '\n> echo Hello >A &&\n> @@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with --no-merged' '\n> test_must_fail git branch --merged HEAD --no-merged HEAD\n> '\n>\n> +test_expect_success '--list during rebase' '\n> + test_when_finished \"reset_rebase\" &&\n> + git checkout master &&\n> + FAKE_LINES=\"1 edit 2\" &&\n> + export FAKE_LINES &&\n> + set_fake_editor &&\n> + git rebase -i HEAD~2 &&\n> + git branch --list >actual &&\n> + test_i18ngrep \"rebasing master\" actual\n> +'\n> +\n> +test_expect_success '--list during rebase from detached HEAD' '\n> + test_when_finished \"reset_rebase && git checkout master\" &&\n> + git checkout master^0 &&\n> + oid=$(git rev-parse --short HEAD) &&\n> + FAKE_LINES=\"1 edit 2\" &&\n> + export FAKE_LINES &&\n> + set_fake_editor &&\n> + git rebase -i HEAD~2 &&\n> + git branch --list >actual &&\n> + test_i18ngrep \"rebasing detached HEAD $oid\" actual\n> +'\n> +\n> test_expect_success 'tracking with unexpected .fetch refspec' '\n> rm -rf a b c d &&\n> git init a &&\n> --\n> 2.17.0.484.g0c8726318\n"}]}