All of lore.kernel.org
 help / color / mirror / Atom feed
* git rebase bug?
@ 2010-07-07 15:05 Mike Hommey
  2010-07-07 18:00 ` Björn Steinbrink
  0 siblings, 1 reply; 6+ messages in thread
From: Mike Hommey @ 2010-07-07 15:05 UTC (permalink / raw)
  To: git

Hi,

I got a really weird result from a rebase today, and I'm wondering if
that's a corner case or if that could be considered a bug in rebase.

This is reproducible with the following setup:
$ git clone git://git.debian.org/pkg-mozilla/xulrunner/experimental xulrunner-1.9.2
$ cd xulrunner-1.9.2
$ git remote add upstream git://git.debian.org/pkg-mozilla/upstream
$ git fetch upstream xulrunner/2.0:xulrunner/2.0
$ git checkout -b test upstream/1.9.2.4
$ git cherry-pick 67e469ef725ac3f4cdf043809066c353e6843db4

The patch that is cherry picked here has the following stats:
 security/manager/ssl/public/Makefile.in            |    1 +
 security/manager/ssl/public/nsIBadCertListener.idl |  155 ++++++++++++++++++++
 security/manager/ssl/src/nsNSSIOLayer.cpp          |  103 +++++++++++++-
 security/manager/ssl/src/nsNSSIOLayer.h            |    8 +

$ git rebase --onto xulrunner/2.0 upstream/1.9.2.4 test

The resulting commit has the following stats:
 security/manager/ssl/public/Makefile.in        |    1 +
 security/manager/ssl/src/nsNSSIOLayer.cpp      |  103 ++++++++++++++++-
 security/manager/ssl/src/nsNSSIOLayer.h        |    8 ++
 xulrunner/examples/simple/content/contents.rdf |  155 ++++++++++++++++++++++++

See how the security/manager/ssl/public/nsIBadCertListener.idl file that
was created by the original patch is created as
xulrunner/examples/simple/content/contents.rdf.

Please note that as the xulrunner/2.0 commit has no parent, I also tried
grafting it on top of upstream/1.9.2.4, which didn't change anything.

So, corner case or definite bug?

Cheers,

Mike

PS: this all is with current master

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: git rebase bug?
  2010-07-07 15:05 git rebase bug? Mike Hommey
@ 2010-07-07 18:00 ` Björn Steinbrink
  2010-07-07 20:51   ` Mike Hommey
  0 siblings, 1 reply; 6+ messages in thread
From: Björn Steinbrink @ 2010-07-07 18:00 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git

On 2010.07.07 17:05:45 +0200, Mike Hommey wrote:
> See how the security/manager/ssl/public/nsIBadCertListener.idl file that
> was created by the original patch is created as
> xulrunner/examples/simple/content/contents.rdf.

The "problem" is that nsIBadCertListener.idl wasn't actually created by
the cherry-picked commit, but was modified. It was an empty file before,
created in 4292283190983fa91b875e22664a79a3aa9ea45d.

And as nsIBadCertListener.idl is missing from the xulrunner/2.0 branch,
git does the usual rename detection, finding another empty file and ends
up patching that one instead.

Björn

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: git rebase bug?
  2010-07-07 18:00 ` Björn Steinbrink
@ 2010-07-07 20:51   ` Mike Hommey
  2010-07-07 21:44     ` Jakub Narebski
  0 siblings, 1 reply; 6+ messages in thread
From: Mike Hommey @ 2010-07-07 20:51 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: git

On Wed, Jul 07, 2010 at 08:00:04PM +0200, Björn Steinbrink <B.Steinbrink@gmx.de> wrote:
> On 2010.07.07 17:05:45 +0200, Mike Hommey wrote:
> > See how the security/manager/ssl/public/nsIBadCertListener.idl file that
> > was created by the original patch is created as
> > xulrunner/examples/simple/content/contents.rdf.
> 
> The "problem" is that nsIBadCertListener.idl wasn't actually created by
> the cherry-picked commit, but was modified. It was an empty file before,
> created in 4292283190983fa91b875e22664a79a3aa9ea45d.
> 
> And as nsIBadCertListener.idl is missing from the xulrunner/2.0 branch,
> git does the usual rename detection, finding another empty file and ends
> up patching that one instead.

Oh, makes sense. Thanks. So that's a quite troubling corner case...
I wonder if empty files shouldn't be special cased...

Mike

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: git rebase bug?
  2010-07-07 20:51   ` Mike Hommey
@ 2010-07-07 21:44     ` Jakub Narebski
  2010-07-08  9:47       ` Mike Hommey
  2010-07-08 11:37       ` Jeff King
  0 siblings, 2 replies; 6+ messages in thread
