{"thread":{"id":"61109","subject":"bisect does not respect 'log.date'","startedAt":"2024-03-13T11:07:34Z","lastAt":"2024-03-28T23:18:23Z","messageCount":8,"participants":["Osipov, Michael (IN IT IN)","Junio C Hamano","Peter Krefting","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"490536","messageId":"645c8253-f1ef-410f-8284-7d6c8b6db601@siemens.com","threadId":"61109","inReplyTo":null,"subject":"bisect does not respect 'log.date'","fromName":"Osipov, Michael (IN IT IN)","fromEmail":"michael.osipov@innomotics.com","sentAt":"2024-03-13T11:07:22Z","receivedAt":"2024-03-13T11:07:34Z","isPatch":false,"sender":{"key":"michael.osipov@innomotics.com","avatar":null},"body":"Folks,\n\nI am running git version 2.43.0 and consider the following config:\n> $ git config --system --list\n> core.eol=native\n> log.date=iso-strict\n> color.ui=auto\n\nSo date output looks fine with 'log' and 'show':\n> osipovmi@deblndw011x:~/var/Projekte/tomcat-native ((ba1454e15...)|BISECTING)\n> $ git log\n> commit ba1454e15619a44fe66d86f59c766c0cc25323eb (HEAD)\n> Author: Mark Thomas <markt@apache.org>\n> Date:   2024-02-13T08:27:43+00:00\n> \n>     Fix link\n> \nand\n> osipovmi@deblndw011x:~/var/Projekte/tomcat-native ((ba1454e15...)|BISECTING)\n> $ git show HEAD^\n> commit ef40b6d00c4bdaa23960b5dc0eaac28cce758d29\n> Author: Mark Thomas <markt@apache.org>\n> Date:   2024-02-06T16:40:25+00:00\n> \n>     Update minimum APR version in a few more places\n> \n\nnow let's bisect:\n> osipovmi@deblndw011x:~/var/Projekte/tomcat-native (apache-1.3.x =)\n> $ git bisect start HEAD HEAD~4\n> Binäre Suche: danach noch 1 Commit zum Testen übrig (ungefähr 1 Schritt)\n> [ef40b6d00c4bdaa23960b5dc0eaac28cce758d29] Update minimum APR version in a few more places\n...fast forward\n> osipovmi@deblndw011x:~/var/Projekte/tomcat-native ((ba1454e15...)|BISECTING)\n> $ git bisect bad\n> ba1454e15619a44fe66d86f59c766c0cc25323eb is the first bad commit\n> commit ba1454e15619a44fe66d86f59c766c0cc25323eb\n> Author: Mark Thomas <markt@apache.org>\n> Date:   Tue Feb 13 08:27:43 2024 +0000\n> \n>     Fix link\n> \n>  xdocs/news/project.xml | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n\nThe config for date format is ignored. Is uses the default value.\n\nAn oversight or bug?\n\nMichael\n"},{"id":"490567","messageId":"xmqq7ci6c7mn.fsf@gitster.g","threadId":"61109","inReplyTo":"645c8253-f1ef-410f-8284-7d6c8b6db601@siemens.com","subject":"Re: bisect does not respect 'log.date'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-13T18:24:32Z","receivedAt":"2024-03-13T18:24:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Osipov, Michael (IN IT IN)\" <michael.osipov@innomotics.com> writes:\n\n> An oversight or bug?\n\nNeither, i.e. WAI, I would say.\n\nThe configuration variable log.date is about \"git log\" and its\ndocumentation makes no promises how \"git bisect\" may or may not be\naffected.\n\nHaving said that, I think it is not unreasonable for you to make it\na feature request to add bisect.dateformat or whatever.  The only\ninteresting part from the output being the exact commit object name\nthe problem bisects to, I personally would not see it a high priority\nfeature request, though.\n\nThanks.\n\n"},{"id":"490573","messageId":"4e2b22fb-7496-4f67-a89f-9fcbffc73a1a@siemens.com","threadId":"61109","inReplyTo":"xmqq7ci6c7mn.fsf@gitster.g","subject":"Re: bisect does not respect 'log.date'","fromName":"Osipov, Michael (IN IT IN)","fromEmail":"michael.osipov@innomotics.com","sentAt":"2024-03-13T19:26:37Z","receivedAt":"2024-03-13T19:26:51Z","isPatch":false,"sender":{"key":"michael.osipov@innomotics.com","avatar":null},"body":"On 2024-03-13 19:24, Junio C Hamano wrote:\n> \"Osipov, Michael (IN IT IN)\" <michael.osipov@innomotics.com> writes:\n> \n>> An oversight or bug?\n> \n> Neither, i.e. WAI, I would say.\n> \n> The configuration variable log.date is about \"git log\" and its\n> documentation makes no promises how \"git bisect\" may or may not be\n> affected.\n> \n> Having said that, I think it is not unreasonable for you to make it\n> a feature request to add bisect.dateformat or whatever.  The only\n> interesting part from the output being the exact commit object name\n> the problem bisects to, I personally would not see it a high priority\n> feature request, though.\n\nInteresting thought, but that also means that \"git show\" misbehaves \nbecause it respects \"log.date\" and there is no \"show.date\". I still \nthink that consistency shouldn't be obeyed.\nI'd be happy if someone could consider this improvement.\n\nMichael\n"},{"id":"491526","messageId":"25d716fa-bd32-4ff0-20f2-05ff51750911@softwolves.pp.se","threadId":"61109","inReplyTo":"4e2b22fb-7496-4f67-a89f-9fcbffc73a1a@siemens.com","subject":"Re: bisect does not respect 'log.date'","fromName":"Peter Krefting","fromEmail":"peter@softwolves.pp.se","sentAt":"2024-03-25T20:27:30Z","receivedAt":"2024-03-25T21:31:48Z","isPatch":false,"sender":{"key":"peter@softwolves.pp.se","avatar":"https://avatars.githubusercontent.com/u/990764?v=4"},"body":"Osipov, Michael (IN IT IN):\n\n> Interesting thought, but that also means that \"git show\" misbehaves because \n> it respects \"log.date\" and there is no \"show.date\". I still think that \n> consistency shouldn't be obeyed.\n> I'd be happy if someone could consider this improvement.\n\nI've also been annoyed at this. Everything else respects the date \nsetting.\n\nBisect displays the commit through an invocation of \"git diff-tree \n--pretty\". This command does not respect the log.date setting, but it \ncan be passed the --date parameter to format the date.\n\nThe question is what is the correct way of fixing this; is it to make \n\"git diff-tree --pretty\" respect the \"log.date\" option, or to make \n\"git bisect\" pass a --date pate parameter to the invocation of it?\n\nOr perhaps everything should just be made support the \"TIME_STYLE\" that \nGNU tools use? GNU ls is so much nicer to use with \"TIME_STYLE=long-iso\" \nset.\n\n-- \n\\\\// Peter - http://www.softwolves.pp.se/\n"},{"id":"491530","messageId":"xmqq1q7ygex1.fsf@gitster.g","threadId":"61109","inReplyTo":"25d716fa-bd32-4ff0-20f2-05ff51750911@softwolves.pp.se","subject":"Re: bisect does not respect 'log.date'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-25T21:49:46Z","receivedAt":"2024-03-25T21:49:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Peter Krefting <peter@softwolves.pp.se> writes:\n\n> The question is what is the correct way of fixing this; is it to make\n> \"git diff-tree --pretty\" respect the \"log.date\" option, or to make\n\nThe \"diff-tree\" and other \"plumbing\" commands deliberately ignore\nconfiguration and the point of doing so is to make sure their output\nare stable without getting affected by the end-user configuration.\n\n> \"git bisect\" pass a --date pate parameter to the invocation of it?\n\nIf we were to change how \"bisect\" reports the date of the commit,\nthis is a more reasonable route to go.\n\nAre you sure that nobody is driving \"git bisect\" from a script and\nscraping the output in such a way that a change in the output format\nwould break such a script?  I would say it is unlikely (they may be\nscraping the output to find a commit by looking for 40-hex string,\nthough) that it would cause such a breakage.\n\nBut stepping back a bit.\n\nIf \"git bisect\" were written in the more modern era, I am reasonably\nsure that it wouldn't have used \"git diff-tree\" when reporting the\n\"first bad commit\".  It would have used \"git show\" instead, which is\nat the Porcelain level and will pay attention to the configuration\nvariables.  Instead of focusing too narrowly on the log.date option,\nthat would only tweak the date format, it may be a more fruitful way\nto invest brainwaves in to consider the feasibility of switching to\nuse \"git show\" there.\n\n"},{"id":"491783","messageId":"4727b78c-e45b-da7c-fa6e-85876b50dcde@softwolves.pp.se","threadId":"61109","inReplyTo":"xmqq1q7ygex1.fsf@gitster.g","subject":"[RFC PATCH] bisect: Honor log.date","fromName":"Peter Krefting","fromEmail":"peter@softwolves.pp.se","sentAt":"2024-03-28T20:53:40Z","receivedAt":"2024-03-28T20:53:51Z","isPatch":true,"sender":{"key":"peter@softwolves.pp.se","avatar":"https://avatars.githubusercontent.com/u/990764?v=4"},"body":"When bisect finds the target commit to display, it calls git diff-tree\nto do so. This is a plumbing command that is not affected by the user's\nlog.date setting. Switch to instead use \"git show\", which does honor\nit.\n\nReported-by: Michael Osipov <michael.osipov@innomotics.com>\nSigned-off-By: Peter Krefting <peter@softwolves.pp.se>\n---\n  bisect.c | 26 +++++++++++---------------\n  1 file changed, 11 insertions(+), 15 deletions(-)\n\nJunio C Hamano:\n\n> Instead of focusing too narrowly on the log.date option, that would \n> only tweak the date format, it may be a more fruitful way to invest \n> brainwaves in to consider the feasibility of switching to use \"git \n> show\" there.\n\nIndeed.\n\nHere is a patch that does exactly that.\n\nThis is my first patch to the actual codebase in Git, so it might be \nbit rough; improvements are welcome. I might need to change something \nin the test suite as well?\n\nWith this patch applied, running with log.date=iso and \nlog.decorate=short, I get this output:\n\n  $ ./git-bisect start\n  [...]\n  $ ./git-bisect good v2.43.2\n  [...]\n  $ ./git-bisect bad v2.43.3\n  [...]\n  $ ./git-bisect good\n  0d464a4e6a5a19bd8fbea1deae22d48d14dccb01 is the first bad commit\n  commit 0d464a4e6a5a19bd8fbea1deae22d48d14dccb01 (tag: v2.43.3)\n  Author: Junio C Hamano <gitster@pobox.com>\n  Date:   2024-02-22 16:13:38 -0800\n\n      Git 2.43.3\n\n  Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nwhich is the format I expect.\n\ndiff --git a/bisect.c b/bisect.c\nindex 8487f8cd1b..0f7126c32b 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -959,23 +959,19 @@ static enum bisect_error check_good_are_ancestors_of_bad(struct repository *r,\n  }\n\n  /*\n- * This does \"git diff-tree --pretty COMMIT\" without one fork+exec.\n+ * Runs \"git show\" to display a commit\n   */\n-static void show_diff_tree(struct repository *r,\n-\t\t\t   const char *prefix,\n-\t\t\t   struct commit *commit)\n+static void show_commit(struct commit *commit)\n  {\n-\tconst char *argv[] = {\n-\t\t\"diff-tree\", \"--pretty\", \"--stat\", \"--summary\", \"--cc\", NULL\n-\t};\n-\tstruct rev_info opt;\n+\tstruct child_process show = CHILD_PROCESS_INIT;\n\n-\tgit_config(git_diff_ui_config, NULL);\n-\trepo_init_revisions(r, &opt, prefix);\n-\n-\tsetup_revisions(ARRAY_SIZE(argv) - 1, argv, &opt, NULL);\n-\tlog_tree_commit(&opt, commit);\n-\trelease_revisions(&opt);\n+\t/* Invoke \"git show --pretty=medium --shortstat --no-abbrev-commit --no-patch $object\" */\n+\tstrvec_pushl(&show.args, \"show\", \"--pretty=medium\", \"--shortstat\", \"--no-abbrev-commit\", \"--no-patch\",\n+\t\t     oid_to_hex(&commit->object.oid), NULL);\n+\tshow.git_cmd = 1;\n+\tif (run_command(&show))\n+\t\tdie(_(\"unable to start 'show' for object '%s'\"),\n+\t\t    oid_to_hex(&commit->object.oid));\n  }\n\n  /*\n@@ -1092,7 +1088,7 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)\n  \t\tprintf(\"%s is the first %s commit\\n\", oid_to_hex(bisect_rev),\n  \t\t\tterm_bad);\n\n-\t\tshow_diff_tree(r, prefix, revs.commits->item);\n+\t\tshow_commit(revs.commits->item);\n  \t\t/*\n  \t\t * This means the bisection process succeeded.\n  \t\t * Using BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND (-10)\n-- \n2.39.2\n\n"},{"id":"491788","messageId":"CAPig+cSKbGW57dh13T6p20B_EY_C4K=LiQ3TP59wheMSi4qsQA@mail.gmail.com","threadId":"61109","inReplyTo":"4727b78c-e45b-da7c-fa6e-85876b50dcde@softwolves.pp.se","subject":"Re: [RFC PATCH] bisect: Honor log.date","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-03-28T21:38:18Z","receivedAt":"2024-03-28T21:38:32Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Mar 28, 2024 at 4:54 PM Peter Krefting <peter@softwolves.pp.se> wrote:\n> When bisect finds the target commit to display, it calls git diff-tree\n> to do so. This is a plumbing command that is not affected by the user's\n> log.date setting. Switch to instead use \"git show\", which does honor\n> it.\n>\n> Reported-by: Michael Osipov <michael.osipov@innomotics.com>\n> Signed-off-By: Peter Krefting <peter@softwolves.pp.se>\n> ---\n> diff --git a/bisect.c b/bisect.c\n> @@ -959,23 +959,19 @@ static enum bisect_error check_good_are_ancestors_of_bad(struct repository *r,\n> +       /* Invoke \"git show --pretty=medium --shortstat --no-abbrev-commit --no-patch $object\" */\n> +       strvec_pushl(&show.args, \"show\", \"--pretty=medium\", \"--shortstat\", \"--no-abbrev-commit\", \"--no-patch\",\n> +                    oid_to_hex(&commit->object.oid), NULL);\n> +       show.git_cmd = 1;\n\nNit: The comment doesn't tell the reader anything that the code itself\nisn't already clearly telling the reader, thus the comment is\nredundant and unnecessary. Moreover, the comment is likely to become\noutdated when people adjust the code but forget to update the comment.\nAs such, I'd recommend dropping the comment altogether.\n"},{"id":"491793","messageId":"b3f7c1c7f126a3c0a65f034ed6166d6a@softwolves.pp.se","threadId":"61109","inReplyTo":"CAPig+cSKbGW57dh13T6p20B_EY_C4K=LiQ3TP59wheMSi4qsQA@mail.gmail.com","subject":"Re: [RFC PATCH] bisect: Honor log.date","fromName":"Peter Krefting","fromEmail":"peter@softwolves.pp.se","sentAt":"2024-03-28T23:18:17Z","receivedAt":"2024-03-28T23:18:23Z","isPatch":true,"sender":{"key":"peter@softwolves.pp.se","avatar":"https://avatars.githubusercontent.com/u/990764?v=4"},"body":"2024-03-28 22:38 skrev Eric Sunshine:\n\n> Nit: The comment doesn't tell the reader anything that the code itself\n> isn't already clearly telling the reader, thus the comment is\n> redundant and unnecessary. Moreover, the comment is likely to become\n> outdated when people adjust the code but forget to update the comment.\n> As such, I'd recommend dropping the comment altogether.\n\nYes, that does make sense. I copied the code for invoking \"git show\" \nfrom builtin/notes.c, which does have that type of redundant comment.\n\nI'll remove it.\n\n-- \n\\\\// Peter - http://www.softwolves.pp.se/\n"}]}