{"thread":{"id":"5991","subject":"renames in StGIT","startedAt":"2006-10-22T01:39:43Z","lastAt":"2006-10-24T10:25:07Z","messageCount":6,"participants":["Karl Hasselström","Catalin Marinas"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"29587","messageId":"20061022013943.GA16341@diana.vm.bytemark.co.uk","threadId":"5991","inReplyTo":null,"subject":"renames in StGIT","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2006-10-22T01:39:43Z","receivedAt":"2006-10-22T01:39:43Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"It doesn't seem like StGIT uses any of git's rename tracking stuff.\nSpecifically, pushing patches doesn't seem to use rename-aware\nmerging, and there is no way to tell diff to detect renames and\ncopies.\n\nShould this perhaps be an item in the TODO list?\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"29729","messageId":"b0943d9e0610230947j79449a4dm8736f480f039c230@mail.gmail.com","threadId":"5991","inReplyTo":"20061022013943.GA16341@diana.vm.bytemark.co.uk","subject":"Re: renames in StGIT","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2006-10-23T16:47:06Z","receivedAt":"2006-10-23T16:47:06Z","isPatch":false,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 22/10/06, Karl Hasselström wrote:\n> It doesn't seem like StGIT uses any of git's rename tracking stuff.\n> Specifically, pushing patches doesn't seem to use rename-aware\n> merging, and there is no way to tell diff to detect renames and\n> copies.\n\nThey are indeed not supported by StGIT. One of the reasons is that the\npush operation would probably take (much) longer (I haven't looked at\nthe algorithm in detail but some comments in the git-diff\ndocumentation suggest that this is very expensive). Another reason is\nthat I (used to) send patches to people not using GIT and therefore\nrenames in the diff output were not welcomed.\n\n> Should this perhaps be an item in the TODO list?\n\nOnly if it doesn't drastically affect the performance.\n\n-- \nCatalin\n"},{"id":"29826","messageId":"20061024081732.GA29265@diana.vm.bytemark.co.uk","threadId":"5991","inReplyTo":"20061023125344.f82426ad.seanlkml@sympatico.ca","subject":"Re: renames in StGIT","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2006-10-24T08:17:32Z","receivedAt":"2006-10-24T08:17:32Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2006-10-23 12:53:44 -0400, Sean wrote:\n\n> On Mon, 23 Oct 2006 17:47:06 +0100\n> \"Catalin Marinas\" wrote:\n>\n> > On 22/10/06, Karl Hasselström wrote:\n> >\n> > > Should this perhaps be an item in the TODO list?\n> >\n> > Only if it doesn't drastically affect the performance.\n>\n> There are some situation where it would be really quite handy. The\n> performance of the human having to hand resolve a failed push\n> because of renames is often worse ;o) If it does become a\n> performance problem, perhaps you could make it an optional parameter\n> to \"stg push\".\n\nYes, this is my opinion too, both for patch generation and pushing.\nHaving it always on is a bad idea at least for patch generation for\nobvious reasons, and may be a bad idea for pushing for performance\nreasons, but I definitely think there should be a flag to enable it.\nThere are situations when you know your upstream can read git patches,\nand being able to retry a failed push with rename detection instead of\nhaving to resolve the conflict manually could save lots of time, as\nSean said. Besides, Linus is always bragging about how fast git\nmerging is, and we all trust Linus, right? :-)\n\nAn optional flag is already available for the very handy \"check if\npatch was merged upstream\" feature, and it's definitely the right\nthing to do there. (Though I end up always using that flag even when I\nknow my patches aren't in upstream, since the extra cost is barely\nnoticeable.)\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"29830","messageId":"b0943d9e0610240148s15d6ec5ch6114360a603fcd71@mail.gmail.com","threadId":"5991","inReplyTo":"20061024081732.GA29265@diana.vm.bytemark.co.uk","subject":"Re: renames in StGIT","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2006-10-24T08:48:44Z","receivedAt":"2006-10-24T08:48:44Z","isPatch":false,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 24/10/06, Karl Hasselström wrote:\n> On 2006-10-23 12:53:44 -0400, Sean wrote:\n> > There are some situation where it would be really quite handy. The\n> > performance of the human having to hand resolve a failed push\n> > because of renames is often worse ;o) If it does become a\n> > performance problem, perhaps you could make it an optional parameter\n> > to \"stg push\".\n>\n> Yes, this is my opinion too, both for patch generation and pushing.\n> Having it always on is a bad idea at least for patch generation for\n> obvious reasons, and may be a bad idea for pushing for performance\n> reasons, but I definitely think there should be a flag to enable it.\n\nActually, it might not be a big performance impact. For diff\ngeneration in the export and mail commands, we should have a flag.\n\nThe push operation might not need a flag since it will only checks\nrenames if all the other operations failed. A push with merge\ndetection has the steps below (if one succeeds, push is finished):\n\n1. check (exact) patch merges by reverse-applying the diff\n2. apply the diff with git-diff | git-apply since this is faster than\na three-way merge\n3. try a three-way merge between head, bottom and top\n\nStep 3 above is handled per file by the stgit.gitmergeonefile.merge()\nfunction. This is the place where we should have the rename detection.\nSince, the majority of the patches don't rename files and, in most\ncases, the push finishes at step 2, it is probably safe to extend this\nfunction and the users won't notice a speed difference.\n\nI'll add it to the TODO list.\n\n-- \nCatalin\n"},{"id":"29833","messageId":"20061024091620.GB29265@diana.vm.bytemark.co.uk","threadId":"5991","inReplyTo":"b0943d9e0610240148s15d6ec5ch6114360a603fcd71@mail.gmail.com","subject":"Re: renames in StGIT","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2006-10-24T09:16:20Z","receivedAt":"2006-10-24T09:16:20Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2006-10-24 09:48:44 +0100, Catalin Marinas wrote:\n\n> Actually, it might not be a big performance impact. For diff\n> generation in the export and mail commands, we should have a flag.\n\nAgreed.\n\n> The push operation might not need a flag since it will only checks\n> renames if all the other operations failed. A push with merge\n> detection has the steps below (if one succeeds, push is finished):\n>\n> 1. check (exact) patch merges by reverse-applying the diff\n> 2. apply the diff with git-diff | git-apply since this is faster\n> than a three-way merge\n> 3. try a three-way merge between head, bottom and top\n>\n> Step 3 above is handled per file by the\n> stgit.gitmergeonefile.merge() function. This is the place where we\n> should have the rename detection. Since, the majority of the patches\n> don't rename files and, in most cases, the push finishes at step 2,\n> it is probably safe to extend this function and the users won't\n> notice a speed difference.\n>\n> I'll add it to the TODO list.\n\nSounds good. I had a feeling it ought to be basically free in the\nmajority of cases, so I'm glad to learn I'm right. :-)\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"29844","messageId":"b0943d9e0610240325v5bc23078w7fc977e091e99779@mail.gmail.com","threadId":"5991","inReplyTo":"20061024091620.GB29265@diana.vm.bytemark.co.uk","subject":"Re: renames in StGIT","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2006-10-24T10:25:07Z","receivedAt":"2006-10-24T10:25:07Z","isPatch":false,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 24/10/06, Karl Hasselström wrote:\n> On 2006-10-24 09:48:44 +0100, Catalin Marinas wrote:\n> > Step 3 above is handled per file by the\n> > stgit.gitmergeonefile.merge() function. This is the place where we\n> > should have the rename detection. Since, the majority of the patches\n> > don't rename files and, in most cases, the push finishes at step 2,\n> > it is probably safe to extend this function and the users won't\n> > notice a speed difference.\n> >\n> > I'll add it to the TODO list.\n>\n> Sounds good. I had a feeling it ought to be basically free in the\n> majority of cases, so I'm glad to learn I'm right. :-)\n\nMight be even simpler for 'push' but I need to do more tests - instead\nof calling git-read-tree in git.merge(), just call git-merge-recursive\nwhich handles renames and it's fully tested. My simple test detected\nrenames when pushing patches (both rename in base and rename in\npatch). I still have to do some tweaking and write proper tests (and\nprobably make it less verbose).\n\n-- \nCatalin\n"}]}