You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by hb...@apache.org on 2018/12/27 00:29:23 UTC

[maven] 03/04: [MNG-6533] Prefer passing the interim project in ProjectBuildingResult

This is an automated email from the ASF dual-hosted git repository.

hboutemy pushed a commit to branch MNG-6533-2
in repository https://gitbox.apache.org/repos/asf/maven.git

commit c41962abc7a8c04f162eaf51b171a4abd47e1159
Author: Mickael Istria <mi...@redhat.com>
AuthorDate: Thu Nov 29 22:21:29 2018 +0100

    [MNG-6533] Prefer passing the interim project in ProjectBuildingResult
    
    Initialize the interim project with "simple" items (ie do not build
    not reference parent if it's not yet in the projectIndex) and returns
    it when installation fails further.
    This give a partial validation of the file, pretty convenient in IDEs.
---
 .../maven/project/DefaultProjectBuilder.java       | 185 ++++++++++++---------
 1 file changed, 103 insertions(+), 82 deletions(-)

diff --git a/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java b/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
index 0d2887b..5e8626a 100644
--- a/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
+++ b/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
@@ -172,8 +172,8 @@ public class DefaultProjectBuilder
 
                 modelProblems = result.getProblems();
 
-                initProject( project, Collections.<String, MavenProject>emptyMap(), result,
-                             new HashMap<File, Boolean>(), projectBuildingRequest );
+                initProject( project, Collections.<String, MavenProject>emptyMap(), true,
+                             result, new HashMap<File, Boolean>(), projectBuildingRequest );
             }
             else if ( projectBuildingRequest.isResolveDependencies() )
             {
@@ -418,6 +418,7 @@ public class DefaultProjectBuilder
         ModelBuildingRequest request = getModelBuildingRequest( config );
 
         MavenProject project = new MavenProject();
+        project.setFile( pomFile );
 
         request.setPomFile( pomFile );
         request.setTwoPhaseBuilding( true );
@@ -427,106 +428,116 @@ public class DefaultProjectBuilder
             new DefaultModelBuildingListener( project, projectBuildingHelper, config.request );
         request.setModelBuildingListener( listener );
 
+        ModelBuildingResult result;
         try
         {
-            ModelBuildingResult result = modelBuilder.build( request );
+            result = modelBuilder.build( request );
+        }
+        catch ( ModelBuildingException e )
+        {
+            result = e.getResult();
+            if ( result == null || result.getEffectiveModel() == null )
+            {
+                 results.add( new DefaultProjectBuildingResult( e.getModelId(), pomFile, e.getProblems() ) );
 
-            Model model = result.getEffectiveModel();
+                 return false;
+            }
+            // validation error, continue project building and delay failing to help IDEs
+            // result.getProblems().addAll(e.getProblems()) ?
+            noErrors = false;
+        }
 
-            projectIndex.put( result.getModelIds().get( 0 ), project );
+        Model model = result.getEffectiveModel();
+        // first pass: build without building parent.
+        initProject( project, projectIndex, false, result, new HashMap<File, Boolean>( 0 ), config.request );
 
-            InterimResult interimResult = new InterimResult( pomFile, request, result, listener, isRoot );
-            interimResults.add( interimResult );
+        projectIndex.put( result.getModelIds().get( 0 ), project );
 
-            if ( recursive && !model.getModules().isEmpty() )
-            {
-                File basedir = pomFile.getParentFile();
+        InterimResult interimResult = new InterimResult( pomFile, request, result, listener, isRoot );
+        interimResults.add( interimResult );
+
+        if ( recursive && !model.getModules().isEmpty() )
+        {
+            File basedir = pomFile.getParentFile();
 
-                List<File> moduleFiles = new ArrayList<>();
+            List<File> moduleFiles = new ArrayList<>();
 
-                for ( String module : model.getModules() )
+            for ( String module : model.getModules() )
+            {
+                if ( StringUtils.isEmpty( module ) )
                 {
-                    if ( StringUtils.isEmpty( module ) )
-                    {
-                        continue;
-                    }
+                    continue;
+                }
 
-                    module = module.replace( '\\', File.separatorChar ).replace( '/', File.separatorChar );
+                module = module.replace( '\\', File.separatorChar ).replace( '/', File.separatorChar );
 
-                    File moduleFile = new File( basedir, module );
+                File moduleFile = new File( basedir, module );
 
-                    if ( moduleFile.isDirectory() )
-                    {
-                        moduleFile = modelProcessor.locatePom( moduleFile );
-                    }
+                if ( moduleFile.isDirectory() )
+                {
+                    moduleFile = modelProcessor.locatePom( moduleFile );
+                }
 
-                    if ( !moduleFile.isFile() )
-                    {
-                        ModelProblem problem =
-                            new DefaultModelProblem( "Child module " + moduleFile + " of " + pomFile
-                                + " does not exist", ModelProblem.Severity.ERROR, ModelProblem.Version.BASE, model, -1,
-                                                     -1, null );
-                        result.getProblems().add( problem );
+                if ( !moduleFile.isFile() )
+                {
+                    ModelProblem problem =
+                        new DefaultModelProblem( "Child module " + moduleFile + " of " + pomFile
+                            + " does not exist", ModelProblem.Severity.ERROR, ModelProblem.Version.BASE, model, -1,
+                                                 -1, null );
+                    result.getProblems().add( problem );
 
-                        noErrors = false;
+                    noErrors = false;
 
-                        continue;
-                    }
+                    continue;
+                }
 
-                    if ( Os.isFamily( Os.FAMILY_WINDOWS ) )
+                if ( Os.isFamily( Os.FAMILY_WINDOWS ) )
+                {
+                    // we don't canonicalize on unix to avoid interfering with symlinks
+                    try
                     {
-                        // we don't canonicalize on unix to avoid interfering with symlinks
-                        try
-                        {
-                            moduleFile = moduleFile.getCanonicalFile();
-                        }
-                        catch ( IOException e )
-                        {
-                            moduleFile = moduleFile.getAbsoluteFile();
-                        }
+                        moduleFile = moduleFile.getCanonicalFile();
                     }
-                    else
+                    catch ( IOException e )
                     {
-                        moduleFile = new File( moduleFile.toURI().normalize() );
+                        moduleFile = moduleFile.getAbsoluteFile();
                     }
+                }
+                else
+                {
+                    moduleFile = new File( moduleFile.toURI().normalize() );
+                }
 
-                    if ( aggregatorFiles.contains( moduleFile ) )
+                if ( aggregatorFiles.contains( moduleFile ) )
+                {
+                    StringBuilder buffer = new StringBuilder( 256 );
+                    for ( File aggregatorFile : aggregatorFiles )
                     {
-                        StringBuilder buffer = new StringBuilder( 256 );
-                        for ( File aggregatorFile : aggregatorFiles )
-                        {
-                            buffer.append( aggregatorFile ).append( " -> " );
-                        }
-                        buffer.append( moduleFile );
-
-                        ModelProblem problem =
-                            new DefaultModelProblem( "Child module " + moduleFile + " of " + pomFile
-                                + " forms aggregation cycle " + buffer, ModelProblem.Severity.ERROR,
-                                                     ModelProblem.Version.BASE, model, -1, -1, null );
-                        result.getProblems().add( problem );
-
-                        noErrors = false;
-
-                        continue;
+                        buffer.append( aggregatorFile ).append( " -> " );
                     }
+                    buffer.append( moduleFile );
 
-                    moduleFiles.add( moduleFile );
-                }
+                    ModelProblem problem =
+                        new DefaultModelProblem( "Child module " + moduleFile + " of " + pomFile
+                            + " forms aggregation cycle " + buffer, ModelProblem.Severity.ERROR,
+                                                 ModelProblem.Version.BASE, model, -1, -1, null );
+                    result.getProblems().add( problem );
 
-                interimResult.modules = new ArrayList<>();
-
-                if ( !build( results, interimResult.modules, projectIndex, moduleFiles, aggregatorFiles, false,
-                             recursive, config ) )
-                {
                     noErrors = false;
+
+                    continue;
                 }
+
+                moduleFiles.add( moduleFile );
             }
-        }
-        catch ( ModelBuildingException e )
-        {
-            results.add( new DefaultProjectBuildingResult( e.getModelId(), pomFile, e.getProblems() ) );
 
-            noErrors = false;
+            interimResult.modules = new ArrayList<>();
+
+            if ( !build( results, interimResult.modules, projectIndex, moduleFiles, aggregatorFiles, false,
+                         recursive, config ) )
+            {
+                noErrors = false;
+            }
         }
 
         return noErrors;
@@ -579,12 +590,13 @@ public class DefaultProjectBuilder
 
         for ( InterimResult interimResult : interimResults )
         {
+            MavenProject project = interimResult.listener.getProject();
             try
             {
                 ModelBuildingResult result = modelBuilder.build( interimResult.request, interimResult.result );
 
-                MavenProject project = interimResult.listener.getProject();
-                initProject( project, projectIndex, result, profilesXmls, request );
+                // 2nd pass of initialization: resolve and build parent if necessary
+                initProject( project, projectIndex, true, result, profilesXmls, request );
 
                 List<MavenProject> modules = new ArrayList<>();
                 noErrors =
@@ -606,8 +618,16 @@ public class DefaultProjectBuilder
             }
             catch ( ModelBuildingException e )
             {
-                results.add( new DefaultProjectBuildingResult( e.getModelId(), interimResult.pomFile,
-                                                               e.getProblems() ) );
+                DefaultProjectBuildingResult result = null;
+                if ( project == null )
+                {
+                    result = new DefaultProjectBuildingResult( e.getModelId(), interimResult.pomFile, e.getProblems() );
+                }
+                else
+                {
+                    result = new DefaultProjectBuildingResult( project, e.getProblems(), null );
+                }
+                results.add( result );
 
                 noErrors = false;
             }
@@ -617,7 +637,8 @@ public class DefaultProjectBuilder
     }
 
     @SuppressWarnings( "checkstyle:methodlength" )
-    private void initProject( MavenProject project, Map<String, MavenProject> projects, ModelBuildingResult result,
+    private void initProject( MavenProject project, Map<String, MavenProject> projects,
+                              boolean buildParentIfNotExisting, ModelBuildingResult result,
                               Map<File, Boolean> profilesXmls, ProjectBuildingRequest projectBuildingRequest )
     {
         Model model = result.getEffectiveModel();
@@ -626,7 +647,7 @@ public class DefaultProjectBuilder
         project.setOriginalModel( result.getRawModel() );
         project.setFile( model.getPomFile() );
 
-        initParent( project, projects, result, projectBuildingRequest );
+        initParent( project, projects, buildParentIfNotExisting, result, projectBuildingRequest );
 
         Artifact projectArtifact =
             repositorySystem.createArtifact( project.getGroupId(), project.getArtifactId(), project.getVersion(), null,
@@ -803,8 +824,8 @@ public class DefaultProjectBuilder
         }
     }
 
-    private void initParent( MavenProject project, Map<String, MavenProject> projects, ModelBuildingResult result,
-                             ProjectBuildingRequest projectBuildingRequest )
+    private void initParent( MavenProject project, Map<String, MavenProject> projects, boolean buildParentIfNotExisting,
+                             ModelBuildingResult result, ProjectBuildingRequest projectBuildingRequest )
     {
         Model parentModel = result.getModelIds().size() > 1 && !result.getModelIds().get( 1 ).isEmpty()
                                 ? result.getRawModel( result.getModelIds().get( 1 ) )
@@ -823,7 +844,7 @@ public class DefaultProjectBuilder
             String parentModelId = result.getModelIds().get( 1 );
             File parentPomFile = result.getRawModel( parentModelId ).getPomFile();
             MavenProject parent = projects.get( parentModelId );
-            if ( parent == null )
+            if ( parent == null && buildParentIfNotExisting )
             {
                 //
                 // At this point the DefaultModelBuildingListener has fired and it populates the