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

Re: [PATCH 2/2] git-rebase: error out when incompatible options passed

From
PWPhillip Wood <phillip.wood@talktalk.net>
Date
Jun 12, 2018, 09:45 UTC
Message-ID
<cd30ac59-4715-dc33-6de4-aa009134ac87@talktalk.net>
In-Reply-To
<CABPp-BF6HapQ=tcWu4j=L-sjgTRbdLnhYXBWwdWm+oThav_zJw@mail.gmail.com>
On 11/06/18 16:49, Elijah Newren wrote:
Show 59 quoted lines
> Another thing I missed...
> 
> On Sun, Jun 10, 2018 at 12:40 PM, Phillip Wood
> <phillip.wood@talktalk.net> wrote:
>> On 07/06/18 06:06, Elijah Newren wrote:
> 
>>> Some exceptions I left out:
>>>
>>>    * --merge and --interactive are technically incompatible since they are
>>>      supposed to run different underlying scripts, but with a few small
>>>      changes, --interactive can do everything that --merge can.  In fact,
>>>      I'll shortly be sending another patch to remove git-rebase--merge and
>>>      reimplement it on top of git-rebase--interactive.
>>
>> Excellent I've wondered about doing that but never got round to it. One
>> thing I was slightly concerned about was that someone maybe parsing the
>> output of git-rebase--merge and they'll get a nasty shock when that output
>> changes as a result of using the sequencer.
> 
> I can see the minor worry, but I think upon inspection it's not
> something that concerns me, for a few reasons:
> 
> In terms of use, given that rebase --merge was introduced to handle
> renames in mid-2006, but plain rebase has been able to handle them
> since late 2008 (though directory renames changes that again), the
> utility of rebase --merge has been somewhat limited.  I think that
> limits the exposure.  But to address the 'break' more directly...
> 
> If we were to agree that we needed to support folks parsing rebase
> output, that would be a really strict requirement that I think would
> prevent lots of fixes.  And if so, it's one we've violated a number of
> times.  For example, I certainly wasn't thinking about rebase when I
> modified messages in merge-recursive.c over the years, but they'd leak
> through for rebase --merge.  (Those messages would not leak through to
> rebase --interactive as much, since the sequencer sets o.buffer_output
> and then only conditionally shows the output.)  Also, changes that
> have occurred in the past, like adding 'git gc --auto' to rebase,
> modifying error messages directly found in git-rebase--merge.sh would
> have been considered breaks.
> 
> Finally, looking over all the differences in output while fixing up
> testcases makes me think we've done much less around designing the
> output based on what we want the user to see, and more around what
> minimal fixups can we do to these lower level commands that provide
> useful functionality to the user?  We linearize history differently
> for different rebase modes, have different entries in the reflog
> depending on which mode, and we often times implement features for
> just one mode and then sometimes add it to others.  In fact, I think
> the primary reason that am-based and merge-based rebases had a --quiet
> option and the interactive rebases didn't, is mostly attributable to
> the defaults of the lower level commands these three were built on top
> of (git-am vs. git-merge-recursive vs. git-cherry-pick).  The noiser
> two merited a quiet option, and the option was never added for the
> last one.
> 
> Anyway, that's my rationale.  I'm curious to hear if folks disagree or
> see things I'm overlooking or have reasons I might be weighting
> tradeoffs less than optimally.
> 

I agree that there are already plenty of inconsistencies, (it's great to see you reducing them). If we can avoid emulating the ouput of git-rebase--merge.sh in sequencer.c that would definitely be my preferred option (the code is already a bit hard to follow in places where there it's doing slightly different things for cherry-pick and rebase -i). Hopefully no one is relying on the output, as you say it's just whatever the underlying plumbing prints rather than designed for a specific purpose.

