You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "gnodet (via GitHub)" <gi...@apache.org> on 2023/03/16 19:16:51 UTC

[GitHub] [maven] gnodet opened a new pull request, #1062: [MNG-6303] Interpolate user supplied properties

gnodet opened a new pull request, #1062:
URL: https://github.com/apache/maven/pull/1062

   Following this checklist to help us incorporate your
   contribution quickly and easily:
   
    - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed
          for the change (usually before you start working on it).  Trivial changes like typos do not
          require a JIRA issue. Your pull request should address just this issue, without
          pulling in other changes.
    - [ ] Each commit in the pull request should have a meaningful subject line and body.
    - [ ] Format the pull request title like `[MNG-XXX] SUMMARY`,
          where you replace `MNG-XXX` and `SUMMARY` with the appropriate JIRA issue.
    - [ ] Also format the first line of the commit message like `[MNG-XXX] SUMMARY`.
          Best practice is to use the JIRA issue title in both the pull request title and in the first line of the commit message.
    - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [ ] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
          be performed on your pull request automatically.
    - [ ] You have run the [Core IT][core-its] successfully.
   
   If your pull request is about ~20 lines of code you don't need to sign an
   [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
   please ask on the developers list.
   
   To make clear that you license your contribution under
   the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   you have to acknowledge this by using the following check-box.
   
    - [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   
    - [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   [core-its]: https://maven.apache.org/core-its/core-it-suite/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] elharo commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1140831831


##########
maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java:
##########
@@ -544,6 +544,19 @@ public void populatePropertiesOverwrite() throws Exception {
         assertThat(request.getUserProperties().getProperty("x"), is("false"));
     }
 
+    @Test
+    public void testPropertiesInterpolation() throws Exception {

Review Comment:
   There should also be a test for the exceptional case. 



##########
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -562,6 +566,21 @@ private void commands(CliRequest cliRequest) {
     // Maybe it's better to move some of those methods to separate class (SoC).
     void properties(CliRequest cliRequest) {
         populateProperties(cliRequest.commandLine, cliRequest.systemProperties, cliRequest.userProperties);
+
+        // now that we have properties, interpolate all arguments
+        BasicInterpolator interpolator = createInterpolator(cliRequest.systemProperties, cliRequest.userProperties);
+        CommandLine.Builder commandLineBuilder = new CommandLine.Builder();
+        cliRequest.commandLine.getArgList().stream()
+                .map(s -> {
+                    try {
+                        return interpolator.interpolate(s);
+                    } catch (InterpolationException e) {
+                        throw new RuntimeException("Unable to interpolate", e);

Review Comment:
   This should not be a runtime exception since it comes from input external to the program. InterpolationException might be OK, or perhaps a different checked exception. 
   
   If this means, the code doesn;t use lambdas, that's fine. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] gnodet commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1185871681


##########
maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java:
##########
@@ -552,6 +553,38 @@ public void findRootProjectWithAttribute() {
         assertEquals(test, new DefaultRootLocator().findRoot(test.resolve("child")));
     }
 
+    @Test
+    public void testPropertiesInterpolation() throws Exception {
+        // Arrange
+        CliRequest request = new CliRequest(
+                new String[] {
+                    "-Dfoo=bar",
+                    "-DvalFound=s${foo}i",
+                    "-DvalNotFound=s${foz}i",
+                    "-DvalRootDirectory=${session.rootDirectory}/.mvn/foo",
+                    "-DvalTopDirectory=${session.topDirectory}/pom.xml",
+                    "-f",
+                    "${session.rootDirectory}/my-child",
+                    "prefix:3.0.0:${foo}",
+                    "validate"

Review Comment:
   Not sure what you mean exactly.  The two properties `request.rootDirectory` and `request.topDirectory` are set just below.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] cstamas commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1185917296


##########
maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java:
##########
@@ -552,6 +553,38 @@ public void findRootProjectWithAttribute() {
         assertEquals(test, new DefaultRootLocator().findRoot(test.resolve("child")));
     }
 
+    @Test
+    public void testPropertiesInterpolation() throws Exception {
+        // Arrange
+        CliRequest request = new CliRequest(
+                new String[] {
+                    "-Dfoo=bar",
+                    "-DvalFound=s${foo}i",
+                    "-DvalNotFound=s${foz}i",
+                    "-DvalRootDirectory=${session.rootDirectory}/.mvn/foo",
+                    "-DvalTopDirectory=${session.topDirectory}/pom.xml",
+                    "-f",
+                    "${session.rootDirectory}/my-child",
+                    "prefix:3.0.0:${foo}",
+                    "validate"

Review Comment:
   They (user properties) are first gathered, then interpolated. So yes, it is possible. Next, using interpolated user properties the rest (everything but user properties "-D") of CLI is interpolated.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] elharo commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1167472442


##########
maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##########
@@ -494,14 +504,51 @@ public interface MavenExecutionRequest {
 
     /**
      * @since 3.3.0
+     * @deprecated use {@link #setRootdir(Path)} instead
      */
+    @Deprecated
     void setMultiModuleProjectDirectory(File file);
 
     /**
      * @since 3.3.0
+     * @deprecated use {@link #getRootdir()} instead
      */
+    @Deprecated
     File getMultiModuleProjectDirectory();
 
+    /**
+     * Sets the top dir of the project.
+     *
+     * @since 4.0.0
+     */
+    MavenExecutionRequest setTopdir(Path topdir);

Review Comment:
   dir --> Directory, here and elsewhere
   
   That is, don't abbreviate. 



##########
maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##########
@@ -494,14 +504,51 @@ public interface MavenExecutionRequest {
 
     /**
      * @since 3.3.0
+     * @deprecated use {@link #setRootdir(Path)} instead
      */
+    @Deprecated
     void setMultiModuleProjectDirectory(File file);
 
     /**
      * @since 3.3.0
+     * @deprecated use {@link #getRootdir()} instead
      */
+    @Deprecated
     File getMultiModuleProjectDirectory();
 
+    /**
+     * Sets the top dir of the project.
+     *
+     * @since 4.0.0
+     */
+    MavenExecutionRequest setTopdir(Path topdir);
+
+    /**
+     * Gets the directory of the topmost project being built, usually the current directory or the
+     * directory pointed at by the {@code -f/--file} command line argument.

Review Comment:
   pointed at by --> specified by 



##########
maven-core/src/main/java/org/apache/maven/execution/MavenSession.java:
##########
@@ -141,10 +142,30 @@ public List<MavenProject> getProjects() {
         return projects;
     }
 
+    /**
+     * @deprecated use {@link #getTopdir()}
+     */
+    @Deprecated
     public String getExecutionRootDirectory() {
         return request.getBaseDirectory();
     }
 
+    /**
+     * @see MavenExecutionRequest#getTopdir()
+     * @since 4.0.0
+     */
+    public Path getTopdir() {
+        return request.getTopdir();
+    }
+
+    /**
+     * @see MavenExecutionRequest#getRootdir()
+     * @since 4.0.0
+     */

Review Comment:
   RootDirectory



##########
api/maven-api-core/src/main/java/org/apache/maven/api/Project.java:
##########
@@ -86,8 +86,42 @@ default String getId() {
         return getModel().getId();
     }
 
+    /**
+     * @deprecated use {@link #isTopProject()} instead
+     */
+    @Deprecated
     boolean isExecutionRoot();
 
+    /**
+     * Returns a boolean indicating if the project is the top level project for
+     * this reactor build.  The top level project may be different from the
+     * {@code rootDirProject}, especially if a subtree of the project is being
+     * built, either because Maven has been launched in a subdirectory or using
+     * a {@code -f} option.
+     *
+     * @return {@code true} if the project is the top level project for this build
+     */
+    boolean isTopProject();
+
+    /**
+     * Returns a boolean indicating if the project is a root project,
+     * meaning that .

Review Comment:
   incomplete



##########
src/mdo/writer-ex.vm:
##########
@@ -210,8 +210,15 @@ public class ${className}
       #set ( $fieldCapName = $Helper.capitalise( $field.name ) )
       #if ( $field.type == "String" )
             writeAttr( "$fieldTagName", ${classLcapName}.get${fieldCapName}(), serializer );
+      #elseif ( $field.type == "boolean" )
+        #set ( $def = ${field.defaultValue} )
+        #if ( ${def} == "true" )
+            writeAttr( "$fieldTagName", ${classLcapName}.is${fieldCapName}() ? null : "false", serializer );
+        #else
+            writeAttr( "$fieldTagName", ${classLcapName}.is${fieldCapName}() ? "true" : null, serializer );
+        #end
       #else
-        // TODO: type=${field.type} to=${field.to} multiplicity=${field.multiplicity}
+            // TODO: type=${field.type} to=${field.to} multiplicity=${field.multiplicity}

Review Comment:
   is this TODO still relevant? Can it be removed?



##########
src/mdo/writer.vm:
##########
@@ -192,8 +192,15 @@ public class ${className}
       #set ( $fieldCapName = $Helper.capitalise( $field.name ) )
       #if ( $field.type == "String" )
             writeAttr( "$fieldTagName", ${classLcapName}.get${fieldCapName}(), serializer );
+      #elseif ( $field.type == "boolean" )
+        #set ( $def = ${field.defaultValue} )
+        #if ( ${def} == "true" )
+            writeAttr( "$fieldTagName", ${classLcapName}.is${fieldCapName}() ? null : "false", serializer );
+        #else
+            writeAttr( "$fieldTagName", ${classLcapName}.is${fieldCapName}() ? "true" : null, serializer );
+        #end
       #else
-        // TODO: type=${field.type} to=${field.to} multiplicity=${field.multiplicity}
+            // TODO: type=${field.type} to=${field.to} multiplicity=${field.multiplicity}

Review Comment:
   is this TODO still relevant? Can it be removed?



##########
maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##########
@@ -91,8 +92,17 @@ public interface MavenExecutionRequest {
     // ----------------------------------------------------------------------
 
     // Base directory
+
+    /**
+     * @deprecated use {@link #setTopdir(Path)} instead

Review Comment:
   Are the base directory and the top directory different? if so, how? (Please don't answer in github comments. Instead add this info to the code.) If not, why this change?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1185874264


##########
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -31,16 +31,8 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;

Review Comment:
   OK, if spotless does not complain, I won't either, but that's not the usual approach for our code.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #1062:
URL: https://github.com/apache/maven/pull/1062#issuecomment-1520753004

   I need a bit more time to review it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] gnodet commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1166721249


##########
maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java:
##########
@@ -544,6 +544,19 @@ public void populatePropertiesOverwrite() throws Exception {
         assertThat(request.getUserProperties().getProperty("x"), is("false"));
     }
 
+    @Test
+    public void testPropertiesInterpolation() throws Exception {

Review Comment:
   Added some tests.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1185873548


##########
maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java:
##########
@@ -552,6 +553,38 @@ public void findRootProjectWithAttribute() {
         assertEquals(test, new DefaultRootLocator().findRoot(test.resolve("child")));
     }
 
+    @Test
+    public void testPropertiesInterpolation() throws Exception {
+        // Arrange
+        CliRequest request = new CliRequest(
+                new String[] {
+                    "-Dfoo=bar",
+                    "-DvalFound=s${foo}i",
+                    "-DvalNotFound=s${foz}i",
+                    "-DvalRootDirectory=${session.rootDirectory}/.mvn/foo",
+                    "-DvalTopDirectory=${session.topDirectory}/pom.xml",
+                    "-f",
+                    "${session.rootDirectory}/my-child",
+                    "prefix:3.0.0:${foo}",
+                    "validate"

Review Comment:
   I mean `foo`. You cannot foo before it has been defined, just like symbols in C. It does not perform a late eval. So is the following possible?
   
   ```
   -Dbla=${foo}
   -Dfoo=blubb
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] cstamas commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1185917296


##########
maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java:
##########
@@ -552,6 +553,38 @@ public void findRootProjectWithAttribute() {
         assertEquals(test, new DefaultRootLocator().findRoot(test.resolve("child")));
     }
 
+    @Test
+    public void testPropertiesInterpolation() throws Exception {
+        // Arrange
+        CliRequest request = new CliRequest(
+                new String[] {
+                    "-Dfoo=bar",
+                    "-DvalFound=s${foo}i",
+                    "-DvalNotFound=s${foz}i",
+                    "-DvalRootDirectory=${session.rootDirectory}/.mvn/foo",
+                    "-DvalTopDirectory=${session.topDirectory}/pom.xml",
+                    "-f",
+                    "${session.rootDirectory}/my-child",
+                    "prefix:3.0.0:${foo}",
+                    "validate"

Review Comment:
   They (user properties) are first gathered, then interpolated. So yes, it is possible. Next, using interpolated user properties the rest of CLI is interpolated.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] elharo commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1167475025


##########
maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##########
@@ -494,14 +504,51 @@ public interface MavenExecutionRequest {
 
     /**
      * @since 3.3.0
+     * @deprecated use {@link #setRootdir(Path)} instead
      */
+    @Deprecated
     void setMultiModuleProjectDirectory(File file);
 
     /**
      * @since 3.3.0
+     * @deprecated use {@link #getRootdir()} instead
      */
+    @Deprecated
     File getMultiModuleProjectDirectory();
 
+    /**
+     * Sets the top dir of the project.
+     *
+     * @since 4.0.0
+     */
+    MavenExecutionRequest setTopdir(Path topdir);

Review Comment:
   It's not the case in the existing methods like setBaseDirectory and it's not the Java approach. Non-abbreviated names are more readable and clear. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1167475281


##########
maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##########
@@ -494,14 +504,51 @@ public interface MavenExecutionRequest {
 
     /**
      * @since 3.3.0
+     * @deprecated use {@link #setRootdir(Path)} instead
      */
+    @Deprecated
     void setMultiModuleProjectDirectory(File file);
 
     /**
      * @since 3.3.0
+     * @deprecated use {@link #getRootdir()} instead
      */
+    @Deprecated
     File getMultiModuleProjectDirectory();
 
+    /**
+     * Sets the top dir of the project.
+     *
+     * @since 4.0.0
+     */
+    MavenExecutionRequest setTopdir(Path topdir);

Review Comment:
   Then we need to review everything in Core and make it consistent.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] gnodet merged pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #1062:
URL: https://github.com/apache/maven/pull/1062


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] elharo commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1167478598


##########
maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##########
@@ -494,14 +504,51 @@ public interface MavenExecutionRequest {
 
     /**
      * @since 3.3.0
+     * @deprecated use {@link #setRootdir(Path)} instead
      */
+    @Deprecated
     void setMultiModuleProjectDirectory(File file);
 
     /**
      * @since 3.3.0
+     * @deprecated use {@link #getRootdir()} instead
      */
+    @Deprecated
     File getMultiModuleProjectDirectory();
 
+    /**
+     * Sets the top dir of the project.
+     *
+     * @since 4.0.0
+     */
+    MavenExecutionRequest setTopdir(Path topdir);

Review Comment:
   That would be nice, but it's not a prerequisitie for the choosing the more readable name here. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1167473448


##########
maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##########
@@ -494,14 +504,51 @@ public interface MavenExecutionRequest {
 
     /**
      * @since 3.3.0
+     * @deprecated use {@link #setRootdir(Path)} instead
      */
+    @Deprecated
     void setMultiModuleProjectDirectory(File file);
 
     /**
      * @since 3.3.0
+     * @deprecated use {@link #getRootdir()} instead
      */
+    @Deprecated
     File getMultiModuleProjectDirectory();
 
+    /**
+     * Sets the top dir of the project.
+     *
+     * @since 4.0.0
+     */
+    MavenExecutionRequest setTopdir(Path topdir);

Review Comment:
   That is everywhere the case and the properties end with `dir` as well.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] gnodet commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1185869307


##########
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -31,16 +31,8 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;

Review Comment:
   I think the spotless _auto_ formatter that when there are more than 10 classes imported from the same package.  This would have to be changed in maven parent's pom if we want to.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on a diff in pull request #1062: [MNG-6303] Interpolate user supplied properties and command line arguments

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1185850625


##########
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -31,16 +31,8 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;

Review Comment:
   Do we really want blanket import?



##########
maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java:
##########
@@ -552,6 +553,38 @@ public void findRootProjectWithAttribute() {
         assertEquals(test, new DefaultRootLocator().findRoot(test.resolve("child")));
     }
 
+    @Test
+    public void testPropertiesInterpolation() throws Exception {
+        // Arrange
+        CliRequest request = new CliRequest(
+                new String[] {
+                    "-Dfoo=bar",
+                    "-DvalFound=s${foo}i",
+                    "-DvalNotFound=s${foz}i",
+                    "-DvalRootDirectory=${session.rootDirectory}/.mvn/foo",
+                    "-DvalTopDirectory=${session.topDirectory}/pom.xml",
+                    "-f",
+                    "${session.rootDirectory}/my-child",
+                    "prefix:3.0.0:${foo}",
+                    "validate"

Review Comment:
   As far as I understand, they need to be defined ahead of time, right?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org