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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Another thing I missed...

On Sun, Jun 10, 2018 at 12:40 PM, Phillip Wood
<phillip.wood@xxxxxxxxxxxx> 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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux