From: Elijah Newren <email@example.com> To: Junio C Hamano <firstname.lastname@example.org> Cc: Elijah Newren via GitGitGadget <email@example.com>, Git Mailing List <firstname.lastname@example.org>, Phillip Wood <email@example.com>, Johannes Schindelin <Johannes.Schindelin@gmx.de>, Bryan Turner <firstname.lastname@example.org>, Sami Boukortt <email@example.com> Subject: Re: [PATCH v2 0/3] rebase -i: mark commits that begin empty in todo editor Date: Fri, 10 Apr 2020 15:11:57 -0700 Message-ID: <CABPp-BECuio43GScBsPx3er60nHmrYnLMzOtibdodJW+KGFU5Q@mail.gmail.com> (raw) In-Reply-To: <firstname.lastname@example.org> On Fri, Apr 10, 2020 at 2:14 PM Junio C Hamano <email@example.com> wrote: > > Junio C Hamano <firstname.lastname@example.org> writes: > > > This is a total tangent, but when I tried to rebase > > jt/rebase-allow-duplicate that builds directly on top of v2.25.0 to > > a newer base, after resolving conflicts, "commit -a" and "rebase > > --continue", somewhere I seem to have mangled the authorship. It > > could entirely be a driver error, or it may be a bug in "rebase", > > especially with recent backend change. I am planning to come back > > to it later to figure out if there is such a bug, but I'd need to > > recover from the authorship screwup first, so it may take some > > time. > > I think I know how it happens now. > > 52e8d1942c662 == jt/rebase-allow-duplicate > > Attempting to rebase this on top of the 'master', i.e. > > $ git rebase --onto master 52e8d1942c662^ 52e8d1942c662 > > results in a merge conflict, but it is easy to resolve in the > working tree. Then after "git add -u" to record the resolution > in the index > > $ git commit > $ git rebase --continue > > gives me the authorship credit. > > Back when the default rebase backend was 'apply', making the above > "mistake" was not even possible; "git commit" would have given me an > empty log buffer to edit, without pre-filling anything, to help me > realize that I shouldn't commit. "was not even possible" and "to help me realize that I shouldn't commit" seem slightly at odds, but I think you're saying that the UI was a bit better at helping you abort before you continued with such an accident. Perhaps it could be improved even more while we're here? > With the sequencer backend, however, the above procedure happily > records the commit under the author's name, it seems. > > I am not sure if that is a bug. I am inclined to say so. I am inclined to say so too, but it does get a bit more complicated...with just a few cases I can think of off the top of my head. In the case of the merge backend, specifically with --interactive, it does make sense to use "break" or "edit" commands and then allow the user to run "git commit" in a stopped rebase. It also could make sense in some cases to hit a conflict, allow the user to run "git reset HEAD" and then start fixing and staging pieces of the changes for a commit and then manually commit the pieces (which then gain a different author and different commit message and rearranged/subsetted/modified contents, with the expectation that they'd probably do a Patched-based-on-work-by tag or equivalent). But if we're in the middle of a rebase-apply, or in the middle of a rebase-merge and resolving a conflict (i.e. not cases like the "break" or "edit" or just ran "git reset HEAD" cases), or in the middle of a cherry-pick and resolving a conflict, then I'm inclined to agree with you that calling "git commit" represents an accident by the user that should result in an error. For my purposes here, "git add -u" shouldn't mean we're no longer "busy resolving a conflict" (otherwise git commit would have already stopped you); that state shouldn't go away until "rebase --continue" or "cherry-pick --continue" or "git reset HEAD" is executed. But it gets slightly weird, though, since "git reset HEAD" operates differently in merge and apply modes (with apply still attempting to commit something afterwards and merge realizing that a reset means that commit was tossed). So it might take a little more care and history digging to make sure that the various cases are handled sanely.
next prev parent reply index Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-09 23:26 [PATCH] " Elijah Newren via GitGitGadget 2020-04-10 0:50 ` Junio C Hamano 2020-04-10 2:06 ` Bryan Turner 2020-04-10 4:57 ` Junio C Hamano 2020-04-10 17:51 ` [PATCH v2 0/3] " Elijah Newren via GitGitGadget 2020-04-10 17:51 ` [PATCH v2 1/3] " Elijah Newren via GitGitGadget 2020-04-10 17:51 ` [PATCH v2 2/3] rebase: reinstate --no-keep-empty Elijah Newren via GitGitGadget 2020-04-10 20:37 ` Junio C Hamano 2020-04-10 21:41 ` Elijah Newren 2020-04-10 17:51 ` [PATCH v2 3/3] rebase: fix an incompatible-options error message Elijah Newren via GitGitGadget 2020-04-10 20:42 ` [PATCH v2 0/3] rebase -i: mark commits that begin empty in todo editor Junio C Hamano 2020-04-10 21:04 ` Junio C Hamano 2020-04-10 21:14 ` Junio C Hamano 2020-04-10 22:11 ` Elijah Newren [this message] 2020-04-10 21:29 ` Junio C Hamano 2020-04-10 22:13 ` Elijah Newren 2020-04-10 22:30 ` Junio C Hamano 2020-04-11 0:07 ` Elijah Newren 2020-04-11 21:14 ` Junio C Hamano 2020-04-11 2:44 ` [PATCH v3 0/4] " Elijah Newren via GitGitGadget 2020-04-11 2:44 ` [PATCH v3 1/4] " Elijah Newren via GitGitGadget 2020-04-15 20:52 ` Junio C Hamano 2020-04-11 2:44 ` [PATCH v3 2/4] rebase: reinstate --no-keep-empty Elijah Newren via GitGitGadget 2020-04-11 2:44 ` [PATCH v3 3/4] rebase: fix an incompatible-options error message Elijah Newren via GitGitGadget 2020-04-11 2:44 ` [PATCH v3 4/4] rebase --merge: optionally skip upstreamed commits Jonathan Tan via GitGitGadget 2020-04-11 18:12 ` Jonathan Tan 2020-04-14 9:11 ` [PATCH v3 0/4] rebase -i: mark commits that begin empty in todo editor Phillip Wood
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CABPp-BECuio43GScBsPx3er60nHmrYnLMzOtibdodJW+KGFU5Q@mail.gmail.com \ --email@example.com \ --cc=Johannes.Schindelin@gmx.de \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Git Mailing List Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/git/0 git/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 git git/ https://lore.kernel.org/git \ firstname.lastname@example.org public-inbox-index git Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git