From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 19 Jul 2008 05:32:10 GMT
Subject: Re: What's cooking in git.git (topics)
Message-ID: <7vabge30dh.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.DEB.1.00.0807181351370.3932@eeepc-johanness>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Fri, 18 Jul 2008, Junio C Hamano wrote:
>
>> +The 'recursive' strategy can take the following options:
>> +
>> +ours;;
>
> You still have not addressed the issue that you can specify multiple 
> strategies,...

Even though multiple -s parameters are supported, I know you have been
here long enough in git scene to remember how it came about.  I've seen
some third-party documents that talk about our ability to "try multiple
strategies and pick the best one" as one of the unique features, but
anybody who was there knows that it was just a failed experiment that we
did not bother removing.

The thing is, trying multiple strategies was a cute idea and it was quite
straightforward to implement.  But picking the best one is the much more
important part, and judging whose result is the best shouldn't be done
with just a naïve "how many conflicting paths remain there?" metric
(c.f. $gmane/87297 which talks about "stupid" but the argument is exactly
the same --- smaller number of conflicts may not necessarily be the
easiest to resolve nor the right resolution).  I would be surprised if
anybody uses multiple -s options in their daily workflow, even though I
would not be surprised if people tried to use it just as an experiment and
for its entertainment value once or maybe twice.  After all, I invented
the multiple strategy support for amusement, not from any practical real
world needs ;-)

So I do not consider that a convincing argument at all.

> ... or even a single _wrong_ one.  So:
>
> 	$ git merge -s stupid -Xours
>
> would not fail at all, but definitely not do the right thing either (it 
> disobeys a direct command of the user).

It does fail gracefully, though.

    $ git merge -s resolve -Xours next
    Trying really trivial in-index merge...
    error: Untracked working tree file '.gitattributes' would be overwritten by merge.
    Nope.
    fatal: Not a valid object name --ours
    Merge with strategy resolve failed.

I consider this falls into "You say it hurts?  Don't do that, then"
category.

The error message will naturally improve, once we teach the merge strategy
backends that they can be given --<option> in front of the usual
<base>... -- <ents>... parameters, and there is no risk of ambiguity
because no object names begin with a dash.

Having said all that, I do not have any reason to push for -Xours/theirs
myself.  I've made myself very clear from the beginning that what these
options do is a bad idea, just as "-s theirs" is a bad idea.  These
encourage a broken workflow and I do not see a clear upside, however
narrow, and you and Pasky seem to agree with me (heh, isn't it a rare
occasion that all three of us agree on something these days? ;-)

I won't shed tears to see them go.

However, I do think it is wrong to deny that it will eventually be
necessary for us to be able to pass strategy specific options via the
git-merge frontend driver to the strategy backend.  The primary reason why
I wrote "subtree" strategy to _guess_ how to shift trees was because there
was no way to pass "how the end user wants to shift them" to the strategy
backend over "pull -- merge -- merge-subtree" callchain.  Coming up with
the algorithm was fun, but that was secondary.

If we allow users to say -Xsubtree=<path>, it would be a true improvement
to a tool that is used in real life.  Unlike "multiple -s strategy"
support that I think nobody ever uses in practice (on which part of your
objection is based), "-s subtree" has been useful in real life, and you
can verify that claim easily by counting how many times I've used that in
git.git history yourself.

Even though I do not care deeply about the syntax (and if you do not like
the "-X" as the external option introducer, you are welcome to pick a
different notation and send in a patch), it would help for example the
vanilla "recursive" strategy to allow the user, when dealing with really
tricky merge, to influence the rename threshold score it uses by passing
it as a strategy-specific option.

As a conclusion of this discussion, I'll discard xx/merge-in-c-into-next
branch from "next", at the beginning of post-1.6.0 cycle.  We might in the
future need to resurrect only the -X<option> part to allow us to pass
strategy specific options (that are not "ours/theirs"), but there is no
immediate need for it, other than -Xsubtree=<path>.  If somebody wants to
step up and give the custom rename threshold to the recursive strategy,
keeping that code to do -X<option> might help that too, though.