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/11 07:48:02 UTC
[GitHub] [maven] mthmulders commented on a diff in pull request #868: [MNG-7542] Remove misleading warning about multi-module root directory
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