git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] rebase -i: remove CHERRY_PICK_HEAD when cherry-pick failed

From
AWAndrew Wong <andrew.w@sohovfx.com>
Date
Apr 3, 2012, 21:01 UTC
Message-ID
<4F7B650C.9060800@sohovfx.com>
In-Reply-To
<20120403144505.GE15589@burratino>
On 04/03/2012 10:45 AM, Jonathan Nieder wrote:
Show 5 quoted lines
> Ok.  Now the user (sensibly) ignores the message from cherry-pick and
> just runs "git rebase --continue".  The rebase finishes but nobody
> feels it's his responsibility to remove the .git/CHERRY_PICK_HEAD file
> and it gets left behind.
>   
Yes, that's exactly what's happening. That particular rebase will leave
behind CHERRY_PICK_HEAD, which have bad consequences such as:
1. Confuses __git_ps1 (from git-completion.bash) into thinking a
cherry-pick is still in progress. (which is what started this discussion)
2. Cause "cherry-pick --continue" to think a cherry-pick is still in
progress.
3. Similarly, if a user then continue on to modifying their files, and
do a "add" and "commit", "commit" would reuse the message from the
CHERRY_PICK_HEAD.
> I suspect a more appropriate long-term fix would involve "git
> cherry-pick" noticing when a patch has resolved to nothing instead of
> leaving it to "git commit" to detect that.
>   

I actually tried implementing a fix like that too. But then I thought there might be other scenarios where "commit" could fail, and it doesn't seem to make sense for "cherry-pick" to have to detect all possible "commit" failures. Though it also feels like the question of whether or not "cherry-pick" should detects the "empty commit" is a separate issue altogether.

Besides the "empty commit" failure, "cherry-pick" can still run into various errors, such as merge conflict. So it will have to keep a state somehow. And instead of having "cherry-pick" make special cases for "rebase -i" to remove the state, it makes more sense to teach "rebase -i" that "cherry-pick" now keeps a state on failure. So if "cherry-pick" fails, "rebase -i" is responsible for clearing that state. And that's what this patch is supposed to do.

Perhaps I should rephrase my description to reflect this better? Something along the line of: "cherry-pick" now keeps a state on failure. Instead of having a special case inside the sequencer to remove the state, we teach "rebase -i" that we need to clear the state.

Previous: Jonathan NiederNext: Jonathan Nieder
Message 13 of 22 in “Rebase regression in v1.7.9?”
  1. Felipe ContrerasJan 31, 2012
  2. Andrew WongFeb 1, 2012
  3. Felipe ContrerasFeb 1, 2012
  4. rebase -i: remove CHERRY_PICK_HEAD when cherry-pick failedAndrew Wong, Mar 18, 2012
  5. Junio C HamanoMar 19, 2012
  6. Andrew WongMar 19, 2012
  7. Andrew WongMar 24, 2012
  8. Andrew WongApr 2, 2012
  9. Junio C HamanoApr 2, 2012
  10. Junio C HamanoApr 3, 2012
  11. Ramkumar RamachandraApr 3, 2012
  12. Jonathan NiederApr 3, 2012
  13. Andrew WongApr 3, 2012
  14. Jonathan NiederApr 3, 2012
  15. Jonathan NiederApr 3, 2012
  16. Andrew WongApr 3, 2012
  17. Jonathan NiederApr 3, 2012
  18. Andrew WongApr 3, 2012
  19. Jonathan NiederApr 4, 2012
  20. Andrew WongApr 4, 2012
  21. Jonathan NiederApr 4, 2012
  22. Jonathan NiederApr 4, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.