From: Jakub Narebski @ 2010-07-07 21:44 UTC (permalink / raw)
  To: Mike Hommey; +Cc: Björn Steinbrink, git

Mike Hommey <mh@glandium.org> writes:
> On Wed, Jul 07, 2010 at 08:00:04PM +0200, Björn Steinbrink <B.Steinbrink@gmx.de> wrote:
> > On 2010.07.07 17:05:45 +0200, Mike Hommey wrote:

> > > See how the security/manager/ssl/public/nsIBadCertListener.idl file that
> > > was created by the original patch is created as
> > > xulrunner/examples/simple/content/contents.rdf.
> > 
> > The "problem" is that nsIBadCertListener.idl wasn't actually created by
> > the cherry-picked commit, but was modified. It was an empty file before,
> > created in 4292283190983fa91b875e22664a79a3aa9ea45d.
> > 
> > And as nsIBadCertListener.idl is missing from the xulrunner/2.0 branch,
> > git does the usual rename detection, finding another empty file and ends
> > up patching that one instead.
> 
> Oh, makes sense. Thanks. So that's a quite troubling corner case...
> I wonder if empty files shouldn't be special cased...

Well, similarity score (of contents and of filename) is weighted by
contents length, but perhaps empty files / zero length somehow fall
out as an edge case...

I agree that empty files should be special cased... unless filename is
_very_ similar.
-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: git rebase bug?
  2010-07-07 21:44     ` Jakub Narebski
@ 2010-07-08  9:47       ` Mike Hommey
  2010-07-08 11:37       ` Jeff King
  1 sibling, 0 replies; 6+ messages in thread
From: Mike Hommey @ 2010-07-08  9:47 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Björn Steinbrink, git

On Wed, Jul 07, 2010 at 02:44:50PM -0700, Jakub Narebski wrote:
> Mike Hommey <mh@glandium.org> writes:
> > On Wed, Jul 07, 2010 at 08:00:04PM +0200, Björn Steinbrink <B.Steinbrink@gmx.de> wrote:
> > > On 2010.07.07 17:05:45 +0200, Mike Hommey wrote:
> 
> > > > See how the security/manager/ssl/public/nsIBadCertListener.idl file that
> > > > was created by the original patch is created as
> > > > xulrunner/examples/simple/content/contents.rdf.
> > > 
> > > The "problem" is that nsIBadCertListener.idl wasn't actually created by
> > > the cherry-picked commit, but was modified. It was an empty file before,
> > > created in 4292283190983fa91b875e22664a79a3aa9ea45d.
> > > 
> > > And as nsIBadCertListener.idl is missing from the xulrunner/2.0 branch,
> > > git does the usual rename detection, finding another empty file and ends
> > > up patching that one instead.
> > 
> > Oh, makes sense. Thanks. So that's a quite troubling corner case...
> > I wonder if empty files shouldn't be special cased...
> 
> Well, similarity score (of contents and of filename) is weighted by
> contents length, but perhaps empty files / zero length somehow fall
> out as an edge case...
> 
> I agree that empty files should be special cased... unless filename is
> _very_ similar.

Question is, in a case like mine, what should the result be?

Mike

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: git rebase bug?
  2010-07-07 21:44     ` Jakub Narebski
  2010-07-08  9:47       ` Mike Hommey
@ 2010-07-08 11:37       ` Jeff King
  1 sibling, 0 replies; 6+ messages in thread
From: Jeff King @ 2010-07-08 11:37 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Mike Hommey, Björn Steinbrink, git

On Wed, Jul 07, 2010 at 02:44:50PM -0700, Jakub Narebski wrote:

> > Oh, makes sense. Thanks. So that's a quite troubling corner case...
> > I wonder if empty files shouldn't be special cased...
> 
> Well, similarity score (of contents and of filename) is weighted by
> contents length, but perhaps empty files / zero length somehow fall
> out as an edge case...

Exact rename detection is handled before inexact detection, so the
contents length are irrelevant. So it is not about empty files, but
about exact matches. I'm not sure if the basename-matching code is used
for exact matches, though. But ideally we would break exact-match ties
based on filename. I'd have to read through the code and/or perform some
tests to be sure, though.

-Peff

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-07-08 11:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-07 15:05 git rebase bug? Mike Hommey
2010-07-07 18:00 ` Björn Steinbrink
2010-07-07 20:51   ` Mike Hommey
2010-07-07 21:44     ` Jakub Narebski
2010-07-08  9:47       ` Mike Hommey
2010-07-08 11:37       ` Jeff King

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.