{"thread":{"id":"24321","subject":"git rebase bug?","startedAt":"2010-07-07T15:05:45Z","lastAt":"2010-07-08T11:37:31Z","messageCount":6,"participants":["Mike Hommey","Björn Steinbrink","Jakub Narebski","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"145029","messageId":"20100707150545.GA24814@glandium.org","threadId":"24321","inReplyTo":null,"subject":"git rebase bug?","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2010-07-07T15:05:45Z","receivedAt":"2010-07-07T15:05:45Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"Hi,\n\nI got a really weird result from a rebase today, and I'm wondering if\nthat's a corner case or if that could be considered a bug in rebase.\n\nThis is reproducible with the following setup:\n$ git clone git://git.debian.org/pkg-mozilla/xulrunner/experimental xulrunner-1.9.2\n$ cd xulrunner-1.9.2\n$ git remote add upstream git://git.debian.org/pkg-mozilla/upstream\n$ git fetch upstream xulrunner/2.0:xulrunner/2.0\n$ git checkout -b test upstream/1.9.2.4\n$ git cherry-pick 67e469ef725ac3f4cdf043809066c353e6843db4\n\nThe patch that is cherry picked here has the following stats:\n security/manager/ssl/public/Makefile.in            |    1 +\n security/manager/ssl/public/nsIBadCertListener.idl |  155 ++++++++++++++++++++\n security/manager/ssl/src/nsNSSIOLayer.cpp          |  103 +++++++++++++-\n security/manager/ssl/src/nsNSSIOLayer.h            |    8 +\n\n$ git rebase --onto xulrunner/2.0 upstream/1.9.2.4 test\n\nThe resulting commit has the following stats:\n security/manager/ssl/public/Makefile.in        |    1 +\n security/manager/ssl/src/nsNSSIOLayer.cpp      |  103 ++++++++++++++++-\n security/manager/ssl/src/nsNSSIOLayer.h        |    8 ++\n xulrunner/examples/simple/content/contents.rdf |  155 ++++++++++++++++++++++++\n\nSee how the security/manager/ssl/public/nsIBadCertListener.idl file that\nwas created by the original patch is created as\nxulrunner/examples/simple/content/contents.rdf.\n\nPlease note that as the xulrunner/2.0 commit has no parent, I also tried\ngrafting it on top of upstream/1.9.2.4, which didn't change anything.\n\nSo, corner case or definite bug?\n\nCheers,\n\nMike\n\nPS: this all is with current master\n"},{"id":"145039","messageId":"20100707180004.GA3165@atjola.homenet","threadId":"24321","inReplyTo":"20100707150545.GA24814@glandium.org","subject":"Re: git rebase bug?","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2010-07-07T18:00:04Z","receivedAt":"2010-07-07T18:00:04Z","isPatch":false,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2010.07.07 17:05:45 +0200, Mike Hommey wrote:\n> See how the security/manager/ssl/public/nsIBadCertListener.idl file that\n> was created by the original patch is created as\n> xulrunner/examples/simple/content/contents.rdf.\n\nThe \"problem\" is that nsIBadCertListener.idl wasn't actually created by\nthe cherry-picked commit, but was modified. It was an empty file before,\ncreated in 4292283190983fa91b875e22664a79a3aa9ea45d.\n\nAnd as nsIBadCertListener.idl is missing from the xulrunner/2.0 branch,\ngit does the usual rename detection, finding another empty file and ends\nup patching that one instead.\n\nBjörn\n"},{"id":"145056","messageId":"20100707205126.GA11240@glandium.org","threadId":"24321","inReplyTo":"20100707180004.GA3165@atjola.homenet","subject":"Re: git rebase bug?","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2010-07-07T20:51:26Z","receivedAt":"2010-07-07T20:51:26Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Wed, Jul 07, 2010 at 08:00:04PM +0200, Björn Steinbrink <B.Steinbrink@gmx.de> wrote:\n> On 2010.07.07 17:05:45 +0200, Mike Hommey wrote:\n> > See how the security/manager/ssl/public/nsIBadCertListener.idl file that\n> > was created by the original patch is created as\n> > xulrunner/examples/simple/content/contents.rdf.\n> \n> The \"problem\" is that nsIBadCertListener.idl wasn't actually created by\n> the cherry-picked commit, but was modified. It was an empty file before,\n> created in 4292283190983fa91b875e22664a79a3aa9ea45d.\n> \n> And as nsIBadCertListener.idl is missing from the xulrunner/2.0 branch,\n> git does the usual rename detection, finding another empty file and ends\n> up patching that one instead.\n\nOh, makes sense. Thanks. So that's a quite troubling corner case...\nI wonder if empty files shouldn't be special cased...\n\nMike\n"},{"id":"145064","messageId":"m3bpajm0gw.fsf@localhost.localdomain","threadId":"24321","inReplyTo":"20100707205126.GA11240@glandium.org","subject":"Re: git rebase bug?","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-07-07T21:44:50Z","receivedAt":"2010-07-07T21:44:50Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Mike Hommey <mh@glandium.org> writes:\n> On Wed, Jul 07, 2010 at 08:00:04PM +0200, Björn Steinbrink <B.Steinbrink@gmx.de> wrote:\n> > On 2010.07.07 17:05:45 +0200, Mike Hommey wrote:\n\n> > > See how the security/manager/ssl/public/nsIBadCertListener.idl file that\n> > > was created by the original patch is created as\n> > > xulrunner/examples/simple/content/contents.rdf.\n> > \n> > The \"problem\" is that nsIBadCertListener.idl wasn't actually created by\n> > the cherry-picked commit, but was modified. It was an empty file before,\n> > created in 4292283190983fa91b875e22664a79a3aa9ea45d.\n> > \n> > And as nsIBadCertListener.idl is missing from the xulrunner/2.0 branch,\n> > git does the usual rename detection, finding another empty file and ends\n> > up patching that one instead.\n> \n> Oh, makes sense. Thanks. So that's a quite troubling corner case...\n> I wonder if empty files shouldn't be special cased...\n\nWell, similarity score (of contents and of filename) is weighted by\ncontents length, but perhaps empty files / zero length somehow fall\nout as an edge case...\n\nI agree that empty files should be special cased... unless filename is\n_very_ similar.\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"145104","messageId":"20100708094720.GB3720@glandium.org","threadId":"24321","inReplyTo":"m3bpajm0gw.fsf@localhost.localdomain","subject":"Re: git rebase bug?","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2010-07-08T09:47:20Z","receivedAt":"2010-07-08T09:47:20Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Wed, Jul 07, 2010 at 02:44:50PM -0700, Jakub Narebski wrote:\n> Mike Hommey <mh@glandium.org> writes:\n> > On Wed, Jul 07, 2010 at 08:00:04PM +0200, Björn Steinbrink <B.Steinbrink@gmx.de> wrote:\n> > > On 2010.07.07 17:05:45 +0200, Mike Hommey wrote:\n> \n> > > > See how the security/manager/ssl/public/nsIBadCertListener.idl file that\n> > > > was created by the original patch is created as\n> > > > xulrunner/examples/simple/content/contents.rdf.\n> > > \n> > > The \"problem\" is that nsIBadCertListener.idl wasn't actually created by\n> > > the cherry-picked commit, but was modified. It was an empty file before,\n> > > created in 4292283190983fa91b875e22664a79a3aa9ea45d.\n> > > \n> > > And as nsIBadCertListener.idl is missing from the xulrunner/2.0 branch,\n> > > git does the usual rename detection, finding another empty file and ends\n> > > up patching that one instead.\n> > \n> > Oh, makes sense. Thanks. So that's a quite troubling corner case...\n> > I wonder if empty files shouldn't be special cased...\n> \n> Well, similarity score (of contents and of filename) is weighted by\n> contents length, but perhaps empty files / zero length somehow fall\n> out as an edge case...\n> \n> I agree that empty files should be special cased... unless filename is\n> _very_ similar.\n\nQuestion is, in a case like mine, what should the result be?\n\nMike\n"},{"id":"145115","messageId":"20100708113731.GC2294@sigill.intra.peff.net","threadId":"24321","inReplyTo":"m3bpajm0gw.fsf@localhost.localdomain","subject":"Re: git rebase bug?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-07-08T11:37:31Z","receivedAt":"2010-07-08T11:37:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 07, 2010 at 02:44:50PM -0700, Jakub Narebski wrote:\n\n> > Oh, makes sense. Thanks. So that's a quite troubling corner case...\n> > I wonder if empty files shouldn't be special cased...\n> \n> Well, similarity score (of contents and of filename) is weighted by\n> contents length, but perhaps empty files / zero length somehow fall\n> out as an edge case...\n\nExact rename detection is handled before inexact detection, so the\ncontents length are irrelevant. So it is not about empty files, but\nabout exact matches. I'm not sure if the basename-matching code is used\nfor exact matches, though. But ideally we would break exact-match ties\nbased on filename. I'd have to read through the code and/or perform some\ntests to be sure, though.\n\n-Peff\n"}]}