[RFC] custom strategies in builtin-merge

[RFC] custom strategies in builtin-merge

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

Hi,

We talked with Dscho about how would it be possible to allow custom
strategies in builtin-merge and I have a question here.

The problem is that currently merge strategies are always named
git-merge-foo, but not all git-merge-foo is a merge strategy.

So we talked about two solutions here:

1) Maintain a list of commands that has a git-merge- prefix, but not a
strategy. This list would currently contain "base, file, index,
one-file and tree".

2) Require custom strategies to have a different naming scheme, like
if "foo" is a custom strategy, then it would have to be named
git-merge-custom-foo, _not_ git-merge-foo.

Both are doable (I prefer 1) a bit), but I thought it's better to ask
first before I implement any of them.

So, comments?

Thanks.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [RFC] custom strategies in builtin-merge

[Sverre Rabbelier]

On Fri, Jul 25, 2008 at 13:33, Miklos Vajna <vmiklos@frugalware.org> wrote:
> 1) Maintain a list of commands that has a git-merge- prefix, but not a
> strategy. This list would currently contain "base, file, index,
> one-file and tree".

Sounds a bit error prone, and could lead to unexpected results if/when
someone creates a new command ('git merge status' anyone?) which is
then suddenly treated as a merge strategy.

> 2) Require custom strategies to have a different naming scheme, like
> if "foo" is a custom strategy, then it would have to be named
> git-merge-custom-foo, _not_ git-merge-foo.

I think this is cleaner, what would be even nicer is to change the
current names too, so name them all "git-merge-stragegy-foo".

-- 
Cheers,

Sverre Rabbelier

Re: [RFC] custom strategies in builtin-merge

[Junio C Hamano]

"Sverre Rabbelier" <alturin@gmail.com> writes:

> On Fri, Jul 25, 2008 at 13:33, Miklos Vajna <vmiklos@frugalware.org> wrote:
>> 1) Maintain a list of commands that has a git-merge- prefix, but not a
>> strategy. This list would currently contain "base, file, index,
>> one-file and tree".
>
> Sounds a bit error prone, and could lead to unexpected results if/when
> someone creates a new command ('git merge status' anyone?) which is
> then suddenly treated as a merge strategy.
>
>> 2) Require custom strategies to have a different naming scheme, like
>> if "foo" is a custom strategy, then it would have to be named
>> git-merge-custom-foo, _not_ git-merge-foo.
>
> I think this is cleaner, what would be even nicer is to change the
> current names too, so name them all "git-merge-stragegy-foo".

I think you could retroactively impose "git-merge-strategy-" prefix rule
and grandfather the existing ones by maintaining a list of them that by
definition will not grow anymore.

Re: [RFC] custom strategies in builtin-merge

[Johannes Schindelin]

Hi,

On Fri, 25 Jul 2008, Junio C Hamano wrote:

> "Sverre Rabbelier" <alturin@gmail.com> writes:
> 
> > On Fri, Jul 25, 2008 at 13:33, Miklos Vajna <vmiklos@frugalware.org> wrote:
> >> 1) Maintain a list of commands that has a git-merge- prefix, but not a
> >> strategy. This list would currently contain "base, file, index,
> >> one-file and tree".
> >
> > Sounds a bit error prone, and could lead to unexpected results if/when
> > someone creates a new command ('git merge status' anyone?) which is
> > then suddenly treated as a merge strategy.
> >
> >> 2) Require custom strategies to have a different naming scheme, like
> >> if "foo" is a custom strategy, then it would have to be named
> >> git-merge-custom-foo, _not_ git-merge-foo.
> >
> > I think this is cleaner, what would be even nicer is to change the
> > current names too, so name them all "git-merge-stragegy-foo".
> 
> I think you could retroactively impose "git-merge-strategy-" prefix rule 
> and grandfather the existing ones by maintaining a list of them that by 
> definition will not grow anymore.

... especially since I hope that we have them builtin soon, and not only 
that, but have builtin-merge call them as C functions, not with fork() and 
exec().

Ciao,
Dscho

Re: [RFC] custom strategies in builtin-merge

[Junio C Hamano]

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

> ... especially since I hope that we have them builtin soon, and not only 
> that, but have builtin-merge call them as C functions, not with fork() and 
> exec().

Because builtin-merge.c does not directly use fork/exec but uses
run_command() interface, non POSIX platforms can spawn subprocesses just
fine, can't they?

I do not think at this point it is of any high priority to call strategies
internally, avoiding fork/exec.  We may apply hundreds of patches per
minute, but would fork/exec overhead matter for merges?

Especially because some strategies (recursive and perhaps the rumored
"blame" even more so) are quite data intensive operations, libifying them
is not worth it, compared to the nice isolation between processes we get
from running them as a separate program.  We get the necessary clean-up
after strategy did its thing for almost free (well, "at the cost of
fork/exec").

Re: [RFC] custom strategies in builtin-merge

[Johannes Schindelin]

Hi,

On Fri, 25 Jul 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > ... especially since I hope that we have them builtin soon, and not 
> > only that, but have builtin-merge call them as C functions, not with 
> > fork() and exec().
> 
> Because builtin-merge.c does not directly use fork/exec but uses 
> run_command() interface, non POSIX platforms can spawn subprocesses just 
> fine, can't they?

Yes, they can.  Slowly, but they do.

But why should they?  Efficient merging is such a prominent feature of 
Git; I do not agree to let it remain a second-class feature.

Ciao,
Dscho