{"thread":{"id":"14673","subject":"[RFC] custom strategies in builtin-merge","startedAt":"2008-07-25T11:33:16Z","lastAt":"2008-07-26T02:37:02Z","messageCount":6,"participants":["Miklos Vajna","Sverre Rabbelier","Junio C Hamano","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"84868","messageId":"20080725113316.GF32057@genesis.frugalware.org","threadId":"14673","inReplyTo":null,"subject":"[RFC] custom strategies in builtin-merge","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-07-25T11:33:16Z","receivedAt":"2008-07-25T11:33:16Z","isPatch":false,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Hi,\n\nWe talked with Dscho about how would it be possible to allow custom\nstrategies in builtin-merge and I have a question here.\n\nThe problem is that currently merge strategies are always named\ngit-merge-foo, but not all git-merge-foo is a merge strategy.\n\nSo we talked about two solutions here:\n\n1) Maintain a list of commands that has a git-merge- prefix, but not a\nstrategy. This list would currently contain \"base, file, index,\none-file and tree\".\n\n2) Require custom strategies to have a different naming scheme, like\nif \"foo\" is a custom strategy, then it would have to be named\ngit-merge-custom-foo, _not_ git-merge-foo.\n\nBoth are doable (I prefer 1) a bit), but I thought it's better to ask\nfirst before I implement any of them.\n\nSo, comments?\n\nThanks.\n"},{"id":"84870","messageId":"bd6139dc0807250450m25a932b8h68fcee13f8c343dc@mail.gmail.com","threadId":"14673","inReplyTo":"20080725113316.GF32057@genesis.frugalware.org","subject":"Re: [RFC] custom strategies in builtin-merge","fromName":"Sverre Rabbelier","fromEmail":"alturin@gmail.com","sentAt":"2008-07-25T11:50:40Z","receivedAt":"2008-07-25T11:50:40Z","isPatch":false,"sender":{"key":"alturin@gmail.com","avatar":null},"body":"On Fri, Jul 25, 2008 at 13:33, Miklos Vajna <vmiklos@frugalware.org> wrote:\n> 1) Maintain a list of commands that has a git-merge- prefix, but not a\n> strategy. This list would currently contain \"base, file, index,\n> one-file and tree\".\n\nSounds a bit error prone, and could lead to unexpected results if/when\nsomeone creates a new command ('git merge status' anyone?) which is\nthen suddenly treated as a merge strategy.\n\n> 2) Require custom strategies to have a different naming scheme, like\n> if \"foo\" is a custom strategy, then it would have to be named\n> git-merge-custom-foo, _not_ git-merge-foo.\n\nI think this is cleaner, what would be even nicer is to change the\ncurrent names too, so name them all \"git-merge-stragegy-foo\".\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"84971","messageId":"7viqutpjqq.fsf@gitster.siamese.dyndns.org","threadId":"14673","inReplyTo":"bd6139dc0807250450m25a932b8h68fcee13f8c343dc@mail.gmail.com","subject":"Re: [RFC] custom strategies in builtin-merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-26T00:33:17Z","receivedAt":"2008-07-26T00:33:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Sverre Rabbelier\" <alturin@gmail.com> writes:\n\n> On Fri, Jul 25, 2008 at 13:33, Miklos Vajna <vmiklos@frugalware.org> wrote:\n>> 1) Maintain a list of commands that has a git-merge- prefix, but not a\n>> strategy. This list would currently contain \"base, file, index,\n>> one-file and tree\".\n>\n> Sounds a bit error prone, and could lead to unexpected results if/when\n> someone creates a new command ('git merge status' anyone?) which is\n> then suddenly treated as a merge strategy.\n>\n>> 2) Require custom strategies to have a different naming scheme, like\n>> if \"foo\" is a custom strategy, then it would have to be named\n>> git-merge-custom-foo, _not_ git-merge-foo.\n>\n> I think this is cleaner, what would be even nicer is to change the\n> current names too, so name them all \"git-merge-stragegy-foo\".\n\nI think you could retroactively impose \"git-merge-strategy-\" prefix rule\nand grandfather the existing ones by maintaining a list of them that by\ndefinition will not grow anymore.\n"},{"id":"84972","messageId":"alpine.DEB.1.00.0807260250480.11976@eeepc-johanness","threadId":"14673","inReplyTo":"7viqutpjqq.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC] custom strategies in builtin-merge","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-26T00:51:41Z","receivedAt":"2008-07-26T00:51:41Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 25 Jul 2008, Junio C Hamano wrote:\n\n> \"Sverre Rabbelier\" <alturin@gmail.com> writes:\n> \n> > On Fri, Jul 25, 2008 at 13:33, Miklos Vajna <vmiklos@frugalware.org> wrote:\n> >> 1) Maintain a list of commands that has a git-merge- prefix, but not a\n> >> strategy. This list would currently contain \"base, file, index,\n> >> one-file and tree\".\n> >\n> > Sounds a bit error prone, and could lead to unexpected results if/when\n> > someone creates a new command ('git merge status' anyone?) which is\n> > then suddenly treated as a merge strategy.\n> >\n> >> 2) Require custom strategies to have a different naming scheme, like\n> >> if \"foo\" is a custom strategy, then it would have to be named\n> >> git-merge-custom-foo, _not_ git-merge-foo.\n> >\n> > I think this is cleaner, what would be even nicer is to change the\n> > current names too, so name them all \"git-merge-stragegy-foo\".\n> \n> I think you could retroactively impose \"git-merge-strategy-\" prefix rule \n> and grandfather the existing ones by maintaining a list of them that by \n> definition will not grow anymore.\n\n... especially since I hope that we have them builtin soon, and not only \nthat, but have builtin-merge call them as C functions, not with fork() and \nexec().\n\nCiao,\nDscho\n"},{"id":"84977","messageId":"7vvdyto3da.fsf@gitster.siamese.dyndns.org","threadId":"14673","inReplyTo":"alpine.DEB.1.00.0807260250480.11976@eeepc-johanness","subject":"Re: [RFC] custom strategies in builtin-merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-26T01:12:17Z","receivedAt":"2008-07-26T01:12:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> ... especially since I hope that we have them builtin soon, and not only \n> that, but have builtin-merge call them as C functions, not with fork() and \n> exec().\n\nBecause builtin-merge.c does not directly use fork/exec but uses\nrun_command() interface, non POSIX platforms can spawn subprocesses just\nfine, can't they?\n\nI do not think at this point it is of any high priority to call strategies\ninternally, avoiding fork/exec.  We may apply hundreds of patches per\nminute, but would fork/exec overhead matter for merges?\n\nEspecially because some strategies (recursive and perhaps the rumored\n\"blame\" even more so) are quite data intensive operations, libifying them\nis not worth it, compared to the nice isolation between processes we get\nfrom running them as a separate program.  We get the necessary clean-up\nafter strategy did its thing for almost free (well, \"at the cost of\nfork/exec\").\n"},{"id":"84986","messageId":"alpine.DEB.1.00.0807260435470.11976@eeepc-johanness","threadId":"14673","inReplyTo":"7vvdyto3da.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC] custom strategies in builtin-merge","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-26T02:37:02Z","receivedAt":"2008-07-26T02:37:02Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 25 Jul 2008, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > ... especially since I hope that we have them builtin soon, and not \n> > only that, but have builtin-merge call them as C functions, not with \n> > fork() and exec().\n> \n> Because builtin-merge.c does not directly use fork/exec but uses \n> run_command() interface, non POSIX platforms can spawn subprocesses just \n> fine, can't they?\n\nYes, they can.  Slowly, but they do.\n\nBut why should they?  Efficient merging is such a prominent feature of \nGit; I do not agree to let it remain a second-class feature.\n\nCiao,\nDscho\n"}]}