* GIT_COMMITTER_* and reflog @ 2019-10-25 21:49 Luke Dashjr 2019-10-26 2:20 ` Jonathan Nieder 0 siblings, 1 reply; 9+ messages in thread From: Luke Dashjr @ 2019-10-25 21:49 UTC (permalink / raw) To: git It appears the reflog currently allows its log data (name and date) to be overridden by the GIT_COMMITTER_* environment variables. At least for my workflow, this kinda breaks the reflog (as I regularly set GIT_COMMITTER_DATE to produce deterministic commit objects). Is there a need to support this override for the reflog? Is there any reason it can't be changed to use GIT_REFLOG_* instead? Note that the reflog does NOT pull the name/date from the commit object itself, but rather calls git_committer_info() to get a new name/date for reflog purposes. So it does not appear to be intentionally designed to use the commit's date. Thanks for your thoughts, Luke P.S. If the reflog date can be made useful like this, maybe I will finish a minor patch to add reflog-date to the pretty formatting format so it can be viewed nicely... ;) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: GIT_COMMITTER_* and reflog 2019-10-25 21:49 GIT_COMMITTER_* and reflog Luke Dashjr @ 2019-10-26 2:20 ` Jonathan Nieder 2019-10-26 2:43 ` Luke Dashjr 2019-10-26 7:34 ` Junio C Hamano 0 siblings, 2 replies; 9+ messages in thread From: Jonathan Nieder @ 2019-10-26 2:20 UTC (permalink / raw) To: Luke Dashjr; +Cc: git Hi Luke, Luke Dashjr wrote: > It appears the reflog currently allows its log data (name and date) to be > overridden by the GIT_COMMITTER_* environment variables. At least for my > workflow, this kinda breaks the reflog (as I regularly set GIT_COMMITTER_DATE > to produce deterministic commit objects). Can you say more about this? What is the workflow this is part of? Can you describe a sequence of steps and how you are affected during those steps? > Is there a need to support this override for the reflog? Yes. > Is there any reason it can't be changed to use GIT_REFLOG_* instead? Would a new GIT_REFLOG_* set of envvars that overrides GIT_COMMITTER_* work for you? If I understand correctly, you could set GIT_REFLOG_NAME and GIT_REFLOG_EMAIL to an appropriate identity, but you wouldn't have a good value to put in GIT_REFLOG_DATE. If GIT_COMMITTER_{NAME,EMAIL} were used when writing reflogs but GIT_COMMITTER_DATE weren't, would that help with your workflow? Thanks and hope that helps, Jonathan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: GIT_COMMITTER_* and reflog 2019-10-26 2:20 ` Jonathan Nieder @ 2019-10-26 2:43 ` Luke Dashjr 2019-11-07 13:57 ` Philip Oakley 2019-10-26 7:34 ` Junio C Hamano 1 sibling, 1 reply; 9+ messages in thread From: Luke Dashjr @ 2019-10-26 2:43 UTC (permalink / raw) To: Jonathan Nieder; +Cc: git On Saturday 26 October 2019 02:20:39 Jonathan Nieder wrote: > Luke Dashjr wrote: > > It appears the reflog currently allows its log data (name and date) to be > > overridden by the GIT_COMMITTER_* environment variables. At least for my > > workflow, this kinda breaks the reflog (as I regularly set > > GIT_COMMITTER_DATE to produce deterministic commit objects). > > Can you say more about this? What is the workflow this is part of? Can > you describe a sequence of steps and how you are affected during those > steps? I maintain a bleeding-edge variant of a more stable project, which is constantly being rebased on the latest stable version. To make this easier, I use a Perl script to generate the bleeding-edge version's git branches: https://github.com/bitcoinknots/assemble-deriv It uses GIT_COMMITTER_DATE to ensure that I can repeatedly generate the branch until everything merges successfully, without polluting the repository with hundreds of merge commits each attempt. (Which would be annoying, since I literally never prune.) Because git's reflog also uses GIT_COMMITTER_DATE, my reflogs (HEAD in particular) get polluted with incorrect timestamps during this process. > > Is there a need to support this override for the reflog? > > Yes. > > > Is there any reason it can't be changed to use GIT_REFLOG_* instead? > > Would a new GIT_REFLOG_* set of envvars that overrides GIT_COMMITTER_* > work for you? If I understand correctly, you could set > GIT_REFLOG_NAME and GIT_REFLOG_EMAIL to an appropriate identity, but > you wouldn't have a good value to put in GIT_REFLOG_DATE. > > If GIT_COMMITTER_{NAME,EMAIL} were used when writing reflogs but > GIT_COMMITTER_DATE weren't, would that help with your workflow? Yes, it's really only GIT_COMMITTER_DATE that's messing me up personally. I never use GIT_COMMITTER_{NAME,EMAIL}. Luke ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: GIT_COMMITTER_* and reflog 2019-10-26 2:43 ` Luke Dashjr @ 2019-11-07 13:57 ` Philip Oakley 0 siblings, 0 replies; 9+ messages in thread From: Philip Oakley @ 2019-11-07 13:57 UTC (permalink / raw) To: Luke Dashjr, Jonathan Nieder, Johannes Schindelin; +Cc: git copying Dscho, On 26/10/2019 03:43, Luke Dashjr wrote: > On Saturday 26 October 2019 02:20:39 Jonathan Nieder wrote: >> Luke Dashjr wrote: >>> It appears the reflog currently allows its log data (name and date) to be >>> overridden by the GIT_COMMITTER_* environment variables. At least for my >>> workflow, this kinda breaks the reflog (as I regularly set >>> GIT_COMMITTER_DATE to produce deterministic commit objects). >> Can you say more about this? What is the workflow this is part of? Can >> you describe a sequence of steps and how you are affected during those >> steps? > I maintain a bleeding-edge variant of a more stable project, which is > constantly being rebased on the latest stable version. This looks to have a strong similarity to the Git-for-Windows patches which are regularly rebased on top of this here upstream Git. Dscho uses the `garden shears` script [1] to help update the patch set, in conjunction with the 'updated' --rebase-merges option (replaces the deprecated --preserve-merges). > To make this easier, I > use a Perl script to generate the bleeding-edge version's git branches: > https://github.com/bitcoinknots/assemble-deriv > It uses GIT_COMMITTER_DATE to ensure that I can repeatedly generate the branch > until everything merges successfully, without polluting the repository with > hundreds of merge commits each attempt. (Which would be annoying, since I > literally never prune.) > > Because git's reflog also uses GIT_COMMITTER_DATE, my reflogs (HEAD in > particular) get polluted with incorrect timestamps during this process. > >>> Is there a need to support this override for the reflog? >> Yes. >> >>> Is there any reason it can't be changed to use GIT_REFLOG_* instead? >> Would a new GIT_REFLOG_* set of envvars that overrides GIT_COMMITTER_* >> work for you? If I understand correctly, you could set >> GIT_REFLOG_NAME and GIT_REFLOG_EMAIL to an appropriate identity, but >> you wouldn't have a good value to put in GIT_REFLOG_DATE. >> >> If GIT_COMMITTER_{NAME,EMAIL} were used when writing reflogs but >> GIT_COMMITTER_DATE weren't, would that help with your workflow? > Yes, it's really only GIT_COMMITTER_DATE that's messing me up personally. > I never use GIT_COMMITTER_{NAME,EMAIL}. -- Philip [1] https://github.com/git-for-windows/build-extra/blob/master/shears.sh ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: GIT_COMMITTER_* and reflog 2019-10-26 2:20 ` Jonathan Nieder 2019-10-26 2:43 ` Luke Dashjr @ 2019-10-26 7:34 ` Junio C Hamano 2019-10-26 17:37 ` Jeff King 1 sibling, 1 reply; 9+ messages in thread From: Junio C Hamano @ 2019-10-26 7:34 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Luke Dashjr, git Jonathan Nieder <jrnieder@gmail.com> writes: > If GIT_COMMITTER_{NAME,EMAIL} were used when writing reflogs but > GIT_COMMITTER_DATE weren't, would that help with your workflow? Thanks for a thoughtful response. My knee-jerk reaction is that it probably was a design bug that came out of laziness that we used the usual mechanism to obtain the committer date when deciding the timestamp we leave in reflog entries. Given that we say master@{6.hours.ago} etc., we should base the timestamp on something that is coherent with what the end users would give us, e.g. "6.hours.ago". IOW, we should be using the wallclock time without paying attention to GIT_COMMITTER_DATE, i.e. date.c::get_time(). ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: GIT_COMMITTER_* and reflog 2019-10-26 7:34 ` Junio C Hamano @ 2019-10-26 17:37 ` Jeff King 2019-10-27 12:20 ` Junio C Hamano 0 siblings, 1 reply; 9+ messages in thread From: Jeff King @ 2019-10-26 17:37 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jonathan Nieder, Luke Dashjr, git On Sat, Oct 26, 2019 at 04:34:22PM +0900, Junio C Hamano wrote: > Jonathan Nieder <jrnieder@gmail.com> writes: > > > If GIT_COMMITTER_{NAME,EMAIL} were used when writing reflogs but > > GIT_COMMITTER_DATE weren't, would that help with your workflow? > > Thanks for a thoughtful response. > > My knee-jerk reaction is that it probably was a design bug that came > out of laziness that we used the usual mechanism to obtain the > committer date when deciding the timestamp we leave in reflog > entries. Given that we say master@{6.hours.ago} etc., we should > base the timestamp on something that is coherent with what the end > users would give us, e.g. "6.hours.ago". IOW, we should be using > the wallclock time without paying attention to GIT_COMMITTER_DATE, > i.e. date.c::get_time(). FWIW, I was about to write a similar response. If people really wanted a separate reflog ident, I could see introducing $GIT_REFLOG_NAME, etc. But the current date handling just seems buggy (and an unintended consequence of reusing the existing ident code). If somebody wants to pursue a patch, I suspect the solution is probably something like this (totally untested): diff --git a/refs/files-backend.c b/refs/files-backend.c index d60767ab73..2ebf2feeb8 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1658,7 +1658,10 @@ static int files_log_ref_write(struct files_ref_store *refs, if (logfd < 0) return 0; result = log_ref_write_fd(logfd, old_oid, new_oid, - git_committer_info(0), msg); + fmt_ident(getenv("GIT_COMMITTER_NAME"), + getenv("GIT_COMMITTER_EMAIL"), + WANT_COMMITTER_IDENT, NULL, 0), + msg); if (result) { struct strbuf sb = STRBUF_INIT; int save_errno = errno; -Peff ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: GIT_COMMITTER_* and reflog 2019-10-26 17:37 ` Jeff King @ 2019-10-27 12:20 ` Junio C Hamano 2019-10-29 14:05 ` Phillip Wood 2019-10-29 14:34 ` Jeff King 0 siblings, 2 replies; 9+ messages in thread From: Junio C Hamano @ 2019-10-27 12:20 UTC (permalink / raw) To: Jeff King; +Cc: Jonathan Nieder, Luke Dashjr, git Jeff King <peff@peff.net> writes: > If somebody wants to pursue a patch, I suspect the solution is probably > something like this (totally untested): Looks sensible. It is very much unsatisfying that datestamp(), which is used by fmt_ident() when no date string is given, seems to totally bypass date.c::get_time(), which means the framework to give fake timestamp via GIT_TEST_DATE_NOW cannot be used to write reproducible tests. Given that datestamp() is only used by the push certificate and fast-import codepaths and nowhere else, I suspect that "fixing" it retroactively to honor GIT_TEST_DATE_NOW would not have any negative fallout, but that's not something I should be contemplating on during the -rc period ;-) Thanks. > > diff --git a/refs/files-backend.c b/refs/files-backend.c > index d60767ab73..2ebf2feeb8 100644 > --- a/refs/files-backend.c > +++ b/refs/files-backend.c > @@ -1658,7 +1658,10 @@ static int files_log_ref_write(struct files_ref_store *refs, > if (logfd < 0) > return 0; > result = log_ref_write_fd(logfd, old_oid, new_oid, > - git_committer_info(0), msg); > + fmt_ident(getenv("GIT_COMMITTER_NAME"), > + getenv("GIT_COMMITTER_EMAIL"), > + WANT_COMMITTER_IDENT, NULL, 0), > + msg); > if (result) { > struct strbuf sb = STRBUF_INIT; > int save_errno = errno; > > -Peff ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: GIT_COMMITTER_* and reflog 2019-10-27 12:20 ` Junio C Hamano @ 2019-10-29 14:05 ` Phillip Wood 2019-10-29 14:34 ` Jeff King 1 sibling, 0 replies; 9+ messages in thread From: Phillip Wood @ 2019-10-29 14:05 UTC (permalink / raw) To: Junio C Hamano, Jeff King; +Cc: Jonathan Nieder, Luke Dashjr, git Hi Junio/Peff On 27/10/2019 12:20, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > >> If somebody wants to pursue a patch, I suspect the solution is probably >> something like this (totally untested): > > Looks sensible. It is very much unsatisfying that datestamp(), > which is used by fmt_ident() when no date string is given, seems to > totally bypass date.c::get_time(), which means the framework to give > fake timestamp via GIT_TEST_DATE_NOW cannot be used to write > reproducible tests. > > Given that datestamp() is only used by the push certificate and > fast-import codepaths and nowhere else, I suspect that "fixing" it > retroactively to honor GIT_TEST_DATE_NOW would not have any negative > fallout, but that's not something I should be contemplating on > during the -rc period ;-) It would allow for more robust am/rebase --ignore-date tests as we would know what date to expect rather than just checking the timezone. Best Wishes Phillip > > Thanks. > >> >> diff --git a/refs/files-backend.c b/refs/files-backend.c >> index d60767ab73..2ebf2feeb8 100644 >> --- a/refs/files-backend.c >> +++ b/refs/files-backend.c >> @@ -1658,7 +1658,10 @@ static int files_log_ref_write(struct files_ref_store *refs, >> if (logfd < 0) >> return 0; >> result = log_ref_write_fd(logfd, old_oid, new_oid, >> - git_committer_info(0), msg); >> + fmt_ident(getenv("GIT_COMMITTER_NAME"), >> + getenv("GIT_COMMITTER_EMAIL"), >> + WANT_COMMITTER_IDENT, NULL, 0), >> + msg); >> if (result) { >> struct strbuf sb = STRBUF_INIT; >> int save_errno = errno; >> >> -Peff ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: GIT_COMMITTER_* and reflog 2019-10-27 12:20 ` Junio C Hamano 2019-10-29 14:05 ` Phillip Wood @ 2019-10-29 14:34 ` Jeff King 1 sibling, 0 replies; 9+ messages in thread From: Jeff King @ 2019-10-29 14:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jonathan Nieder, Luke Dashjr, git On Sun, Oct 27, 2019 at 09:20:24PM +0900, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > If somebody wants to pursue a patch, I suspect the solution is probably > > something like this (totally untested): > > Looks sensible. It is very much unsatisfying that datestamp(), > which is used by fmt_ident() when no date string is given, seems to > totally bypass date.c::get_time(), which means the framework to give > fake timestamp via GIT_TEST_DATE_NOW cannot be used to write > reproducible tests. > > Given that datestamp() is only used by the push certificate and > fast-import codepaths and nowhere else, I suspect that "fixing" it > retroactively to honor GIT_TEST_DATE_NOW would not have any negative > fallout, but that's not something I should be contemplating on > during the -rc period ;-) Yeah, I agree datestamp() should respect $GIT_TEST_DATE_NOW. This whole topic is not something for the -rc period, I would think. -Peff ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-11-07 13:57 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-25 21:49 GIT_COMMITTER_* and reflog Luke Dashjr 2019-10-26 2:20 ` Jonathan Nieder 2019-10-26 2:43 ` Luke Dashjr 2019-11-07 13:57 ` Philip Oakley 2019-10-26 7:34 ` Junio C Hamano 2019-10-26 17:37 ` Jeff King 2019-10-27 12:20 ` Junio C Hamano 2019-10-29 14:05 ` Phillip Wood 2019-10-29 14:34 ` Jeff King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).