Bugzilla – Full Text Bug Listing
| Summary: | Many errors after import | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] PDE | Reporter: | Dani Megert <daniel_megert> | ||||
| Component: | UI | Assignee: | PDE-UI-Inbox <pde-ui-inbox> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | critical | ||||||
| Priority: | P3 | CC: | curtis.windatt.public, darin.eclipse, john.arthorne, markus.kell.r | ||||
| Version: | 3.6 | Flags: | markus.kell.r:
review+
curtis.windatt.public: review+ daniel_megert: review+ |
||||
| Target Milestone: | 3.6 RC3 | ||||||
| Hardware: | PC | ||||||
| OS: | Linux-GTK | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Dani Megert
Reproduced with I20100520-1744 on Linux. First import was OK, but the second import produced errors again (nothing in the log). The problem appears to come from the fix to bug 309894. Examining the fix to that bug revealed an problem with the code. Will attach patch shortly. Created attachment 170227 [details]
patch
The previous code was using constants from the wrong event class to detect add/remove of bundles. This patch limits asynch updates to change events only (generated when saving a manifest editor). All other updates are performed synchronously (to ensure the same behavior as 3.5 and previous).
I suspect the reason we see different behavior on linux vs. windows is related to the speed of the file operations. If the deletion/creation is faster on linux the deltas being created will be different, executing potentially different code paths or different number of update sequences. Marking as RC3 candidate, will ask for review after we do some more review ourselves. Patch looks good (based on my limited understanding of the PDE code base). I've tested the patch with I20100526-1625 and then updated to I20100527-0800+patch. Both importing and compiling went well (several times) on the Linux machine where it failed often before. I again ran into bug 314419 once (with I20100526-1625+patch), but that seems to be a different issue. +1 for RC3. My findings with the old code (i.e. without the patch) show that plug-in import ends up with compilation errors when a combination of synchronous and asynchronous updates occurr with classpath containers. When all updates are either synch or asynch I end up with no compilation errors. Looking at the classpath container API (JavaCore.setClasspathContainer(...)) and SetContainerOperation, there definitely is code that avoids updates to the same container already being updated. This leads me to suspect that our asynch and synch calls end up executing at the same time leaving the Java model in a bad state (since the classpaths do actually look correct, and manually cleaning/building single projects in the correct order does clean things up). What Markus and Darin said makes sense. The code change also makes sense to me. I would like to see some new tests for that bug that can wait (please file a new bug for that). +1 for RC3. +1 I was able to reproduce the problem, applying the patch fixed the issue (even after trying several imports). Fix makes sense. The last time I looked at the code something seemed wrong with how the flags were checked, but everywhere else in PDE we were treating the flags the same (as an int constant rather than a bit mask). Turns out we were using the wrong event flags. Fixed in HEAD. Verified in I20100527-1700. |