You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Thomas Börkel <TB...@ap-ag.com> on 2005/02/03 14:36:49 UTC

Bug in merge (repro included)

HI!

As requested by Ben Collins-Sussman, I am reposting this from the users list to the dev list.

I think we have found a bug in Subversion's merge command.

I am attaching 3 versions of the same file:
- TransportOutUtil-B1.java = Rev 1 in the branch
- TransportOutUtil-B2.java = Rev 2 in the branch
- TransportOutUtil-T.java = trunk

Now we are merging the diff between B1 and B2 into T.

The unified diff between B1 and B2 looks like this:

--------------------------------------------------

@@ -444,20 +444,20 @@
       if (vkRahmenObj != null) {
         vkRahmenObj.setGeliefert(vkRahmenObj.getGeliefert() - lieferMenge);
         vkRahmenObj.update();
+
+        where = "KARTIKEL = '" + vkRahmenObj.getKartikel()+ "' and " +
+                "EDIPARTNER = '" + vkRahmenObj.getEdipartner() + "' and " +
+                "WERKKUNDE = '" + vkRahmenObj.getWerkkunde() + "' and " +
+                "ABLADESTELLE = '" + vkRahmenObj.getAbladestelle() + "' and
" +
+                "STATUS = 1";
+        lieferabrufInObj = LieferabrufInFactory.search2(manager,
+                                                        where,
+                                                        READ_LOCK,
+                                                        true);
+        LieferabrufInUtil.unDispatchDeliverySchedule(lieferabrufInObj);
+        LieferabrufInUtil.dispatchDeliverySchedule(lieferabrufInObj);
       }

-      where = "KARTIKEL = '" + vkRahmenObj.getKartikel()+ "' and " +
-              "EDIPARTNER = '" + vkRahmenObj.getEdipartner() + "' and " +
-              "WERKKUNDE = '" + vkRahmenObj.getWerkkunde() + "' and " +
-              "ABLADESTELLE = '" + vkRahmenObj.getAbladestelle() + "' and "
+
-              "STATUS = 1";
-      lieferabrufInObj = LieferabrufInFactory.search2(manager,
-                                                      where,
-                                                      READ_LOCK,
-                                                      true);
-      LieferabrufInUtil.unDispatchDeliverySchedule(lieferabrufInObj);
-      LieferabrufInUtil.dispatchDeliverySchedule(lieferabrufInObj);
-
     } finally {
       manager.release();
     }

--------------------------------------------------

The code in the trunk looks slightly different there, so IMHO the merge
should result in a conflict (trying with GNU diff3 results in a conflict).

But it doesn't. After the merge, the new block has just been added without
conflict and the old block has not been removed.

Before:
          if (vkRahmenObj != null) {
            vkRahmenObj.setGeliefert(vkRahmenObj.getGeliefert() -
lieferMenge);
            vkRahmenObj.update();

            where = "KARTIKEL = '" + vkRahmenObj.getKartikel()+ "' and " +
                    "EDIPARTNER = '" + vkRahmenObj.getEdipartner() + "' and
" +
                    "WERKKUNDE = '" + vkRahmenObj.getWerkkunde() + "' and "
+
                    "ABLADESTELLE = '" + vkRahmenObj.getAbladestelle() + "'
and " +
                    "STATUS = 1";
            lieferabrufInObj = LieferabrufInFactory.search2(manager,
                                                            where,
                                                            READ_LOCK,
                                                            false);
            if (lieferabrufInObj != null) {

LieferabrufInUtil.unDispatchDeliverySchedule(lieferabrufInObj);
              LieferabrufInUtil.dispatchDeliverySchedule(lieferabrufInObj);
            }


After the merge:

            where = "KARTIKEL = '" + vkRahmenObj.getKartikel()+ "' and " +
                    "EDIPARTNER = '" + vkRahmenObj.getEdipartner() + "' and
" +
                    "WERKKUNDE = '" + vkRahmenObj.getWerkkunde() + "' and "
+
                    "ABLADESTELLE = '" + vkRahmenObj.getAbladestelle() + "'
and " +
                    "STATUS = 1";
            lieferabrufInObj = LieferabrufInFactory.search2(manager,
                                                            where,
                                                            READ_LOCK,
                                                            false);
            if (lieferabrufInObj != null) {

LieferabrufInUtil.unDispatchDeliverySchedule(lieferabrufInObj);
              LieferabrufInUtil.dispatchDeliverySchedule(lieferabrufInObj);
            }
          }
        }

        where = "KARTIKEL = '" + vkRahmenObj.getKartikel()+ "' and " +
                "EDIPARTNER = '" + vkRahmenObj.getEdipartner() + "' and " +
                "WERKKUNDE = '" + vkRahmenObj.getWerkkunde() + "' and " +
                "ABLADESTELLE = '" + vkRahmenObj.getAbladestelle() + "' and
