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

[Jeff King <peff@peff.net>]

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,
> >         }
> >
> >         /* Person/date information */
> > +       reset_ident_date();
> >         if (!author)
> >                 author = git_author_info(IDENT_STRICT);
> >         strbuf_addf(&buffer, "author %s\n", author);
> 
> But since builtin/commit.c constructs its author ident string before
> calling the editor and then commit_tree_extended(), this would cause
> the resulting commits to have committer timestamps which differ from
> their author timestamps.

Hrm, yeah. I assumed it would only pass in the author string for things
like "-c" or "--amend". But it looks like it unconditionally passes in
the author.  And it would be slightly difficult to have it pass NULL,
because it may actually have _part_ of an author (e.g., "--author" will
come up with name and email but not the date), so it has to sometimes
combine those bits with things like ident_default_date() itself.

I guess one option would be to commit_tree_extended() to take the
broken-down author bits and call fmt_ident() itself. That's what all of
the callers are doing (that, or just passing NULL). It would make the
interface a bit clunkier, but I think the end result would be more
flexible.

I suppose that would be tricky for git-commit, because in addition to
passing the result of fmt_ident() to commit_tree_extended(), it wants to
take the pieces and put them in the environment for hooks to see. And if
the data is available only inside commit_tree_extended(), we don't have
it for the hooks.

> 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 insert resets at
strategic points.

-Peff