Bugzilla – Full Text Bug Listing
| Summary: | NPE in CompareEditorInput.setDirty in Eclipse 3.7 [ID-MAMS9] | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Khaled Al-Akhras <kalakhr> | ||||||
| Component: | Compare | Assignee: | Malgorzata Janczarska <malgorzata.tomczyk> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | major | ||||||||
| Priority: | P3 | CC: | 6lo.olg, a.gurov, bart_vanhaute_, betommybeball, christian.herold, christopher.senior, eclipse, egalvez, gerbrand, jfrantzius, levente.fulop, lloyd.smith, marcel.veselka, mars29200, mauromol, michael.krkoska, nt7121, passmasterml, roshan_ail, silvio.ginter, Szymon.Brandys, tacpub, tomasz.zarna, victor | ||||||
| Version: | 3.7 | Flags: | tomasz.zarna:
review+
|
||||||
| Target Milestone: | 3.8 M2 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows Vista | ||||||||
| Whiteboard: | |||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 354593 | ||||||||
| Attachments: |
|
||||||||
|
Description
Khaled Al-Akhras
In every other place were placed checks if fContentInputPane is null or not, but in the method setDirty() the check is absent. So, this is probably an issue in the CompareEditorInput implementation. Although if there is a difference in how CompareEditorInput should be used in compare to the 3.6.x version please tell me (I will change the Subversive code in order to avoid stepping onto this landmine). *** Bug 349538 has been marked as a duplicate of this bug. *** I got this exception as well. Wound up having to use tortoise to do the edits. Mighty inconvenient. Does it happen every time? If not, could you provide steps to reproduce it? Haven't run into a conflict with the source files again. All I did was the following 1) Select a bunch of projects (I think I had about 10 projects selected) 2) Right Click -> Team -> Synchronize. 3) It goes to the "Team Synchronizing Perspective" 4) Select all the projects; Right click -> Update 5) Right click on the file with the conflict -> Edit conflicts. That's it. That gives the error Hope that helps Same issue here, also with Subversive's "Edit Conflict" during merging. Repeatable. same error while merging with subversive having conflicts I too have this problem with "Edit Conflicts" in Subversive. *** Bug 350902 has been marked as a duplicate of this bug. *** *** Bug 350900 has been marked as a duplicate of this bug. *** I have found important difference between previous and current implementation of setDirty() method. Previous one allowed its usage even if UI parts weren't initialized. New on requires UI initialization first, which is mentioned nowhere. IMHO it IS a bad practice: "dirty" flag is just an configuration option (in our case the editor is "dirty" by default), so there is always exists the use case when you want to initialize it as "dirty" from the start. Now I've changed Subversive code a little in order to call setDirty() after compare UI initialization is called, but it could go bad if initialization is ever to became asynchronous. And so, I think, if there currently is no way to make compare editor "dirty" from the start (without tricky overloads of compare UI initialization methods or unstable solutions that could crash if UI initialization process is ever to became asynchronous), then one should be introduced. In summary: After next development build is published, Subversive will not throw NPE here anymore, but still, in my opinion, it is reasonable to correct the setDirty() method behaviour or to provide configuration option that could be passed to the compare editor through CompareConfiguration (or something around those lines). *** Bug 351436 has been marked as a duplicate of this bug. *** Hi Alexander, when do you expect the next development release to be published and where can I get it? I tried the latest 3.7.x maintenance build from here http://download.eclipse.org/eclipse/downloads/ but the bug still seems to exist in this version. Cheers, Ben (In reply to comment #13) We will publish it at the end of this week. It will be the "Early Access" build (see http://www.eclipse.org/subversive/downloads.php). I'd like to +1 and raise the importance of this fix findings its way into Eclipse 3.7 (w/out awaiting for SR1) We are building a new product feature, for our next commercial product release built on Eclipse 3.7, where we ship Subversive, and we are implementing some compare extensions. We are unable thus to work properly against the "Edit Conflicts feature, as this NPE is always hit. Would it be possible of this fix to find itself slipstreamed into the 'helios' release? Or will we need to, in our build system, pull subversive from a new source control source for this bug fix, but as you can imagine we do not want to be pulling "the latest dev branch"! We just need Helios + this major fix... FWIW, the steps we have were as simple as: 1) Check out/have a java project using subversive 2) Check it out a second time also using subversive (either in the same Eclipse workspace or a different one) 3) Make a change in a java file and check it in. 4) Make a conflicting change in the same java file in the other checked-out project. 5) Team->Update. Get a conflict. 6) Go to the synchronize view and select Edit Conflicts from the content menu. 7) See NPE. (In reply to comment #15) Hmm... I'm sorry, but I don't quite understand why do you need to build Subversive from the source to get the workaround for the issue? You could just take the development build with the workaround. As for the Helios release + this workaround I'm not sure why would anyone want it this way: there were way too many issues fixed from the time of Helios release (if you really mean the "release" and not the patched version from the SR2 launch time). P.S. If what you've mentioned about "release" was related not to the Subversive but to the Eclipse, then I'll just say this: the issue is not related to the Helios in any way, but to Indigo only. forgive me, I fat-fingered two things: 1) SORRY, Indigo. I never meant Helios. 2) clarification: to be precise, we use P2 to install subversive into our product. we certainly do not /build (compile)/ subversive... We do this out of a local mirror of the Indigo release update site, so to speak. So, clarifying we are back to talking about Indigo, what we are trying to do is ship "the stable version of subversive". If you ask us to "take the weekly build snapshot" how can we assert the stability of that release? (rightly so -- it's your snapshot release). I guess another way to put it is, imagine if subversive, indigo release, sitting on eclipse.org right now, had a bug that every third click crashed the eclipse instance. you have a fix, but you know everybody downloading the indigo release will crash in three clicks -- what process do you (does eclipse) have for that? Of course, this isn't that sort of an issue, but it is, I argue, a major issue. Finally, I'd be happy to accept knowing that some update site somewhere gets placed with the indigo release + a fix to this issue, and we will download that release and ship it when we ship our final product (currently scheduled to release before Indigo SR1) Created attachment 200001 [details]
Remove dirty viewers
Earlier we needed UI parts to be initialized because we needed a dirty viewer for each site that was dirty. Recent changes removed that dependency and all we used the viewers for is to check if they are not null. I changed Left/RightDirtyViewer to just boolean, this allowed to set the dirty flag even if UI is not initialized. It also simplifies the code.
I've checked it with Subversive 2.2.2.I20110602-1700 and it solved the problem.
*** Bug 352438 has been marked as a duplicate of this bug. *** *** Bug 352470 has been marked as a duplicate of this bug. *** *** Bug 352802 has been marked as a duplicate of this bug. *** *** Bug 353710 has been marked as a duplicate of this bug. *** *** Bug 354189 has been marked as a duplicate of this bug. *** The patch looks good. Applied to HEAD. Available in builds >= N20110810-2000. Created attachment 201215 [details]
mylyn/context/zip
I'd like to ask this be on the table to release with 3.7 SR1 -- edit conflicts is basically broken, and you're breaking all conflict editors for all file types (such as the one we are building a product for) as soon as SVN is managing the containing project. That point aside, http://download.eclipse.org/technology/subversive/0.7/update-site/ is expected to carry this fix as soon as somebody deems a build suitable for releasing as early access, yes? (In reply to comment #26) > I'd like to ask this be on the table to release with 3.7 SR1 Makes sense to me, if not for 3.7.1 then at least for 3.7.2 [1]. Could you verify the fix once the n-build with it is out i.e. tomorrow morning? [1] It's a little bit late for 3.7.1 fixes, see http://www.eclipse.org/eclipse/development/plans/freeze_plan_3_7_1.php (In reply to comment #26) > That point aside, > http://download.eclipse.org/technology/subversive/0.7/update-site/ is expected > to carry this fix as soon as somebody deems a build suitable for releasing as > early access, yes? Subversive early access build have no issue with the conflict editor within the Eclipse IDE 3.7, but it does not contain fix for the issue itself, but workaround instead, because the issue is related to the Team component and not to the Subversive plug-in. > Subversive early access build have no issue with the conflict editor within the
> Eclipse IDE 3.7, but it does not contain fix for the issue itself, but
> workaround instead, because the issue is related to the Team component and not
> to the Subversive plug-in.
oh, I see, I missed that this turned into a team bug, sorry. Since we release a commercial product, whose next version is based on 3.7, I surely hope this appears in 3.7 SR1 and not later.
(In reply to comment #26) > I'd like to ask this be on the table to release with 3.7 SR1 -- edit conflicts > is basically broken, and you're breaking all conflict editors for all file > types (such as the one we are building a product for) as soon as SVN is > managing the containing project. It should have been marked 'major'. And of course +1 for this backport to 3.7.1. Please raise a backport bug for it. (In reply to comment #30) > Please raise a backport bug for it. It's bug 354593. *** Bug 354607 has been marked as a duplicate of this bug. *** *** Bug 355169 has been marked as a duplicate of this bug. *** *** Bug 363005 has been marked as a duplicate of this bug. *** *** Bug 368796 has been marked as a duplicate of this bug. *** *** Bug 373897 has been marked as a duplicate of this bug. *** *** Bug 380477 has been marked as a duplicate of this bug. *** |