-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use custom debug model #437
Conversation
94042a2
to
38e0d1e
Compare
b99ec7f
to
e407c69
Compare
bundles/io.openliberty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/DevModeOperations.java
Show resolved
Hide resolved
...io.openliberty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/debug/DebugModeHandler.java
Outdated
Show resolved
Hide resolved
...berty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/debug/LibertySourcePathComputer.java
Show resolved
Hide resolved
...rty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/debug/LibertySourceLookupDirector.java
Show resolved
Hide resolved
...berty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/ui/launch/shortcuts/StartAction.java
Show resolved
Hide resolved
...berty.tools.eclipse.ui/src/io/openliberty/tools/eclipse/ui/launch/shortcuts/StartAction.java
Show resolved
Hide resolved
* | ||
* @throws Exception | ||
*/ | ||
public void startDevMode(IProject iProject, ILaunchConfiguration iConfiguration, ILaunch launch, String mode) throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like I mentioned in my StartAction comment, I think I get and like the refactoring overall.
This maybe should not be public. Maybe it should be named 'startViaDevModeOperations' or something different than the method name in DevModeOperations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
launchDevMode
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
15f4018
to
577db4d
Compare
org.eclipse.equinox.preferences, | ||
org.eclipse.swt, | ||
org.eclipse.m2e.maven.runtime, | ||
org.eclipse.m2e.core |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you still need the RB here? Or could these now be removed with the Import-Pkg?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Short answer is I tried without success.... we are importing some org.apache.maven packages which are not found if you try to add them in the Import-Pkg section rather than having the m2e bundles defined... can't say I understand why... As for the other two bundles, we are calling methods on implicit objects rather than creating the objects first. So we arent actually importing packages in the classes. (e.g. object.getSomething().someMethod() ).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK.. it's OK to use the Require-Bundle for the two m2e bundles.
As far as the other two...it sounds like you're saying that you can't easily see at compile time what classes/pkgs are needed to be imported? That in and of itself is arguably not a reason to use RB vs ImportPkg. Is it just one or two packages ? I could see trying once, twice, and then if there's a third just saying let's use RB?
However we do it can we avoid dups across RB and IP? Eg. the IP for m2e should be removed and maybe the swt custom one?
default: | ||
// Return the configuration that was run last. | ||
configuration = getLastRunConfiguration(matchingConfigList); | ||
break; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add the exit trace too here?
129f847
to
230fd96
Compare
Signed-off-by: Adam Wisniewski <[email protected]>
Signed-off-by: Adam Wisniewski <[email protected]>
Signed-off-by: Adam Wisniewski <[email protected]>
Signed-off-by: Adam Wisniewski <[email protected]>
50d977a
to
8830eee
Compare
Signed-off-by: Adam Wisniewski <[email protected]>
8830eee
to
7ec0f1a
Compare
Closing, superseded by #458 |
This PR creates and uses a new custom debug configuration rather than using the Remote Java Application configuration. The main advantages to this are:
Some notable changes:
LaunchConfigurationDelegateLauncher
. In order to get this to work, we need to "launch" a configuration on each action. To do this, I refactored our action launch classes (e.g.StartAction
) to lookup the configuration for the project and runDebugUITools.launch(configuration, mode)
instead of starting dev mode directly. As a consequence, all actions create an ILaunch object which then gets routed to ourLaunchConfigurationDelegateLauncher
. The "start dev mode" logic has now been moved over to the delegate. "This change does not change any external behavior and if anything, it keeps our action flow consistent. We now always "launch" a configuration which always routes to the delegate which is the single location for our "start dev mode" logic.
a. LibertySourcePathComputer - This class contains the logic to create the default source lookup path list. It first uses eclipse JDT APIs to get classpath entries for the project. It then uses m2e for maven projects and gradle tooling for gradle projects to get a list of project dependencies and then checks to see if the dependency exists in the workspace using the artifact coordinates.
b. LibertySourceLookupDirector - This class may not be necessary for what we are doing, but the logic is trivial... it creates a "director" that gathers a list of source lookup "participants" which I believe determine what type of source lookups to create. We are only using the basic JavaSourceLookupParticipant which is for Java code.
NOTE: The source lookup only works ATM for dependencies that are Maven projects... in other words we properly get the list of dependencies for both Maven and Gradle projects, BUT only m2e has APIs that will look up projects in the workspace based on artifact coordinates.... and of course, it only looks up maven projects.... So a gradle app with a dependency that is a maven project will work just fine. but if the dependency project is a gradle app, that dependency app will not be added to the source lookup list.