Summary: | [search] Hyperlinks from the Java Stack Trace console should search the workspace in CLASSPATH order | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Nikolay Botev <nikolaybotev.b> | ||||||||||
Component: | Core | Assignee: | Satyam Kandula <satyam.kandula> | ||||||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||||||
Severity: | normal | ||||||||||||
Priority: | P3 | CC: | amj87.iitr, elatllat, markus.kell.r, Olivier_Thomann, srikanth_sankaran | ||||||||||
Version: | 3.7 | ||||||||||||
Target Milestone: | 3.8 M2 | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
See Also: | https://bugs.eclipse.org/bugs/show_bug.cgi?id=567521 | ||||||||||||
Whiteboard: | |||||||||||||
Attachments: |
|
Description
Nikolay Botev
2011-06-07 03:44:24 EDT
Created attachment 197481 [details]
patch with fix
this is a fix that modifies the 2 places in the code where the classpath entries are added into sets to use LinkedHashSet instead of whatever set they were using before. this effectively fixes the problem by defining a simple ordering rule for merging classpaths of multiple projects.
Satyam, please follow up for 3.8. Thanks! (In reply to comment #1) Thanks for your patch and this is good. Some minor comments: 1. However, I think it is better to extend to even multiple projects. Here are the steps to reproduce: 1. Create two projects: ProjectA and ProjectB and create some classes in both of them. 2. Export ProjectA into a jar and put as a dependency for projectB. 3. Export Project B into a jar and put as a dependency for projectA. 4. Make code changes such that the exception has both the classes in the exception. 5. Run the main program and click on the hyperlinks from the stacktrace Let me know if you want to fix this problem too in your patch. 2. IndexSelector.java - The change of locations.toArray() is good enough and the subsequent for loop is not necessary and can be removed. 3. Can you please add your name in the copyright of the file? I will try to add a test and also try to run some performance tests to make sure that there is no performance regression. (In reply to comment #3) Satyam, 1. I will attempt to update the patch to pull all the source classpath entries up to the front. If there is more than one project containing the same java class, there is still an ambiguity as to which one will get selected. At that point it might be best to pull up the Open Type dialog, but I don't know how easy that would be and not sure if I will have the time to spend on this. For my purpose the current state of the fix even though incomplete is good enough. 2. I will update the patch to avoid the double-copy of the array. Thanks for the quick feedback > (In reply to comment #1) > Thanks for your patch and this is good. Some minor comments: > > 1. However, I think it is better to extend to even multiple projects. Here are > the steps to reproduce: > 1. Create two projects: ProjectA and ProjectB and create some classes in > both of them. > 2. Export ProjectA into a jar and put as a dependency for projectB. > 3. Export Project B into a jar and put as a dependency for projectA. > 4. Make code changes such that the exception has both the classes in the > exception. > 5. Run the main program and click on the hyperlinks from the stacktrace > > Let me know if you want to fix this problem too in your patch. > > 2. IndexSelector.java - The change of locations.toArray() is good enough and > the subsequent for loop is not necessary and can be removed. > > 3. Can you please add your name in the copyright of the file? > > I will try to add a test and also try to run some performance tests to make > sure that there is no performance regression. Satyam, I have incorporated all of your feedback. Please let me know how it looks. Thanks (In reply to comment #3) > (In reply to comment #1) > Thanks for your patch and this is good. Some minor comments: > > 1. However, I think it is better to extend to even multiple projects. Here are > the steps to reproduce: > 1. Create two projects: ProjectA and ProjectB and create some classes in > both of them. > 2. Export ProjectA into a jar and put as a dependency for projectB. > 3. Export Project B into a jar and put as a dependency for projectA. > 4. Make code changes such that the exception has both the classes in the > exception. > 5. Run the main program and click on the hyperlinks from the stacktrace > > Let me know if you want to fix this problem too in your patch. > > 2. IndexSelector.java - The change of locations.toArray() is good enough and > the subsequent for loop is not necessary and can be removed. > > 3. Can you please add your name in the copyright of the file? > > I will try to add a test and also try to run some performance tests to make > sure that there is no performance regression. Created attachment 198304 [details]
updated patch
for your review.
Created attachment 198306 [details] workspace wtih testcase involving 2 projects ... as described in comment #3 See also bug 138377. Hi, This is a critical issue for us at my company. Can this be integrated in a 3.7 point release? Is there anything else that is needed from me? Thanks (In reply to comment #9) > Hi, > > This is a critical issue for us at my company. > > Can this be integrated in a 3.7 point release? Olivier, what is your opinion on this ? Created attachment 201811 [details]
Junit test
Junit test for this problem
All tests pass with the patch. We were busy with Java 7 support and hence this got left out. Now, this does seem to be a little late for 3.7.1. Added to this, we generally put only regressions and very serious problems into a service release and this doesn't qualify that criteria. Hence, we are targeting this for 3.8M2. Released the patch and the Junit test on HEAD Hi, Thank you for the quick response and integrating this patch. We look forward to seeing this in the next 3.8 milestone release. Verified for 3.8M2 using build I20110911-2000. *** Bug 363512 has been marked as a duplicate of this bug. *** (In reply to comment #17) > *** Bug 363512 has been marked as a duplicate of this bug. *** link to the latest builds: http://download.eclipse.org/eclipse/downloads/eclipse3x.php (In reply to comment #18) > (In reply to comment #17) > > *** Bug 363512 has been marked as a duplicate of this bug. *** > > link to the latest builds: > > http://download.eclipse.org/eclipse/downloads/eclipse3x.php Did you want to mention something else? I don't understand what you want to tell. (In reply to comment #19) > (In reply to comment #18) > > (In reply to comment #17) > > > *** Bug 363512 has been marked as a duplicate of this bug. *** > > > > link to the latest builds: > > > > http://download.eclipse.org/eclipse/downloads/eclipse3x.php > Did you want to mention something else? I don't understand what you want to > tell. I was just providing a link so that people who have this bug and went it fixed can find something download. (In reply to comment #20) > I was just providing a link so that people who have this bug and went it fixed > can find something download. Thank you. |