[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_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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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