{"thread":{"id":"45319","subject":"BUG Report: git branch ignore --no-abbrev flag","startedAt":"2017-03-08T18:14:50Z","lastAt":"2017-03-10T16:45:31Z","messageCount":8,"participants":["Guillaume Wenzek","Junio C Hamano","Jeff King","Jakub Narębski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"313528","messageId":"CAAvNd=ir1qNQVaKphdg51AfGnsNgTrfvW2L6cca3SHiZrWNgHA@mail.gmail.com","threadId":"45319","inReplyTo":null,"subject":"BUG Report: git branch ignore --no-abbrev flag","fromName":"Guillaume Wenzek","fromEmail":"guillaume.wenzek@gmail.com","sentAt":"2017-03-08T18:14:24Z","receivedAt":"2017-03-08T18:14:50Z","isPatch":false,"sender":{"key":"guillaume.wenzek@gmail.com","avatar":null},"body":"Hi,\nAfter updating to git 2.12.0 on Monday I noticed that the \"git branch\"\nwasn't behaving as usual.\nAs of today `git branch -vv --no-abbrev` outputs short hashes instead\nof long one (as requested by --no-abbrev)[1]\n\ngit branch -vv --no-abbrev\n* (HEAD detached at 2.12.1)                        1c69bf2 Add\nrecap-release since previous release messages didn't go out.\n  master                                           eb70249\n[origin/master] Fix: support parsing \"git://\" remote URI to Github URL\n\n\nExpected output:\n\ngit branch --vv --no-abbrev\n* (HEAD detached at 2.12.1)\n1c69bf24be6de096d801435378be85a936ab0f29 Add recap-release since\nprevious release messages didn't go out.\n  master\neb70249e724f933255b4d8c7096092f2764942b4 [origin/master] Fix: support\nparsing \"git://\" remote URI to Github URL\n\n[1] https://git-scm.com/docs/git-branch#git-branch---no-abbrev\n\nI don't have any relevant configuration set.\n"},{"id":"313530","messageId":"xmqqlgsf39fg.fsf@gitster.mtv.corp.google.com","threadId":"45319","inReplyTo":"CAAvNd=ir1qNQVaKphdg51AfGnsNgTrfvW2L6cca3SHiZrWNgHA@mail.gmail.com","subject":"Re: BUG Report: git branch ignore --no-abbrev flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-08T18:33:39Z","receivedAt":"2017-03-08T18:34:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Guillaume Wenzek <guillaume.wenzek@gmail.com> writes:\n\n> After updating to git 2.12.0 on Monday I noticed that the \"git branch\"\n> wasn't behaving as usual.\n\nAre you sure you are trying 2.12?  v2.12.0 and before should behave\nthe same way and honor --no-abbrev as far as I know.\n\nOn the other hand, 'master' has 93e8cd8b (\"Merge branch\n'kn/ref-filter-branch-list'\", 2017-02-27), which seems to introduce\nthe regression.\n\nKarthik?\n"},{"id":"313560","messageId":"xmqqzigv1lc3.fsf@gitster.mtv.corp.google.com","threadId":"45319","inReplyTo":"xmqqlgsf39fg.fsf@gitster.mtv.corp.google.com","subject":"Re: BUG Report: git branch ignore --no-abbrev flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-08T21:59:24Z","receivedAt":"2017-03-08T22:05:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Guillaume Wenzek <guillaume.wenzek@gmail.com> writes:\n>\n>> After updating to git 2.12.0 on Monday I noticed that the \"git branch\"\n>> wasn't behaving as usual.\n>\n> Are you sure you are trying 2.12?  v2.12.0 and before should behave\n> the same way and honor --no-abbrev as far as I know.\n>\n> On the other hand, 'master' has 93e8cd8b (\"Merge branch\n> 'kn/ref-filter-branch-list'\", 2017-02-27), which seems to introduce\n> the regression.\n>\n> Karthik?\n\nI haven't fully checked if filter.abbrev is set correctly, but I\nnoticed the output format is formulated without taking the value of\nfilter.abbrev into account at all, so this is an attempt to fix\nthat omission.\n\nI also notice that filter.abbrev is _ONLY_ used by builtin/branch.c and\nthe actual ref-filter code does not have to know anything about it.\n\nWe probably should eliminate filter.abbrev field from the structure\nand use a regular variable in builtin/branch.c and use it to pass\nthe result of command line parsing from cmd_branch() down to\nbuild_format() as an argument.  \n\nBut that is outside the scope of regression fix.\n\n\n builtin/branch.c | 19 +++++++++++++++----\n 1 file changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex cbaa6d03c0..537c47811a 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -335,9 +335,18 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r\n \t\t    branch_get_color(BRANCH_COLOR_CURRENT));\n \n \tif (filter->verbose) {\n+\t\tstruct strbuf obname = STRBUF_INIT;\n+\n+\t\tif (filter->abbrev < 0)\n+\t\t\tstrbuf_addf(&obname, \"%%(objectname:short)\");\n+\t\telse if (!filter->abbrev)\n+\t\t\tstrbuf_addf(&obname, \"%%(objectname)\");\n+\t\telse\n+\t\t\tstrbuf_addf(&obname, \" %%(objectname:short=%d) \", filter->abbrev);\n+\n \t\tstrbuf_addf(&local, \"%%(align:%d,left)%%(refname:lstrip=2)%%(end)\", maxwidth);\n \t\tstrbuf_addf(&local, \"%s\", branch_get_color(BRANCH_COLOR_RESET));\n-\t\tstrbuf_addf(&local, \" %%(objectname:short=7) \");\n+\t\tstrbuf_addf(&local, \" %s \", obname.buf);\n \n \t\tif (filter->verbose > 1)\n \t\t\tstrbuf_addf(&local, \"%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)\"\n@@ -346,10 +355,12 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r\n \t\telse\n \t\t\tstrbuf_addf(&local, \"%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)\");\n \n-\t\tstrbuf_addf(&remote, \"%s%%(align:%d,left)%s%%(refname:lstrip=2)%%(end)%s%%(if)%%(symref)%%(then) -> %%(symref:short)\"\n-\t\t\t    \"%%(else) %%(objectname:short=7) %%(contents:subject)%%(end)\",\n+\t\tstrbuf_addf(&remote, \"%s%%(align:%d,left)%s%%(refname:lstrip=2)%%(end)%s\"\n+\t\t\t    \"%%(if)%%(symref)%%(then) -> %%(symref:short)\"\n+\t\t\t    \"%%(else) %s %%(contents:subject)%%(end)\",\n \t\t\t    branch_get_color(BRANCH_COLOR_REMOTE), maxwidth, quote_literal_for_format(remote_prefix),\n-\t\t\t    branch_get_color(BRANCH_COLOR_RESET));\n+\t\t\t    branch_get_color(BRANCH_COLOR_RESET), obname.buf);\n+\t\tstrbuf_release(&obname);\n \t} else {\n \t\tstrbuf_addf(&local, \"%%(refname:lstrip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)\",\n \t\t\t    branch_get_color(BRANCH_COLOR_RESET));\n"},{"id":"313567","messageId":"xmqqvarj1kix.fsf_-_@gitster.mtv.corp.google.com","threadId":"45319","inReplyTo":"xmqqzigv1lc3.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] branch: honor --abbrev/--no-abbrev in --list mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-08T22:16:54Z","receivedAt":"2017-03-08T22:33:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When the \"branch --list\" command was converted to use the --format\nfacility from the ref-filter API, we forgot to honor the --abbrev\nsetting in the default output format and instead used a hardcoded\n\"7\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This time with test.  I am building directly this fix on top of\n   the kn/ref-filter-branch-test topic that was merged to 'master'\n   after 2.12, and haven't checked if there will be conflicts with\n   other topics in-flight (I am expecting none, but I do not know\n   until I start today's integration cycle).\n\n builtin/branch.c  | 19 +++++++++++++++----\n t/t3200-branch.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 40 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex cbaa6d03c0..537c47811a 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -335,9 +335,18 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r\n \t\t    branch_get_color(BRANCH_COLOR_CURRENT));\n \n \tif (filter->verbose) {\n+\t\tstruct strbuf obname = STRBUF_INIT;\n+\n+\t\tif (filter->abbrev < 0)\n+\t\t\tstrbuf_addf(&obname, \"%%(objectname:short)\");\n+\t\telse if (!filter->abbrev)\n+\t\t\tstrbuf_addf(&obname, \"%%(objectname)\");\n+\t\telse\n+\t\t\tstrbuf_addf(&obname, \" %%(objectname:short=%d) \", filter->abbrev);\n+\n \t\tstrbuf_addf(&local, \"%%(align:%d,left)%%(refname:lstrip=2)%%(end)\", maxwidth);\n \t\tstrbuf_addf(&local, \"%s\", branch_get_color(BRANCH_COLOR_RESET));\n-\t\tstrbuf_addf(&local, \" %%(objectname:short=7) \");\n+\t\tstrbuf_addf(&local, \" %s \", obname.buf);\n \n \t\tif (filter->verbose > 1)\n \t\t\tstrbuf_addf(&local, \"%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)\"\n@@ -346,10 +355,12 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r\n \t\telse\n \t\t\tstrbuf_addf(&local, \"%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)\");\n \n-\t\tstrbuf_addf(&remote, \"%s%%(align:%d,left)%s%%(refname:lstrip=2)%%(end)%s%%(if)%%(symref)%%(then) -> %%(symref:short)\"\n-\t\t\t    \"%%(else) %%(objectname:short=7) %%(contents:subject)%%(end)\",\n+\t\tstrbuf_addf(&remote, \"%s%%(align:%d,left)%s%%(refname:lstrip=2)%%(end)%s\"\n+\t\t\t    \"%%(if)%%(symref)%%(then) -> %%(symref:short)\"\n+\t\t\t    \"%%(else) %s %%(contents:subject)%%(end)\",\n \t\t\t    branch_get_color(BRANCH_COLOR_REMOTE), maxwidth, quote_literal_for_format(remote_prefix),\n-\t\t\t    branch_get_color(BRANCH_COLOR_RESET));\n+\t\t\t    branch_get_color(BRANCH_COLOR_RESET), obname.buf);\n+\t\tstrbuf_release(&obname);\n \t} else {\n \t\tstrbuf_addf(&local, \"%%(refname:lstrip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)\",\n \t\t\t    branch_get_color(BRANCH_COLOR_RESET));\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 8a833f354e..39bd5ac8fa 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -207,6 +207,31 @@ test_expect_success 'git branch --list -d t should fail' '\n \ttest_path_is_missing .git/refs/heads/t\n '\n \n+test_expect_success 'git branch --list -v with --abbrev' '\n+\ttest_when_finished \"git branch -D t\" &&\n+\tgit branch t &&\n+\tgit branch -v --list t >actual.default &&\n+\tgit branch -v --list --abbrev t >actual.abbrev &&\n+\ttest_cmp actual.default actual.abbrev &&\n+\n+\tgit branch -v --list --no-abbrev t >actual.noabbrev &&\n+\tgit branch -v --list --abbrev=0 t >actual.0abbrev &&\n+\ttest_cmp actual.noabbrev actual.0abbrev &&\n+\n+\tgit branch -v --list --abbrev=36 t >actual.36abbrev &&\n+\t# how many hexdigits are used?\n+\tread name objdefault rest <actual.abbrev &&\n+\tread name obj36 rest <actual.36abbrev &&\n+\tobjfull=$(git rev-parse --verify t) &&\n+\n+\t# are we really getting abbreviations?\n+\ttest \"$obj36\" != \"$objdefault\" &&\n+\texpr \"$obj36\" : \"$objdefault\" >/dev/null &&\n+\ttest \"$objfull\" != \"$obj36\" &&\n+\texpr \"$objfull\" : \"$obj36\" >/dev/null\n+\n+'\n+\n test_expect_success 'git branch --column' '\n \tCOLUMNS=81 git branch --column=column >actual &&\n \tcat >expected <<\\EOF &&\n-- \n2.12.0.246.ga2ecc84866-goog\n\n"},{"id":"313625","messageId":"CAAvNd=gAvONS-XP2vOLExdWiwDXTPbB5beiXikwdig7xC5OKow@mail.gmail.com","threadId":"45319","inReplyTo":"xmqqzigv1lc3.fsf@gitster.mtv.corp.google.com","subject":"Re: BUG Report: git branch ignore --no-abbrev flag","fromName":"Guillaume Wenzek","fromEmail":"guillaume.wenzek@gmail.com","sentAt":"2017-03-09T09:44:42Z","receivedAt":"2017-03-09T10:14:46Z","isPatch":false,"sender":{"key":"guillaume.wenzek@gmail.com","avatar":null},"body":"Thanks for looking into it,\n\nThe full version number is \"2.12.0.246.ga2ecc84866\" but I don't think\nthat's an official number, I'm using my company (Google) apt\nrepository.\n\nThe git package date is 2017/03/03 while previous version was from\n2017/02/16, the commit you identified is actually between the two, so\nthat may be it.\n\nOn 8 March 2017 at 22:59, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Guillaume Wenzek <guillaume.wenzek@gmail.com> writes:\n>>\n>>> After updating to git 2.12.0 on Monday I noticed that the \"git branch\"\n>>> wasn't behaving as usual.\n>>\n>> Are you sure you are trying 2.12?  v2.12.0 and before should behave\n>> the same way and honor --no-abbrev as far as I know.\n>>\n>> On the other hand, 'master' has 93e8cd8b (\"Merge branch\n>> 'kn/ref-filter-branch-list'\", 2017-02-27), which seems to introduce\n>> the regression.\n>>\n>> Karthik?\n>\n> I haven't fully checked if filter.abbrev is set correctly, but I\n> noticed the output format is formulated without taking the value of\n> filter.abbrev into account at all, so this is an attempt to fix\n> that omission.\n>\n> I also notice that filter.abbrev is _ONLY_ used by builtin/branch.c and\n> the actual ref-filter code does not have to know anything about it.\n>\n> We probably should eliminate filter.abbrev field from the structure\n> and use a regular variable in builtin/branch.c and use it to pass\n> the result of command line parsing from cmd_branch() down to\n> build_format() as an argument.\n>\n> But that is outside the scope of regression fix.\n>\n>\n>  builtin/branch.c | 19 +++++++++++++++----\n>  1 file changed, 15 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/branch.c b/builtin/branch.c\n> index cbaa6d03c0..537c47811a 100644\n> --- a/builtin/branch.c\n> +++ b/builtin/branch.c\n> @@ -335,9 +335,18 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r\n>                     branch_get_color(BRANCH_COLOR_CURRENT));\n>\n>         if (filter->verbose) {\n> +               struct strbuf obname = STRBUF_INIT;\n> +\n> +               if (filter->abbrev < 0)\n> +                       strbuf_addf(&obname, \"%%(objectname:short)\");\n> +               else if (!filter->abbrev)\n> +                       strbuf_addf(&obname, \"%%(objectname)\");\n> +               else\n> +                       strbuf_addf(&obname, \" %%(objectname:short=%d) \", filter->abbrev);\n> +\n>                 strbuf_addf(&local, \"%%(align:%d,left)%%(refname:lstrip=2)%%(end)\", maxwidth);\n>                 strbuf_addf(&local, \"%s\", branch_get_color(BRANCH_COLOR_RESET));\n> -               strbuf_addf(&local, \" %%(objectname:short=7) \");\n> +               strbuf_addf(&local, \" %s \", obname.buf);\n>\n>                 if (filter->verbose > 1)\n>                         strbuf_addf(&local, \"%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)\"\n> @@ -346,10 +355,12 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r\n>                 else\n>                         strbuf_addf(&local, \"%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)\");\n>\n> -               strbuf_addf(&remote, \"%s%%(align:%d,left)%s%%(refname:lstrip=2)%%(end)%s%%(if)%%(symref)%%(then) -> %%(symref:short)\"\n> -                           \"%%(else) %%(objectname:short=7) %%(contents:subject)%%(end)\",\n> +               strbuf_addf(&remote, \"%s%%(align:%d,left)%s%%(refname:lstrip=2)%%(end)%s\"\n> +                           \"%%(if)%%(symref)%%(then) -> %%(symref:short)\"\n> +                           \"%%(else) %s %%(contents:subject)%%(end)\",\n>                             branch_get_color(BRANCH_COLOR_REMOTE), maxwidth, quote_literal_for_format(remote_prefix),\n> -                           branch_get_color(BRANCH_COLOR_RESET));\n> +                           branch_get_color(BRANCH_COLOR_RESET), obname.buf);\n> +               strbuf_release(&obname);\n>         } else {\n>                 strbuf_addf(&local, \"%%(refname:lstrip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)\",\n>                             branch_get_color(BRANCH_COLOR_RESET));\n"},{"id":"313627","messageId":"20170309103812.skwmyn4bmkxjwjl4@sigill.intra.peff.net","threadId":"45319","inReplyTo":"CAAvNd=gAvONS-XP2vOLExdWiwDXTPbB5beiXikwdig7xC5OKow@mail.gmail.com","subject":"Re: BUG Report: git branch ignore --no-abbrev flag","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-09T10:38:12Z","receivedAt":"2017-03-09T10:38:37Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 09, 2017 at 10:44:42AM +0100, Guillaume Wenzek wrote:\n\n> The full version number is \"2.12.0.246.ga2ecc84866\" but I don't think\n> that's an official number, I'm using my company (Google) apt\n> repository.\n\nThat's built from the commit at a2ecc84866, which is in the \"next\"\nbranch (and is 246 commits ahead of v2.12.0). So that explains why you\nare seeing the behavior. The code _isn't_ in v2.12.0, but you're running\na much more recent version.\n\n-Peff\n"},{"id":"313640","messageId":"4d09ad56-96ac-75d3-c9df-0c23b805b478@gmail.com","threadId":"45319","inReplyTo":"xmqqvarj1kix.fsf_-_@gitster.mtv.corp.google.com","subject":"Re: [PATCH] branch: honor --abbrev/--no-abbrev in --list mode","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2017-03-09T13:25:08Z","receivedAt":"2017-03-09T13:25:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 08.03.2017 o 23:16, Junio C Hamano pisze:\n \n> diff --git a/builtin/branch.c b/builtin/branch.c\n> index cbaa6d03c0..537c47811a 100644\n> --- a/builtin/branch.c\n> +++ b/builtin/branch.c\n> @@ -335,9 +335,18 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r\n>  \t\t    branch_get_color(BRANCH_COLOR_CURRENT));\n>  \n>  \tif (filter->verbose) {\n> +\t\tstruct strbuf obname = STRBUF_INIT;\n> +\n> +\t\tif (filter->abbrev < 0)\n> +\t\t\tstrbuf_addf(&obname, \"%%(objectname:short)\");\n> +\t\telse if (!filter->abbrev)\n> +\t\t\tstrbuf_addf(&obname, \"%%(objectname)\");\n> +\t\telse\n> +\t\t\tstrbuf_addf(&obname, \" %%(objectname:short=%d) \", filter->abbrev);\n                                              ^                       ^\nI wonder why the last one has leading space --/ and trailing one -----/\nThe rest (for default abbrev and for no abbrev do not).\n\n-- \nJakub Narębski\n\n"},{"id":"313756","messageId":"xmqqinnhytb0.fsf@gitster.mtv.corp.google.com","threadId":"45319","inReplyTo":"4d09ad56-96ac-75d3-c9df-0c23b805b478@gmail.com","subject":"Re: [PATCH] branch: honor --abbrev/--no-abbrev in --list mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-10T16:45:23Z","receivedAt":"2017-03-10T16:45:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narębski <jnareb@gmail.com> writes:\n\n> W dniu 08.03.2017 o 23:16, Junio C Hamano pisze:\n>  \n>> diff --git a/builtin/branch.c b/builtin/branch.c\n>> index cbaa6d03c0..537c47811a 100644\n>> --- a/builtin/branch.c\n>> +++ b/builtin/branch.c\n>> @@ -335,9 +335,18 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r\n>>  \t\t    branch_get_color(BRANCH_COLOR_CURRENT));\n>>  \n>>  \tif (filter->verbose) {\n>> +\t\tstruct strbuf obname = STRBUF_INIT;\n>> +\n>> +\t\tif (filter->abbrev < 0)\n>> +\t\t\tstrbuf_addf(&obname, \"%%(objectname:short)\");\n>> +\t\telse if (!filter->abbrev)\n>> +\t\t\tstrbuf_addf(&obname, \"%%(objectname)\");\n>> +\t\telse\n>> +\t\t\tstrbuf_addf(&obname, \" %%(objectname:short=%d) \", filter->abbrev);\n>                                               ^                       ^\n> I wonder why the last one has leading space --/ and trailing one -----/\n> The rest (for default abbrev and for no abbrev do not).\n\nThanks for spotting; that's just an editor cruft that shouldn't have\nbeen there.\n\n\n"}]}