Community
Participate
Working Groups
For Bug 305037, a behavior change was implemented such that JDT will no longer resolve manifest classpath entries in jars returned by a classpath container. The classpath containers are now expected to resolve manifest classpath themselves if that is desirable. The issue is not so much the design (whether the classpath container or JDT be responsible for manifest classpath resolution), but rather that this change in behavior affects existing container implementations written against Eclipse 3.5. This is not a theoretical problem. We came across this when debugging an issue in our product (Oracle Enterprise Pack for Eclipse). A classpath container that works on Eclipse 3.5 does not work on latest 3.6 builds. There is also a side issue of impact on users as the result of this change impacting user libraries container. If a user has a workspace on Eclipse 3.5 and they define user libraries, they may intentionally or accidentally come to rely on manifest classpath resolution on those jars. Such a workspace when loaded in Eclipse 3.6 will not build. It will not be obvious to users what happened and what they need to correct (explicitly add jars pulled in via manifest classpath). I would recommend trying to come up with a safer approach to solving the issue described in Bug 305037. If it is important to give more control over manifest classpath resolution to container then I would recommend adding new API that allows containers to opt-in for resolution suppression.
Would it be feasible for you to update to your product with an updated ClasspathContainerInitializer?
> Would it be feasible for you to update to your product with an updated > ClasspathContainerInitializer? Yes, we can address this on our side. We have a release planned soon after Helios launch. Other products may not be so fortunate.
Jay, Classpath containers got the referenced entries for free in 3.5. So users that are relying on this behavior might be broken when moving to 3.6 with missing type errors. We discuss that situation at the JDT call and we came up with this plan: - add a system property to be able to revert back to the old (3.5) behavior for the classpath container. - add a readme and a migration guide entry so that we can warn users of potential problems and let them know that there is a workaround using the system property. - we don't want to add support for attributes on these referenced entries inside classpath container. Let me know if you have any question. You can discuss the case with Srikanth. Please prepare a patch and set the review flag to Markus, Srikanth and myself. Thanks.
Created attachment 170061 [details] Proposed patch Patch contains changes mentioned in comment 3 with tests. Tests have been added in ClasspathTests#testBug313965 and ClasspathTests#testBug313965a. Srikanth/Markus/Olivier, Please review. Bug 313890 captures the documentation changes such as Javadoc, migration guide and readme.
I would remove the if statement to write it as: this.resolveReferencedLibrariesForContainers = "true".equalsIgnoreCase(includeContainerReferencedLib); //$NON-NLS-1$
Created attachment 170081 [details] Proposed fix + regression tests New patch based on my last comment.
+1
Created attachment 170086 [details] Proposed fix + regression tests New patch that reuses the constant "true".
(In reply to comment #8) > Created an attachment (id=170086) [details] > Proposed fix + regression tests > > New patch that reuses the constant "true". Shouldn't addToResult(...) be called with the parameter addAsChainedEntry set to true instead of false in the added if block in JavaProject? Similar blocks of code already present in JavaProject.resolveClasspath(...) method call this method with this parameter equals to true... But maybe I missed something here!?
(In reply to comment #9) > (In reply to comment #8) > > Created an attachment (id=170086) [details] [details] > > Proposed fix + regression tests > > > > New patch that reuses the constant "true". > > Shouldn't addToResult(...) be called with the parameter addAsChainedEntry set > to true instead of false in the added if block in JavaProject? > > Similar blocks of code already present in JavaProject.resolveClasspath(...) > method call this method with this parameter equals to true... But maybe I > missed something here!? Since we don't want support for attributes to be persisted for container libraries as Olivier mentioned in comment 3, a false needs to be passed there.
(In reply to comment #10) > Since we don't want support for attributes to be persisted for container > libraries as Olivier mentioned in comment 3, a false needs to be passed there. To make it clearer, the two cases where we are passing true (as value for parameter addAsChainedEntry) to the addToResult() method are in the case of CPE_LIBRARY and CPE_VARIABLE, where we do want to support additional attributes for referenced libraries. In all other cases, we pass 'false'.
Patch looks good. Note that the header comment in both junits need to be fixed as they are referencing a wrong system property name: "includeContainerReferencedLib" instead of the final choice "resolveReferencedLibrariesForContainers"
(In reply to comment #10) OK, thanks for the clarification. Hence, patch looks good to me
+1 for RC3.
Released in HEAD for 3.6RC3 with one minor change - as per comment 12.
verified for 3.6RC3 using build I20100527-1700.
