From 19ba3af30a725a9f3edcec3178924d48d2071d3b Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 26 Feb 2025 11:33:06 +0100 Subject: [PATCH 1/4] [MNG-8594] Add atFile option Where user can create ad-hoc command line parms. The difference between .mvn/maven.conf and this file is that this file allws goals as well. The CLI and atFile are merged, in this order, hence, goals in atFile will come AFTER goals specified on CLI. --- https://issues.apache.org/jira/browse/MNG-8594 --- .../maven/api/cli/mvn/MavenOptions.java | 5 ++++ .../invoker/mvn/CommonsCliMavenOptions.java | 14 +++++++++++ .../invoker/mvn/LayeredMavenOptions.java | 5 ++++ .../maven/cling/invoker/mvn/MavenParser.java | 25 ++++++++++++++++++- 4 files changed, 48 insertions(+), 1 deletion(-) diff --git a/api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java b/api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java index 21f241750e26..e290114dec6c 100644 --- a/api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java +++ b/api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java @@ -207,6 +207,11 @@ public interface MavenOptions extends Options { @Nonnull Optional ignoreTransitiveRepositories(); + /** + * Allows {@code @filename} to read options from file. + */ + Optional atFile(); + /** * Returns the list of goals and phases to execute. * diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java index 0ba2e6e0ab5a..51f2fe285366 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java @@ -238,6 +238,14 @@ public Optional ignoreTransitiveRepositories() { return Optional.empty(); } + @Override + public Optional atFile() { + if (commandLine.hasOption(CLIManager.AT_FILE)) { + return Optional.of(commandLine.getOptionValue(CLIManager.AT_FILE)); + } + return Optional.empty(); + } + @Override public Optional> goals() { if (!commandLine.getArgList().isEmpty()) { @@ -273,6 +281,7 @@ protected static class CLIManager extends CommonsCliOptions.CLIManager { public static final String CACHE_ARTIFACT_NOT_FOUND = "canf"; public static final String STRICT_ARTIFACT_DESCRIPTOR_POLICY = "sadp"; public static final String IGNORE_TRANSITIVE_REPOSITORIES = "itr"; + public static final String AT_FILE = "af"; @Override protected void prepareOptions(org.apache.commons.cli.Options options) { @@ -375,6 +384,11 @@ protected void prepareOptions(org.apache.commons.cli.Options options) { .longOpt("ignore-transitive-repositories") .desc("If set, Maven will ignore remote repositories introduced by transitive dependencies.") .build()); + options.addOption(Option.builder(AT_FILE) + .longOpt("at-file") + .hasArg() + .desc("If set, Maven load and merge command line options from the file as well.") + .build()); } } } diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/LayeredMavenOptions.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/LayeredMavenOptions.java index 353f795913a4..d88e08b04721 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/LayeredMavenOptions.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/LayeredMavenOptions.java @@ -154,6 +154,11 @@ public Optional ignoreTransitiveRepositories() { return returnFirstPresentOrEmpty(MavenOptions::ignoreTransitiveRepositories); } + @Override + public Optional atFile() { + return returnFirstPresentOrEmpty(MavenOptions::atFile); + } + @Override public Optional> goals() { return collectListIfPresentOrEmpty(MavenOptions::goals); diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java index 7de8e0d51889..9e55bb1e7d5a 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java @@ -37,7 +37,17 @@ public class MavenParser extends BaseParser { protected List parseCliOptions(LocalContext context) { ArrayList result = new ArrayList<>(); // CLI args - result.add(parseMavenCliOptions(context.parserRequest.args())); + MavenOptions cliOptions = parseMavenCliOptions(context.parserRequest.args()); + result.add(cliOptions); + // atFile option + if (cliOptions.atFile().isPresent()) { + Path file = context.cwd.resolve(cliOptions.atFile().orElseThrow()); + if (Files.isRegularFile(file)) { + result.add(parseMavenAtFileOptions(file)); + } else { + throw new IllegalArgumentException("Specified atFile does not exists (" + file + ")"); + } + } // maven.config; if exists Path mavenConfig = context.rootDirectory != null ? context.rootDirectory.resolve(".mvn/maven.config") : null; if (mavenConfig != null && Files.isRegularFile(mavenConfig)) { @@ -54,6 +64,19 @@ protected MavenOptions parseMavenCliOptions(List args) { } } + protected MavenOptions parseMavenAtFileOptions(Path atFile) { + try (Stream lines = Files.lines(atFile, Charset.defaultCharset())) { + List args = + lines.filter(arg -> !arg.isEmpty() && !arg.startsWith("#")).toList(); + return parseArgs("atFile", args); + } catch (ParseException e) { + throw new IllegalArgumentException( + "Failed to parse arguments from file (" + atFile + "): " + e.getMessage(), e.getCause()); + } catch (IOException e) { + throw new IllegalStateException("Error reading config file: " + atFile, e); + } + } + protected MavenOptions parseMavenConfigOptions(Path configFile) { try (Stream lines = Files.lines(configFile, Charset.defaultCharset())) { List args = From 037a34f44e59fddbc381d8bac22250d35eba03f6 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 26 Feb 2025 15:25:44 +0100 Subject: [PATCH 2/4] Add IT and fix a bug --- .../maven/cling/invoker/LayeredOptions.java | 4 +- .../maven/it/MavenITmng8594AtFileTest.java | 64 +++++++++++++++++++ .../apache/maven/it/TestSuiteOrdering.java | 1 + .../src/test/resources/mng-8594/cmd.txt | 3 + .../src/test/resources/mng-8594/pom.xml | 34 ++++++++++ 5 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8594AtFileTest.java create mode 100644 its/core-it-suite/src/test/resources/mng-8594/cmd.txt create mode 100644 its/core-it-suite/src/test/resources/mng-8594/pom.xml diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LayeredOptions.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LayeredOptions.java index 4d9d7b6c09ee..2f9c367c4786 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LayeredOptions.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LayeredOptions.java @@ -181,7 +181,9 @@ protected Optional> collectMapIfPresentOrEmpty( Optional> up = getter.apply(option); if (up.isPresent()) { had++; - items.putAll(up.get()); + for (Map.Entry entry : up.get().entrySet()) { + items.putIfAbsent(entry.getKey(), entry.getValue()); + } } } return had == 0 ? Optional.empty() : Optional.of(Map.copyOf(items)); diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8594AtFileTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8594AtFileTest.java new file mode 100644 index 000000000000..a0ad34011e01 --- /dev/null +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8594AtFileTest.java @@ -0,0 +1,64 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.it; + +import java.nio.file.Path; +import java.util.List; + +import org.junit.jupiter.api.Test; + +import static org.junit.Assert.assertTrue; + +/** + * This is a test set for MNG-8594. + */ +class MavenITmng8594AtFileTest extends AbstractMavenIntegrationTestCase { + + MavenITmng8594AtFileTest() { + super("[4.0.0-rc-3-SNAPSHOT,)"); + } + + /** + * Verify Maven picks up params/goals from atFile. + */ + @Test + void testIt() throws Exception { + Path basedir = extractResources("/mng-8594").getAbsoluteFile().toPath(); + + Verifier verifier = newVerifier(basedir.toString()); + verifier.addCliArgument("-af"); + verifier.addCliArgument("cmd.txt"); + verifier.addCliArgument("-Dcolor1=green"); + verifier.addCliArgument("-Dcolor2=blue"); + verifier.addCliArgument("clean"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + // clean did run + verifier.verifyTextInLog("(default-clean) @ root"); + // validate bound plugin did run + verifier.verifyTextInLog("(eval) @ root"); + + // validate properties + List properties = verifier.loadLines("target/pom.properties"); + assertTrue(properties.contains("session.executionProperties.color1=green")); // CLI only + assertTrue(properties.contains("session.executionProperties.color2=blue")); // both + assertTrue(properties.contains("session.executionProperties.color3=yellow")); // cmd.txt only + } +} diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java b/its/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java index a303f19ca3dd..101ddd853ea4 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java @@ -100,6 +100,7 @@ public TestSuiteOrdering() { * the tests are to finishing. Newer tests are also more likely to fail, so this is * a fail fast technique as well. */ + suite.addTestSuite(MavenITmng8594AtFileTest.class); suite.addTestSuite(MavenITmng8561SourceRootTest.class); suite.addTestSuite(MavenITmng8523ModelPropertiesTest.class); suite.addTestSuite(MavenITmng8527ConsumerPomTest.class); diff --git a/its/core-it-suite/src/test/resources/mng-8594/cmd.txt b/its/core-it-suite/src/test/resources/mng-8594/cmd.txt new file mode 100644 index 000000000000..2d638e72a64a --- /dev/null +++ b/its/core-it-suite/src/test/resources/mng-8594/cmd.txt @@ -0,0 +1,3 @@ +validate +-Dcolor2=gray +-Dcolor3=yellow \ No newline at end of file diff --git a/its/core-it-suite/src/test/resources/mng-8594/pom.xml b/its/core-it-suite/src/test/resources/mng-8594/pom.xml new file mode 100644 index 000000000000..fa94de4747bc --- /dev/null +++ b/its/core-it-suite/src/test/resources/mng-8594/pom.xml @@ -0,0 +1,34 @@ + + + + 4.0.0 + + org.apache.maven.it.mng8594 + root + 1.0.0 + + + + + org.apache.maven.its.plugins + maven-it-plugin-expression + 2.1-SNAPSHOT + + target/pom.properties + + session/executionProperties + + + + + eval + + eval + + validate + + + + + + From 787385215456942f17260eafca1265a3222fde0a Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 26 Feb 2025 15:59:51 +0100 Subject: [PATCH 3/4] Javadoc and message tidy up --- .../main/java/org/apache/maven/api/cli/mvn/MavenOptions.java | 4 +++- .../maven/cling/invoker/mvn/CommonsCliMavenOptions.java | 2 +- .../java/org/apache/maven/cling/invoker/mvn/MavenParser.java | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java b/api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java index e290114dec6c..2387105a3a24 100644 --- a/api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java +++ b/api/maven-api-cli/src/main/java/org/apache/maven/api/cli/mvn/MavenOptions.java @@ -208,7 +208,9 @@ public interface MavenOptions extends Options { Optional ignoreTransitiveRepositories(); /** - * Allows {@code @filename} to read options from file. + * Specifies "@file"-like file, to load up command line from. It may contain goals as well. Format is one parameter + * per line (similar to {@code maven.conf}) and {@code '#'} (hash) marked comment lines are allowed. Goals, if + * present, are appended, to those specified on CLI input, if any. */ Optional atFile(); diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java index 51f2fe285366..8fe493cfa6cf 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java @@ -387,7 +387,7 @@ protected void prepareOptions(org.apache.commons.cli.Options options) { options.addOption(Option.builder(AT_FILE) .longOpt("at-file") .hasArg() - .desc("If set, Maven load and merge command line options from the file as well.") + .desc("If set, Maven will load command line options from the specified file and merge with CLI specified ones.") .build()); } } diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java index 9e55bb1e7d5a..fd030ba1858c 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java @@ -45,7 +45,7 @@ protected List parseCliOptions(LocalContext context) { if (Files.isRegularFile(file)) { result.add(parseMavenAtFileOptions(file)); } else { - throw new IllegalArgumentException("Specified atFile does not exists (" + file + ")"); + throw new IllegalArgumentException("Specified file does not exists (" + file + ")"); } } // maven.config; if exists From d253608eef185f975accbbe2573fd688dc1a8e66 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 26 Feb 2025 17:23:37 +0100 Subject: [PATCH 4/4] Reformat --- .../apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java index 8fe493cfa6cf..180d756e4d73 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/CommonsCliMavenOptions.java @@ -387,7 +387,8 @@ protected void prepareOptions(org.apache.commons.cli.Options options) { options.addOption(Option.builder(AT_FILE) .longOpt("at-file") .hasArg() - .desc("If set, Maven will load command line options from the specified file and merge with CLI specified ones.") + .desc( + "If set, Maven will load command line options from the specified file and merge with CLI specified ones.") .build()); } }