You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/11/10 12:25:04 UTC
[GitHub] [maven] AzezSalami opened a new pull request, #868: Move the misleading logger from multiModuleProjectPom to collectProjects
AzezSalami opened a new pull request, #868:
URL: https://github.com/apache/maven/pull/868
JIRA issue: https://issues.apache.org/jira/browse/MNG-7542
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.
- [x] 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)
--
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
Re: [PR] [MNG-7542] Remove misleading warning about multi-module root directory [maven]
Posted by "AzezSalami (via GitHub)" <gi...@apache.org>.
AzezSalami closed pull request #868: [MNG-7542] Remove misleading warning about multi-module root directory
URL: https://github.com/apache/maven/pull/868
--
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] mthmulders commented on a diff in pull request #868: [MNG-7542] Remove misleading warning about multi-module root directory
Posted by GitBox <gi...@apache.org>.
mthmulders commented on code in PR #868:
URL: https://github.com/apache/maven/pull/868#discussion_r1019955847
##########
maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java:
##########
@@ -64,23 +65,36 @@ public MultiModuleCollectionStrategy( ModelLocator modelLocator, ProjectsSelecto
@Override
public List<MavenProject> collectProjects( MavenExecutionRequest request ) throws ProjectBuildingException
{
- File moduleProjectPomFile = getMultiModuleProjectPomFile( request );
- List<File> files = Collections.singletonList( moduleProjectPomFile.getAbsoluteFile() );
+ Optional<File> moduleProjectPomFile = getMultiModuleProjectPomFile( request );
+ File absoluteFile = moduleProjectPomFile.orElse( request.getPom() ).getAbsoluteFile();
+ List<File> files = Collections.singletonList( absoluteFile );
try
{
List<MavenProject> projects = projectsSelector.selectProjects( files, request );
+ if ( moduleProjectPomFile.isPresent()
Review Comment:
Are you sure you need to check for `moduleProjectPomFile.isPresent()`? Because previously, the warning would be emitted if the expected POM file did _not_ exist - in which case we now return `Optional.empty()`.
##########
maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java:
##########
@@ -103,25 +118,20 @@ public List<MavenProject> collectProjects( MavenExecutionRequest request ) throw
}
}
- private File getMultiModuleProjectPomFile( MavenExecutionRequest request )
+ private Optional<File> getMultiModuleProjectPomFile( MavenExecutionRequest request ) // return Optional<File>
Review Comment:
```suggestion
private Optional<File> getMultiModuleProjectPomFile( MavenExecutionRequest request )
```
##########
maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java:
##########
@@ -64,23 +65,36 @@ public MultiModuleCollectionStrategy( ModelLocator modelLocator, ProjectsSelecto
@Override
public List<MavenProject> collectProjects( MavenExecutionRequest request ) throws ProjectBuildingException
{
- File moduleProjectPomFile = getMultiModuleProjectPomFile( request );
- List<File> files = Collections.singletonList( moduleProjectPomFile.getAbsoluteFile() );
+ Optional<File> moduleProjectPomFile = getMultiModuleProjectPomFile( request );
+ File absoluteFile = moduleProjectPomFile.orElse( request.getPom() ).getAbsoluteFile();
+ List<File> files = Collections.singletonList( absoluteFile );
try
{
List<MavenProject> projects = projectsSelector.selectProjects( files, request );
+ if ( moduleProjectPomFile.isPresent()
+ && projects.stream()
Review Comment:
For better understanding of the `if` statement, I think it would help if the second part would be refactored to a separate boolean variable with a meaningful name.
##########
maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java:
##########
@@ -91,7 +105,8 @@ public List<MavenProject> collectProjects( MavenExecutionRequest request ) throw
if ( fallThrough )
{
- LOGGER.debug( "Multi module project collection failed:{}"
+ LOGGER.debug(
Review Comment:
It seems to me that these lines are unchanged apart from some whitespace. If that is the case, could you please revert the lines that aren't changed?
##########
maven-core/src/main/java/org/apache/maven/project/collector/MultiModuleCollectionStrategy.java:
##########
@@ -64,23 +65,36 @@ public MultiModuleCollectionStrategy( ModelLocator modelLocator, ProjectsSelecto
@Override
public List<MavenProject> collectProjects( MavenExecutionRequest request ) throws ProjectBuildingException
{
- File moduleProjectPomFile = getMultiModuleProjectPomFile( request );
- List<File> files = Collections.singletonList( moduleProjectPomFile.getAbsoluteFile() );
+ Optional<File> moduleProjectPomFile = getMultiModuleProjectPomFile( request );
+ File absoluteFile = moduleProjectPomFile.orElse( request.getPom() ).getAbsoluteFile();
+ List<File> files = Collections.singletonList( absoluteFile );
try
{
List<MavenProject> projects = projectsSelector.selectProjects( files, request );
+ if ( moduleProjectPomFile.isPresent()
+ && projects.stream()
+ .filter( MavenProject::hasParent )
+ .noneMatch( p -> p.getParent().getBasedir() == null ) )
+ {
+ LOGGER.info( "Maven detected that the requested POM file is part of a multi-module project, "
+ + "but could not find a pom.xml file in the multi-module root directory '{}'.",
+ request.getMultiModuleProjectDirectory() );
+ LOGGER.info( "The reactor is limited to all projects under: " + request.getPom().getParent() );
+ }
+
boolean isRequestedProjectCollected = isRequestedProjectCollected( request, projects );
if ( isRequestedProjectCollected )
{
return projects;
}
else
{
- LOGGER.debug( "Multi module project collection failed:{}"
- + "Detected a POM file next to a .mvn directory in a parent directory ({}). "
- + "Maven assumed that POM file to be the parent of the requested project ({}), but it turned "
- + "out that it was not. Another project collection strategy will be executed as result.",
- System.lineSeparator(), moduleProjectPomFile.getAbsolutePath(),
+ LOGGER.debug(
Review Comment:
It seems to me that these lines are unchanged apart from some whitespace. If that is the case, could you please revert the lines that aren't changed?
--
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] mthmulders commented on pull request #868: Move the misleading logger from multiModuleProjectPom to collectProjects
Posted by GitBox <gi...@apache.org>.
mthmulders commented on PR #868:
URL: https://github.com/apache/maven/pull/868#issuecomment-1311334950
Note to self: commit message could include
> Move the misleading logger from determining the multi-module project root POM to collecting the projects inside that 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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org