Community
Participate
Working Groups
Using Platform RC1a, 1) Start a new workspace 2) Add http://download.eclipse.org/callisto/releases/site-byProject.xml as a new update site 3) Select the TPTP project 4) Click on "Selected Required". You still get errors about plugins not selected. Requested operation cannot be performed because it would invalidate the current configuration. See details for more information. TPTP Server Log Import (4.1.100.v200604131758) requires plug-in "org.eclipse.wst.server.core". TPTP Platform Trace (4.2.0.v200604131758) requires plug-in "org.eclipse.pde.core". TPTP Profile on Server (4.1.100.v200604131758) requires plug-in "org.eclipse.jst.server.tomcat.core". BIRT Reporting for TPTP Feature (4.2.0.v200604131758) requires plug-in "org.eclipse.birt.core". Java Code Review Feature (4.2.0.v200604131758) requires plug-in "org.eclipse.pde.core". This should not happen since I explicitely clicked on "Select Required".
It looks like the "Select Required" doesn't do a transitive closure. I think this is a major limitation in order to get new users of Eclipse through Callisto. If it is painful to do the update, users will give up.
There are several connected problems on this page some of them seem to be special cases. i am working on them I hope i will have fix for tonight’s build.
This problem is very important to be done right, and we don't feel we had enough runaway to release it for RC2. We will fix it in HEAD and let you know so that you can test it on real data before we propose it for RC3.
Created attachment 40327 [details] 138802forCoreAndUIpatch.txt Some of the features in Callisto put requirements on plugins that are in included features which "Select Required" would not pick up. This patch allow us to check requirements from features on plugins as well as on included features and their plugins. Risk is very minimal and benefit is that among other thing Callisto users can use "Select Required". This patch also resolves bug 130667.
*** Bug 130667 has been marked as a duplicate of this bug. ***
there's alot of code there. Need someone to review in detail and. I am conceptually ok with releasing a fix for this bug but we need to understand the risk
1. Since 99% of changes are in ReviewPage it can affect only UI. 2. 99% of the code changes are in methods called only from "Select Required" thus only this functionality is at risk 3. "Select Require" did not function properly or at all through RC1 and RC2 so we can only get improvements here. 4. The only notable change that might affect other code paths (still localized in ReviewPage) is that we now process status of validation but this was taken out by accident during rc1 and this functionality as it is after this patched enabled it is exactly the same as it was in 3.1 and as far as I know in 3.0 and perhaps even before. To summarize: The use cases that might be affected are the ones with usage of "Select Require" however, since this was not working before this patch it can only be positive impact.
Jeff, we are OK with someone reviewing, but I suggest you nominate that 'someone' soon or we will miss the window to react to possible change requests.
Branko, I reviewed the patch and the scariest part is at the bottom of ReviewPage.java. Considering that it does not work now, I don't think we can make it worse. Is there an update site I can point at to compare 'before the patch' and 'after the patch' behaviour? Regardless of this testing, I have noticed several snippets like this: } catch (CoreException e) { e.printStackTrace(); ... } I would definitely prefer if we do something more intelligent with the exception (log it or something) rather than dumping it on stdout.
current callisto site just be patient it is slow. you select for instance test an performance category and run "Select Required". As comments in the code say those two exceptions are not important on this level (and they should already happen be logged by this point in code) but, they still had to be caught. I left those printStackTraces by accident. If you prefer I can log this exception or just ignore them. I am fine with either. However this will not change behavior of patch so you can continue testing and i will submit new one as soon as i am aware of popular opinion.
My test case is described in comment 0. If you don't get any error about missing features after step 4 then it is definitely an improvement.
Some comments on the patch: UpdateManagerUtils: - You changed both rules to return "compatible" instead of "perfect" when a null parameter is provided. It seems strange to be relying on the rule returned for a null parameter, and it seems to contract the javadoc that says the default is "perfect". InstallOperation: - You have defined equality as equality of the version identifier of the feature object, but for hashCode it is just using the native default hashcode of the feature object. Thus, it appears the hashCode() and equals() methods will give inconsistent results in the case where the feature objects are different but the versioned identifier is the same. ReviewPage: - I don't understand all the details, but I don't see any particular problems. I strongly agree with Dejan's comment that exceptions must be logged and not dumped on stdout. If there are any problems with this fix we will need to be able to diagnose them quickly. Other than that, +1 to put this fix in.
John, 1) This is how we define it our documentation: match (perfect | equivalent | compatible | greaterOrEqual) "compatible" so this is long standing problem that floated up when “non-perfect” callisto features came on line 2. it is actually consistent with feature which does not define hashCode but only equals that was the reason for me to do it. So, both Feature and InstallOperation are doing the same thing they use VersionedIndetifier for equals and ignore hashCode. Update has a general problem with not defined equals and hashCode so I just try not to break it in this regard by doing what was done in more relevant classes around the class i am working on. 3. patch is on the way with exception logging. Thanks for your thorough look at this patch.
Created attachment 40425 [details] with exceptions being logged not printed
+1 on the latest patch. I suggest entering a separate bug for the equals/hashCode issue (to be resolved in a future release). When equals and hashCode are inconsistent it can cause *really* subtle bugs that are very hard to track down. It's worth revisiting that when there is time. The fact that it's consistently inconsistent isn't much reassurance ;)
+1 - let's wrap this up.
Done.
