-
Notifications
You must be signed in to change notification settings - Fork 159
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 project's java runtime to launch the application #319
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ | |
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
import java.util.concurrent.CompletableFuture; | ||
|
||
import org.apache.commons.lang3.StringUtils; | ||
|
@@ -78,11 +79,79 @@ public static IDebugSession launch(VirtualMachineManager vmManager, | |
envVars); | ||
} | ||
|
||
/** | ||
* Launch a debuggee in suspend mode. | ||
* @see #launch(VirtualMachineManager, String, String, String, String, String, String, String, String[]) | ||
*/ | ||
public static IDebugSession launch(VirtualMachineManager vmManager, | ||
String javaExec, | ||
String mainClass, | ||
String programArguments, | ||
String vmArguments, | ||
List<String> modulePaths, | ||
List<String> classPaths, | ||
String cwd, | ||
String[] envVars) | ||
throws IOException, IllegalConnectorArgumentsException, VMStartException { | ||
return DebugUtility.launch(vmManager, | ||
javaExec, | ||
mainClass, | ||
programArguments, | ||
vmArguments, | ||
String.join(File.pathSeparator, modulePaths), | ||
String.join(File.pathSeparator, classPaths), | ||
cwd, | ||
envVars); | ||
} | ||
|
||
/** | ||
* Launches a debuggee in suspend mode. | ||
* | ||
* @param vmManager | ||
* the virtual machine manager. | ||
* @param mainClass | ||
* the main class. | ||
* @param programArguments | ||
* the program arguments. | ||
* @param vmArguments | ||
* the vm arguments. | ||
* @param modulePaths | ||
* the module paths. | ||
* @param classPaths | ||
* the class paths. | ||
* @param cwd | ||
* the working directory of the program. | ||
* @param envVars | ||
* array of strings, each element of which has environment variable settings in the format name=value. | ||
* or null if the subprocess should inherit the environment of the current process. | ||
* @return an instance of IDebugSession. | ||
* @throws IOException | ||
* when unable to launch. | ||
* @throws IllegalConnectorArgumentsException | ||
* when one of the arguments is invalid. | ||
* @throws VMStartException | ||
* when the debuggee was successfully launched, but terminated | ||
* with an error before a connection could be established. | ||
*/ | ||
public static IDebugSession launch(VirtualMachineManager vmManager, | ||
String mainClass, | ||
String programArguments, | ||
String vmArguments, | ||
String modulePaths, | ||
String classPaths, | ||
String cwd, | ||
String[] envVars) | ||
throws IOException, IllegalConnectorArgumentsException, VMStartException { | ||
return launch(vmManager, null, mainClass, programArguments, vmArguments, modulePaths, classPaths, cwd, envVars); | ||
} | ||
|
||
/** | ||
* Launches a debuggee in suspend mode. | ||
* | ||
* @param vmManager | ||
* the virtual machine manager. | ||
* @param javaExec | ||
* the java executable path. If not defined, then resolve from java home. | ||
* @param mainClass | ||
* the main class. | ||
* @param programArguments | ||
|
@@ -108,6 +177,7 @@ public static IDebugSession launch(VirtualMachineManager vmManager, | |
* with an error before a connection could be established. | ||
*/ | ||
public static IDebugSession launch(VirtualMachineManager vmManager, | ||
String javaExec, | ||
String mainClass, | ||
String programArguments, | ||
String vmArguments, | ||
|
@@ -164,7 +234,12 @@ public static IDebugSession launch(VirtualMachineManager vmManager, | |
arguments.get(ENV).setValue(encodeArrayArgument(envVars)); | ||
} | ||
|
||
if (StringUtils.isNotEmpty(DebugSettings.getCurrent().javaHome)) { | ||
if (isValidJavaExec(javaExec)) { | ||
String vmExec = new File(javaExec).getName(); | ||
String javaHome = new File(javaExec).getParentFile().getParentFile().getAbsolutePath(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks tricky. Do we really need to pass this value to launch the JVM since we already have the path to the executable? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is in order to keep compatible with JDI Socket LaunchingConnectorImpl https://github.com/eclipse/eclipse.jdt.debug/blob/master/org.eclipse.jdt.debug/jdi/org/eclipse/jdi/internal/connect/SocketLaunchingConnectorImpl.java#L188, where it uses Currently when launching in debug console, we're still using SocketLaunchingConnector to construct the command line, so need resolve the javaHome and vmExec from the path and pass them as the connector arguments. Another option is to use ListeningConnector instead, like what launchInTerminal did, the debugger can leverage the javaExec path directly. Let's track the refactor improvement in the issue microsoft/vscode-java-debug#760 |
||
arguments.get(HOME).setValue(javaHome); | ||
arguments.get(EXEC).setValue(vmExec); | ||
} else if (StringUtils.isNotEmpty(DebugSettings.getCurrent().javaHome)) { | ||
arguments.get(HOME).setValue(DebugSettings.getCurrent().javaHome); | ||
} | ||
|
||
|
@@ -179,6 +254,19 @@ public static IDebugSession launch(VirtualMachineManager vmManager, | |
return new DebugSession(vm); | ||
} | ||
|
||
private static boolean isValidJavaExec(String javaExec) { | ||
if (StringUtils.isBlank(javaExec)) { | ||
return false; | ||
} | ||
|
||
File file = new File(javaExec); | ||
if (!file.exists() || !file.isFile()) { | ||
return false; | ||
} | ||
|
||
return Objects.equals(file.getParentFile().getName(), "bin"); | ||
testforstephen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/** | ||
* Attach to an existing debuggee VM. | ||
* @param vmManager | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
/******************************************************************************* | ||
* Copyright (c) 2020 Microsoft Corporation and others. | ||
* All rights reserved. This program and the accompanying materials | ||
* are made available under the terms of the Eclipse Public License v1.0 | ||
* which accompanies this distribution, and is available at | ||
* http://www.eclipse.org/legal/epl-v10.html | ||
* | ||
* Contributors: | ||
* Microsoft Corporation - initial API and implementation | ||
*******************************************************************************/ | ||
|
||
package com.microsoft.java.debug.plugin.internal; | ||
|
||
import java.io.File; | ||
import java.util.List; | ||
import java.util.Objects; | ||
import java.util.logging.Level; | ||
import java.util.logging.Logger; | ||
|
||
import org.apache.commons.lang3.StringUtils; | ||
import org.eclipse.core.runtime.CoreException; | ||
import org.eclipse.jdt.core.IJavaProject; | ||
import org.eclipse.jdt.launching.IVMInstall; | ||
import org.eclipse.jdt.launching.JavaRuntime; | ||
|
||
import com.microsoft.java.debug.core.Configuration; | ||
|
||
public class ResolveJavaExecutableHandler { | ||
private static final Logger logger = Logger.getLogger(Configuration.LOGGER_NAME); | ||
private static final String[] candidateJavaExecs = { | ||
testforstephen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"java", | ||
"java.exe", | ||
"javaw", | ||
"javaw.exe", | ||
"j9", | ||
"j9.exe", | ||
"j9w", | ||
"j9w.exe" | ||
}; | ||
private static final String[] candidateJavaBins = { | ||
testforstephen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
File.separator, | ||
"bin" + File.separatorChar, | ||
"jre" + File.separatorChar + "bin" + File.separatorChar | ||
}; | ||
|
||
/** | ||
* Resolve the java executable path from the project's java runtime. | ||
*/ | ||
public static String resolveJavaExecutable(List<Object> arguments) throws Exception { | ||
try { | ||
String mainClass = (String) arguments.get(0); | ||
String projectName = (String) arguments.get(1); | ||
IJavaProject targetProject = null; | ||
if (StringUtils.isNotBlank(projectName)) { | ||
targetProject = JdtUtils.getJavaProject(projectName); | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a project is not specified, should we just use whatever JVM that the debugger is using to launch the app? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In launch.json, |
||
List<IJavaProject> targetProjects = ResolveClasspathsHandler.getJavaProjectFromType(mainClass); | ||
if (!targetProjects.isEmpty()) { | ||
targetProject = targetProjects.get(0); | ||
} | ||
} | ||
|
||
if (targetProject == null) { | ||
return null; | ||
} | ||
|
||
IVMInstall vmInstall = JavaRuntime.getVMInstall(targetProject); | ||
if (vmInstall == null || vmInstall.getInstallLocation() == null) { | ||
return null; | ||
} | ||
|
||
File exe = findJavaExecutable(vmInstall.getInstallLocation()); | ||
if (exe == null) { | ||
return null; | ||
} | ||
|
||
return exe.getAbsolutePath(); | ||
} catch (CoreException e) { | ||
logger.log(Level.SEVERE, "Failed to resolve java executable: " + e.getMessage(), e); | ||
} | ||
|
||
return null; | ||
} | ||
|
||
private static File findJavaExecutable(File vmInstallLocation) { | ||
boolean isBin = Objects.equals("bin", vmInstallLocation.getName()); | ||
for (int i = 0; i < candidateJavaExecs.length; i++) { | ||
for (int j = 0; j < candidateJavaBins.length; j++) { | ||
if (!isBin && j == 0) { | ||
continue; | ||
} | ||
File javaFile = new File(vmInstallLocation, candidateJavaBins[j] + candidateJavaExecs[i]); | ||
testforstephen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (javaFile.isFile()) { | ||
return javaFile; | ||
} | ||
} | ||
} | ||
|
||
return null; | ||
} | ||
} |
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.
When adding a parameter to the signature, it's suggested that the new param is added to the end of the list. Other languages do that because they support default parameters. While Java does not have that, it's still recommended to keep the convention.
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.
Nice catch!