From: Kristoffer Haugsbakk Date: Thu, 09 Oct 2025 14:31:40 GMT Subject: Re: [PATCH] doc: warn against --committer-date-is-author-date Message-ID: In-Reply-To: <3a8dfd13-982d-4c83-b675-1e9a63bb6ab0@gmail.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 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!