Community
Participate
Working Groups
Created attachment 209239 [details] proposed change I have a java class that has two JSR45 strata -- 'java' and 'm2e'. The standard 'java' stratum holds java source name and path information. 'm2e' stratum holds additional maven-specific information and allows more precise and efficient source code lookup for classes generated with Maven. 'm2e' stratum interpretation is implemented in ISourceLocator associated with the launch. The same ISourceLocator is also able to handle string-based. As of 3.8M4, JavaLineBreakpoint.computeJavaProject implementation evaluates standard 'java' stratum first and only allows ISourceLocator evaluate IJavaStackFrame if no standard source/path information was found. I believe this implementation is not correct and propose to reverse the evaluation order, i.e. allow ISourceLocator evaluate IJavaStackFrame first, and fall back to the generic java source code lookup after that (see attached proposed change).
Created attachment 223712 [details] proposed change Updated the patch for the current R3_8_maintenance. I find complete lack of feedback on this issue rather unfortunate. This leaves me little choice but include jdt feature patch in m2e 1.3 we plan to released as part of juno sr2.
(In reply to comment #1) > Created attachment 223712 [details] > proposed change > > Updated the patch for the current R3_8_maintenance. > > > I find complete lack of feedback on this issue rather unfortunate. This > leaves me little choice but include jdt feature patch in m2e 1.3 we plan to > released as part of juno sr2. I don't see a problem with the patch, but I guess I just don't understand why it fails for your case. Is it because your director cannot compute the source paths for all your stratums? Or is it because of this line: sourceElement = ((ISourceLookupDirector) locator).getSourceElement(sourcePaths[0]); where you do correctly compute what you want but it is in the paths array greater than the first position? The patch works fine for the Java debugger, because our only locator is a director, so in either case our (and other) participants will still be consulted properly.
Created attachment 223732 [details] updated patch This patch is updated against 4.3 as the other patch did not apply completely
In case of m2e, classes usually contain standard 'java' stratum with .java file name information and 'm2e' stratum with additional metadata that allows m2e disambiguate multiple versions of the same class present at runtime and also download corresponding sources from artifact repositories if necessary. The following two line in JavaLineBreakpoint.computeJavaProject effectively bypass m2e-specific logic and force use of default 'java' stratum regardless if 'm2e' stratum is present or not. String[] sourcePaths = ((IJavaReferenceType) thisType).getSourcePaths(null); sourceElement = ((ISourceLookupDirector) locator).getSourceElement(sourcePaths[0]); By first calling "sourceElement = locator.getSourceElement(stackFrame);", JavaLineBreakpoint will allow locator evaluate 'm2e' stratum if it is present, while still falling back to the current behaviour if getSourceElement(stackFrame) returns null. There is a similar problem in org.eclipse.jdt.internal.debug.core.logicalstructures.JavaLogicalStructure.getLogicalStructure(IValue), which also needs to be fixed. I can provide updated patch against 4.3 if this helps. Btw, does debug ui actually need to have separate code path for "if locator instanceof ISourceLookupDirector"? Why not always delegate to locator.getSourceElement(stackFrame) and let the locator decide how to handle it? This seems to provide better separation of concerns between ui and locator.
(In reply to comment #4) > The following two line in JavaLineBreakpoint.computeJavaProject effectively > bypass m2e-specific logic and force use of default 'java' stratum regardless > if 'm2e' stratum is present or not. Thanks for the info, this code was added in bug 44758 to help resolve code from external projects, but I confirmed this fix does not cause a regression for that fix. > > There is a similar problem in > org.eclipse.jdt.internal.debug.core.logicalstructures.JavaLogicalStructure. > getLogicalStructure(IValue), which also needs to be fixed. I can provide > updated patch against 4.3 if this helps. That would be very helpful. > Btw, does debug ui actually need to have separate code path for "if locator > instanceof ISourceLookupDirector"? Why not always delegate to > locator.getSourceElement(stackFrame) and let the locator decide how to > handle it? This seems to provide better separation of concerns between ui > and locator. It looks like it no longer needs this code, I have commented it out + tested + run the regression tests and it all works fine. For what its worth we could also get rid of the JavaLineBreakpoint#computeJavaProject method as well, the only place it is used is in the JavaLineBreakpoint#getJavaProject method.
Created attachment 223783 [details] proposed change to JavaLineBreakpoint and JavaLineBreakpoint I've updated my patch to make similar change to both JavaLineBreakpoint and JavaLineBreakpoint. In both cases I did not remove "if locator instanceof ISourceLookupDirector", but can rework the patch to do this if you want. The patch is against current master HEAD and was produced using git-diff. My original patch was produced with git-format-patch, but you were not able to apply it cleanly as far as I understood. I'll be happy to provide the patch in whatever format you prefer. Btw, specific m2e extension that relies on this patch is opensource and available from github https://github.com/ifedorenko/com.ifedorenko.m2e.sourcelookup . I can even do short demo of it in action if you are interested :-)
Comment on attachment 223783 [details] proposed change to JavaLineBreakpoint and JavaLineBreakpoint Patch works fine
Pushed patch to: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=de526943120915e44f4ea1792f6445ffd1ef0efa Thanks for the patch Igor!
Reopening, this patch makes bug 327497 much worse.
Do you plan to keep my changes or should I look for alternative ways to distribute them in M4 and beyond? Anything I can do to help keep the changes in?
(In reply to comment #10) > Do you plan to keep my changes or should I look for alternative ways to > distribute them in M4 and beyond? Anything I can do to help keep the changes > in? We have a fix that I am testing to see if this is the case: (http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?id=6969f6131bc0b5b970509047133b72ec67d8b3c0). Ultimately though, I would envision an alternate way to resolve the Java project that does not involve using source lookup at all - this would make our evaluations faster as well
(In reply to comment #11) > (In reply to comment #10) > > Do you plan to keep my changes or should I look for alternative ways to > > distribute them in M4 and beyond? Anything I can do to help keep the changes > > in? > > We have a fix that I am testing to see if this is the case: > (http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/ > ?id=6969f6131bc0b5b970509047133b72ec67d8b3c0). > This will work for my case ;-) > Ultimately though, I would envision an alternate way to resolve the Java > project that does not involve using source lookup at all - this would make > our evaluations faster as well Are you ready to share your ideas? I am particularly interested to understand how project lookup will work when the same class is present multiple workspace projects. m2e and I believe pde are able to disambiguate the classes by querying the target jvm, which appears to require source lookup.
(In reply to comment #12) > Are you ready to share your ideas? I am particularly interested to > understand how project lookup will work when the same class is present > multiple workspace projects. m2e and I believe pde are able to disambiguate > the classes by querying the target jvm, which appears to require source > lookup. The problem is I don't have any concrete ideas. But while I was thinking about it I did recall we have a framework method for resolving Java elements using source lookup that we could have extended to resolve Java projects. I pushed an updated fix to: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=7136543e2c30b1e1f1d953e43649aac46ad30202 Igor, can you try the latest code changes to see if it still works for your project resolution?
re-marking this as fixed. The problem that the lookup jobs are not properly serialized (bug 327497) is really only made worse by one dialog, providing you have a conditional breakpoint in one of the duplicate classes.
Verified in: Version: 4.3.0 Build id: I20121212-0800
