Re: whither merge-tree?
Jeff King writes: > 2. Rip out the weird add/add conflict resolution. This gets rid of the > buggy code, makes merge-tree more like the rest of git, and I think > lets us even drop the EMIT_COMMON stuff from xdiff). That is a nice bonus. git-merge-resolve (rather, git-merge-one-file) attempts the same "resolve add/add by taking the common" thing, but it implements it in quite a different way. > That lets people keep using merge-tree if they have found it useful > over the years. > 3. Drop merge-tree completely. This deletes even more code, and helps > the people in (2) realize that it is utterly unmaintained. :) > > I think at this point I am waffling between (2) and (3). I did (1) in a > hope that I could avoid looking deeper into the code at all, but now > that I have, I do not think (2) would be so bad. I'm happy to work up a > patch, but won't bother if we think that (3) is viable. Yup, between 2 and 3, 2 would certainly be safer, and I agree that it is not too bad (I have this feeling that add-add conflict is not the only funny this code has, though). Let's wait and see how many "please don't"s we hear, perhaps, before deciding to go 3.? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: whither merge-tree?
On Mon, Feb 22, 2016 at 02:45:45PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > 2. Rip out the weird add/add conflict resolution. This gets rid of the > > buggy code, makes merge-tree more like the rest of git, and I think > > lets us even drop the EMIT_COMMON stuff from xdiff). > > That is a nice bonus. > > git-merge-resolve (rather, git-merge-one-file) attempts the same > "resolve add/add by taking the common" thing, but it implements it > in quite a different way. I suppose the end result of what merge-tree is trying to do makes sense. It's definitely a conflict, but we are interested in showing the minimal content-level conflict. But I think xdl_merge() takes care of that for us, if we simply feed an empty base. And that is what merge-recursive does. I do see that merge-one-file tries create_virtual_base(), which does some magic with diff. But I'm having trouble conceiving of a case where that would do something different or useful. > > That lets people keep using merge-tree if they have found it useful > > over the years. > > > 3. Drop merge-tree completely. This deletes even more code, and helps > > the people in (2) realize that it is utterly unmaintained. :) > > > > I think at this point I am waffling between (2) and (3). I did (1) in a > > hope that I could avoid looking deeper into the code at all, but now > > that I have, I do not think (2) would be so bad. I'm happy to work up a > > patch, but won't bother if we think that (3) is viable. > > Yup, between 2 and 3, 2 would certainly be safer, and I agree that > it is not too bad (I have this feeling that add-add conflict is not > the only funny this code has, though). Yeah, I do not mind doing 2, but I have no idea what else is lurking, and I have very little interest in digging into it. > Let's wait and see how many "please don't"s we hear, perhaps, before > deciding to go 3.? I'm guessing we won't see much either way. Even Stefan, the original reporter, does not seem to actively be using it, but rather relaying a report. We'd probably get more response by doing 2 for now, then adding a deprecation warning to the manpage (and possibly the program itself) for the next release. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: whither merge-tree?
On Tue, Feb 23, 2016 at 12:02:10AM -0500, Jeff King wrote: > > git-merge-resolve (rather, git-merge-one-file) attempts the same > > "resolve add/add by taking the common" thing, but it implements it > > in quite a different way. > > I suppose the end result of what merge-tree is trying to do makes sense. > It's definitely a conflict, but we are interested in showing the minimal > content-level conflict. But I think xdl_merge() takes care of that for > us, if we simply feed an empty base. And that is what merge-recursive > does. > > I do see that merge-one-file tries create_virtual_base(), which does > some magic with diff. But I'm having trouble conceiving of a case where > that would do something different or useful. I dug this all the way down to your cb93c19 (merge-one-file: use common as base, instead of emptiness., 2005-11-09), which states that the goal is just to get: common file contents... << FILENAME version from our branch... == version from their branch... >> .merge_file_XX more common file contents... But that seems to be what we produce now. Did all of this simply predate xdl_merge, and the crappy rcs merge did not bother minimizing the diff? That certainly seems to be the case in my tests. If that is the case, I think we can get rid of the complex create_virtual_base(), as well. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: whither merge-tree?
On Tue, Feb 23, 2016 at 12:14:02AM -0500, Jeff King wrote: > I dug this all the way down to your cb93c19 (merge-one-file: use common > as base, instead of emptiness., 2005-11-09), which states that the goal > is just to get: > > common file contents... > << FILENAME > version from our branch... > == > version from their branch... > >> .merge_file_XX > more common file contents... > > But that seems to be what we produce now. Did all of this simply predate > xdl_merge, and the crappy rcs merge did not bother minimizing the diff? > That certainly seems to be the case in my tests. > > If that is the case, I think we can get rid of the complex > create_virtual_base(), as well. So here is what I would propose: [1/3]: merge-one-file: use empty blob for add/add base [2/3]: merge-tree: drop generate_common strategy [3/3]: xdiff: drop XDL_EMIT_COMMON I briefly wondered if there were any bugs in merge-one-file around this "no newline at end of file" issue. But there shouldn't be. It generates the common file by applying the diff to the first file with "--no-add", which should do the right thing, I think. I stopped short of dropping the create_virtual_base function in the first patch, for reasons explained there. But if we were to do so, I suspect we could deprecated and eventually drop "apply --no-add", too. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: whither merge-tree?
Jeff King writes: > On Tue, Feb 23, 2016 at 12:14:02AM -0500, Jeff King wrote: > >> I dug this all the way down to your cb93c19 (merge-one-file: use common >> as base, instead of emptiness., 2005-11-09), which states that the goal >> is just to get: >> >> common file contents... >> << FILENAME >> version from our branch... >> == >> version from their branch... >> >> .merge_file_XX >> more common file contents... >> >> But that seems to be what we produce now. Did all of this simply predate >> xdl_merge, and the crappy rcs merge did not bother minimizing the diff? >> That certainly seems to be the case in my tests. >> >> If that is the case, I think we can get rid of the complex >> create_virtual_base(), as well. > > So here is what I would propose: > > [1/3]: merge-one-file: use empty blob for add/add base > [2/3]: merge-tree: drop generate_common strategy > [3/3]: xdiff: drop XDL_EMIT_COMMON > > I briefly wondered if there were any bugs in merge-one-file around this > "no newline at end of file" issue. But there shouldn't be. It generates > the common file by applying the diff to the first file with "--no-add", > which should do the right thing, I think. > > I stopped short of dropping the create_virtual_base function in the > first patch, for reasons explained there. But if we were to do so, I > suspect we could deprecated and eventually drop "apply --no-add", too. Thanks. I think the "virtual" stuff started its life outside sh-setup but later was moved there for p4merge. The log message for 4549162e (mergetools/p4merge: create a base if none available, 2013-03-13) seems to indicate that using an empty file does not work well over there as you suspected, so unfortunately we cannot lose it just yet. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: whither merge-tree?
On Mon, Feb 22, 2016 at 10:35:34PM -0800, Junio C Hamano wrote: > > I stopped short of dropping the create_virtual_base function in the > > first patch, for reasons explained there. But if we were to do so, I > > suspect we could deprecated and eventually drop "apply --no-add", too. > > Thanks. I think the "virtual" stuff started its life outside > sh-setup but later was moved there for p4merge. The log message for > 4549162e (mergetools/p4merge: create a base if none available, > 2013-03-13) seems to indicate that using an empty file does not work > well over there as you suspected, so unfortunately we cannot lose it > just yet. Oh, indeed. I did find that commit in my git-blaming, but failed to actually read it carefully. :) With that, yeah, I agree that we cannot drop the virtual-base thing, and my 3 patches are fine as-is. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: whither merge-tree?
On 23.02.2016 at 06:02 Jeff King wrote: Let's wait and see how many "please don't"s we hear, perhaps, before deciding to go 3.? I'm guessing we won't see much either way. Even Stefan, the original reporter, does not seem to actively be using it, but rather relaying a report. I _am_ actively using it. Maybe I was unclear on that topic. I'm in favour of keeping it, because this means I don't have to rewrite Chris' Code in order to be able to use the Python library that uses merge-tree (Acidfs). But as a sensible human being I want what's best in the long run. I leave that up to you as I have no way of assessing that. So that's a "please don't" leave the code as-is but provide a (transitional) solution that fixes the reported bug and has the best chances of not causing any more headaches :) We'd probably get more response by doing 2 for now, then adding a deprecation warning to the manpage (and possibly the program itself) for the next release. A deprecation warning would be very welcome. Thanks, Stefan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: whither merge-tree?
Hi Peff, On Tue, 23 Feb 2016, Jeff King wrote: > On Mon, Feb 22, 2016 at 02:45:45PM -0800, Junio C Hamano wrote: > > > Jeff King writes: > > > > > 3. Drop merge-tree completely. This deletes even more code, and > > > helps the people in (2) realize that it is utterly unmaintained. > > > :) > > > > Let's wait and see how many "please don't"s we hear, perhaps, before > > deciding to go 3.? > > I'm guessing we won't see much either way. We could encourage more voices by issuing a warning when merge-tree is called, e.g.: warning: merge-tree is unsupported (please contact git@vger.kernel.org if you use it actively) Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: whither merge-tree?
On Tue, Feb 23, 2016 at 7:36 PM, Johannes Schindelin wrote: > We could encourage more voices by issuing a warning when merge-tree is > called, e.g.: > > warning: merge-tree is unsupported (please contact > git@vger.kernel.org if you use it actively) Maybe motivate them to reach git@vger by s/use it actively/do not want it gone/. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: whither merge-tree?
On di, 2016-02-23 at 10:49 +0100, Stefan Frühwirth wrote: > On 23.02.2016 at 06:02 Jeff King wrote: > > > Let's wait and see how many "please don't"s we hear, perhaps, > > > before > > > deciding to go 3.? > > > > I'm guessing we won't see much either way. Even Stefan, the > > original > > reporter, does not seem to actively be using it, but rather > > relaying a > > report. > > I _am_ actively using it. Maybe I was unclear on that topic. I'm in > favour of keeping it, because this means I don't have to rewrite > Chris' > Code in order to be able to use the Python library that uses merge- > tree > (Acidfs). But as a sensible human being I want what's best in the > long > run. I leave that up to you as I have no way of assessing that. > > So that's a "please don't" leave the code as-is but provide a > (transitional) solution that fixes the reported bug and has the best > chances of not causing any more headaches :) I am also actively using it. It's the only way (I know of) of trying to preview a merge result without attempting the actual merge, which is useful in some of my scripts. -- Dennis Kaarsemaker www.kaarsemaker.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: whither merge-tree?
On Wed, Feb 24, 2016 at 08:28:59AM +0100, Dennis Kaarsemaker wrote: > > So that's a "please don't" leave the code as-is but provide a > > (transitional) solution that fixes the reported bug and has the best > > chances of not causing any more headaches :) > > I am also actively using it. It's the only way (I know of) of trying to > preview a merge result without attempting the actual merge, which is > useful in some of my scripts. OK. I guess we can live with the series I posted earlier (to simplify the add/add behavior and fix the overflow), and leave it in place. I _do_ think it's a useful concept, and there isn't something quite like it in git right now. At one point I contemplated teaching unpack-trees to do the content-level merges too, so you could do this via read-tree, but I didn't ever polish it[1]. So I am sympathetic to the goal, and there's not another way to do it as efficiently. And now I feel like both of you have been warned that the code might be crufty. :) I'm not sure what we should do about warning others. Since people are using it, I don't want to put a deprecation warning. It's not "this will be removed in the next version" anymore, but "watch out, this might be crufty". I guess that can go in the manpage, but I'm not quite sure how to word it. -Peff [1] It looks like my patch has been collecting dust since 2011: git://github.com/peff/git.git jk/read-tree-content-merge-wip It's been rebased along with all of my other topics since then, but otherwise I have no clue if it works or even compiles (it's not part of my day-to-day build). Anyone is welcome to use it as a base if they want to pursue it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: whither merge-tree?
On Tue, Feb 23, 2016 at 10:49:34AM +0100, Stefan Frühwirth wrote: > On 23.02.2016 at 06:02 Jeff King wrote: > >>Let's wait and see how many "please don't"s we hear, perhaps, before > >>deciding to go 3.? > > > >I'm guessing we won't see much either way. Even Stefan, the original > >reporter, does not seem to actively be using it, but rather relaying a > >report. > > I _am_ actively using it. Maybe I was unclear on that topic. I'm in favour > of keeping it, because this means I don't have to rewrite Chris' Code in > order to be able to use the Python library that uses merge-tree (Acidfs). > But as a sensible human being I want what's best in the long run. I leave > that up to you as I have no way of assessing that. Ah, sorry for the confusion. I had thought you were just relaying bugs on behalf of acidfs folks, but it makes sense that you are using it indirectly through the library. > So that's a "please don't" leave the code as-is but provide a (transitional) > solution that fixes the reported bug and has the best chances of not causing > any more headaches :) I think the series I posted does what you want, then, and we can stop short of deprecating for now. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
whither merge-tree? (was: What's cooking in git.git (Feb 2016, #05; Wed, 17))
On Wed, Feb 17, 2016 at 02:34:08PM -0800, Junio C Hamano wrote: > * jk/merge-tree-merge-blobs (2016-02-16) 1 commit > - merge_blobs: use strbuf instead of manually-sized mmfile_t > > "git merge-tree" (a throw-away demonstration) did not work very > well when merging "both sides added a new file at the same path" > case. > > Undecided; we might be better off deleting it altogether. What do we want to do with this? I think there are basically three options: 1. Take the patch you queued above. That's the minimal fix with no user-visible changes. 2. Rip out the weird add/add conflict resolution. This gets rid of the buggy code, makes merge-tree more like the rest of git, and I think lets us even drop the EMIT_COMMON stuff from xdiff). That lets people keep using merge-tree if they have found it useful over the years. 3. Drop merge-tree completely. This deletes even more code, and helps the people in (2) realize that it is utterly unmaintained. :) I think at this point I am waffling between (2) and (3). I did (1) in a hope that I could avoid looking deeper into the code at all, but now that I have, I do not think (2) would be so bad. I'm happy to work up a patch, but won't bother if we think that (3) is viable. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