Best Wishes
Phillip
Previous: Elijah NewrenNext: Elijah Newren
Message 18 of 130 in “RFC: rebase inconsistency in 2.18 -- course of action?”
  1. Elijah NewrenJun 7, 2018
  2. t3401: add directory rename testcases for rebase and amElijah Newren, Jun 7, 2018
  3. Elijah NewrenJun 25, 2018
  4. apply: fix grammar error in commentElijah Newren, Jun 7, 2018
  5. t5407: fix test to cover intended argumentsElijah Newren, Jun 7, 2018
  6. git-rebase--merge: modernize "git-$cmd" to "git $cmd"Elijah Newren, Jun 7, 2018
  7. Elijah NewrenJun 7, 2018
  8. git-rebase--merge: modernize "git-$cmd" to "git $cmd"Elijah Newren, Jun 27, 2018
  9. Johannes SchindelinJul 12, 2018
  10. Elijah NewrenJul 13, 2018
  11. Johannes SchindelinJul 14, 2018
  12. 1/2 t3422: new testcases for checking when incompatible options passedElijah Newren, Jun 7, 2018
  13. 2/2 git-rebase: error out when incompatible options passedElijah Newren, Jun 7, 2018
  14. Phillip WoodJun 10, 2018
  15. Elijah NewrenJun 11, 2018
  16. Phillip WoodJun 11, 2018
  17. Elijah NewrenJun 11, 2018
  18. Phillip WoodJun 12, 2018
  19. 0/7 Document/fix/warn about rebase incompatibilities and inconsistencesElijah Newren, Jun 17, 2018
  20. 1/7 git-rebase.txt: document incompatible optionsElijah Newren, Jun 17, 2018
  21. Phillip WoodJun 17, 2018
  22. Elijah NewrenJun 20, 2018
  23. Eric SunshineJun 17, 2018
  24. Elijah NewrenJun 20, 2018
  25. 2/7 git-rebase.sh: update help messages a bitElijah Newren, Jun 17, 2018
  26. 3/7 t3422: new testcases for checking when incompatible options passedElijah Newren, Jun 17, 2018
  27. 4/7 git-rebase: error out when incompatible options passedElijah Newren, Jun 17, 2018
  28. 7/7 git-rebase: make --allow-empty-message the defaultElijah Newren, Jun 17, 2018
  29. Phillip WoodJun 17, 2018
  30. Elijah NewrenJun 20, 2018
  31. 6/7 git-rebase.txt: address confusion between --no-ff vs --force-rebaseElijah Newren, Jun 17, 2018
  32. 5/7 git-rebase.txt: document behavioral inconsistencies between modesElijah Newren, Jun 17, 2018
  33. Phillip WoodJun 17, 2018
  34. 0/7 Document/fix/warn about rebase incompatibilities and inconsistenciesElijah Newren, Jun 21, 2018
  35. 7/7 git-rebase: make --allow-empty-message the defaultElijah Newren, Jun 21, 2018
  36. 3/7 t3422: new testcases for checking when incompatible options passedElijah Newren, Jun 21, 2018
  37. Junio C HamanoJun 21, 2018
  38. Elijah NewrenJun 21, 2018
  39. Eric SunshineJun 21, 2018
  40. 2/7 git-rebase.sh: update help messages a bitElijah Newren, Jun 21, 2018
  41. Junio C HamanoJun 21, 2018
  42. Elijah NewrenJun 21, 2018
  43. 5/7 git-rebase.txt: document behavioral inconsistencies between modesElijah Newren, Jun 21, 2018
  44. Junio C HamanoJun 21, 2018
  45. Elijah NewrenJun 21, 2018
  46. 1/7 git-rebase.txt: document incompatible optionsElijah Newren, Jun 21, 2018
  47. Junio C HamanoJun 21, 2018
  48. Elijah NewrenJun 21, 2018
  49. SZEDER GáborJun 22, 2018
  50. 4/7 git-rebase: error out when incompatible options passedElijah Newren, Jun 21, 2018
  51. Junio C HamanoJun 21, 2018
  52. Elijah NewrenJun 21, 2018
  53. 6/7 git-rebase.txt: address confusion between --no-ff vs --force-rebaseElijah Newren, Jun 21, 2018
  54. Junio C HamanoJun 21, 2018
  55. Elijah NewrenJun 21, 2018
  56. 0/9 Document/fix/warn about rebase incompatibilities and inconsistenciesElijah Newren, Jun 25, 2018
  57. 3/9 t3422: new testcases for checking when incompatible options passedElijah Newren, Jun 25, 2018
  58. Junio C HamanoJun 26, 2018
  59. Elijah NewrenJun 27, 2018
  60. 2/9 git-rebase.sh: update help messages a bitElijah Newren, Jun 25, 2018
  61. 5/9 git-rebase.txt: address confusion between --no-ff vs --force-rebaseElijah Newren, Jun 25, 2018
  62. 9/9 git-rebase: make --allow-empty-message the defaultElijah Newren, Jun 25, 2018
  63. 8/9 t3401: add directory rename testcases for rebase and amElijah Newren, Jun 25, 2018
  64. 7/9 git-rebase.txt: document behavioral differences between modesElijah Newren, Jun 25, 2018
  65. 1/9 git-rebase.txt: document incompatible optionsElijah Newren, Jun 25, 2018
  66. 6/9 directory-rename-detection.txt: technical docs on abilities and limitationsElijah Newren, Jun 25, 2018
  67. 4/9 git-rebase: error out when incompatible options passedElijah Newren, Jun 25, 2018
  68. 0/9 Document/fix/warn about rebase incompatibilities and inconsistenciesElijah Newren, Jun 27, 2018
  69. 2/9 git-rebase.sh: update help messages a bitElijah Newren, Jun 27, 2018
  70. 1/9 git-rebase.txt: document incompatible optionsElijah Newren, Jun 27, 2018
  71. 7/9 git-rebase.txt: document behavioral differences between modesElijah Newren, Jun 27, 2018
  72. 9/9 git-rebase: make --allow-empty-message the defaultElijah Newren, Jun 27, 2018
  73. 2.19.0 regression: leaving editor with empty commit message doesn't stop rebase [was: Re: [RFC PATCH v5 9/9] git-rebase: make --allow-empty-message the default]SZEDER Gábor, Sep 12, 2018
  74. Elijah NewrenSep 12, 2018
  75. sequencer: fix --allow-empty-message behavior, make it smarterElijah Newren, Sep 12, 2018
  76. Junio C HamanoSep 13, 2018
  77. 6/9 directory-rename-detection.txt: technical docs on abilities and limitationsElijah Newren, Jun 27, 2018
  78. 8/9 t3401: add directory rename testcases for rebase and amElijah Newren, Jun 27, 2018
  79. 4/9 git-rebase: error out when incompatible options passedElijah Newren, Jun 27, 2018
  80. 5/9 git-rebase.txt: address confusion between --no-ff vs --force-rebaseElijah Newren, Jun 27, 2018
  81. 3/9 t3422: new testcases for checking when incompatible options passedElijah Newren, Jun 27, 2018
  82. Phillip WoodJun 29, 2018
  83. git-rebase.sh: handle keep-empty like all other optionsElijah Newren, Jun 7, 2018
  84. Phillip WoodJun 10, 2018
  85. Elijah NewrenJun 11, 2018
  86. Phillip WoodJun 11, 2018
  87. Elijah NewrenJun 11, 2018
  88. Phillip WoodJun 12, 2018
  89. 1/2 t3418: add testcase showing problems with rebase -i and strategy optionsElijah Newren, Jun 7, 2018
  90. 2/2 Fix use of strategy options with interactive rebasesElijah Newren, Jun 7, 2018
  91. 0/2 Fix use of strategy options with interactive rebasesElijah Newren, Jun 27, 2018
  92. 1/2 t3418: add testcase showing problems with rebase -i and strategy optionsElijah Newren, Jun 27, 2018
  93. Eric SunshineJun 27, 2018
  94. Elijah NewrenJun 27, 2018
  95. Junio C HamanoJun 27, 2018
  96. Elijah NewrenJun 27, 2018
  97. Johannes SixtJun 27, 2018
  98. Eric SunshineJun 27, 2018
  99. SZEDER GáborJun 27, 2018
  100. Junio C HamanoJun 27, 2018
  101. Junio C HamanoJun 27, 2018
  102. 2/2 Fix use of strategy options with interactive rebasesElijah Newren, Jun 27, 2018
  103. 0/2 Fix use of strategy options with interactive rebasesElijah Newren, Jun 27, 2018
  104. 2/2 Fix use of strategy options with interactive rebasesElijah Newren, Jun 27, 2018
  105. Johannes SchindelinJul 12, 2018
  106. Elijah NewrenJul 13, 2018
  107. 1/2 t3418: add testcase showing problems with rebase -i and strategy optionsElijah Newren, Jun 27, 2018
  108. SZEDER GáborJun 27, 2018
  109. SZEDER GáborJul 11, 2018
  110. Elijah NewrenJul 11, 2018
  111. 0/3 Send more rebases through interactive machineryElijah Newren, Jun 7, 2018
  112. 1/3 git-rebase, sequencer: add a --quiet option for the interactive machineryElijah Newren, Jun 7, 2018
  113. Johannes SchindelinJun 9, 2018
  114. Elijah NewrenJun 9, 2018
  115. 3/3 git-rebase.sh: make git-rebase--interactive the defaultElijah Newren, Jun 7, 2018
  116. Johannes SchindelinJun 9, 2018
  117. Elijah NewrenJun 10, 2018
  118. Johannes SchindelinJun 17, 2018
  119. Elijah NewrenJun 20, 2018
  120. Johannes SchindelinJun 21, 2018
  121. Elijah NewrenJun 21, 2018
  122. Johannes SchindelinJan 21, 2019
  123. Elijah NewrenJan 21, 2019
  124. Johannes SchindelinJan 22, 2019
  125. 2/3 rebase: Implement --merge via git-rebase--interactiveElijah Newren, Jun 7, 2018
  126. Johannes SchindelinJun 9, 2018
  127. Elijah NewrenJun 10, 2018
  128. Phillip WoodJun 11, 2018
  129. Junio C HamanoJun 11, 2018
  130. Elijah NewrenJun 11, 2018

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.