{"thread":{"id":"52122","subject":"GIT_COMMITTER_* and reflog","startedAt":"2019-10-25T21:57:19Z","lastAt":"2019-11-07T13:57:27Z","messageCount":9,"participants":["Luke Dashjr","Jonathan Nieder","Junio C Hamano","Jeff King","Phillip Wood","Philip Oakley"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"384889","messageId":"201910252149.23787.luke@dashjr.org","threadId":"52122","inReplyTo":null,"subject":"GIT_COMMITTER_* and reflog","fromName":"Luke Dashjr","fromEmail":"luke@dashjr.org","sentAt":"2019-10-25T21:49:22Z","receivedAt":"2019-10-25T21:57:19Z","isPatch":false,"sender":{"key":"luke@dashjr.org","avatar":null},"body":"It appears the reflog currently allows its log data (name and date) to be \noverridden by the GIT_COMMITTER_* environment variables. At least for my \nworkflow, this kinda breaks the reflog (as I regularly set GIT_COMMITTER_DATE \nto produce deterministic commit objects).\n\nIs there a need to support this override for the reflog?\nIs there any reason it can't be changed to use GIT_REFLOG_* instead?\n\nNote that the reflog does NOT pull the name/date from the commit object \nitself, but rather calls git_committer_info() to get a new name/date for \nreflog purposes. So it does not appear to be intentionally designed to use \nthe commit's date.\n\nThanks for your thoughts,\n\nLuke\n\nP.S. If the reflog date can be made useful like this, maybe I will finish a \nminor patch to add reflog-date to the pretty formatting format so it can be \nviewed nicely... ;)\n"},{"id":"384900","messageId":"20191026022039.GE39574@google.com","threadId":"52122","inReplyTo":"201910252149.23787.luke@dashjr.org","subject":"Re: GIT_COMMITTER_* and reflog","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2019-10-26T02:20:39Z","receivedAt":"2019-10-26T02:20:45Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Luke,\n\nLuke Dashjr wrote:\n\n> It appears the reflog currently allows its log data (name and date) to be\n> overridden by the GIT_COMMITTER_* environment variables. At least for my\n> workflow, this kinda breaks the reflog (as I regularly set GIT_COMMITTER_DATE\n> to produce deterministic commit objects).\n\nCan you say more about this? What is the workflow this is part of? Can\nyou describe a sequence of steps and how you are affected during those\nsteps?\n\n> Is there a need to support this override for the reflog?\n\nYes.\n\n> Is there any reason it can't be changed to use GIT_REFLOG_* instead?\n\nWould a new GIT_REFLOG_* set of envvars that overrides GIT_COMMITTER_*\nwork for you? If I understand correctly, you could set\nGIT_REFLOG_NAME and GIT_REFLOG_EMAIL to an appropriate identity, but\nyou wouldn't have a good value to put in GIT_REFLOG_DATE.\n\nIf GIT_COMMITTER_{NAME,EMAIL} were used when writing reflogs but\nGIT_COMMITTER_DATE weren't, would that help with your workflow?\n\nThanks and hope that helps,\nJonathan\n"},{"id":"384902","messageId":"201910260243.23627.luke@dashjr.org","threadId":"52122","inReplyTo":"20191026022039.GE39574@google.com","subject":"Re: GIT_COMMITTER_* and reflog","fromName":"Luke Dashjr","fromEmail":"luke@dashjr.org","sentAt":"2019-10-26T02:43:22Z","receivedAt":"2019-10-26T02:43:32Z","isPatch":false,"sender":{"key":"luke@dashjr.org","avatar":null},"body":"On Saturday 26 October 2019 02:20:39 Jonathan Nieder wrote:\n> Luke Dashjr wrote:\n> > It appears the reflog currently allows its log data (name and date) to be\n> > overridden by the GIT_COMMITTER_* environment variables. At least for my\n> > workflow, this kinda breaks the reflog (as I regularly set\n> > GIT_COMMITTER_DATE to produce deterministic commit objects).\n>\n> Can you say more about this? What is the workflow this is part of? Can\n> you describe a sequence of steps and how you are affected during those\n> steps?\n\nI maintain a bleeding-edge variant of a more stable project, which is \nconstantly being rebased on the latest stable version. To make this easier, I \nuse a Perl script to generate the bleeding-edge version's git branches:\n https://github.com/bitcoinknots/assemble-deriv\nIt uses GIT_COMMITTER_DATE to ensure that I can repeatedly generate the branch \nuntil everything merges successfully, without polluting the repository with \nhundreds of merge commits each attempt. (Which would be annoying, since I \nliterally never prune.)\n\nBecause git's reflog also uses GIT_COMMITTER_DATE, my reflogs (HEAD in \nparticular) get polluted with incorrect timestamps during this process.\n\n> > Is there a need to support this override for the reflog?\n>\n> Yes.\n>\n> > Is there any reason it can't be changed to use GIT_REFLOG_* instead?\n>\n> Would a new GIT_REFLOG_* set of envvars that overrides GIT_COMMITTER_*\n> work for you? If I understand correctly, you could set\n> GIT_REFLOG_NAME and GIT_REFLOG_EMAIL to an appropriate identity, but\n> you wouldn't have a good value to put in GIT_REFLOG_DATE.\n>\n> If GIT_COMMITTER_{NAME,EMAIL} were used when writing reflogs but\n> GIT_COMMITTER_DATE weren't, would that help with your workflow?\n\nYes, it's really only GIT_COMMITTER_DATE that's messing me up personally.\nI never use GIT_COMMITTER_{NAME,EMAIL}.\n\nLuke\n"},{"id":"384904","messageId":"xmqqv9scark1.fsf@gitster-ct.c.googlers.com","threadId":"52122","inReplyTo":"20191026022039.GE39574@google.com","subject":"Re: GIT_COMMITTER_* and reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-26T07:34:22Z","receivedAt":"2019-10-26T07:34:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> If GIT_COMMITTER_{NAME,EMAIL} were used when writing reflogs but\n> GIT_COMMITTER_DATE weren't, would that help with your workflow?\n\nThanks for a thoughtful response.\n\nMy knee-jerk reaction is that it probably was a design bug that came\nout of laziness that we used the usual mechanism to obtain the\ncommitter date when deciding the timestamp we leave in reflog\nentries. Given that we say master@{6.hours.ago} etc., we should\nbase the timestamp on something that is coherent with what the end\nusers would give us, e.g. \"6.hours.ago\". IOW, we should be using\nthe wallclock time without paying attention to GIT_COMMITTER_DATE,\ni.e. date.c::get_time().\n\n"},{"id":"384914","messageId":"20191026173702.GA5522@sigill.intra.peff.net","threadId":"52122","inReplyTo":"xmqqv9scark1.fsf@gitster-ct.c.googlers.com","subject":"Re: GIT_COMMITTER_* and reflog","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-10-26T17:37:02Z","receivedAt":"2019-10-26T17:39:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 26, 2019 at 04:34:22PM +0900, Junio C Hamano wrote:\n\n> Jonathan Nieder writes:\n> \n> > If GIT_COMMITTER_{NAME,EMAIL} were used when writing reflogs but\n> > GIT_COMMITTER_DATE weren't, would that help with your workflow?\n> \n> Thanks for a thoughtful response.\n> \n> My knee-jerk reaction is that it probably was a design bug that came\n> out of laziness that we used the usual mechanism to obtain the\n> committer date when deciding the timestamp we leave in reflog\n> entries. Given that we say master@{6.hours.ago} etc., we should\n> base the timestamp on something that is coherent with what the end\n> users would give us, e.g. \"6.hours.ago\". IOW, we should be using\n> the wallclock time without paying attention to GIT_COMMITTER_DATE,\n> i.e. date.c::get_time().\n\nFWIW, I was about to write a similar response. If people really wanted a\nseparate reflog ident, I could see introducing $GIT_REFLOG_NAME, etc.\nBut the current date handling just seems buggy (and an unintended\nconsequence of reusing the existing ident code).\n\nIf somebody wants to pursue a patch, I suspect the solution is probably\nsomething like this (totally untested):\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex d60767ab73..2ebf2feeb8 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1658,7 +1658,10 @@ static int files_log_ref_write(struct files_ref_store *refs,\n \tif (logfd < 0)\n \t\treturn 0;\n \tresult = log_ref_write_fd(logfd, old_oid, new_oid,\n-\t\t\t\t git_committer_info(0), msg);\n+\t\t\t\t fmt_ident(getenv(\"GIT_COMMITTER_NAME\"),\n+\t\t\t\t\t getenv(\"GIT_COMMITTER_EMAIL\"),\n+\t\t\t\t\t WANT_COMMITTER_IDENT, NULL, 0),\n+\t\t\t\t msg);\n \tif (result) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \t\tint save_errno = errno;\n\n-Peff\n"},{"id":"384930","messageId":"xmqqr22ybcs7.fsf@gitster-ct.c.googlers.com","threadId":"52122","inReplyTo":"20191026173702.GA5522@sigill.intra.peff.net","subject":"Re: GIT_COMMITTER_* and reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-27T12:20:24Z","receivedAt":"2019-10-27T12:20:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> If somebody wants to pursue a patch, I suspect the solution is probably\n> something like this (totally untested):\n\nLooks sensible. It is very much unsatisfying that datestamp(),\nwhich is used by fmt_ident() when no date string is given, seems to\ntotally bypass date.c::get_time(), which means the framework to give\nfake timestamp via GIT_TEST_DATE_NOW cannot be used to write\nreproducible tests.\n\nGiven that datestamp() is only used by the push certificate and\nfast-import codepaths and nowhere else, I suspect that \"fixing\" it\nretroactively to honor GIT_TEST_DATE_NOW would not have any negative\nfallout, but that's not something I should be contemplating on\nduring the -rc period ;-)\n\nThanks.\n\n>\n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index d60767ab73..2ebf2feeb8 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -1658,7 +1658,10 @@ static int files_log_ref_write(struct files_ref_store *refs,\n> \tif (logfd < 0)\n> \t\treturn 0;\n> \tresult = log_ref_write_fd(logfd, old_oid, new_oid,\n> -\t\t\t\t git_committer_info(0), msg);\n> +\t\t\t\t fmt_ident(getenv(\"GIT_COMMITTER_NAME\"),\n> +\t\t\t\t\t getenv(\"GIT_COMMITTER_EMAIL\"),\n> +\t\t\t\t\t WANT_COMMITTER_IDENT, NULL, 0),\n> +\t\t\t\t msg);\n> \tif (result) {\n> \t\tstruct strbuf sb = STRBUF_INIT;\n> \t\tint save_errno = errno;\n>\n> -Peff\n"},{"id":"385082","messageId":"08310159-41f9-43e2-5d54-a48fd403ad07@gmail.com","threadId":"52122","inReplyTo":"xmqqr22ybcs7.fsf@gitster-ct.c.googlers.com","subject":"Re: GIT_COMMITTER_* and reflog","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-10-29T14:05:53Z","receivedAt":"2019-10-29T14:05:59Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio/Peff\n\nOn 27/10/2019 12:20, Junio C Hamano wrote:\n> Jeff King writes:\n> \n>> If somebody wants to pursue a patch, I suspect the solution is probably\n>> something like this (totally untested):\n> \n> Looks sensible. It is very much unsatisfying that datestamp(),\n> which is used by fmt_ident() when no date string is given, seems to\n> totally bypass date.c::get_time(), which means the framework to give\n> fake timestamp via GIT_TEST_DATE_NOW cannot be used to write\n> reproducible tests.\n> \n> Given that datestamp() is only used by the push certificate and\n> fast-import codepaths and nowhere else, I suspect that \"fixing\" it\n> retroactively to honor GIT_TEST_DATE_NOW would not have any negative\n> fallout, but that's not something I should be contemplating on\n> during the -rc period ;-)\n\nIt would allow for more robust am/rebase --ignore-date tests as we would \nknow what date to expect rather than just checking the timezone.\n\nBest Wishes\n\nPhillip\n\n> \n> Thanks.\n> \n>>\n>> diff --git a/refs/files-backend.c b/refs/files-backend.c\n>> index d60767ab73..2ebf2feeb8 100644\n>> --- a/refs/files-backend.c\n>> +++ b/refs/files-backend.c\n>> @@ -1658,7 +1658,10 @@ static int files_log_ref_write(struct files_ref_store *refs,\n>> \tif (logfd < 0)\n>> \t\treturn 0;\n>> \tresult = log_ref_write_fd(logfd, old_oid, new_oid,\n>> -\t\t\t\t git_committer_info(0), msg);\n>> +\t\t\t\t fmt_ident(getenv(\"GIT_COMMITTER_NAME\"),\n>> +\t\t\t\t\t getenv(\"GIT_COMMITTER_EMAIL\"),\n>> +\t\t\t\t\t WANT_COMMITTER_IDENT, NULL, 0),\n>> +\t\t\t\t msg);\n>> \tif (result) {\n>> \t\tstruct strbuf sb = STRBUF_INIT;\n>> \t\tint save_errno = errno;\n>>\n>> -Peff\n"},{"id":"385089","messageId":"20191029143420.GB3683@sigill.intra.peff.net","threadId":"52122","inReplyTo":"xmqqr22ybcs7.fsf@gitster-ct.c.googlers.com","subject":"Re: GIT_COMMITTER_* and reflog","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-10-29T14:34:20Z","receivedAt":"2019-10-29T14:34:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 27, 2019 at 09:20:24PM +0900, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > If somebody wants to pursue a patch, I suspect the solution is probably\n> > something like this (totally untested):\n> \n> Looks sensible. It is very much unsatisfying that datestamp(),\n> which is used by fmt_ident() when no date string is given, seems to\n> totally bypass date.c::get_time(), which means the framework to give\n> fake timestamp via GIT_TEST_DATE_NOW cannot be used to write\n> reproducible tests.\n> \n> Given that datestamp() is only used by the push certificate and\n> fast-import codepaths and nowhere else, I suspect that \"fixing\" it\n> retroactively to honor GIT_TEST_DATE_NOW would not have any negative\n> fallout, but that's not something I should be contemplating on\n> during the -rc period ;-)\n\nYeah, I agree datestamp() should respect $GIT_TEST_DATE_NOW. This whole\ntopic is not something for the -rc period, I would think.\n\n-Peff\n"},{"id":"385707","messageId":"3d6e0a08-673d-e83d-d18d-8153840d185d@iee.email","threadId":"52122","inReplyTo":"201910260243.23627.luke@dashjr.org","subject":"Re: GIT_COMMITTER_* and reflog","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2019-11-07T13:57:24Z","receivedAt":"2019-11-07T13:57:27Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"copying Dscho,\n\nOn 26/10/2019 03:43, Luke Dashjr wrote:\n> On Saturday 26 October 2019 02:20:39 Jonathan Nieder wrote:\n>> Luke Dashjr wrote:\n>>> It appears the reflog currently allows its log data (name and date) to be\n>>> overridden by the GIT_COMMITTER_* environment variables. At least for my\n>>> workflow, this kinda breaks the reflog (as I regularly set\n>>> GIT_COMMITTER_DATE to produce deterministic commit objects).\n>> Can you say more about this? What is the workflow this is part of? Can\n>> you describe a sequence of steps and how you are affected during those\n>> steps?\n> I maintain a bleeding-edge variant of a more stable project, which is\n> constantly being rebased on the latest stable version.\nThis looks to have a strong similarity to the Git-for-Windows patches \nwhich are regularly rebased on top of this here upstream Git.\n\nDscho uses the `garden shears` script [1] to help update the patch set, \nin conjunction with the 'updated' --rebase-merges option (replaces the \ndeprecated --preserve-merges).\n\n> To make this easier, I\n> use a Perl script to generate the bleeding-edge version's git branches:\n> https://github.com/bitcoinknots/assemble-deriv\n> It uses GIT_COMMITTER_DATE to ensure that I can repeatedly generate the branch\n> until everything merges successfully, without polluting the repository with\n> hundreds of merge commits each attempt. (Which would be annoying, since I\n> literally never prune.)\n>\n> Because git's reflog also uses GIT_COMMITTER_DATE, my reflogs (HEAD in\n> particular) get polluted with incorrect timestamps during this process.\n>\n>>> Is there a need to support this override for the reflog?\n>> Yes.\n>>\n>>> Is there any reason it can't be changed to use GIT_REFLOG_* instead?\n>> Would a new GIT_REFLOG_* set of envvars that overrides GIT_COMMITTER_*\n>> work for you? If I understand correctly, you could set\n>> GIT_REFLOG_NAME and GIT_REFLOG_EMAIL to an appropriate identity, but\n>> you wouldn't have a good value to put in GIT_REFLOG_DATE.\n>>\n>> If GIT_COMMITTER_{NAME,EMAIL} were used when writing reflogs but\n>> GIT_COMMITTER_DATE weren't, would that help with your workflow?\n> Yes, it's really only GIT_COMMITTER_DATE that's messing me up personally.\n> I never use GIT_COMMITTER_{NAME,EMAIL}.\n\n-- \nPhilip\n\n[1] https://github.com/git-for-windows/build-extra/blob/master/shears.sh\n\n"}]}