You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "michael-o (via GitHub)" <gi...@apache.org> on 2023/03/23 14:53:10 UTC
[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
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 directory of the reactor, usually containing
+ * the {@code .mvn} directory.
+ *
+ * @since 4.0.0
+ */
+ Path getRootdir();
+
+ /**
+ * Sets the top dir of the project.
+ *
+ * @since 4.0.0
+ */
+ MavenExecutionRequest setTopdir(Path topdir);
+
+ /**
+ * Gets the top dir of the project, which is the up-most directory of the reactor projects.
Review Comment:
This gets confusing
##########
maven-embedder/src/main/java/org/apache/maven/cli/CLIReportingUtils.java:
##########
@@ -136,6 +136,13 @@ static Properties getBuildProperties() {
}
public static void showError(Logger logger, String message, Throwable e, boolean showStackTrace) {
+ if (logger == null) {
Review Comment:
But this is unrelated to this change, right?
##########
maven-model-builder/src/site/apt/index.apt:
##########
@@ -190,6 +190,10 @@ Maven Model Builder
*----+------+------+
| <<<*>>> | model properties, such as project properties set in the pom | <<<$\{any.key\}>>> |
*----+------+------+
+| <<<session.rootdir>>> | the directory containing the root <<<pom.xml>>> file of a multi module project, in a single module project this is the same as <<<project.basedir>>> | <<<$\{session.rootdir\}>>> |
+*----+------+------+
+| <<<session.topdir>>> | the directory containing the top-level <<<pom.xml>>> in this reactor build | <<<$\{session.topdir\}>>> |
+*----+------+------+
Review Comment:
Same concern as above.
##########
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:
I don't understand this loop, why not use `cliRequest.commandLine for this? Like
```
String alternatePomFile = null;
if (commandLine.hasOption(CLIManager.ALTERNATE_POM_FILE)) {
alternatePomFile = commandLine.getOptionValue(CLIManager.ALTERNATE_POM_FILE);
}
```
##########
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
Review Comment:
I have a problem what explaining the rootdir with a term "topmost". That leads to confusion with the other property.
##########
maven-core/src/main/java/org/apache/maven/plugin/PluginParameterExpressionEvaluatorV4.java:
##########
@@ -166,7 +166,11 @@ public Object evaluate(String expr, Class<?> type) throws ExpressionEvaluationEx
return expression.replace("$$", "$");
}
- 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();
+ } else if ("localRepository".equals(expression)) {
Review Comment:
Same here
##########
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:
I wonder now, we don't have an expression for ther items in `session` which starts with `session.`. Why should they be prefixed, by not make then unqualified like the rest? `session.foo` will work via bean introspection, no?
--
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