This is an automated email from the ASF dual-hosted git repository. michaelo pushed a commit to branch MNG-7651 in repository https://gitbox.apache.org/repos/asf/maven.git
commit 5af08d0052acb4f422e62fbfd0f3ba334fd87583 Author: Michael Osipov <[email protected]> AuthorDate: Mon Dec 26 18:29:11 2022 +0100 [MNG-7651] Simplify and document merge of maven.config file and CLI args This closes #939 --- .../main/java/org/apache/maven/cli/MavenCli.java | 44 ++++++++++++---------- .../java/org/apache/maven/cli/MavenCliTest.java | 6 +-- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java index 509b02ede..b3dacf8e3 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java @@ -339,35 +339,33 @@ public class MavenCli { cliManager = new CLIManager(); - List<String> args = new ArrayList<>(); CommandLine mavenConfig = null; try { File configFile = new File(cliRequest.multiModuleProjectDirectory, MVN_MAVEN_CONFIG); if (configFile.isFile()) { - for (String arg : Files.readAllLines(configFile.toPath(), Charset.defaultCharset())) { - if (!arg.isEmpty()) { - args.add(arg); - } - } - - mavenConfig = cliManager.parse(args.toArray(new String[0])); + String[] args = Files.lines(configFile.toPath(), Charset.defaultCharset()) + .filter(arg -> !arg.isEmpty()) + .toArray(size -> new String[size]); + mavenConfig = cliManager.parse(args); List<?> unrecognized = mavenConfig.getArgList(); if (!unrecognized.isEmpty()) { - throw new ParseException("Unrecognized maven.config entries: " + unrecognized); + // This file can only contain options, not args (goals or phases) + throw new ParseException("Unrecognized maven.config file entries: " + unrecognized); } } } catch (ParseException e) { - System.err.println("Unable to parse maven.config: " + e.getMessage()); + System.err.println("Unable to parse maven.config file options: " + e.getMessage()); cliManager.displayHelp(System.out); throw e; } try { + CommandLine mavenCli = cliManager.parse(cliRequest.args); if (mavenConfig == null) { - cliRequest.commandLine = cliManager.parse(cliRequest.args); + cliRequest.commandLine = mavenCli; } else { - cliRequest.commandLine = cliMerge(cliManager.parse(cliRequest.args), mavenConfig); + cliRequest.commandLine = cliMerge(mavenConfig, mavenCli); } } catch (ParseException e) { System.err.println("Unable to parse command line options: " + e.getMessage()); @@ -392,20 +390,26 @@ public class MavenCli { } } - private CommandLine cliMerge(CommandLine mavenArgs, CommandLine mavenConfig) { + private CommandLine cliMerge(CommandLine mavenConfig, CommandLine mavenCli) { CommandLine.Builder commandLineBuilder = new CommandLine.Builder(); - // the args are easy, cli first then config file - for (String arg : mavenArgs.getArgs()) { - commandLineBuilder.addArg(arg); - } - for (String arg : mavenConfig.getArgs()) { + // the args are easy, CLI only since maven.config file can only contain options + for (String arg : mavenCli.getArgs()) { commandLineBuilder.addArg(arg); } - // now add all options, except for -D with cli first then config file + /* Although this looks wrong in terms of order Commons CLI stores the value of options in + * an array and when a value is potentionally overriden it is added to the array. The single + * arg option value is retrieved and instead of returning values[values.length-1] it returns + * values[0] which means that the original value instead of the overridden one is returned + * (first wins). With properties values are truely overriden since at the end a map is used + * to merge which means last wins. + * + * TODO Report this behavioral bug with Commons CLI + */ + // now add all options, except for user properties with CLI first then maven.config file List<Option> setPropertyOptions = new ArrayList<>(); - for (Option opt : mavenArgs.getOptions()) { + for (Option opt : mavenCli.getOptions()) { if (String.valueOf(CLIManager.SET_USER_PROPERTY).equals(opt.getOpt())) { setPropertyOptions.add(opt); } else { diff --git a/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java b/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java index 1f1da2e7c..54e937b31 100644 --- a/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java +++ b/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java @@ -244,7 +244,7 @@ public class MavenCliTest { cli.cli(request); cli.properties(request); - String revision = System.getProperty("revision"); + String revision = request.getUserProperties().getProperty("revision"); assertEquals("8.1.0", revision); } @@ -273,7 +273,7 @@ public class MavenCliTest { cli.cli(request); cli.properties(request); - String revision = System.getProperty("revision"); + String revision = request.getUserProperties().getProperty("revision"); assertEquals("8.2.0", revision); } @@ -308,7 +308,7 @@ public class MavenCliTest { assertEquals("3", request.commandLine.getOptionValue(CLIManager.THREADS)); - String revision = System.getProperty("revision"); + String revision = request.getUserProperties().getProperty("revision"); assertEquals("8.2.0", revision); assertEquals("bar ", request.getUserProperties().getProperty("foo"));
