[3.9.x] [MNG-7720] Wrong build order of forked projects #1038
Conversation
The original fix MNG-7672 matched the "scope" but missed the "order". project.collectedProjects are in order as loaded (POM order), is not topologically sorted. Fix is to use DAG of projects, ask for downstream projects (will return more then we need but sorted) and narrow that list to only contain collected projects. Related commit: 48cac1c --- https://issues.apache.org/jira/browse/MNG-7720
| return getProjectAndSubModules(project).collect(Collectors.toList()); | ||
| List<MavenProject> downstreamProjects = session.getProjectDependencyGraph() | ||
| .getDownstreamProjects(project, true); // sorted but more than needed | ||
| List<MavenProject> collectedProjects = project.getCollectedProjects(); // not sorted but what we need |
There was a problem hiding this comment.
This should be getProjectAndSubModules(project).collect(Collectors.toList()) instead of project.getCollectedProjects(), else the content will be very different.
The first one will return the project and all its submodules (recursively), while the second will just return the modules of the current project.
There was a problem hiding this comment.
Ah, so only 1 level... right
There was a problem hiding this comment.
I cannot confirm this statement with 3.8.7: https://github.com/apache/maven-doxia-sitetools/pull/94/files#r1136260250
There was a problem hiding this comment.
This would mean we don't need to recurse.
There was a problem hiding this comment.
The question is what is expected here? I debugged with Maven SCM and all modules were in that list.
There was a problem hiding this comment.
I think I misread the code at
Reading more closely, the projects list will contain all submodules recursively...
| private static Stream<MavenProject> getProjectAndSubModules(MavenProject project) { | ||
| return Stream.concat( | ||
| Stream.of(project), | ||
| project.getCollectedProjects().stream().flatMap(LifecycleDependencyResolver::getProjectAndSubModules)); |
There was a problem hiding this comment.
Note how this method is recursive...
| getProjectAndSubModules(project).collect(Collectors.toList()); // not sorted but what we need | ||
| return downstreamProjects.stream() | ||
| .filter(projectAndSubmodules::contains) | ||
| .collect(Collectors.toList()); // sorted what we need |
There was a problem hiding this comment.
The returned list should contain the project instance.
There was a problem hiding this comment.
I can propose the following which may be slightly easier to read
List<MavenProject> projectAndSubmodules =
getProjectAndSubModules(project).collect(Collectors.toList());
return Stream.concat(
Stream.of(project),
session.getProjectDependencyGraph().getDownstreamProjects(project, true).stream())
.filter(projectAndSubmodules::contains)
.collect(Collectors.toList());
There was a problem hiding this comment.
This is exactly what I figured out yesterday as well
cstamas
changed the title
|
Liquibase build started failing when we switcthed to Maven 3.9.0 , it's a similar issue using the aggregate-jar javadoc goal. Here is a failed build link: https://github.com/liquibase/liquibase/actions/runs/4196586092/jobs/7277765171 . |
As NOTHING guarantees that none is null, it is really just a "detail" of current DefaultProjectBuilder code (implementation).
The original fix MNG-7672 matched the "scope" but missed the "order".
project.collectedProjectsare in order as loaded (POM order), is not topologically sorted.Fix is to use DAG of projects, ask for downstream projects (will return more then we need but sorted) and narrow that list to only contain collected projects.
Related commit: 48cac1c
https://issues.apache.org/jira/browse/MNG-7720