Skip to content

Commit

Permalink
Issue #5681 - Improved warning on unrecognized Command Line Options
Browse files Browse the repository at this point in the history 
Signed-off-by: Joakim Erdfelt <[email protected]>
  • Loading branch information
@joakime
joakime committed Mar 24, 2022
1 parent 132df88 commit e9f32b3
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 24 deletions.
6 changes: 3 additions & 3 deletions jetty-start/src/main/java/org/eclipse/jetty/start/Main.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,11 +501,11 @@ public void run()

if (args.hasJvmArgs() || args.hasSystemProperties())
{
StartLog.warn("System properties and/or JVM args set. Consider using --dry-run or --exec");
StartLog.warn("Unknown Arguments detected. Consider using --dry-run or --exec");
if (args.hasSystemProperties())
args.getSystemProperties().forEach((k, v) -> StartLog.warn(" Detected JVM System Property: %s=%s", k, System.getProperty(k)));
args.getSystemProperties().forEach((k, v) -> StartLog.warn(" Argument: -D%s=%s (interpreted as a System property, from %s)", k, System.getProperty(k), v));
if (args.hasJvmArgs())
args.getJvmArgs().forEach((jvmArg) -> StartLog.warn(" Detected JVM Arg: %s", jvmArg));
args.getJvmArgSources().forEach((jvmArg, source) -> StartLog.warn(" Argument: %s (interpreted as a JVM argument, from %s)", jvmArg, source));
}

ClassLoader cl = classpath.getClassLoader();
Expand Down
54 changes: 37 additions & 17 deletions jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public class StartArgs
/**
* JVM arguments, found via command line and in all active [exec] sections from enabled modules
*/
private final List<String> jvmArgs = new ArrayList<>();
private final Map<String, String> jvmArgSources = new LinkedHashMap<>();

/**
* List of all xml references found directly on command line or start.ini
Expand Down Expand Up @@ -331,21 +331,23 @@ public void dumpEnvironment(PrintStream out)

public void dumpJvmArgs(PrintStream out)
{
if (jvmArgs.isEmpty())
if (jvmArgSources.isEmpty())
return;

out.println();
out.println("Forked JVM Arguments:");
out.println("---------------------");

for (String jvmArgKey : jvmArgs)
{
String value = System.getProperty(jvmArgKey);
if (value != null)
out.printf(" %s = %s%n", jvmArgKey, value);
else
out.printf(" %s%n", jvmArgKey);
}
jvmArgSources.forEach((key, sourceRef) ->
{
String value = System.getProperty(key);
String source = StartLog.isDebugEnabled() ? '(' + sourceRef + ')' : "";
if (value != null)
out.printf(" %s = %s %s%n", key, value, source);
else
out.printf(" %s %s%n", key, source);
}
);
}

public void dumpProperties(PrintStream out)
Expand Down Expand Up @@ -544,7 +546,7 @@ public void expandModules(List<Module> activeModules) throws IOException
for (String jvmArg : module.getJvmArgs())
{
exec = true;
jvmArgs.add(jvmArg);
jvmArgSources.put(jvmArg, String.format("module[%s|jvm]", module.getName()));
}

// Find and Expand XML files
Expand Down Expand Up @@ -655,9 +657,25 @@ public List<FileArg> getFiles()
return files;
}

/**
* Gets the List of JVM arguments detected.
*
* @deprecated use {@link #getJvmArgSources()} instead, as it will return source references with each arg.
*/
@Deprecated
public List<String> getJvmArgs()
{
return jvmArgs;
return new ArrayList<>(jvmArgSources.keySet());
}

/**
* Return ordered Map of JVM arguments to Source (locations)
*
* @return the ordered map of JVM Argument to Source (locations)
*/
public Map<String, String> getJvmArgSources()
{
return jvmArgSources;
}

public CommandLineBuilder getMainArgs(Set<String> parts) throws IOException
Expand All @@ -681,7 +699,7 @@ public CommandLineBuilder getMainArgs(Set<String> parts) throws IOException
cmd.addRawArg("-Djetty.home=" + baseHome.getHome());
cmd.addRawArg("-Djetty.base=" + baseHome.getBase());

