Skip to content
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

Merged
merged 3 commits into from
Feb 17, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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,
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

String mainClass,
String programArguments,
String vmArguments,
Expand Down Expand Up @@ -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();
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 javaHome + separator + vmExec to construct the command line.

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);
}

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,14 @@ public static String[] constructLaunchCommands(LaunchArguments launchArguments,
if (launchArguments.launcherScript != null) {
launchCmds.add(launchArguments.launcherScript);
}
final String javaHome = StringUtils.isNotEmpty(DebugSettings.getCurrent().javaHome) ? DebugSettings.getCurrent().javaHome
: System.getProperty("java.home");
launchCmds.add(javaHome + slash + "bin" + slash + "java");

if (StringUtils.isNotBlank(launchArguments.javaExec)) {
launchCmds.add(launchArguments.javaExec);
} else {
final String javaHome = StringUtils.isNotEmpty(DebugSettings.getCurrent().javaHome) ? DebugSettings.getCurrent().javaHome
: System.getProperty("java.home");
launchCmds.add(javaHome + slash + "bin" + slash + "java");
testforstephen marked this conversation as resolved.
Show resolved Hide resolved
}
if (StringUtils.isNotEmpty(address)) {
launchCmds.add(String.format("-agentlib:jdwp=transport=dt_socket,server=%s,suspend=y,address=%s", serverMode ? "y" : "n", address));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ public Process launch(LaunchArguments launchArguments, IDebugAdapterContext cont

IDebugSession debugSession = DebugUtility.launch(
vmProvider.getVirtualMachineManager(),
launchArguments.javaExec,
launchArguments.mainClass,
launchArguments.args,
launchArguments.vmArgs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ public static class LaunchArguments extends LaunchBaseArguments {
public CONSOLE console = CONSOLE.integratedTerminal;
public ShortenApproach shortenCommandLine = ShortenApproach.NONE;
public String launcherScript;
public String javaExec;
}

public static class AttachArguments extends LaunchBaseArguments {
Expand Down
1 change: 1 addition & 0 deletions com.microsoft.java.debug.plugin/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
<command id="vscode.java.resolveElementAtSelection"/>
<command id="vscode.java.resolveBuildFiles"/>
<command id="vscode.java.isOnClasspath"/>
<command id="vscode.java.resolveJavaExecutable"/>
</delegateCommandHandler>
</extension>
</plugin>
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public class JavaDebugDelegateCommandHandler implements IDelegateCommandHandler
public static final String RESOLVE_ELEMENT_AT_SELECTION = "vscode.java.resolveElementAtSelection";
public static final String RESOLVE_BUILD_FILES = "vscode.java.resolveBuildFiles";
public static final String IS_ON_CLASSPATH = "vscode.java.isOnClasspath";
public static final String RESOLVE_JAVA_EXECUTABLE = "vscode.java.resolveJavaExecutable";

@Override
public Object executeCommand(String commandId, List<Object> arguments, IProgressMonitor progress) throws Exception {
Expand Down Expand Up @@ -78,6 +79,8 @@ public Object executeCommand(String commandId, List<Object> arguments, IProgress
return getBuildFiles();
case IS_ON_CLASSPATH:
return isOnClasspath(arguments);
case RESOLVE_JAVA_EXECUTABLE:
return ResolveJavaExecutableHandler.resolveJavaExecutable(arguments);
default:
break;
}
Expand Down
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 {
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In launch.json, project info is optional. In the case of undefined, the debugger will try to resolve the project info from the main class. See the else branch.

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;
}
}