Incorrect Handling of Insertion / Deletion Conflicts between Files

[ttben]

Hello!

I am trying to perform a 3-way merge with JDime to show it to my students (and show what Git would have done), and I can't make it work as expected.

The context is easy and is the following:

• a base that contains a Class C with only one dummy method.
• a right branch that renames, and only renames, Class C into Class D
• a left branch that add a single method to Class C (since it exists in her branch)

After building JDime, I run the following:

./JDime -m linebased -p -r ~/Desktop/test/left/ ~/Desktop/test/base/ ~/Desktop/test/right/
(by the way, the option unstructured is specified in the readme but does not exist)

I expected

• a full deletion of C and full addition of D by the right branch,
• an small addition in C by the left branch
• no conflict yielded

Instead, the merge result is:

• only the right branch has been applied. ie: the method added on the left branch is not in the merged result and the class C no longer exists.

I tried to run this command:
./JDime -m structured -p -r ~/Desktop/test/left/ ~/Desktop/test/base/ ~/Desktop/test/right/
and I have exactly the same result as before.

Am I doing something wrong here?

Thanks!

[xai]

Thx for reporting! That doesn't sound right indeed. I'll look into this and try to reproduce it. I'll get back to you.

[xai]

@ttben One question w.r.t. the renaming, just to make sure I'm on the same page here: In the right branch, did you rename the whole file (i.e., C.java -> D.java), or just the class (i.e., still C.java, but with content of class D)?

[ttben]

I renamed both the file itself and the name of the class

[GSeibt]

We are on it right now. It's one of those things that we know about but don't run into often enough to fix. Now we have a reason ;)

[ttben]

Oh ok :) Thanks!

[GSeibt]

I expected

• a full deletion of C and full addition of D by the right branch,
• an small addition in C by the left branch
• no conflict yielded

What files would you expect as a result of the merge (C.java and D.java or D.java with both methods)? According to 3 way merge rules this situation is an insertion-deletion conflict. This is tricky to represent on the file level.

What we 'do' now (really it's an oversight in our merge routine) is incorrect though.

[ttben]

Actually, the insertion-deletion conflict should not occur IMO, since methods are unordered in Java.
Therefore, left added a method, right renamed the class, merged should have both. So I think I'll wait merged version to have D only, or yielding the conflict you specified, or otherwise tell me why it failed :)

I also tried to generate the diff only, to answer this very specific question, but executing JDime with the -d flag returned nothing (or empty). Is it linked to the same issue?

[ttben]

Actually I would be interested that JDime states that a insertion-deletion conflict occured, and present me the diffs between base/left and base/right ; like a classical git-merge,
wdyt?

[GSeibt]

First of all, the -d parameter will disable merging and only perform the matching part of the algorithm. We have options (such as lookahead) which produce matchings that the merger cannot cope with currently. If you want to see any output you would have to increase the log level to something like FINE in order to look at the AST diff.

However, we don't get down to the AST level in this case since the C.java and D.java files are never connected by the matchers. If we were to merge their ASTs, the order of methods would not matter, that is correct. What JDime sees (before it gets to ASTs) is a tree of files and on this level the matchings imply that C was deleted and D was added. We fail to notice that C has also changed (this bug is specific to recursive merging of directories).

Actually I would be interested that JDime states that a insertion-deletion conflict occured, and present me he diffs between base/left and base/right ; like a classical git-merge,
wdyt?

Exactly, the only feasible solution right now is to produce a file tree containing a conflict that involves C.java and notify the user.

[ttben]

Thanks for your answer and on the clarifications about -d flag and why it has not been triggered.
So if I understand correctly, all of this would have not happened if right renamed a method (eg dummy) instead of the class C itself?

Thank you again for your clarification between the identified bug and the expected behavior.

Exactly, the only feasible solution right now is to produce a file tree containing a conflict that involves C.java and notify the user.
I am not quite sure of what kind of conflict will be yielded? Left added a method but right deleted all the wrapping-class (thus failing to detect renaming)?

[GSeibt]

For demonstration purposes, one of the best ways to show the advantages of structured merging is to reorder methods. Git will fail spectacularly in this case while JDime can merge the methods. Renaming is not fully supported by JDime at the moment. Structured merging will give you both methods by default. For git it depends on the context of the changed lines.

We have ways of matching, e.g., renamed method declaration nodes (that contain the name of the method) if all their children are matched. This is one of the situations that the merger cannot handle currently. You can experiment with the -lookahead full parameter and -d. There are also some other ways (look for MCESubtreeMatcher and CostModelMatcher).

[ttben]

Thank you for your answer.
i'll take your scenario to show the pros and cons of such tools :)

I am not quite sure how to experiment what you propose.
Context:

• Base contains a class C with method dummy
• Right renames dummy into notiamnot
• Left adds a method to C

Then,

./JDime -m structured -p -r -d -lookahead full ~/Desktop/test/left/ ~/Desktop/test/base/ ~/Desktop/test/right/

should yield the diffs that exist?

Because when I am running it, it returns nothing..

Thanks again for your quick answers!

[GSeibt]

In this case merging works without lookahead (./JDime -m structured -p -r ~/Desktop/test/left/ ~/Desktop/test/base/ ~/Desktop/test/right/) because the matchers report a deleted method (dummy) and an added method (notiamnot) from the right branch, and an added method from the left branch. As the order of methods is insignificant, the result contains both notiamnot and the new method from the left branch.

As for -d, this is used mainly for debugging purposes and to see any output you have to increase the log level (-log FINE). We tested it just now for recursive merging and it terminates the merge too soon, you will only see the matched directory trees. For non-recursive merging it prints all relevant tree dumps. We will work on providing more useful output when -d is given.

[ttben]

Ok thank you for all your answers!

[xai]

Insertion/Deletion conflicts should be fixed in master 61d7dc1 (v0.4.3.2) and develop 87c83bb.
Instead of a warning, we settled for leaving the file in the directory, but creating a conflict over the whole file. This way, other tools should automatically be able to detect that there was a major conflict.
Thx again for reporting this issue!