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

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Sep 28, 2024, 09:49 UTC
Message-ID: <aa981bb7-dd3b-4e63-9769-0fc2559983e6@gmail.com>
In-Reply-To: <6af09726-e3bf-4903-87ae-9524ad334678@kdbg.org>

Hi Johannes

On 28/09/2024 07:59, Johannes Sixt wrote:

Show 10 quoted lines

> 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/1237409629-4289-1-git-send-email-barra_cuda@katamail.com/

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