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

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Kristoffer

On 08/10/2025 20:45, kristofferhaugsbakk@fastmail.com wrote:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
> 
> This option has legitimate uses but could create a commit history which
> violates the assumption that commits are strictly increasing in terms of
> commit timestamps. Warn against that in both git-am(1) and git-rebase(1).
> 
> ❦

What's this?

> 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]
> 
>      I've a big set of patches in a mbox file: there's sufficient info
>      inside for git-am to work.
> 
>      Yet, each time I do import these, my sha1sums are changing because of
>      different commit dates.
> 
>      I'd like to force the commit date to match the info/date from the time
>      I received the email (and therefore always get back the right
>      sha1sums).
> 
> 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.>
> [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.

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.

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

s/lie/override/ ?

> +	commits on top of a base which commit is older (in terms of the
> +	commit date) than the oldest patch you are applying.
> ++
> +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.
>   
>   --ignore-date::
>   	By default the command records the date from the e-mail
> 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.

> +	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.

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

Phillip