feat: --debug now allow multiple key/values. --debug=server=n,suspend…

…=y (#1642)
jbangdev · Jul 8, 2023 · d87feaa · d87feaa
1 parent d81bed7
commit d87feaa
Show file tree

Hide file tree

Showing 11 changed files with 89 additions and 41 deletions.
diff --git a/docs/modules/ROOT/pages/debugging.adoc b/docs/modules/ROOT/pages/debugging.adoc
@@ -27,6 +27,11 @@ This will make it use port 4321 and make it listen on all ('*') network interfac
 
 NOTE: Be sure to put a breakpoint in your IDE/debugger before you connect to make the debugger actually stop when you need it.
 
+Additionally you can pass in a additional key/value options to the debug string, e.g., `--debug=server=n,suspend=y`. To have the debugger
+connect to the IDE and suspend the execution when it connects as opposed to having the IDE connect.
+
+You can see the key/value options supported in OpenJDK JPDA https://docs.oracle.com/en/java/javase/11/docs/specs/jpda/conninv.html[documentation].
+
 == Debug info
 
 JBang since version 0.100 compiles with debug info enabled by default. You can use `-C=-g` to tweak how much debug info is included: `jbang -C=-g=lines,vars,source` or to turn it off use `jbang -C=-g=none".

diff --git a/src/main/java/dev/jbang/catalog/Alias.java b/src/main/java/dev/jbang/catalog/Alias.java
@@ -40,7 +40,7 @@ public class Alias extends CatalogItem {
 	@SerializedName(value = "native-options")
 	public final List<String> nativeOptions;
 	public final String jfr;
-	public final String debug;
+	public final Map<String, String> debug;
 	public final Boolean cds;
 	public final Boolean interactive;
 	@SerializedName(value = "enable-preview")
@@ -92,7 +92,7 @@ public Alias(String scriptRef,
 			Boolean nativeImage,
 			List<String> nativeOptions,
 			String jfr,
-			String debug,
+			Map<String, String> debug,
 			Boolean cds,
 			Boolean interactive,
 			Boolean enablePreview,
@@ -211,7 +211,7 @@ private static Alias merge(Alias a1, String name, Function<String, Alias> findUn
 					: a2.nativeOptions;
 			Boolean nimg = a1.nativeImage != null ? a1.nativeImage : a2.nativeImage;
 			String jfr = a1.jfr != null ? a1.jfr : a2.jfr;
-			String debug = a1.debug != null ? a1.debug : a2.debug;
+			Map<String, String> debug = a1.debug != null ? a1.debug : a2.debug;
 			Boolean cds = a1.cds != null ? a1.cds : a2.cds;
 			Boolean inter = a1.interactive != null ? a1.interactive : a2.interactive;
 			Boolean ep = a1.enablePreview != null ? a1.enablePreview : a2.enablePreview;

diff --git a/src/main/java/dev/jbang/cli/KeyValueConsumer.java b/src/main/java/dev/jbang/cli/KeyValueConsumer.java
@@ -18,7 +18,6 @@ public void consumeParameters(Stack<String> args, CommandLine.Model.ArgSpec argS
 		String arg = args.pop();
 		Matcher m = p.matcher(arg);
 		if (m.matches()) {
-
 			Map<String, String> kv = argSpec.getValue();
 
 			if (kv == null) {

diff --git a/src/main/java/dev/jbang/cli/Run.java b/src/main/java/dev/jbang/cli/Run.java
@@ -1,13 +1,7 @@
 package dev.jbang.cli;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.Stack;
+import java.util.*;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -170,12 +164,37 @@ private static Map<String, String> handleRemoteFiles(Map<String, String> slots)
 	 * Helper class to peek ahead at `--debug` to pickup --debug=5000, --debug 5000,
 	 * --debug *:5000 as debug parameters but not --debug somefile.java
 	 */
-	static class DebugFallbackConsumer extends PatternFallbackConsumer {
-		private static final Pattern p = Pattern.compile("(.*?:)?(\\d+)");
+	static class DebugFallbackConsumer implements CommandLine.IParameterConsumer {
+
+		private static Pattern p = Pattern.compile("(?<address>(.*?:)?(\\d+))|(?<key>\\S*)=(?<value>\\S+)");
 
 		@Override
-		protected Pattern getValuePattern() {
-			return p;
+		public void consumeParameters(Stack<String> args, CommandLine.Model.ArgSpec argSpec,
+				CommandLine.Model.CommandSpec commandSpec) {
+			String arg = args.peek();
+			Matcher m = p.matcher(arg);
+
+			if (!m.matches()) {
+				m = p.matcher(((CommandLine.Model.OptionSpec) argSpec).fallbackValue());
+			} else {
+				args.pop();
+			}
+
+			if (m.matches()) {
+				Map<String, String> kv = argSpec.getValue();
+
+				if (kv == null) {
+					kv = new LinkedHashMap<>();
+				}
+
+				String address = m.group("address");
+				if (address != null) {
+					kv.put("address", address);
+				} else {
+					kv.put(m.group("key"), m.group("value"));
+				}
+				argSpec.setValue(kv);
+			}
 		}
 	}
 
@@ -190,6 +209,7 @@ static class KeyValueFallbackConsumer extends PatternFallbackConsumer {
 		protected Pattern getValuePattern() {
 			return p;
 		}
+
 	}
 
 	static abstract class PatternFallbackConsumer implements CommandLine.IParameterConsumer {

diff --git a/src/main/java/dev/jbang/cli/RunMixin.java b/src/main/java/dev/jbang/cli/RunMixin.java
@@ -16,8 +16,8 @@ public class RunMixin {
 	public String flightRecorderString;
 
 	@CommandLine.Option(names = { "-d",
-			"--debug" }, fallbackValue = "${default.run.debug}", parameterConsumer = Run.DebugFallbackConsumer.class, arity = "0..1", description = "Launch with java debug enabled on specified port (default: ${FALLBACK-VALUE}) ")
-	public String debugString;
+			"--debug" }, fallbackValue = "${default.run.debug}", parameterConsumer = Run.DebugFallbackConsumer.class, arity = "0..1", description = "Launch with java debug enabled. Set host/port or provide key/value list of JPDA options (default: ${FALLBACK-VALUE}) ")
+	public Map<String, String> debugString;
 
 	// should take arguments for package/classes when picocli fixes its flag
 	// handling bug in release 4.6.

diff --git a/src/main/java/dev/jbang/source/CmdGeneratorBuilder.java b/src/main/java/dev/jbang/source/CmdGeneratorBuilder.java
@@ -1,9 +1,6 @@
 package dev.jbang.source;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-import java.util.Optional;
+import java.util.*;
 import java.util.stream.Collectors;
 
 import dev.jbang.catalog.Alias;
@@ -26,7 +23,7 @@ public class CmdGeneratorBuilder {
 	private Boolean enableAssertions;
 	private Boolean enableSystemAssertions;
 	private String flightRecorderString;
-	private String debugString;
+	private Map<String, String> debugString;
 	private Boolean classDataSharing;
 
 	CmdGeneratorBuilder(Project project, BuildContext ctx) {
@@ -87,7 +84,7 @@ public CmdGeneratorBuilder flightRecorderString(String flightRecorderString) {
 		return this;
 	}
 
-	public CmdGeneratorBuilder debugString(String debugString) {
+	public CmdGeneratorBuilder debugString(Map<String, String> debugString) {
 		this.debugString = debugString;
 		return this;
 	}

diff --git a/src/main/java/dev/jbang/source/generators/BaseCmdGenerator.java b/src/main/java/dev/jbang/source/generators/BaseCmdGenerator.java
@@ -12,7 +12,7 @@ public abstract class BaseCmdGenerator<T extends CmdGenerator> implements CmdGen
 	protected final BuildContext ctx;
 
 	protected List<String> arguments = Collections.emptyList();
-	protected String debugString;
+	protected Map<String, String> debugString;
 	protected String flightRecorderString;
 
 	protected Util.Shell shell = Util.getShell();
@@ -33,7 +33,7 @@ public T shell(Util.Shell shell) {
 	}
 
 	@SuppressWarnings("unchecked")
-	public T debugString(String debugString) {
+	public T debugString(Map<String, String> debugString) {
 		this.debugString = debugString != null && !debugString.isEmpty() ? debugString : null;
 		return (T) this;
 	}

diff --git a/src/main/java/dev/jbang/source/generators/JarCmdGenerator.java b/src/main/java/dev/jbang/source/generators/JarCmdGenerator.java
@@ -3,12 +3,8 @@
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
+import java.util.*;
+import java.util.stream.Collectors;
 
 import dev.jbang.Settings;
 import dev.jbang.cli.BaseCommand;
@@ -88,8 +84,20 @@ protected List<String> generateCommandLineList() throws IOException {
 		addPropertyFlags(project.getProperties(), "-D", optionalArgs);
 
 		if (debugString != null) {
+			Map<String, String> fallbackDebug = new LinkedHashMap<>();
+			fallbackDebug.put("transport", "dt_socket");
+			fallbackDebug.put("server", "y");
+			fallbackDebug.put("suspend", "y");
+			fallbackDebug.putAll(debugString);
+			// needed even though there is a fallbackvalue as user might have set some other
+			// key/value
+			// i.e. --debug=server=n
+			fallbackDebug.put("address", "4004");
 			optionalArgs.add(
-					"-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=" + debugString);
+					"-agentlib:jdwp=" + fallbackDebug	.entrySet()
+														.stream()
+														.map(e -> e.getKey() + "=" + e.getValue())
+														.collect(Collectors.joining(",")));
 		}
 
 		if (assertions) {

diff --git a/src/test/java/dev/jbang/TestConfiguration.java b/src/test/java/dev/jbang/TestConfiguration.java
@@ -162,15 +162,15 @@ public void testCommandEditOpenFallbackEnv() {
 	public void testCommandFallbacks() {
 		CommandLine.ParseResult pr = JBang.getCommandLine().parseArgs("run", "dummy");
 		Run run = (Run) pr.subcommand().commandSpec().userObject();
-		assertThat(run.runMixin.debugString, emptyOrNullString());
+		assertThat(run.runMixin.debugString, is(nullValue()));
 		assertThat(run.runMixin.flightRecorderString, emptyOrNullString());
 	}
 
 	@Test
 	public void testCommandFallbacks2() {
 		CommandLine.ParseResult pr = JBang.getCommandLine().parseArgs("run", "--jfr", "--debug", "dummy");
 		Run run = (Run) pr.subcommand().commandSpec().userObject();
-		assertThat(run.runMixin.debugString, equalTo("4004"));
+		assertThat(run.runMixin.debugString, allOf(hasEntry("address", "4004"), is(aMapWithSize(1))));
 		assertThat(run.runMixin.flightRecorderString, equalTo("dummyjfr.cfg"));
 	}
 

diff --git a/src/test/java/dev/jbang/cli/TestArguments.java b/src/test/java/dev/jbang/cli/TestArguments.java
@@ -1,8 +1,7 @@
 package dev.jbang.cli;
 
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.*;
 
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -37,7 +36,7 @@ public void testBasicArguments() {
 		Run run = (Run) pr.subcommand().commandSpec().userObject();
 
 		assert run.helpRequested;
-		assertThat(run.runMixin.debugString, notNullValue());
+		assertThat(run.runMixin.debugString, hasEntry("address", "4004"));
 		assertThat(run.scriptMixin.scriptOrFile, is("myfile.java"));
 		assertThat(run.userParams.size(), is(0));
 
@@ -48,7 +47,7 @@ public void testDoubleDebug() {
 		CommandLine.ParseResult pr = cli.parseArgs("run", "--debug", "test.java", "--debug", "wonka");
 		Run run = (Run) pr.subcommand().commandSpec().userObject();
 
-		assertThat(run.runMixin.debugString, is("4004"));
+		assertThat(run.runMixin.debugString, hasEntry("address", "4004"));
 
 		assertThat(run.scriptMixin.scriptOrFile, is("test.java"));
 		assertThat(run.userParams, is(Arrays.asList("--debug", "wonka")));
@@ -96,7 +95,8 @@ public void testDebugPort() {
 
 		assertThat(run.scriptMixin.scriptOrFile, is("test.java"));
 		assertThat(run.runMixin.debugString, notNullValue());
-		assertThat(run.runMixin.debugString, is("*:5000"));
+		assertThat(run.runMixin.debugString, hasEntry("address", "*:5000"));
+		assertThat(run.runMixin.debugString.size(), is(1));
 	}
 
 	@Test
@@ -106,7 +106,7 @@ public void testDebugPortSeperateValue() {
 
 		assertThat(run.scriptMixin.scriptOrFile, is("test.java"));
 		assertThat(run.runMixin.debugString, notNullValue());
-		assertThat(run.runMixin.debugString, is("xyz.dk:5005"));
+		assertThat(run.runMixin.debugString, hasEntry("address", "xyz.dk:5005"));
 	}
 
 	@Test

diff --git a/src/test/java/dev/jbang/cli/TestRun.java b/src/test/java/dev/jbang/cli/TestRun.java
@@ -614,7 +614,6 @@ void testWithSourcesShell() throws IOException {
 
 	@Test
 	void testDebug() throws IOException {
-
 		environmentVariables.clear("JAVA_HOME");
 		String arg = examplesTestFolder.resolve("helloworld.java").toAbsolutePath().toString();
 		CommandLine.ParseResult pr = JBang.getCommandLine().parseArgs("run", "--debug", arg);
@@ -634,6 +633,26 @@ void testDebug() throws IOException {
 		assertThat(result, not(containsString("  ")));
 	}
 
+	@Test
+	void testDebugHost() throws IOException {
+		environmentVariables.clear("JAVA_HOME");
+		String arg = examplesTestFolder.resolve("helloworld.java").toAbsolutePath().toString();
+		CommandLine.ParseResult pr = JBang.getCommandLine().parseArgs("run", "--debug=server=n", arg);
+		Run run = (Run) pr.subcommand().commandSpec().userObject();
+
+		ProjectBuilder pb = run.createProjectBuilderForRun();
+		pb.mainClass("fakemain");
+		Project prj = pb.build(arg);
+
+		String result = run.updateGeneratorForRun(CmdGenerator.builder(prj)).build().generate();
+
+		assertThat(result, matchesPattern("^.*java(.exe)? .*$"));
+		assertThat(result, containsString("helloworld.java"));
+		assertThat(result, containsString("classpath"));
+		assertThat(result, containsString("-agentlib:jdwp=transport=dt_socket,server=n,suspend=y,address=4004"));
+		assertThat(result, not(containsString("  ")));
+	}
+
 	@Test
 	void testDependencies() throws IOException {