Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 312085

Summary: [target] removing location results in errors
Product: [Eclipse Project] PDE Reporter: Jeff McAffer <jeffmcaffer>
Component: UIAssignee: Curtis Windatt <curtis.windatt.public>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: curtis.windatt.public, darin.eclipse
Version: 3.6Flags: darin.eclipse: review+
Target Milestone: 3.6 RC1   
Hardware: PC   
OS: Mac OS X - Carbon (unsup.)   
Whiteboard:
Bug Depends on:    
Bug Blocks: 312710    
Attachments:
Description Flags
Fix none

Description Jeff McAffer CLA 2010-05-07 11:30:21 EDT 
in M7

1) create a target that has several locations and various features selected from those locations
2) switch to the content tab and deselect some bundles
3) go back to the definition tab and delete one of the locations
4) notice that there are a mess of errors now in the locations are detailing that all the bundles in the location you just deleted cannot be loaded.

The root problem seems to be that when you change anything in the Content tab, the target file then has a complete list of the bundles that are in the target (as opposed to the list of bundles that should be excluded). As a result, the target things that all those bundles should be in the target but the location supplying various ones has been deleted.

Given that the primary management of target content is by adding locations (and then perhaps selecting features), modifying the Contents should create a list of things to remove from the coarser grained locaiton and feature contributions.

Once a user gets into this situation there is no way to recover the .target file except
- open it as a text file and hack out the bundle list
- re add a location that supplies the bundles in question, then go to the Content tab and laboriously deselect the individual bundles they dont want and then delete the location again.
Comment 1 Curtis Windatt CLA 2010-05-10 12:09:42 EDT 
Listing the included vs excluded bundles was a discussion point from the start of reworking the target platform.  We went with included because once the user has defined a target platform as a certain set of bundles we think they will want their target to stay the same.

If you have used the content tab to say "These 28 bundles are my target" and then the locations are changed so that those bundles are not available, that is an error.

Trying this out, there is a more urgent bug here.  You shouldn't have to hack the text file to remove the errors.  The included (but not found) bundles are supposed to be listed in the content tab, with error icons.  Simply unchecking them should remove them from the include list and remove the errors.  However, trying this out, I see that it somehow got broken.

Marking this for RC1, even though we are really stretched for time this milestone.
Comment 2 Jeff McAffer CLA 2010-05-10 13:37:21 EDT 
Thanks for the clarification.  FWIW, the workflow that has users deselecting the now missing bundles from the content tab is very error prone.  For example, my target has 500+ bundles in it and there are several dozen bundles to remove. Chances of the user getting this right  are slim to none when they have to flip between the two tabs then hunt and deselect.

Mitigating approaches:
- on the Definition tab where the errors show up have a context menu entry that says "remove from target" or some such.  So "thanks, yes, the are problems, toss them"

- on the Content tab show error marks beside the bundles with problem and all users to deselect them.  Ideally there would be some bulk removal workflow since the numbers here can get very large
Comment 3 Curtis Windatt CLA 2010-05-10 14:13:38 EDT 
(In reply to comment #2)
> Thanks for the clarification.  FWIW, the workflow that has users deselecting
> the now missing bundles from the content tab is very error prone.  For example,
> my target has 500+ bundles in it and there are several dozen bundles to remove.
> Chances of the user getting this right  are slim to none when they have to flip
> between the two tabs then hunt and deselect.

Users shouldn't have to hunt, as they all show up with error icons and are at the top of the list.  This does require using the content tab.

> 
> Mitigating approaches:
> - on the Definition tab where the errors show up have a context menu entry that
> says "remove from target" or some such.  So "thanks, yes, the are problems,
> toss them"
> 

Yes, I could see that, the error management in the target is definitely not as robust as we would like.  Errors should be associated in some way to the location and/or plug-in they are caused by.  When working with p2 based targets the problem can be even worse as the p2 errors are verbose, but aren't associated with any of the actual target content.  This is beyond the scope of anything we are doing in 3.6

> - on the Content tab show error marks beside the bundles with problem and all
> users to deselect them.  Ideally there would be some bulk removal workflow
> since the numbers here can get very large

The steps would be:
1) Open the content tab
2) See the errors, select them all
3) Press the deselect button
Comment 4 Curtis Windatt CLA 2010-05-12 16:29:04 EDT 
Created attachment 168256 [details]
Fix

Fixes the content group so that it displays entries for missing bundles/features.  I had to separate the does not exist status code into two codes for plugins vs features.  Also fixes an NPE from updating the counter.
Comment 5 Curtis Windatt CLA 2010-05-12 16:36:51 EDT 
Darin, please review/apply the patch.

I opened bug 312710 for an edge case that isn't working.  I don't see a simple fix for it, so it won't make it for RC1.
Comment 6 Curtis Windatt CLA 2010-05-12 16:46:01 EDT 
There is one trivial change to the tests that didn't get included in the patch.

On line 366 of TargetDefinitionFeatureResolutionTests.testMissingMixed(), the returned status code will be IResolveBundle.STATUS_FEATURE_DOES_NOT_EXIST not the plugin does not exist constant.
Comment 7 Darin Wright CLA 2010-05-12 16:58:03 EDT 
+1
Comment 8 Curtis Windatt CLA 2010-05-12 17:18:57 EDT 
Fixed in HEAD.

Filed bug 312718 for a related issue Darin noticed.