Re: [PATCH] doc: warn against --committer-date-is-author-date
- From
- Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
- Date
- Oct 9, 2025, 14:31 UTC
- Message-ID
- <aae39545-461a-44f0-b01f-bb40b53b1858@app.fastmail.com>
- In-Reply-To
- <3a8dfd13-982d-4c83-b675-1e9a63bb6ab0@gmail.com>
On Thu, Oct 9, 2025, at 15:46, Phillip Wood wrote:
Show 6 quoted lines
> On 08/10/2025 20:45, kristofferhaugsbakk@fastmail.com wrote: >>[snip] >> >> ❦ > > What's this?
A thematic break.
Show 14 quoted lines
> >> 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/
Show 12 quoted lines
>> [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?
Show 21 quoted lines
> > 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.
Show 5 quoted lines
> >> You should only use >> + this option to lie about the committer date when applying > > s/lie/override/ ?
I’ll make that change.
Show 20 quoted lines
>>[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.
Show 6 quoted lines
> >> + 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!