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

Re: [PATCH v4 0/7] rebase -i: impove handling of failed commands

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Sep 7, 2023, 09:56 UTC
Message-ID
<6b927687-cf6e-d73e-78fb-bd4f46736928@gmx.de>
In-Reply-To
<pull.1492.v4.git.1694013771.gitgitgadget@gmail.com>
Hi Phillip,
On Wed, 6 Sep 2023, Phillip Wood via GitGitGadget wrote:
Show 85 quoted lines
> Range-diff vs v3:
>
>  1:  1ab1ad2ef07 ! 1:  ae4f873b3d0 rebase -i: move unlink() calls
>      @@ Metadata
>        ## Commit message ##
>           rebase -i: move unlink() calls
>
>      -    At the start of each iteration the loop that picks commits removes
>      -    state files from the previous pick. However some of these are only
>      -    written if there are conflicts and so we break out of the loop after
>      -    writing them. Therefore they only need to be removed when the rebase
>      -    continues, not in each iteration.
>      +    At the start of each iteration the loop that picks commits removes the
>      +    state files from the previous pick. However some of these files are only
>      +    written if there are conflicts in which case we exit the loop before the
>      +    end of the loop body. Therefore they only need to be removed when the
>      +    rebase continues, not at the start of each iteration.
>
>           Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>
>  2:  e2a758eb4a5 ! 2:  f540ed1d607 rebase -i: remove patch file after conflict resolution
>      @@ Commit message
>           now used in two different places rebase_path_patch() is added and used
>           to obtain the path for the patch.
>
>      +    To construct the path write_patch() previously used get_dir() which
>      +    returns different paths depending on whether we're rebasing or
>      +    cherry-picking/reverting. As this function is only called when
>      +    rebasing it is safe to use a hard coded string for the directory
>      +    instead. An assertion is added to make sure we don't starting calling
>      +    this function when cherry-picking in the future.
>      +
>           Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>
>        ## sequencer.c ##
>      @@ sequencer.c: static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend")
>         * For the post-rewrite hook, we make a list of rewritten commits and
>         * their new sha1s.  The rewritten-pending list keeps the sha1s of
>       @@ sequencer.c: static int make_patch(struct repository *r,
>      + 	char hex[GIT_MAX_HEXSZ + 1];
>      + 	int res = 0;
>      +
>      ++	if (!is_rebase_i(opts))
>      ++		BUG("make_patch should only be called when rebasing");
>      ++
>      + 	oid_to_hex_r(hex, &commit->object.oid);
>      + 	if (write_message(hex, strlen(hex), rebase_path_stopped_sha(), 1) < 0)
>        		return -1;
>        	res |= write_rebase_head(&commit->object.oid);
>
>  3:  8f6c0e40567 ! 3:  818bdaf772d sequencer: use rebase_path_message()
>      @@ Commit message
>           made function to get the path name instead. This was the last
>           remaining use of the strbuf so remove it as well.
>
>      +    As with the previous patch we now use a hard coded string rather than
>      +    git_dir() when constructing the path. This is safe for the same
>      +    reason (make_patch() is only called when rebasing) and is protected by
>      +    the assertion added in the previous patch.
>      +
>           Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>
>        ## sequencer.c ##
>  4:  a1fad70f4b9 = 4:  bd67765a864 sequencer: factor out part of pick_commits()
>  5:  df401945866 ! 5:  f6f330f7063 rebase: fix rewritten list for failed pick
>      @@ Commit message
>           disabled the user will see the messages from the merge machinery
>           detailing the problem.
>
>      -    To simplify writing REBASE_HEAD in this case pick_one_commit() is
>      -    modified to avoid duplicating the code that adds the failed command
>      -    back into the todo list.
>      +    The code to add a failed command back into the todo list is duplicated
>      +    between pick_one_commit() and the loop in pick_commits(). Both sites
>      +    print advice about the command being rescheduled, decrement the current
>      +    item and save the todo list. To avoid duplicating this code
>      +    pick_one_commit() is modified to set a flag to indicate that the command
>      +    should be rescheduled in the main loop. This simplifies things as only
>      +    the remaining copy of the code needs to be modified to set REBASE_HEAD
>      +    rather than calling error_with_patch().
>
>           Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>
>  6:  2ed7cbe5fff = 6:  0ca5fccca17 rebase --continue: refuse to commit after failed command
>  7:  bbe0afde512 = 7:  8d5f6d51e19 rebase -i: fix adding failed command to the todo list
Thank you for indulging me. This iteration looks good to me!

Ciao, Johannes

Previous: Junio C HamanoNext: Junio C Hamano
Message 79 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.