You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by mi...@apache.org on 2022/12/29 20:51:22 UTC
[maven] branch maven-3.9.x updated: [MNG-7651] Simplify and document merge of maven.config file and CLI args
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a commit to branch maven-3.9.x
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/maven-3.9.x by this push:
new 074879ff2 [MNG-7651] Simplify and document merge of maven.config file and CLI args
074879ff2 is described below
commit 074879ff28c1649106304a1004115bf59c3f8aa2
Author: Michael Osipov <mi...@apache.org>
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 | 48 ++++++++++++----------
.../java/org/apache/maven/cli/MavenCliTest.java | 6 +--
2 files changed, 29 insertions(+), 25 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 1a38e6409..e4f320a44 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
@@ -326,35 +326,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]));
- List<?> unrecongized = mavenConfig.getArgList();
- if (!unrecongized.isEmpty()) {
- throw new ParseException("Unrecognized maven.config entries: " + unrecongized);
+ 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()) {
+ // 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());
@@ -379,20 +377,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 9fb867a5b..9b9ce2602 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
@@ -175,7 +175,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);
}
@@ -204,7 +204,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);
}
@@ -239,7 +239,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"));