Re: [RFC PATCH] ident: don't cache default date

> Hi Phillip,
> 
> On Thu, 19 Apr 2018, Phillip Wood wrote:
> 
>> On 18/04/18 19:15, Johannes Sixt wrote:
>>> Am 18.04.2018 um 19:47 schrieb Phillip Wood:
>>>> On 18/04/18 12:27, Ævar Arnfjörð Bjarmason wrote:
>>>>> On Wed, Apr 18 2018, Phillip Wood wrote:
>>>>>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>>>>> as it is created by running an separate instance of 'git commit'.  If
>>>>>> the reworded commit is follow by further picks, those later commits
>>>>>> will have an earlier committer date than the reworded one. This is
>>>>>> caused by git caching the default date used when GIT_COMMITTER_DATE is
>>>>>> not set. Fix this by not caching the date.
>>>>>>
>>>>>> Users expect commits to have the same author and committer dates when
>>>>>> the don't explicitly set them. As the date is now updated each time
>>>>>> git_author_info() or git_committer_info() is run it is possible to end
>>>>>> up with different author and committer dates. Fix this for
>>>>>> 'commit-tree', 'notes' and 'merge' by using a single date in
>>>>>> commit_tree_extended() and passing it explicitly to the new functions
>>>>>> git_author_info_with_date() and git_committer_info_with_date() when
>>>>>> neither the author date nor the committer date are explicitly
>>>>>> set. 'commit' always passes the author date to commit_tree_extended()
>>>>>> and relied on the date caching to have the same committer and author
>>>>>> dates when neither was specified. Fix this by setting
>>>>>> GIT_COMMITTER_DATE to be the same as the author date passed to
>>>>>> commit_tree_extended().
>>>>>>
>>>>>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>>>>>> Reported-by: Johannes Sixt <j6t@kdbg.org>
>>>>>> ---
>>>>>>
>>>>>> I'm slightly nervous that setting GIT_COMMITTER_DATE in
>>>>>> builtin/commit.c will break someone's hook script. Maybe it would be
>>>>>> better to add a committer parameter to commit_tree() and
>>>>>> commit_tree_extended().
>>>
>>> While I like the basic theme of your patch, I think we should fix this
>>> case in a much simpler way, namely, use the infrastructure that was
>>> introduced for git-am.
>>>
>>> I've shamelessly lifted the commit message from your patch.
>>
>> Thanks, that is a better way (I'm annoyed with myself for not having
>> noticed reset_ident_date() when I edited the function above it)
> 
> Don't be too annoyed. I did remember that The Linus had complained about
> something similar and assumed that it had been fixed in the meantime, but
> I failed to find it within 30 minutes where I tried to dig through
> public-inbox and pu.

> Thanks Hannes for remembering, and for coming up with the final form of
> the patch!
> 
> Ciao,
> Dscho
>