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

Bug 323174

Summary: Existence of classpath dependencies should not break single-root
Product: [WebTools] WTP Java EE Tools Reporter: Jason Peterson <jasonpet>
Component: jst.j2eeAssignee: Jason Peterson <jasonpet>
Status: RESOLVED FIXED QA Contact: Chuck Bridgham <cbridgha>
Severity: normal    
Priority: P3 CC: ccc, david_williams, jsholl, stryker
Version: 3.2Flags: david_williams: pmc_approved+
jasonpet: pmc_approved? (raghunathan.srinivasan)
jasonpet: pmc_approved? (naci.dai)
jasonpet: pmc_approved? (deboer)
jasonpet: pmc_approved? (neil.hauge)
jasonpet: pmc_approved? (kaloyan)
cbridgha: review+
Target Milestone: 3.2.2   
Hardware: PC   
OS: Windows XP   
Whiteboard: PMC_approved
Attachments:
Description Flags
patch
none
junit_patch
none
junit_patch_updated
none
junit_patch_v3 none

Description Jason Peterson CLA 2010-08-19 12:24:43 EDT
WTP 3.2 introduced a new check in the single-root utility (JavaEESingleRootCallback to be specific) that would check for the existence of any classpath dependencies.  If any were found it would return that the project's structure was not single-root.  The reason this was added in 3.2 was most likely due to the fact that when single-root classpath dependencies with a deploy path other than "../" were not being returned by members.

The problem with the added check is that it hurt an adopter's publishing performance since these projects were now marked as non single-root.  Publishing to a single-root project is much faster for the adopter.  The bigger issue is that this caused a regression since these same projects were considered single-root in earlier WTP versions.  

The solution is to remove the classpath dependencies check from the single-root utility and add the necessary delegate participants to the single-root participant.  This would involve adding the already existing classpath participants and creating a new one to handle classpath dependencies with a deploy path other than "../".
Comment 1 Jason Peterson CLA 2010-08-31 15:20:39 EDT
Created attachment 177880 [details]
patch
Comment 2 Jason Peterson CLA 2010-08-31 15:22:14 EDT
Created attachment 177881 [details]
junit_patch
Comment 3 Chuck Bridgham CLA 2010-08-31 15:44:42 EDT
approved
Comment 4 Jason Peterson CLA 2010-08-31 16:17:22 EDT
 * 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. 

Performance is hurt on some servers when a project is not considered to be single-root.  Projects that have classpath dependencies did not break single-root in the past and because this check was added in 8.0 a regression in performance will occur for some servers.  
   

    * Is there a work-around? If so, why do you believe the work-around is
insufficient? 

No workaround exists for users that tag classpath entries with the classpath dependency flag.

    * How has the fix been tested? Is there a test case attached to the
bugzilla record? Has a JUnit Test been added? 

JUnit testcase is attached.  I have added a new testcase.

    * Give a brief technical overview. Who has reviewed this fix? 

The solution was to remove the classpath dependencies check from the single-root
utility and add the necessary participants to the single-root code path for finding the classpath dependencies. 

Chuck has reviewed this fix.

    * What is the risk associated with this fix? 

minimal - regression tests have been run and a new testcase added.
Comment 5 Jason Peterson CLA 2010-08-31 17:11:33 EDT
Created attachment 177894 [details]
junit_patch_updated
Comment 6 Jason Peterson CLA 2010-08-31 17:39:23 EDT
Created attachment 177896 [details]
junit_patch_v3
Comment 7 David Williams CLA 2010-08-31 21:34:56 EDT
agreed this performance regression is urgent to fix.
Comment 8 Carl Anderson CLA 2010-08-31 23:42:13 EDT
Committed to HEAD for WTP 3.2.2 and WTP 3.3
Comment 9 Carl Anderson CLA 2010-08-31 23:43:54 EDT
Note that this also reinstated the ClasspathDependencyWebTests.suite() tests that were disabled in org/eclipse/jst/j2ee/classpath/tests/AllTests.java for bug 234409