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

Re: [PATCH] rebase -i: do not update "done" when rescheduling command

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Aug 23, 2023, 08:54 UTC
Message-ID
<03d6f4df-d175-350c-7e75-ba78be80ba4e@gmx.de>
In-Reply-To
<d20ffc1f-c3b5-0f64-d508-976098f418e0@gmail.com>
Hi Phillip,
On Thu, 3 Aug 2023, Phillip Wood wrote:
Show 60 quoted lines
> On 27/03/2023 08:04, Johannes Schindelin wrote:
> >
> > On Sun, 19 Mar 2023, Phillip Wood via GitGitGadget wrote:
> >
> > > From: Phillip Wood <phillip.wood@dunelm.org.uk>
> > >
> > > As the sequencer executes todo commands it appends them to
> > > .git/rebase-merge/done. This file is used by "git status" to show the
> > > recently executed commands. Unfortunately when a command is rescheduled
> > > the command preceding it is erroneously appended to the "done" file.
> > > This means that when rebase stops after rescheduling "pick B" the "done"
> > > file contains
> > >
> > >  pick A
> > >  pick B
> > >  pick A
> > >
> > > instead of
> > >
> > >  pick A
> > >  pick B
> > >
> > > Fix this by not updating the "done" file when adding a rescheduled
> > > command back into the "todo" file. A couple of the existing tests are
> > > modified to improve their coverage as none of them trigger this bug or
> > > check the "done" file.
> >
> > I am purposefully not yet focusing on the patch, as I have a concern about
> > the reasoning above.
> >
> > When a command fails that needs to be rescheduled, I actually _like_ that
> > there is a record in `done` about said command. It is very much like a
> > `pick` that failed (but was not rescheduled) and was then `--skip`ed: it
> > still shows up on `done`.
>
> We still do that after this patch. What changes is that when "pick B" fails we
> don't add "pick A" to the "done" file when "pick B" is added back into
> "git-rebase-todo"
>
> > I do understand the concern that the rescheduled command now shows up in
> > both `done` and `git-rebase-todo` (which is very different from the failed
> > `pick` command that would show up _only_ in `git-rebase-todo`). So maybe
> > we can find some compromise, e.g. adding a commented-out line to `done` à
> > la:
> >
> >  # rescheduled: pick A
> >
> > What do you think?
>
> If a commit is rescheduled we still end up with multiple entries in the
> "done". In the example above if "pick B" fails the first time it is executed
> and then succeeds on the second attempt "done" will contain
>
> 	pick A
> 	pick B
> 	pick B
>
> It might be nice to mark it as rescheduled as you suggest but this series
> focuses on removing the incorrect entry from the "done" file, not
> de-duplicating the "done" entities when a command fails.

After having had plenty of time to let this issue simmer in the back of my head, and after reviewing the latest iteration of the patch series, I am no longer concerned, as it now sounds more logical to me, too, that rescheduled commands don't show up in `done`.

Thank you, Johannes