" +
                "STATUS = 1";
        lieferabrufInObj = LieferabrufInFactory.search2(manager,
                                                        where,
                                                        READ_LOCK,
                                                        true);
        LieferabrufInUtil.unDispatchDeliverySchedule(lieferabrufInObj);
        LieferabrufInUtil.dispatchDeliverySchedule(lieferabrufInObj);


Are we thinking wrong?

Thanks!

Thomas

 <<TransportOutUtil-B2.java>>  <<TransportOutUtil-B1.java>>  <<TransportOutUtil-T.java>> 

Re: Bug in merge (repro included)

Posted by Thomas Börkel <tb...@ap-ag.com>.
HI!

"Sander Striker" <st...@apache.org> wrote in message
news:25908.8096012876$1107451820@news.gmane.org...

> GNU diff3 is not the same as Subversion.  The results may differ.
> I don't feel it is wrong per se.

IMHO, the diff between B1 and B2 can be seen in 2 different ways:

1. Respecting whitespaces (see the unified diff)
There you can see the removal of one big block and the addition of another
big block.

2. Ignoring whitespaces
There you can see, what really was done: Moving one "}" to another line and
adjusting the indentation accordingly.

AFAIK, svn merge does not ignore whitespaces. But then it has not only to
add the new block, but also to remove the obsolete block. But svn merge just
does the add, not the remove. If it would have tried to remove the big
block, it would have resulted in a conflict, because that block is slightly
different in T and so cannot be removed.

Thomas




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Bug in merge (repro included)

Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@codematters.co.uk> writes:

> $ diff3 -m foo-t foo-b1 foo-b2

  a       Oops!  I missed a line out.

> b
> B
> c
> <<<<<<< foo-b1
> d
> =======
>>>>>>>> foo-b2
> e

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Bug in merge (repro included)

Posted by Philip Martin <ph...@codematters.co.uk>.
Ben Collins-Sussman <su...@collab.net> writes:

> Sander Striker:  what do you think?  Is this a bug in our diff3
> algorithm?
>
> Thomas says that GNU diff3 produces a conflict with these files also.

Subversion doesn't use the exact same algorithm as GNU diff3.  There
are cases where one generates a conflict and the other does not,
here's an example that is similar to Thomas'

$ cat foo-b1
a
b
c
d
e
$ cat foo-b2
a
b
B
c
e
$ diff -u foo-b1 foo-b2
--- foo-b1      Thu Feb  3 16:56:02 2005
+++ foo-b2      Thu Feb  3 16:56:29 2005
@@ -1,5 +1,5 @@
 a
 b
+B
 c
-d
 e
$ cat foo-t
a
b
c
e
$ diff3 -m foo-t foo-b1 foo-b2
b
B
c
<<<<<<< foo-b1
d
=======
>>>>>>> foo-b2
e
$ subversion/tests/libsvn_diff/diff3-test foo-t foo-b1 foo-b2
a
b
B
c
e

Different results, but it's not clear that GNU is right and Subversion
is wrong.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

RE: Bug in merge (repro included)

Posted by Sander Striker <st...@apache.org>.
> From: Ben Collins-Sussman [mailto:sussman@collab.net] 
> Sent: Thursday, February 03, 2005 5:47 PM

> Sander Striker:  what do you think?  Is this a bug in our diff3 algorithm?

Not nessecarily.  Our range of conflict is smaller.  We could make this
configurable.  It only affects the output function, calculating what
the common parts are doesn't change.

> Thomas says that GNU diff3 produces a conflict with these files also.

GNU diff3 is not the same as Subversion.  The results may differ.
I don't feel it is wrong per se.

Sander


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Bug in merge (repro included)

Posted by Ben Collins-Sussman <su...@collab.net>.
Sander Striker:  what do you think?  Is this a bug in our diff3 
algorithm?

Thomas says that GNU diff3 produces a conflict with these files also.


On Feb 3, 2005, at 8:36 AM, Thomas Börkel wrote:

> HI!
>
> As requested by Ben Collins-Sussman, I am reposting this from the 
> users list to the dev list.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org