Subject: Re: How dangerous is --committer-date-is-author-date these days?

From: Phillip Wood <[email-masked]>
Date: Sat, 28 Sep 2024 10:49:17 +0100

Hi Johannes

On 28/09/2024 07:59, Johannes Sixt wrote:
> The option --committer-date-is-author-date of git-rebase rewrites the
> committer dates like its name suggests. It is not uncommon that commits
> are rearranged and cherry-picked. Then, as a consequence, author dates
> are not decreasing when walking back in history. Now, if such a history
> with a non-monotonic author date is rebased one final time with
> --committer-date-is-author-date, this creates a history with
> non-monotonic committer dates. I recall that this is not a good thing to
> have since it can confuse our history walker.
> 
> - Why do we have --committer-date-is-author-date in a porcelain command?

Support was added to the sequencer to reduce the differences between the 
two rebase backends in the hope that one day we'll be able to remove the 
apply based backend. Support was added to the shell based rebase in 
570ccad33e (rebase: add options passed to git-am, 2009-03-18), there is 
not much discussion in the commit message or mailing list thread [1] 
about the motivation for adding this support.

[1] 
https://lore.kernel.org/git/[email-masked]/

> - Should we remove it?

It is only a problem when re-arranging commits - even then I think the 
commit walk machinery has some tolerance to commit dates that do not 
increase monotonically in order accommodate clocks that are out of sync. 
It is perfectly fine for non-interactive rebases (or just squashing 
fixups) so removing it seems like throwing out the baby with the bathwater.

> - Should we require an explicit --force instead of implying it?

I think we'd want a convincing reason to change the behavior - the other 
options that require the history to be rewritten all imply "--force" 
rather than requiring the user to pass it separately.

> - Should we issue a big warning about the consequences?

It would certainly be worth adding a warning to the documentation. To 
issue a warning at run-time would require us to check that the commits 
are actually being re-arranged as there are plenty of reasons to use 
"--interactive" without changing the order of commits.

Best Wishes

Phillip