Previous: Phillip WoodNext: Phillip Wood via GitGitGadget
Message 9 of 80 in “rebase -i: do not update "done" when rescheduling command”
  1. rebase -i: do not update "done" when rescheduling commandPhillip Wood via GitGitGadget, Mar 19, 2023
  2. Stefan HallerMar 20, 2023
  3. Junio C HamanoMar 20, 2023
  4. Phillip WoodMar 24, 2023
  5. Junio C HamanoMar 24, 2023
  6. Phillip WoodMar 24, 2023
  7. Johannes SchindelinMar 27, 2023
  8. Phillip WoodAug 3, 2023
  9. Johannes SchindelinAug 23, 2023
  10. 0/6 rebase -i: impove handling of failed commandsPhillip Wood via GitGitGadget, Apr 21, 2023
  11. 1/6 rebase -i: move unlink() callsPhillip Wood via GitGitGadget, Apr 21, 2023
  12. Junio C HamanoApr 21, 2023
  13. Phillip WoodApr 27, 2023
  14. 2/6 rebase -i: remove patch file after conflict resolutionPhillip Wood via GitGitGadget, Apr 21, 2023
  15. Junio C HamanoApr 21, 2023
  16. Phillip WoodApr 27, 2023
  17. Glen ChooJun 21, 2023
  18. Phillip WoodJul 14, 2023
  19. Junio C HamanoJul 14, 2023
  20. Phillip WoodJul 17, 2023
  21. 3/6 sequencer: factor out part of pick_commits()Phillip Wood via GitGitGadget, Apr 21, 2023
  22. Eric SunshineApr 21, 2023
  23. Junio C HamanoApr 21, 2023
  24. Phillip WoodApr 21, 2023
  25. Junio C HamanoApr 21, 2023
  26. 4/6 rebase --continue: refuse to commit after failed commandPhillip Wood via GitGitGadget, Apr 21, 2023
  27. Eric SunshineApr 21, 2023
  28. Junio C HamanoApr 21, 2023
  29. Glen ChooJun 21, 2023
  30. 5/6 rebase: fix rewritten list for failed pickPhillip Wood via GitGitGadget, Apr 21, 2023
  31. Glen ChooJun 21, 2023
  32. Phillip WoodJul 25, 2023
  33. Glen ChooJul 25, 2023
  34. Phillip WoodJul 26, 2023
  35. Glen ChooJul 26, 2023
  36. Phillip WoodJul 28, 2023
  37. 6/6 rebase -i: fix adding failed command to the todo listPhillip Wood via GitGitGadget, Apr 21, 2023
  38. Glen ChooJun 21, 2023
  39. Junio C HamanoApr 21, 2023
  40. Glen ChooJun 21, 2023
  41. 0/7 rebase -i: impove handling of failed commandsPhillip Wood via GitGitGadget, Aug 1, 2023
  42. 2/7 rebase -i: remove patch file after conflict resolutionPhillip Wood via GitGitGadget, Aug 1, 2023
  43. Junio C HamanoAug 1, 2023
  44. Phillip WoodAug 1, 2023
  45. 1/7 rebase -i: move unlink() callsPhillip Wood via GitGitGadget, Aug 1, 2023
  46. Junio C HamanoAug 1, 2023
  47. Phillip WoodAug 1, 2023
  48. Junio C HamanoAug 1, 2023
  49. 3/7 sequencer: use rebase_path_message()Phillip Wood via GitGitGadget, Aug 1, 2023
  50. Junio C HamanoAug 1, 2023
  51. Phillip WoodAug 1, 2023
  52. Junio C HamanoAug 2, 2023
  53. 4/7 sequencer: factor out part of pick_commits()Phillip Wood via GitGitGadget, Aug 1, 2023
  54. Johannes SchindelinAug 23, 2023
  55. 5/7 rebase: fix rewritten list for failed pickPhillip Wood via GitGitGadget, Aug 1, 2023
  56. Johannes SchindelinAug 23, 2023
  57. Phillip WoodSep 4, 2023
  58. 6/7 rebase --continue: refuse to commit after failed commandPhillip Wood via GitGitGadget, Aug 1, 2023
  59. Johannes SchindelinAug 23, 2023
  60. Phillip WoodSep 4, 2023
  61. Johannes SchindelinSep 5, 2023
  62. Junio C HamanoSep 5, 2023
  63. Phillip WoodSep 5, 2023
  64. 7/7 rebase -i: fix adding failed command to the todo listPhillip Wood via GitGitGadget, Aug 1, 2023
  65. Junio C HamanoAug 2, 2023
  66. Phillip WoodAug 3, 2023
  67. Phillip WoodAug 9, 2023
  68. Glen ChooAug 7, 2023
  69. Phillip WoodAug 9, 2023
  70. 0/7 rebase -i: impove handling of failed commandsPhillip Wood via GitGitGadget, Sep 6, 2023
  71. 3/7 sequencer: use rebase_path_message()Phillip Wood via GitGitGadget, Sep 6, 2023
  72. 2/7 rebase -i: remove patch file after conflict resolutionPhillip Wood via GitGitGadget, Sep 6, 2023
  73. 1/7 rebase -i: move unlink() callsPhillip Wood via GitGitGadget, Sep 6, 2023
  74. 4/7 sequencer: factor out part of pick_commits()Phillip Wood via GitGitGadget, Sep 6, 2023
  75. 5/7 rebase: fix rewritten list for failed pickPhillip Wood via GitGitGadget, Sep 6, 2023
  76. 6/7 rebase --continue: refuse to commit after failed commandPhillip Wood via GitGitGadget, Sep 6, 2023
  77. 7/7 rebase -i: fix adding failed command to the todo listPhillip Wood via GitGitGadget, Sep 6, 2023
  78. Junio C HamanoSep 6, 2023
  79. Johannes SchindelinSep 7, 2023
  80. Junio C HamanoSep 7, 2023

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.