Community
Participate
Working Groups
Currently if a user assembles an EAR 5/6 project to include a classpath container mapped to the lib folder it is also necessary for the user to add that classpath container to each module's classpath. This bug will remove that required user step by taking advantage of the EAR Libraries classpath container to add the appropriate classpath artifacts to each module's classpath. Similarly assembling a classpath container to a Web project's WEB-INF/lib folder will now automatically resolve the classpath via the Web App Libraries classpath container instead of requiring an extra step by the user. The changes in J2EEComponentClasspathContainer handle the EAR case. The changes in ClasspathContainerVirtualComponent, FlexibleProjectContainer, and WebAppLibrariesContainer handle the Web case. The changes in VirtualReference handle a NPE I encountered while testing the above with the ModuleAssemblies page and is unrelated to the other changes.
Created attachment 167002 [details] patch
I approve this patch - Please add PMC review
Rob can you also take a look?
I haven't tested it, but by looking at the patch, it looks like it's doing 100% what I would expect it to do... Specifically the changes look good and seem to appropriately handle both used and consumed scenerios in both classpath containers. It'd be nice to see some unit tests but I don't believe they're necessary for me to vote +1 on this, so, +1. ;)
* Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such. Jars (from consumed references or otherwise) which may be bundled and exported in one project are not currently properly added to the classpath of other related projects. * Is there a work-around? If so, why do you believe the work-around is insufficient? The workaround is to manually add such jars to the classpath. This is very inefficient for the user and would require them adding a jar to assembly and then again adding it to the classpath. * How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? I don't believe there are unit tests for this bug. * Give a brief technical overview. Who has reviewed this fix? Briefly, the classpath containers will now also scan the virtual components for other suitable jars and automatically add those to the classpath. This has been reviewed and approved by Chuck, and also reviewed by myself. * What is the risk associated with this fix? I see little risk with this patch currently but I have not tested it thorougly so its possible some tiny defect shows through. As currently coded, it seems a well-written and proper patch for the behaviour of these classpath containers.
*** Bug 311620 has been marked as a duplicate of this bug. ***
code checked into HEAD for RC1
I see a cycle dependency error when testing this out for a project that is not a web or EAR project. I added a user library to a connector project and instead of adding the correct jar/s the project itself was added to the EAR libraries container causing a cycle dependency.
The problem mentioned in comment 8 is unrelated; you can reproduce it without these changes; That problem will be handled with a separate bug.
Re-opening because it turns out that the change to VirtualResource address the NPE doesn't work for the use case in Bug 311620. In that case, the first test on line 165 (i.e. "getArchiveName() == null && otherRef.getArchiveName() == null") will evaluate to false with getArchiveName() returning null and otherRef.getArchiveName() returning non-null. In this case, the companion "or" test takes place and generates the NPE. I think the patch in Bug 311620 would work better. I'll attach an updated version here.
Created attachment 167422 [details] Additional patch to address NPE still present in VirtualReference For convenience, the patch also updates the copyright year.
What is the process for re-submitting this bug for approvals? Should I reset the flags? Which ones? Would it simply be easier to put this in a separate bug?
Ouch; can't believe I made that mistake with the and/or logic. Thanks for catching. Considering this is such an obvious error and simple that needs to be fixed, I'm checking this in based on the original PMC review.
