Re: [PATCH] reset cached ident date before creating objects

2016-08-01 Thread Jeff King
On Mon, Aug 01, 2016 at 02:54:47PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > I also didn't go looking for other spots which might want similar > > treatment. Junio mentioned that the sequencer code still uses an > > external commit process, so cherry-pick/revert are

Re: [PATCH] reset cached ident date before creating objects

2016-08-01 Thread Junio C Hamano
Jeff King writes: > I also didn't go looking for other spots which might want similar > treatment. Junio mentioned that the sequencer code still uses an > external commit process, so cherry-pick/revert are OK. git-fast-import > creates a stream of commits, but already deals with

Re: [PATCH] reset cached ident date before creating objects

2016-08-01 Thread Jeff King
On Mon, Aug 01, 2016 at 02:00:47PM -0400, Jeff King wrote: > So at this point I think I'd lean towards programs which have multiple > logical commit operations explicitly saying "I am starting a new > operation". It may be that we end up attaching more to that in the long > run than just

Re: [PATCH] reset cached ident date before creating objects

2016-08-01 Thread Jeff King
On Mon, Aug 01, 2016 at 10:49:12AM -0700, Junio C Hamano wrote: > Jeff King writes: > > >> So maybe we would have to put reset_ident_date() at the end of the > >> function instead, at least after git_committer_info() is called. > > > > Yes, although "reset and end" still feels a

Re: [PATCH] reset cached ident date before creating objects

2016-08-01 Thread Jeff King
On Mon, Aug 01, 2016 at 02:12:34PM -0400, Jeff King wrote: > Before I switched to "reset at the beginning" in my original patch, I > also noticed this issue, but decided the other way: to only reset after > a successful creation. > > I think in most cases it wouldn't matter anyway, because the

Re: [PATCH] reset cached ident date before creating objects

2016-08-01 Thread Jeff King
On Mon, Aug 01, 2016 at 10:58:47AM -0700, Junio C Hamano wrote: > diff --git a/commit.c b/commit.c > index b02f3c4..db24013 100644 > --- a/commit.c > +++ b/commit.c > @@ -1543,7 +1543,6 @@ int commit_tree_extended(const char *msg, size_t > msg_len, > } > > /* Person/date

Re: [PATCH] reset cached ident date before creating objects

2016-08-01 Thread Junio C Hamano
Junio C Hamano writes: > Jeff King writes: > >>> So maybe we would have to put reset_ident_date() at the end of the >>> function instead, at least after git_committer_info() is called. >> >> Yes, although "reset and end" still feels a bit weird to me. >> >> I'd

Re: [PATCH] reset cached ident date before creating objects

2016-08-01 Thread Junio C Hamano
Jeff King writes: >> So maybe we would have to put reset_ident_date() at the end of the >> function instead, at least after git_committer_info() is called. > > Yes, although "reset and end" still feels a bit weird to me. > > I'd almost prefer to just have long-running programs

Re: [PATCH] reset cached ident date before creating objects

2016-07-29 Thread Jeff King
On Sat, Jul 30, 2016 at 10:11:56AM +0800, Paul Tan wrote: > > diff --git a/commit.c b/commit.c > > index 71a360d..7ddbffe 100644 > > --- a/commit.c > > +++ b/commit.c > > @@ -1548,6 +1548,7 @@ int commit_tree_extended(const char *msg, size_t > > msg_len, > > } > > > > /*

Re: [PATCH] reset cached ident date before creating objects

2016-07-29 Thread Paul Tan
Hi Jeff, On Sat, Jul 30, 2016 at 2:05 AM, Jeff King wrote: > When we compute the date to put in author/committer lines of > commits, or tagger lines of tags, we get the current date > once and then cache it for the rest of the program. This is > a good thing in some cases, like

Re: [PATCH] reset cached ident date before creating objects

2016-07-29 Thread Jeff King
On Fri, Jul 29, 2016 at 11:15:39AM -0700, Junio C Hamano wrote: > > It does feel a little backwards to cache by default, and then try to > > catch all the places that want to reset. Another way of thinking about > > it would be to almost _never_ cache, but let a few callsites like (the > > commit

Re: [PATCH] reset cached ident date before creating objects

2016-07-29 Thread Junio C Hamano
Jeff King writes: > Linus suggested resetting the timestamp after making the commit, to > clear the way for the next commit. But if we reset any cached value > _before_ making the commit, this has a few advantages: I guess our mails crossed ;-). > It does feel a little backwards

Re: [PATCH] reset cached ident date before creating objects

2016-07-29 Thread Linus Torvalds
On Fri, Jul 29, 2016 at 11:05 AM, Jeff King wrote: > > Linus suggested resetting the timestamp after making the commit, to > clear the way for the next commit. But if we reset any cached value > _before_ making the commit, this has a few advantages: Looks fine to me. It should

[PATCH] reset cached ident date before creating objects

2016-07-29 Thread Jeff King
On Fri, Jul 29, 2016 at 10:15:26AM -0700, Junio C Hamano wrote: > > One obvious impact would be reflog entries, since we would reset the > > time between the object creation and the ref write (so your reflog entry > > would sometimes be a second or more after the commit time it writes). > > I'm