for (String x : getJvmArgs())
for (String x : getJvmArgSources().keySet())
{
if (x.startsWith("-D"))
{
Expand Down Expand Up @@ -903,7 +921,7 @@ public List<Path> getXmlFiles()

public boolean hasJvmArgs()
{
return !jvmArgs.isEmpty();
return !jvmArgSources.isEmpty();
}

public boolean hasSystemProperties()
Expand Down Expand Up @@ -1339,10 +1357,12 @@ public void parse(final String rawarg, String source)
// Anything else with a "-" is considered a JVM argument
if (arg.startsWith("-"))
{
StartLog.debug("Unrecognized Arg: %s (from %s)", arg, source);

// Only add non-duplicates
if (!jvmArgs.contains(arg))
if (!jvmArgSources.containsKey(arg))
{
jvmArgs.add(arg);
jvmArgSources.put(arg, source);
}
return;
}
Expand Down Expand Up @@ -1563,6 +1583,6 @@ public void setRun(boolean run)
public String toString()
{
return String.format("%s[enabledModules=%s, xmlRefs=%s, properties=%s, jvmArgs=%s]",
getClass().getSimpleName(), modules, xmlRefs, properties, jvmArgs);
getClass().getSimpleName(), modules, xmlRefs, properties, jvmArgSources.keySet());
}
}
15 changes: 12 additions & 3 deletions jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.io.File;
import java.io.PrintStream;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Iterator;
Expand Down Expand Up @@ -98,7 +99,13 @@ public void testUnknownDistroCommand() throws Exception
{
List<String> cmdLineArgs = new ArrayList<>();
File testJettyHome = MavenTestingUtils.getTestResourceDir("dist-home");
Path testJettyBase = MavenTestingUtils.getTargetTestingPath("base-example-unknown");
FS.ensureDirectoryExists(testJettyBase);
Path zedIni = testJettyBase.resolve("start.d/zed.ini");
FS.ensureDirectoryExists(zedIni.getParent());
Files.writeString(zedIni, "--zed-0-zed");
cmdLineArgs.add("jetty.home=" + testJettyHome);
cmdLineArgs.add("jetty.base=" + testJettyBase);
cmdLineArgs.add("main.class=" + PropertyDump.class.getName());
cmdLineArgs.add("--module=base");
cmdLineArgs.add("--foople");
Expand Down Expand Up @@ -126,11 +133,13 @@ public void testUnknownDistroCommand() throws Exception

// Test a System Property that comes from JVM
List<String> warnings = output.stream().filter((line) -> line.startsWith("WARN")).collect(Collectors.toList());
// warnings.forEach(System.out::println);
Iterator<String> warningIter = warnings.iterator();

assertThat("Announcement", warningIter.next(), containsString("System properties and/or JVM args set."));
assertThat("System Prop Detail", warningIter.next(), containsString("Detected JVM System Property: zed.key=0.value"));
assertThat("JVM Arg Detail", warningIter.next(), containsString("Detected JVM Arg: --foople"));
assertThat("Announcement", warningIter.next(), containsString("Unknown Arguments detected."));
assertThat("System Prop on command line detail", warningIter.next(), containsString("Argument: -Dzed.key=0.value (interpreted as a System property, from <command-line>"));
assertThat("JVM Arg in ini detail", warningIter.next(), containsString("Argument: --zed-0-zed (interpreted as a JVM argument, from " + zedIni));
assertThat("JVM Arg on command line detail", warningIter.next(), containsString("Argument: --foople (interpreted as a JVM argument, from <command-line>"));
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion jetty-start/src/test/java/org/eclipse/jetty/start/usecases/BasicTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ public void testWithCommandLine() throws Exception
"-Xms1g",
"-Xmx1g"
);
List<String> actualJvmArgs = results.startArgs.getJvmArgs();
List<String> actualJvmArgs = new ArrayList<>(results.startArgs.getJvmArgSources().keySet());
assertThat("JVM Args", actualJvmArgs, contains(expectedJvmArgs.toArray()));
}

Expand Down

0 comments on commit e9f32b3

Please sign in to comment.