Community
Participate
Working Groups
We need to verify that when an error because an IU can't be resolved because of filter (either applicability filter in the IU or on the requirement) the explanation is still decent.
I need some examples. Should this be taken care at the encoding level or at the explanation level?
We have some JUnit tests for cases like this. See AutomatedDirectorTest.java. If the filter on the required capability is not satisfied this shouldn't cause an error (the dependency is just removed). I think testInstallPlatformFilterUnsatisfied is the interesting failure case.
Here is the current explanation: Cannot complete the install because one or more required items could not be found. null children=[Status ERROR: org.eclipse.equinox.p2.director code=0 Missing requirement: 1239048285281 0.0.0.1239048285281 requires 'toInstall.testInstallPlatformFilterUnsatisfied [1.0.0]' but it could not be found null]
We also have IUWithFilter*
John, How do you get an explanation from a director object? (i.e. how do you get that explanation in testInstallPlatformFilterUnsatisfied)
That method invokes IDirector#provision, which returns an IStatus. The explanation is inside that status object. The message I printed above was just the "toString()" of that status object.
Thanks John. In that particular example, executing the following code for req="toInstall.testInstallPlatformFilterUnsatisfied [1.0.0]": Collector matches = picker.query(new CapabilityQuery(req), new Collector(), null); the matches ends up empty. I do not see the filter in action in Projector code (filter field is always set to null, both in the installable unit and in the requirement).
I believe that the code in Projector: private List getApplicableMatches(IRequiredCapability req) { List target = new ArrayList(); Collector matches = picker.query(new CapabilityQuery(req), new Collector(), null); for (Iterator iterator = matches.iterator(); iterator.hasNext();) { IInstallableUnit match = (IInstallableUnit) iterator.next(); if (isApplicable(match)) { target.add(match); } } return target; } Could be simplified by: private List getApplicableMatches(IRequiredCapability req) { List target = new ArrayList(); Collector matches = picker.query(new CapabilityQuery(req), new Collector(), null); for (Iterator iterator = matches.iterator(); iterator.hasNext();) { IInstallableUnit match = (IInstallableUnit) iterator.next(); target.add(match); } return target; } I haven't found one test case in planner tests in which isApplicable returns false. It looks like filters are taken into account in picker.query(). On my side, I believe I cannot do much to improve the explanation message since I do not have access to the filters.
We need to also review what can be done to improve the error message This is the message I get when attempting to install a linux feature with this filter <feature id="com.test.F101.feature.pFilter.os.linux" label="test.F101.feature.pFilter.os.linux" version="1.0.0" provider-name="IBM" os="linux"> "Cannot complete the install because one or more required items could not be found. Missing requirement: 1240951705984 0.0.0.1240951705984 requires 'com.test.F101.feature.pFilter.os.linux.feature.group [1.0.0]' but it could not be found"
I have a added a test called TopLevelFilterTest that shows the problem. It is not enabled.
See also #277170
*** Bug 287168 has been marked as a duplicate of this bug. ***
Can you bump up the severity on this bug? It's causing a lot of confusion in installing things e.g. installing on a Mac when a feature was meant for Linux. Users typically select "all" then get a cryptic error message that doesn't make sense
Agreed, this is a pretty major problem. Many of our potential users are new to Eclipse and aren't "sold" on it anyway -- and since some of our features are platform-specific, if they check the entire feature set, they get a cryptic error, and nothing installs. We are losing potential users because, as far as they can tell, the installation "doesn't work." Of course, we can document these things in the installation process, but no one reads that, and the experience leaves a poor first impression anyway. -Jeff
Frankly, this is not only a problem for end users. This issue even impacts developers. I just spent about 2 hours troubleshooting the update site for a new plug-in until the post at http://www.eclipse.org/forums/index.php?t=msg&goto=490349 finally pointed me into the right direction. The existing error messages are of absolutely no help in identifying what the actual problem is. These sorts of cryptic errors can waste a lot of peoples' time, and I assume it would be relatively easy to provide an error message that is more helpful. So, please bump up this bug in the priority list. Thanks! Mirko
Created attachment 165851 [details] Improved error message for Root IU and direct Root filter problem detection. Here is a patch that displays a specific message when the missing requirement is coming from the Root IU. That idea may be reused for all explanation messages. By using the additions in the slicer, direct requirements with no matching filters are directly detected in the slicer and reports an appropriate error message in the multistatus: The IU ThingWithFilter 1.0.0 can't be installed in this environment because its filter does not match. The current problem is that message is a children of the multistatus, not the main message.
Thanks for working on this, Daniel. > The IU ThingWithFilter 1.0.0 can't be installed in this environment because its > filter does not match. Apologies... I don't completely understand the previous comment. :-) Is this the improved error message? I was really hoping that the example in Comment 9 would result in an error message like, "The feature com.test.F101.feature.pFilter.os.linux can only be installed on Linux," i.e., a message that an end user could comprehend and act on. I don't know if that's technically feasible or not, but it would be very nice. Just my 2 cents worth...
Well, the problem is that the filter might be something more specific: (& (os=linux) (ws=gtk)),(& (os=win32) (ws=win32) (arch=x86)), etc. we could display directly that information: The feature ThingWithFilter 1.0.0 can't be installed in this environment because its filter (& (os=linux) (ws=gtk)) does not match the current system. We can hardly translate that into plain English: The feature ThingWithFilter 1.0.0 can't be installed in this environment because it requires a Linux operating system with GTK installed.
> we could display directly that information: > > The feature ThingWithFilter 1.0.0 can't be installed in this environment > because its filter (& (os=linux) (ws=gtk)) does not match the current system. That seems reasonable to me. I might suggest the following wording (which is perhaps a bit more end user-oriented), but of course it's not my decision: :-) The feature ThingWithFilter 1.0.0 cannot be installed. To install this feature, your system must meet the following requirements: (& (os=linux) (ws=gtk)),(& (os=win32) (ws=win32) (arch=x86)) Thanks again for your help on this. We really appreciate it.
Created attachment 166030 [details] Improved message and minimized impact on Projector class. Updated patch after discussion with Pascal.
I was just about to tag and realized that we introduced a regression. I have reverted the patch in HEAD and released test case Bug270668 showing the regression.
Created attachment 166038 [details] New patch Before any attempt to fix this problem, the failure was raised on the projector. With the previous patch, we were trying to catch the error in the slicer but forgot to take into account the case where some IUs have a non applicable filter but are installed optionally. The idea behind this patch is to let the IU that have been requested to be added go to the projector whereas in the past they would have been filtered out, and then handle the situation to create a special explanation. I'm not super happy with the creation of the CompoundQueryable in the SimplePlanner and we could probably do something reusing the newRootIUs passed to the Projector#encode. Daniel, please review.
Created attachment 166082 [details] simplified solution A slightly modified version of Pascal's patch that avoid creating a new query. Note that now the root IUs are detected as strict requirements of the entryPoint IU. renamed the field rootIU by isEntryPoint in MissingIU.
(In reply to comment #21) > I was just about to tag and realized that we introduced a regression. I have > reverted the patch in HEAD and released test case Bug270668 showing the > regression. Sorry for the confusion on my part, I see that you reverted this in HEAD, but you did not tag HEAD for the M7 build. The committed patch is still part of the latest M7 build. Is that intended?
AFAIK, Pascal noticed the problem before tagging the patch for inclusion in the IBuild.
(In reply to comment #25) > AFAIK, Pascal noticed the problem before tagging the patch for inclusion in the > IBuild. I see where my confusion is now. Pascal backed out the change to the java code in HEAD and reverted the tag used in the p2.map for the director back to v20100424-0100.
Daniel, feel free to release the patch as well as enable the test cases.
Done.
