Community
Participate
Working Groups
Created attachment 195885 [details] testcase I found a regression from 342 to 362, similar to what bug# 297039 describes. I made sure that I have the changes for bug# 297039 in my environment when I use 362. The scenario: When I debug the code for my application I see that the java files is open instead of the class file and I can't inspect the content of the 'this' variable. I get an error saying "Unable to evaluate the selected expression: ...." The setup: I have two projects, JavaPrj (a Java project) contains my code and it makes use of a class inside of jar. The jar is located inside of SimplePrj\lib (a simple project). The jar contains the source and the binary code. JavaPrj makes a reference to the jar through the Java Build Path -> Libraries > Add Jars... button The Run: (using the attach testcase) - Add a breakpoint to RunMe.java at the initUtil method. - Debug the application - When the Eclipse stops at the breakpoint step inside of u.putAttributes - Once inside try to inspect 'this'
Quickly tested this on 3.7 and it works as expected: 1. import the test case and use the steps given 2. after stepping in, the source from the archive is shown [OK] 3. I can inspect 'this' without issue [OK]. I will load up my 3.6.2 test workspace and try there...
The same steps also work for me using: Version: 3.6.2 Build id: M20110210-1200 Angel, since the .java file source is available in the archive why are you expecting to see the .class file? Note: showing the class file or the Java file from the archive does not affect the inspect action.
When I tried inspecting 'this' in the .java file it didn't worked for me. I got the problem that I documented in the description. I tried this on Win32, I wouldn't expect that the platform makes a difference, but just in case. Looking through the code the problem seemed to be that in previous versions JDT org.eclipse.jdt.internal.launching.JavaSourceLookupUtil.translate(IRuntimeClasspathEntry[]) would return a PackageFragmentRootSourceContainer because a root exists for that project, but in 362 a root doesn't exists so it creates a ExternalArchiveSourceContainer.
I am using 362 M20110210-1200
(In reply to comment #2) > The same steps also work for me using: > > Version: 3.6.2 > Build id: M20110210-1200 > > Angel, since the .java file source is available in the archive why are you > expecting to see the .class file? Note: showing the class file or the Java file > from the archive does not affect the inspect action. The critical missing step to reproduce is that you have to select 'this' in the editor and try to inspect it.
Debugging this I found that part of the fix for bug 297039 moved a call to RuntimeClasspathEntry#getPath() out of a double-nested for-loop and in doing so changed the path that would be compared: was: if (!root.isExternal() && root.getPath().equals(entry.getPath())) { .. } changed to: if (root.isExternal() && root.getPath().equals(entryPath)) { .. } where entryPath is defined as: Path entryPath = new Path(entry.getLocation()); I will have to change the definition of entryPath to be based on entry.getPath() and see if it affects the fix for bug 297039
(In reply to comment #6) > I will have to change the definition of entryPath to be based on > entry.getPath() and see if it affects the fix for bug 297039 Michael, What is the status on this one? Did you get a chance to try out things?
(In reply to comment #7) > (In reply to comment #6) > > I will have to change the definition of entryPath to be based on > > entry.getPath() and see if it affects the fix for bug 297039 > > Michael, > > What is the status on this one? Did you get a chance to try out things? I did, I reverted the logic to compare the correct path elements and removed the check for if the class path entry is external or not, and it seems to be working - much more smoke testing is required and I want to add some JUnit tests for this scenario. I'll post a patch
Created attachment 196120 [details] testing patch Here is a patch of the code I have been playing with.
I ran through my scenario and it seems to working OK, I am certainly concern about the risk of regression as per comment# 6. I will leave it up to you for your testing to confirm that this is the right fix and that it doesn't break other scenarios or that introduces regressions.
Created attachment 196680 [details] proposed fix Here is a complete patch with new tests. I am still on the fence about inclusion in RC3 / RC4 for fear of regression in another edge case, although smoke testing did not uncover any other issues.
Created attachment 196682 [details] test projects Here are the test projects that must be un-zipped in the org.eclipse.jdt.debug.tests/testresources/ folder. Using these projects the new tests set up a scenario as described in this bug, and perform source look up, classpath entry translation and stepping / inspecting.
Curtis, Dani, Markus, please review the patch
Code-wise, the proposed fix is really the same as the testing patch. It looks good, and I verified that it works for internal and external JARs, and that it doesn't break bug 297039. However, this problem has not been reported so far, although bug 297039 has been in since 3.5.2. Angel, do you really think it's critical for 3.7, or could this wait till 3.7.1?
The fix looks OK to me but I can't guarantee that there are no unwanted side effects. Given we're in RC4 and the bug is in since 3.5.2 I -1 this fix for 3.7 RC4.
(In reply to comment #15) > Given we're in RC4 and the bug is in since 3.5.2 I -1 this fix for 3.7 > RC4. I'm fine with that. We can target it for 3.7.1.
The fix works for me, but since we are moving this to 3.7.1 I will hold off on my +1.
Applied patch + tests to HEAD
Created attachment 198834 [details] patch for 3.7.1
committed fix to 3.7.1
