Re: [PATCH] Build in merge

From: Miklos Vajna <vmiklos@frugalware.org>
Date: Jul 8, 2008, 01:00 UTC
Message-ID: <20080708010007.GA10347@genesis.frugalware.org>
In-Reply-To: <7v63rhz03x.fsf@gitster.siamese.dyndns.org>

On Mon, Jul 07, 2008 at 05:32:50PM -0700, Junio C Hamano <gitster@pobox.com> wrote:

Show 18 quoted lines

> > So here is a version, this time without the use_strategies list.
> 
> That is not what I meant.  I am afraid perhaps I misunderstood what you
> were talking about.
> 
> When/if you allow user defined new strategies, then you have a choice:
> 
>  (1) find "git-merge-*" in path, add them to the single all_strategies[]
>      list (but you will do the ALLOC_GROW() business so you would need to
>      use the one you currently have as static form to prime the real list),
>      and look for "foo" strategy when "-s foo" is given from that single
>      list, or
> 
>  (2) find "git-merge-*" in path, add them to a separate user_strategies[]
>      list, and look for "foo" strategy when "-s foo" is given from the
>      user_strategies[] list and all_strategies[] list (all_strategies[]
>      should perhaps be renamed to builtin_strategies[] if you go that
>      route).

OK, I see now. Actually I think the primary problem with a custom strategy is that we do not know its flags, IOW if it handles an octopus, etc. So I think there are two questions here:

1) How to tell git-merge the flags of a custom strategy? Or: is it
necessary at all?

I could imagine the following situations:

1) A project has code in a repo, some documentation and po files.

The first two can be merged with builtin strategies, the later probably needs a custom merge driver.

So, in most cases recursive is fine, but sometimes the maintainer wants to say 'git pull -s po'.

In this case the flags of 'po' does not really matter.

2) Someone is not happy with the current recursive strategy and writes
from scratch a new one. He/she puts it to pull.twohead, so it will be
tried before recursive.

To sum up: I am not sure what would be the benefit of being able to specify flags for strategies. However, if we want so, it would be good to discuss how it should be done.

Show 9 quoted lines

> The part that records which strategies were specified from the command
> line *in what order* via "-s foo" switches should remain list of pointers
> into "struct strategy", which is called "struct strategy **use_strategies"
> in the code and corresponds to the $use_strategies variable in the
> scripted version.  The order of these is important, as that defines in
> which order the strategies are tried [*1*].  If you go route (1), these
> pointers will all be pointing at elements in all_strategies[]; with route
> (2) they may be pointing at either all_strageties[] element or
> user_strategies[] element.

I see the problem, I lost the order at the moment.

Show 10 quoted lines

> If you are never going to say "available strategies are these" after you
> start supporting user-defined strategy, then you do not necessarily need
> to do the "find 'git-merge-*' in path, add them to ..." step above, in
> which case it would be Ok not to scan the path and add them to
> all_strategies[] (in route (1)) nor user_strategies[] (in route (2)).
> Instead, you would just create a new "struct strategy" instance lazily
> when the user gave "-s foo" and "foo" is not one of the built-in strategy.
> You would put that at the tail of "struct strategy **use_strategy" array,
> and iterate over use_strategy in the order they are given on the command
> line.

I guess it would be nice to adjust the error message and scanning path, but that's something - I guess - that should be done after the rewrite is complete and there are no more issues with it.