| Summary: | [Examples] ArrayIndexOOBE in AbstractMatching.dist | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Aaron Digulla <digulla> | ||||||||
| Component: | Compare | Assignee: | Platform-Compare-Inbox <platform-compare-inbox> | ||||||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P4 | CC: | defert, Michael.Valenta, mveurman, remy.suen, Szymon.Brandys, tomasz.zarna | ||||||||
| Version: | 3.2.2 | Keywords: | helpwanted | ||||||||
| Target Milestone: | --- | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| Whiteboard: | stalebug | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Aaron Digulla
This also happens with the CVS plugin and I'm only seeing this with XML files. Michael made some modifications in the RangeComparatorLCS class in November last year. Would you mind downloading the latest, stable build from 3.4 stream[1] and checking if it still occurs there? [1] 3.4M5: http://download.eclipse.org/eclipse/downloads/drops/S-3.4M5-200802071530/index.php Created attachment 91502 [details]
Two files which can be compared against each other
As I said, this is a regression between 3.2.1.1 and the latest 3.2.2 release and I need it fixed in 3.2.
But I managed to create a test case. The bug is not related to CVS or SVN but to the contents of the files. Just put the two files in an Eclipse project and use "Compare With -> Each Other" to trigger the bug.
The index error moves with the number of elements in the XML file (it gets bigger when there are more elements). The attached file has 6 XML elements (counting the XML version header) and the error happens with index 7.
It goes away when you add any other content to the files.
I've just reproduce it on N20080228-0010. Thanks for the test case Aaron. I'm on it. Created attachment 91743 [details]
Fix
After some investigation, I suspect that the problem is located in the dist[1] method of the AbstractMatching class. It uses indexOfLN and indexOfRN methods to determine indexes of given XMLNodes in currently compared node collections. The problem arises when RangeComparatorLCS calls the rangeEquals method in line 167. The comparator for which the method is called is the same as the one passed as the parameter. This leads to a situation when either indexOfLN or indexOfRN won't return a proper index since they look for the node in a wrong collection.
The fix introduces an additional dist method which accepts two indexes if they are known. It's used in the XMLComparator.rangesEqual method.
However, I find this fix more as a workaround than a real fix since I'm not sure how the change affects the distance table[4] used in the algorithm. To make sure this is a proper fix, I would need to spent a little bit more time on it, which unfortunately I don't have at this moment. Patches will be accepted.
[1] org.eclipse.compare.examples.xml.AbstractMatching.dist(XMLNode, XMLNode)
[2] comparator.rangesEqual(nextLine - 1, comparator, lcsSide[j] - 1)
[3] org.eclipse.compare.examples.xml.AbstractMatching.XMLComparator.rangesEqual(int, IRangeComparator, int)
[4] org.eclipse.compare.examples.xml.AbstractMatching.fDT
Created attachment 91744 [details]
mylyn/context/zip
Given that this happens only in an example plug-in, I'm lowering priority. This "example" plugin is used as the standard file compare component throughout Eclipse (i.e. in "Compare With", compare in the Synchronize view, etc). Actually, this is a blocker bug for me to upgrade to 3.2.2! Aaron, I think you are over-stating the importance of syntax coloring when comparing XML files. Also, given that this is example code and is not shipped with the SDK, blocker is not an appropriate severity for this bug. You can use Eclipse 3.3.2 but simply not installing the examples. Of course, you are at liberty to set the severity back to blocker if you want (since this is a field for use by the reporter) but this will not cause the fix to be back ported to the 3.3.2 stream. If you feel you really need this feature with 3.3.2, another option is for you to check out the 3.3.2 version of the example from CVS and apply Tomek's patch to it. You can then create a usable version of the plug-in using the Export/PDE/Deployable Plug-ins and Fragments wizard. Guys, listen ... for the last time ... I'M GETTING THIS WITH THE OFFICIAL ECLIPSE 3.2.2 UPDATE! IT'S A REGRESSION INTRODUCED BY INCLUDING EXPERIMENTAL CODE IN THE RELEASE! Sorry for yelling but you're not listening to what I say. I'm not using any example code. All I do is fire up the official release downloaded from the Europa Update Site and double click on an XML file in one of my projects! I have no idea how example code ended up in the official release but this is not a milestone build and I also haven't checked out snippet or something like that. The stacktrace is from the Eclipse error log, not from the console. So please fix it and push out a new 3.2 release! If you still don't get it: 1. Download and install Eclipse from http://www.eclipse.org/downloads/ 2. Compare the two file I attached to this bug report Damn ... it's not 3.2 but Europa. 3.3.2, I think. I'm not at the computer right now, so I can't check. I'm not sure if I have the examples installed (can't check either) but in my book, installing the SDK examples shouldn't have an influence on the compare plug-in used by the JDT, Team and other plug-ins. I mean, "Compare With" is such basic functionality, it has to work. The class that is causing the problem is "org.eclipse.compare.examples.xml.AbstractMatching". It is contained in the Compare examples that are available as a separate download from Eclipse.org. The bug in the example code was surfaced by a change that was made in Eclipse 3.3.2. As for the error happening in Compare With, the compare example registers an XML compare viewer that will be used whenever XML files are compared. This is the nature of a pluggable architecture. The purpose of the example is to illustrate how to provide a compare editor for a particular file type. It is unfortunate that a common file type was chosen for the example. Perhaps it would be prudent to change the file extension used by the example to a custom extension to avoid interfering with real file types (Tomek, do you think this is a good idea?) Unfortunately, it is too late to fix this for 3.3.2 since it has already been released. I'm not sure if and when a 3.3.3 release is planned but I'm sure we could get the fix included in that release. However, given that the bug is in example code, I don't think the PMC would agree to create a release specifically to provide a fix for this bug. (In reply to comment #10) > I'M GETTING THIS WITH THE OFFICIAL ECLIPSE 3.2.2 UPDATE! IT'S A REGRESSION > INTRODUCED BY INCLUDING EXPERIMENTAL CODE IN THE RELEASE! We've just passed the Europa Winter Maintenance and there is no plan for another one. Since this bug is not considered as a blocker and AFAIK the change would affect only the example plug-in, we don't plan to release the fix. (In reply to comment #12) > Unfortunately, it is too late to fix this for 3.3.2 since it has already been > released. I'm not sure if and when a 3.3.3 release is planned but I'm sure we > could get the fix included in that release. However, given that the bug is in > example code, I don't think the PMC would agree to create a release > specifically to provide a fix for this bug. If there is a plan for 3.3.3 we will try to incorporate the fix to that release. So far the plan is to fix it in Eclipse 3.4 and this means that the fix will be available in Ganymede release. Please don't play ping-pong with me. This bug is a show stopper since it means I can't update to the latest Europa release (and thus I can't get the other bug fixes). No matter how you turn it: It shows up when the user compares XML files and she doesn't care that this comes from the examples. As far as my users are concerned, Eclipse is broken. What you do behind the scenes it completely up to you. Pluggable architecture means: Whoever writes a plugin must make sure it behaves well. Especially since I (as a user) can't tell Eclipse what to do in this case (or at least I wouldn't know how to do that and since you didn't mention any workaround, I guess you don't know either). So what do we have: You supply code examples and when we install them, Eclipse breaks. You say that you can't provide an official fix since that would mean a new Eclipse release, PMC approval, etc. But ... The bug is in the example code. This means we're not talking full-blown Eclipse major huge release, we're talking about a new release of a tiny bug fix of the SDK examples (either attaching the code to a different file extension or fixing the compare code). I can't see why the PMC would care much about such a tiny issue. It's probably faster to fix than to talk about it. *** Bug 222029 has been marked as a duplicate of this bug. *** Could I then make a request to move the xml compare editor out of the example and make it really something that eclipse supports? We also have a lot of users that depend on this to work. Now my only workaround is to remove the org.eclipse.compare.examples.xml plugin from the product before we can migrate to Eclipse 3.3.x. The example is there to illustrate how to register a compare merge viewer and other viewers. The purpose is not to provide an XML compare. And in this case I think I will go for changing the extension as Michael suggested in comment 12. This way, with the examples installed, a user would never actually use the example to compare real XML files. However, the issue reported by Aaron should be investigated as indicated in comment 5. I think I will open a separate bug for it. Just as a follow up to comment 16, if you want to see real XML compare support, the best place to request it would be from the WTP project which provides a full blown XML editor. At one point, I tried to make a real XML compare editor based of the the code used by the XML editor provided by WTP. Unfortunately, the architecture of the WTP SSE (Structured Source Editor) was not compatible with the architecture of the compare frameworks TextMergeViewer. The specific problem was that the viewers used by SSE have hardcoded dependencies on the underlying document (i.e. the underlying document must be the one provided by SSE). This bug will be opened until we find an optimal solution for the issue reported by Aaron. In the meantime I've opened bug 232272 which changes the file extension used in the example. This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. |