[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-04-19 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1171610999


##
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##
@@ -324,10 +324,14 @@ void initialize(CliRequest cliRequest) throws 
ExitException {
 }
 }
 
+// We need to locate the top level project which may be pointed at 
using
+// the -f/--file option.  However, the command line isn't parsed yet, 
so
+// we need to iterator through the args to find it and act upon it.

Review Comment:
   iterate



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-04-19 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1171609823


##
maven-model-builder/src/main/java/org/apache/maven/model/root/DefaultRootLocator.java:
##
@@ -0,0 +1,53 @@
+/*
+ * 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.model.root;
+
+import javax.inject.Named;
+
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.codehaus.plexus.util.xml.pull.MXParser;
+
+@Named
+public class DefaultRootLocator implements RootLocator {
+
+public boolean isRootDirectory(Path dir) {
+if (Files.isDirectory(dir.resolve(".mvn"))) {
+return true;
+}
+// we're too early to use the modelProcessor ...
+Path pom = dir.resolve("pom.xml");
+try (InputStream is = Files.newInputStream(pom)) {
+MXParser parser = new MXParser();
+parser.setInput(is, null);
+if (parser.nextTag() == MXParser.START_TAG && 
parser.getName().equals("project")) {
+for (int i = 0; i < parser.getAttributeCount(); i++) {
+if ("root".equals(parser.getAttributeName(i))) {
+return 
Boolean.parseBoolean(parser.getAttributeValue(i));
+}
+}
+}
+} catch (Exception e) {
+// Ignore

Review Comment:
   Yes, depicting that a later stage will handle this better



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-04-19 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1171516507


##
maven-model-builder/src/main/java/org/apache/maven/model/root/DefaultRootLocator.java:
##
@@ -0,0 +1,53 @@
+/*
+ * 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.model.root;
+
+import javax.inject.Named;
+
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.codehaus.plexus.util.xml.pull.MXParser;
+
+@Named
+public class DefaultRootLocator implements RootLocator {
+
+public boolean isRootDirectory(Path dir) {
+if (Files.isDirectory(dir.resolve(".mvn"))) {
+return true;
+}
+// we're too early to use the modelProcessor ...
+Path pom = dir.resolve("pom.xml");
+try (InputStream is = Files.newInputStream(pom)) {
+MXParser parser = new MXParser();
+parser.setInput(is, null);
+if (parser.nextTag() == MXParser.START_TAG && 
parser.getName().equals("project")) {
+for (int i = 0; i < parser.getAttributeCount(); i++) {
+if ("root".equals(parser.getAttributeName(i))) {
+return 
Boolean.parseBoolean(parser.getAttributeValue(i));
+}
+}
+}
+} catch (Exception e) {
+// Ignore

Review Comment:
   Can we really swallow this?



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-04-19 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1171512493


##
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##
@@ -320,6 +324,49 @@ void initialize(CliRequest cliRequest) throws 
ExitException {
 }
 }
 
+Path topDirectory = Paths.get(cliRequest.workingDirectory);
+boolean isAltFile = false;
+for (String arg : cliRequest.args) {
+if (isAltFile) {
+Path path = Paths.get(arg);
+if (Files.isDirectory(path)) {
+topDirectory = path;
+} else if (Files.isRegularFile(topDirectory)) {
+topDirectory = path.getParent();
+if (!Files.isDirectory(topDirectory)) {
+System.err.println("Directory " + topDirectory
++ " extracted from the -f/--file command-line 
argument " + arg + " does not exist");
+throw new ExitException(1);
+}
+} else {
+System.err.println(
+"POM file " + arg + " specified with the -f/--file 
command line argument does not exist");
+throw new ExitException(1);
+}
+break;
+} else {
+isAltFile = 
arg.equals(String.valueOf(CLIManager.ALTERNATE_POM_FILE)) || arg.equals("file");

Review Comment:
   Aha, then a comment should accompany that 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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-04-19 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1171338733


##
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

Review Comment:
   missed this one



##
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



##
maven-model-builder/src/main/java/org/apache/maven/model/root/RootLocator.java:
##
@@ -0,0 +1,69 @@
+/*
+ * 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.model.root;
+
+import java.nio.file.Path;
+
+import org.apache.maven.api.annotations.Nonnull;
+import org.apache.maven.api.annotations.Nullable;
+
+/**
+ * Interface used to locate the root directory for a given project.
+ *
+ * The root locator is usually looked up from the plexus container.
+ * One notable exception is the computation of the early {@code 
session.rootDirectory}
+ * property which happens very early.  The implementation used in this case
+ * will be discovered using the JDK service mechanism.
+ *
+ * The default implementation will look for a {@code .mvn} child directory
+ * or a {@code pom.xml} containing the {@code root="true"} attribute.
+ *
+ * @see DefaultRootLocator
+ */
+public interface RootLocator {
+
+String UNABLE_TO_FIND_ROOT_PROJECT_MESSAGE = "Unable to find the root 
directory. "
++ "Create a .mvn directory in the root directory or add the 
root=\"true\""
++ " attribute on the root project's model to identify it.";
+
+@Nonnull
+default Path findMandatoryRoot(Path basedir) {
+Path rootDirectory = findRoot(basedir);
+if (rootDirectory == null) {
+throw new IllegalStateException(getNoRootMessage());
+}
+return rootDirectory;
+}
+
+@Nullable
+default Path findRoot(Path basedir) {
+Path rootDirectory = basedir;
+while (rootDirectory != null && !isRootDir(rootDirectory)) {
+rootDirectory = rootDirectory.getParent();
+}
+return rootDirectory;
+}
+
+@Nonnull
+default String getNoRootMessage() {
+return UNABLE_TO_FIND_ROOT_PROJECT_MESSAGE;
+}
+
+boolean isRootDir(Path dir);

Review Comment:
   missed this one



##
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 

[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-04-19 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1171331743


##
maven-model-builder/src/site/apt/index.apt:
##
@@ -183,13 +183,17 @@ Maven Model Builder
 | <<>>\
 <<>> () | the directory containing the <<>> 
file as URI | <<<$\{project.baseUri\}>>> |
 *+--+--+
+| <<>> | the project's root directory (containing a 
<<<.mvn>>> directory or with the <<>> xml attribute | 
<<<$\{project.rootdir\}>>> |
+*+--+--+
 | <<>>\
 <<>> | the UTC timestamp of build start, in 
<<>> default format, which can be overridden with 
<<>> POM property | 
<<<$\{maven.build.timestamp\}>>> |
 *+--+--+
 | <<<*>>> | user properties, set from CLI with <<<-Dproperty=value>>> | 
<<<$\{skipTests\}>>> |
 *+--+--+
 | <<<*>>> | model properties, such as project properties set in the pom | 
<<<$\{any.key\}>>> |
 *+--+--+
+| <<>> | the directory containing the root <<>> 
file, identified by the existence of a <<<.mvn>>> child directory | 
<<<$\{session.rootdir\}>>> |
+*+--+--+

Review Comment:
   Right, my bad!



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-04-19 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1171150547


##
maven-model-builder/src/site/apt/index.apt:
##
@@ -183,13 +183,17 @@ Maven Model Builder
 | <<>>\
 <<>> () | the directory containing the <<>> 
file as URI | <<<$\{project.baseUri\}>>> |
 *+--+--+
+| <<>> | the project's root directory (containing a 
<<<.mvn>>> directory or with the <<>> xml attribute | 
<<<$\{project.rootdir\}>>> |
+*+--+--+
 | <<>>\
 <<>> | the UTC timestamp of build start, in 
<<>> default format, which can be overridden with 
<<>> POM property | 
<<<$\{maven.build.timestamp\}>>> |
 *+--+--+
 | <<<*>>> | user properties, set from CLI with <<<-Dproperty=value>>> | 
<<<$\{skipTests\}>>> |
 *+--+--+
 | <<<*>>> | model properties, such as project properties set in the pom | 
<<<$\{any.key\}>>> |
 *+--+--+
+| <<>> | the directory containing the root <<>> 
file, identified by the existence of a <<<.mvn>>> child directory | 
<<<$\{session.rootdir\}>>> |
+*+--+--+

Review Comment:
   Where is the documentation for the top directory?



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-04-12 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1163863120


##
api/maven-api-core/src/main/java/org/apache/maven/api/Project.java:
##
@@ -116,6 +116,16 @@ default String getId() {
  */
 boolean isRootProject();
 
+/**
+ * Gets the root directory of the project, which is the parent directory 
containing the {@code .mvn} directory
+ * or flagged with {@code root="true"}. If there's no such file, an {@code 
IllegalStateException} will be thrown.

Review Comment:
   The last sentence duplicates the `@throws`



##
api/maven-api-core/src/main/java/org/apache/maven/api/Session.java:
##
@@ -101,6 +92,17 @@ public interface Session {
 @Nonnull
 Path getTopdir();
 
+/**
+ * Gets the root directory of the session, which is the root directory for 
the topdir project.
+ * If there's no such file, an {@code IllegalStateException} will be 
thrown.

Review Comment:
   same here



##
maven-core/src/main/java/org/apache/maven/project/MavenProject.java:
##
@@ -1679,4 +1681,13 @@ public ProjectBuildingRequest 
getProjectBuildingRequest() {
 public void setProjectBuildingRequest(ProjectBuildingRequest 
projectBuildingRequest) {
 this.projectBuilderConfiguration = projectBuildingRequest;
 }
+
+/**
+ * @since 4.0.0
+ * @return the rootdir for this project
+ * @throws IllegalArgumentException if the rootdir cannot be found

Review Comment:
   Here it is IAE but else ISE?



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-04-07 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1160802840


##
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 root dir of the project.
+ *
+ * @since 4.0.0
+ */
+MavenExecutionRequest setRootdir(Path rootdir);
+
+/**
+ * Gets the root directory of the project, which is the parent directory 
containing the {@code .mvn} directory
+ * or a {@code pom.xml} file with the {@code root="true"} attribute.
+ * If there's no such file, an {@code IllegalStateException} will be 
thrown.
+ *
+ * @throws IllegalStateException if the root directory could not be found
+ * @since 4.0.0
+ */
+Path getRootdir() throws IllegalStateException;

Review Comment:
   The documentation is already present in Javadoc. We don't declare REs 
anywhere, but Javadoc. It should be consistent throughout.



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-04-07 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1160779967


##
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 root dir of the project.
+ *
+ * @since 4.0.0
+ */
+MavenExecutionRequest setRootdir(Path rootdir);
+
+/**
+ * Gets the root directory of the project, which is the parent directory 
containing the {@code .mvn} directory
+ * or a {@code pom.xml} file with the {@code root="true"} attribute.
+ * If there's no such file, an {@code IllegalStateException} will be 
thrown.
+ *
+ * @throws IllegalStateException if the root directory could not be found
+ * @since 4.0.0
+ */
+Path getRootdir() throws IllegalStateException;

Review Comment:
   That's a runtime exception and redundant in the signature.



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-03-29 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1152181028


##
maven-core/src/main/java/org/apache/maven/plugin/PluginParameterExpressionEvaluator.java:
##
@@ -165,7 +165,11 @@ public Object evaluate(String expr, Class type) throws 
ExpressionEvaluationEx
 
 MojoDescriptor mojoDescriptor = mojoExecution.getMojoDescriptor();
 
-if ("localRepository".equals(expression)) {
+if ("rootdir".equals(expression) || 
"session.rootdir".equals(expression)) {
+value = session.getRootdir();
+} else if ("topdir".equals(expression) || 
"session.topdir".equals(expression)) {
+value = session.getTopdir();

Review Comment:
   Very good, agreed.



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-03-29 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1152079980


##
maven-core/src/main/java/org/apache/maven/plugin/PluginParameterExpressionEvaluator.java:
##
@@ -165,7 +165,11 @@ public Object evaluate(String expr, Class type) throws 
ExpressionEvaluationEx
 
 MojoDescriptor mojoDescriptor = mojoExecution.getMojoDescriptor();
 
-if ("localRepository".equals(expression)) {
+if ("rootdir".equals(expression) || 
"session.rootdir".equals(expression)) {
+value = session.getRootdir();
+} else if ("topdir".equals(expression) || 
"session.topdir".equals(expression)) {
+value = session.getTopdir();

Review Comment:
   > That's true, if we don't support the non prefixed expression, we can 
simply use the code which follows and which should handle `session.rootdir` and 
`session.topdir` perfectly. Would you prefer that ?
   
   Yes, strive for consistency 



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-03-29 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1152077350


##
maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java:
##
@@ -1051,16 +1057,44 @@ public MavenExecutionRequest setToolchains(Map> too
 return this;
 }
 
+@Deprecated
 @Override
 public void setMultiModuleProjectDirectory(File directory) {
 this.multiModuleProjectDirectory = directory;
 }
 
+@Deprecated
 @Override
 public File getMultiModuleProjectDirectory() {
 return multiModuleProjectDirectory;
 }
 
+@Override
+public Path getRootdir() {
+if (rootdir == null) {
+throw new IllegalArgumentException("Unable to find the root 
directory. "

Review Comment:
   Accepted 



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-03-29 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1152018222


##
maven-core/src/main/java/org/apache/maven/plugin/PluginParameterExpressionEvaluator.java:
##
@@ -165,7 +165,11 @@ public Object evaluate(String expr, Class type) throws 
ExpressionEvaluationEx
 
 MojoDescriptor mojoDescriptor = mojoExecution.getMojoDescriptor();
 
-if ("localRepository".equals(expression)) {
+if ("rootdir".equals(expression) || 
"session.rootdir".equals(expression)) {
+value = session.getRootdir();
+} else if ("topdir".equals(expression) || 
"session.topdir".equals(expression)) {
+value = session.getTopdir();

Review Comment:
   How does this then collide with `${project.basedir}`? The devs should be 
able to acces the current module project basedir. This should not 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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-03-29 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1152016383


##
maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java:
##
@@ -1051,16 +1057,44 @@ public MavenExecutionRequest setToolchains(Map> too
 return this;
 }
 
+@Deprecated
 @Override
 public void setMultiModuleProjectDirectory(File directory) {
 this.multiModuleProjectDirectory = directory;
 }
 
+@Deprecated
 @Override
 public File getMultiModuleProjectDirectory() {
 return multiModuleProjectDirectory;
 }
 
+@Override
+public Path getRootdir() {
+if (rootdir == null) {
+throw new IllegalArgumentException("Unable to find the root 
directory. "

Review Comment:
   But rather fail early than after 30 min of a build?



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-03-29 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1152000103


##
maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##
@@ -494,14 +504,50 @@ 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 root dir of the project.
+ *
+ * @since 4.0.0
+ */
+MavenExecutionRequest setRootdir(Path rootdir);
+
+/**
+ * Gets the root directory of the project, which is the parent directory 
containing the {@code .mvn} directory.
+ * If there's no such file, an {@code IllegalArgumentException} will be 
thrown.
+ *
+ * @throws IllegalArgumentException if the root directory could not be 
found

Review Comment:
   Isn't this `IllegalStateException`?



##
maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java:
##
@@ -133,6 +134,21 @@ public Object getValue(String expression) {
 valueSources.add(new 
BuildTimestampValueSource(config.getBuildStartTime(), modelProperties));
 }
 
+valueSources.add(new AbstractValueSource(false) {
+@Override
+public Object getValue(String expression) {
+if ("rootdir".equals(expression)) {
+Path path = config.getRootdir();
+if (path != null) {
+return path.toString();
+}
+throw new IllegalArgumentException("Unable to find the 
root directory. "

Review Comment:
   ISE rather? The expression isn't invalid but the state within, no?



##
maven-model-builder/src/main/java/org/apache/maven/model/path/ProfileActivationFilePathInterpolator.java:
##
@@ -79,6 +80,25 @@ public Object getValue(String expression) {
 return null;
 }
 
+interpolator.addValueSource(new AbstractValueSource(false) {
+@Override
+public Object getValue(String expression) {
+/*
+ * We intentionally only support ${rootdir} and not 
${session.rootdir} as the latter form
+ * would suggest that other session.* expressions can be used 
which is beyond the design.
+ */
+if ("rootdir".equals(expression)) {
+Path path = context.getRootdir();
+if (path != null) {
+return path.toString();
+}
+throw new IllegalArgumentException("Unable to find the 
root directory. "

Review Comment:
   Same here



##
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##
@@ -321,6 +323,46 @@ void initialize(CliRequest cliRequest) throws 
ExitException {
 }
 }
 
+Path topdir = Paths.get(cliRequest.workingDirectory);
+boolean isAltFile = false;
+for (String arg : cliRequest.args) {
+if (isAltFile) {
+Path path = Paths.get(arg);
+if (Files.isDirectory(path)) {

Review Comment:
   Granted.



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-03-29 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1151997995


##
maven-core/src/main/java/org/apache/maven/execution/DefaultMavenExecutionRequest.java:
##
@@ -1051,16 +1057,44 @@ public MavenExecutionRequest setToolchains(Map> too
 return this;
 }
 
+@Deprecated
 @Override
 public void setMultiModuleProjectDirectory(File directory) {
 this.multiModuleProjectDirectory = directory;
 }
 
+@Deprecated
 @Override
 public File getMultiModuleProjectDirectory() {
 return multiModuleProjectDirectory;
 }
 
+@Override
+public Path getRootdir() {
+if (rootdir == null) {
+throw new IllegalArgumentException("Unable to find the root 
directory. "

Review Comment:
   This needs to be documented since it looks like a hard requirement now.



-- 
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-unsubscr...@maven.apache.org

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



[GitHub] [maven] michael-o commented on a diff in pull request #1061: [MNG-7038] Introduce public property to point to a root directory of (multi-module) project

2023-03-23 Thread via GitHub


michael-o commented on code in PR #1061:
URL: https://github.com/apache/maven/pull/1061#discussion_r1146267900


##
api/maven-api-core/src/main/java/org/apache/maven/api/Project.java:
##
@@ -86,8 +86,34 @@ default String getId() {
 return getModel().getId();
 }
 
+/**
+ * @deprecated use {@link #isTopdirProject()} instead
+ */
+@Deprecated
 boolean isExecutionRoot();
 
+/**
+ * Returns a boolean indicating if the project is the top leve 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 isTopdirProject();
+
+/**
+ * Returns a boolean indicating if the project is the project from the root
+ * directory of this reactor. The root project is the top-most project 
containing
+ * the {@code .mvn} directory.  If only a subtree of the reactor is build 
(because
+ * the current directory is in a subtree or because the {@code -f} option 
has been
+ * specified, then there may be no rootDirProject in this reactor.
+ *
+ * @return {@code true} if the project is the root dir project
+ */
+boolean isRootdirProject();

Review Comment:
   Same here



##
api/maven-api-core/src/main/java/org/apache/maven/api/Project.java:
##
@@ -86,8 +86,34 @@ default String getId() {
 return getModel().getId();
 }
 
+/**
+ * @deprecated use {@link #isTopdirProject()} instead
+ */
+@Deprecated
 boolean isExecutionRoot();
 
+/**
+ * Returns a boolean indicating if the project is the top leve 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 isTopdirProject();

Review Comment:
   Why not `isTopProject()` or `isTopLevelProject()`?



##
maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##
@@ -494,14 +504,49 @@ 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 root dir of the project.
+ *
+ * @since 4.0.0
+ */
+MavenExecutionRequest setRootdir(Path rootdir);
+
+/**
+ * Gets the root dir of the project, which is the up-most directory of the 
reactor, usually containing

Review Comment:
   Here you use the term "up-most" which is likely better



##
api/maven-api-core/src/main/java/org/apache/maven/api/Project.java:
##
@@ -86,8 +86,34 @@ default String getId() {
 return getModel().getId();
 }
 
+/**
+ * @deprecated use {@link #isTopdirProject()} instead
+ */
+@Deprecated
 boolean isExecutionRoot();
 
+/**
+ * Returns a boolean indicating if the project is the top leve project for

Review Comment:
   level



##
api/maven-api-core/src/main/java/org/apache/maven/api/Project.java:
##
@@ -86,8 +86,34 @@ default String getId() {
 return getModel().getId();
 }
 
+/**
+ * @deprecated use {@link #isTopdirProject()} instead
+ */
+@Deprecated
 boolean isExecutionRoot();
 
+/**
+ * Returns a boolean indicating if the project is the top leve 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

Review Comment:
   Maven



##
maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##
@@ -494,14 +504,49 @@ 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 root dir of the project.
+ *
+ * @since 4.0.0
+ */
+MavenExecutionRequest setRootdir(Path rootdir);
+
+/**
+ * Gets the root dir of the project, which is the up-most