Re: [PATCH] doc: warn against --committer-date-is-author-date

["Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com>]

On Thu, Oct 9, 2025, at 15:46, Phillip Wood wrote:
> On 08/10/2025 20:45, kristofferhaugsbakk@fastmail.com wrote:
>>[snip]
>>
>> ❦
>
> What's this?

A thematic break.

>
>> The genesis of this option is 3f01ad66 (am: Add --committer-date-is-
>> author-date option, 2009-01-22). The commit message doesn’t give us an
>> example of a use case, but the thread starter does:[1]
>>
>>[snip quote]
>>
>> So the motivation was to treat git-am(1) as an import command that
>> creates the same commit IDs given the same base and committer.
>
> That seems like a reasonable thing for "git am" to do. I'd be interested
> to know what the rationale was for adding it to "git rebase". In
> retrospect I feel it was a mistake to port this option over to the
> sequencer just to match what the am based rebase did.

There isn’t any more discussion on the patch:

https://lore.kernel.org/git/1237399558-27289-3-git-send-email-barra_cuda@katamail.com/

>> [1]: https://lore.kernel.org/git/46d6db660901221441q60eb90bdge601a7a250c3a247@mail.gmail.com/
>>
>>      I thought about marking it as deprecated but eventually found out why it
>>      was added. And it wasn’t for some (still unknown) dedication or
>>      not-explained *want* to keep the committer date and author date in synch
>>      just-because (as I thought[1]).
>
> We should maybe think about deprecating it for "git rebase" though as it
> is a lot less clear that it is sensible there. If you're rebasing a
> branch then there is a very high likely hood that the upstream committer
> dates of the commits the branch is being rebased onto will be newer that
> the author dates of the commits in your branch.

That makes sense. If there is no use case then it should be deprecated.

I could mark it as such in the next version.

Anyone else have an opinion on this?

>
> I've left a couple of comments below
>> diff --git a/Documentation/git-am.adoc b/Documentation/git-am.adoc
>> index 221070de481..c36ae679cfb 100644
>> --- a/Documentation/git-am.adoc
>> +++ b/Documentation/git-am.adoc
>> @@ -156,11 +156,18 @@ Valid <action> for the `--whitespace` option are:
>>   	See also linkgit:githooks[5].
>>
>>   --committer-date-is-author-date::
>> -	By default the command records the date from the e-mail
>> -	message as the commit author date, and uses the time of
>> -	commit creation as the committer date. This allows the
>> -	user to lie about the committer date by using the same
>> -	value as the author date.
>> +	NOTE: The history walking machinery assumes that commits have
>> +	strictly increasing commit timestamps, with some tolerance for
>> +	clock skew (see linkgit:git-rev-list[1]).
>
> Is there a particuaar section of the rev-list man page you had in mind
> here? I had a quick look and I couldn't see anything about clock skew.

No, I just thought linking to the “history walking command” was apropos.
I’ll remove it in the next version.

>
>>  You should only use
>> +	this option to lie about the committer date when applying
>
> s/lie/override/ ?

I’ll make that change.

>>[snip]
>> diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
>> index 956d3048f5a..336ee90f7e3 100644
>> --- a/Documentation/git-rebase.adoc
>> +++ b/Documentation/git-rebase.adoc
>> @@ -504,9 +504,17 @@ merge backend;;
>>   See also INCOMPATIBLE OPTIONS below.
>>
>>   --committer-date-is-author-date::
>> -	Instead of using the current time as the committer date, use
>> -	the author date of the commit being rebased as the committer
>> -	date. This option implies `--force-rebase`.
>> +	NOTE: The history walking machinery assumes that commits have
>> +	strictly increasing commit timestamps, with some tolerance for
>> +	clock skew (see linkgit:git-rev-list[1]). You should only use
>> +	this option to lie about the committer date when applying
>> +	commits on top of a base which commit is older (in terms of the
>
> The comments above apply here as well. In addition s/applying
> commits/rebasing commits/ for this command I think.

Okay, thanks.

>
>> +	commit date) than the oldest commit you are applying (in
>> +	terms of the author date).
>
> We should also warn against using this option when rearranging commits
> with "git rebase -i" as well.

Okay but what does that mean? Should this “note” call out `-i`
specifically? And if so why is that?

> Thanks for working on this, it is a very good idea to add a warning to
> the documentation for this option. I'm going to be off the list for the
> next 10 days or so, I'll look at any re-roll when I return.

Thanks for the review!