Fixing the case where there were exceptions in the build, but for whatever reason they didn't trigger a reactor-manager build failure flag...in cases where there isn't really a project against which to log the error, for instance.

git-svn-id: https://svn.apache.org/repos/asf/maven/components/trunk@616968 13f79535-47bb-0310-9956-ffa450edef68

maven-core/src/main/aspect/org/apache/maven/errors/LifecycleErrorReporterAspect.aj

@@ -75,6 +75,7 @@ public privileged aspect LifecycleErrorReporterAspect
         execution( private * DefaultLifecycleExecutor.getMojoDescriptorForDirectInvocation( String, MavenSession, MavenProject ) )
         && args( task, session, project )
     {
+        System.out.println( "BINGO" );
         getReporter().reportInvalidPluginForDirectInvocation( task, session, project, cause );
     }
 

maven-core/src/main/java/org/apache/maven/errors/DefaultCoreErrorReporter.java

@@ -1685,6 +1685,7 @@ public class DefaultCoreErrorReporter
         writer.write( NEWLINE );
         writer.write( NEWLINE );
         writer.write( "Error message:" );
+        writer.write( NEWLINE );
         writer.write( err.getMessage() );
 
         addTips( CoreErrorTips.getInvalidPluginForDirectInvocationTips( task, session, project, err ), writer );

maven-core/src/main/java/org/apache/maven/lifecycle/plan/BuildPlan.java

@@ -8,14 +8,17 @@ import org.apache.maven.lifecycle.NoSuchPhaseException;
 import org.apache.maven.lifecycle.model.LifecycleBinding;
 import org.apache.maven.lifecycle.model.LifecycleBindings;
 import org.apache.maven.lifecycle.model.MojoBinding;
+import org.apache.maven.lifecycle.model.Phase;
 import org.apache.maven.lifecycle.statemgmt.StateManagementUtils;
 
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.Stack;
 
 public class BuildPlan
@@ -32,6 +35,10 @@ public class BuildPlan
 
     private final Map directInvocationBindings;
 
+    private Set fullyResolvedBindings = new HashSet();
+
+    private boolean includingReports = false;
+
     public BuildPlan( final LifecycleBindings packaging, final LifecycleBindings projectBindings,
                       final LifecycleBindings defaults, final List tasks )
         throws LifecycleSpecificationException, LifecycleLoaderException
@@ -50,15 +57,66 @@ public class BuildPlan
     }
 
     private BuildPlan( final LifecycleBindings bindings, final Map forkedDirectInvocations, final Map forkedPhases,
-                       final Map directInvocationBindings, final List tasks )
+                       final Map directInvocationBindings, final Set fullyResolvedMojoBindings, final List tasks,
+                       boolean includingReports )
     {
         this.bindings = LifecycleUtils.cloneBindings( bindings );
         this.forkedDirectInvocations = new HashMap( forkedDirectInvocations );
         this.forkedPhases = new HashMap( forkedPhases );
+        fullyResolvedBindings = new HashSet( fullyResolvedMojoBindings );
         this.tasks = tasks;
+        this.includingReports = includingReports;
         this.directInvocationBindings = new HashMap( directInvocationBindings );
     }
 
+    public void markAsIncludingReports()
+    {
+        includingReports = true;
+    }
+
+    public boolean isIncludingReports()
+    {
+        return includingReports;
+    }
+
+    public boolean isFullyResolved( final MojoBinding mojoBinding )
+    {
+        String key = MojoBindingUtils.createMojoBindingKey( mojoBinding, false );
+
+        return fullyResolvedBindings.contains( key );
+    }
+
+    public void markFullyResolved()
+    {
+        for ( Iterator bindingIterator = bindings.getBindingList().iterator(); bindingIterator.hasNext(); )
+        {
+            LifecycleBinding binding = (LifecycleBinding) bindingIterator.next();
+
+            for ( Iterator phaseIterator = binding.getPhasesInOrder().iterator(); phaseIterator.hasNext(); )
+            {
+                Phase phase = (Phase) phaseIterator.next();
+
+                for ( Iterator mojoBindingIterator = phase.getBindings().iterator(); mojoBindingIterator.hasNext(); )
+                {
+                    MojoBinding mojoBinding = (MojoBinding) mojoBindingIterator.next();
+
+                    String key = MojoBindingUtils.createMojoBindingKey( mojoBinding, false );
+
+                    fullyResolvedBindings.add( key );
+                }
+            }
+        }
+
+        for ( Iterator it = directInvocationBindings.values().iterator(); it.hasNext(); )
+        {
+            MojoBinding mojoBinding = (MojoBinding) it.next();
+
+            String key = MojoBindingUtils.createMojoBindingKey( mojoBinding, false );
+
+            fullyResolvedBindings.add( key );
+        }
+    }
+
     public void addLifecycleOverlay( final LifecycleBindings overlay )
     {
         bindings = LifecycleUtils.mergeBindings( overlay, bindings, null, true, true );
@@ -135,7 +193,7 @@ public class BuildPlan
 
     public BuildPlan copy( final List newTasks )
     {
-        return new BuildPlan( bindings, forkedDirectInvocations, forkedPhases, directInvocationBindings, newTasks );
+        return new BuildPlan( bindings, forkedDirectInvocations, forkedPhases, directInvocationBindings, fullyResolvedBindings, newTasks, includingReports );
     }
 
     public void resetExecutionProgress()

maven-core/src/main/java/org/apache/maven/lifecycle/plan/DefaultBuildPlanner.java

@@ -102,6 +102,8 @@ public class DefaultBuildPlanner
         addForkedLifecycleModifiers( plan, project, session, new LinkedList() );
         addReportingLifecycleModifiers( plan, project, session, new LinkedList() );
 
+        plan.markFullyResolved();
+
         // TODO: Inject relative-ordered project/plugin executions as plan modifiers.
 
         return plan;
@@ -152,7 +154,10 @@ public class DefaultBuildPlanner
         {
             MojoBinding mojoBinding = (MojoBinding) it.next();
 
-            findForkModifiers( mojoBinding, plan, project, session, callStack );
+            if ( !plan.isFullyResolved( mojoBinding ) )
+            {
+                findForkModifiers( mojoBinding, plan, project, session, callStack );
+            }
         }
     }
 
@@ -197,6 +202,11 @@ public class DefaultBuildPlanner
                                                  LinkedList callStack )
         throws LifecyclePlannerException, LifecycleSpecificationException, LifecycleLoaderException
     {
+        if ( plan.isIncludingReports() )
+        {
+            return;
+        }
+
         List planBindings = plan.renderExecutionPlan( new Stack() );
         plan.resetExecutionProgress();
 
@@ -204,6 +214,11 @@ public class DefaultBuildPlanner
         {
             MojoBinding mojoBinding = (MojoBinding) it.next();
 
+            if ( plan.isFullyResolved( mojoBinding ) )
+            {
+                continue;
+            }
+
             PluginDescriptor pluginDescriptor = loadPluginDescriptor( mojoBinding,
                                                                       plan,
                                                                       project,
@@ -234,6 +249,11 @@ public class DefaultBuildPlanner
                     {
                         MojoBinding reportBinding = (MojoBinding) reportBindingIt.next();
 
+                        if ( plan.isFullyResolved( reportBinding ) )
+                        {
+                            continue;
+                        }
+
                         PluginDescriptor pd = loadPluginDescriptor( reportBinding,
                                                                     plan,
                                                                     project,
@@ -252,6 +272,8 @@ public class DefaultBuildPlanner
                     }
                 }
 
+                plan.markAsIncludingReports();
+
                 // NOTE: the first sighting of a mojo requiring reports should satisfy this condition.
                 // therefore, we can break out as soon as we find one.
                 break;

maven-core/src/test/java/org/apache/maven/errors/DefaultCoreErrorReporterTest.java

@@ -0,0 +1,35 @@
+package org.apache.maven.errors;
+
+import org.apache.maven.InvalidTaskException;
+import org.apache.maven.lifecycle.TaskValidationResult;
+import org.apache.maven.plugin.InvalidPluginException;
+
+import junit.framework.TestCase;
+
+public class DefaultCoreErrorReporterTest
+    extends TestCase
+{
+
+    public void testReportInvalidPluginForDirectInvocation()
+    {
+        CoreErrorReporter reporter = new DefaultCoreErrorReporter();
+
+        InvalidPluginException err = new InvalidPluginException( "Test message" );
+        reporter.reportInvalidPluginForDirectInvocation( "test", null, null, err );
+
+        TaskValidationResult tvr = new TaskValidationResult( "test", "This is a test invalid task.", err );
+        InvalidTaskException exception = tvr.generateInvalidTaskException();
+
+        Throwable realCause = reporter.findReportedException( exception );
+
+        assertSame( err, realCause );
+
+        String message = reporter.getFormattedMessage( realCause );
+
+        System.out.println( message );
+
+        assertNotNull( message );
+        assertTrue( message.length() > 0 );
+    }
+
+}

maven-embedder/src/main/java/org/apache/maven/cli/CLIReportingUtils.java

@@ -95,7 +95,7 @@ public final class CLIReportingUtils
         logReactorSummary( reactorManager, logger );
 
         boolean printSuccess = true;
-        if ( ( reactorManager == null ) || reactorManager.hasBuildFailures() )
+        if ( result.hasExceptions() )
         {
             for ( Iterator i = result.getExceptions().iterator(); i.hasNext(); )
             {

maven-embedder/src/test/error-reporting-projects/missing-direct-invoke-mojo/plugin/pom.xml

@@ -0,0 +1,49 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<project>
+  <modelVersion>4.0.0</modelVersion>
+  <groupId>org.apache.maven.errortest</groupId>
+  <artifactId>missing-direct-invoke-mojo-maven-plugin</artifactId>
+  <packaging>maven-plugin</packaging>
+  <version>1</version>
+  <dependencies>
+    <dependency>
+      <groupId>org.apache.maven</groupId>
+      <artifactId>maven-plugin-api</artifactId>
+      <version>2.0</version>
+    </dependency>
+  </dependencies>
+  <build>
+    <pluginManagement>
+      <plugins>
+        <plugin>
+          <artifactId>maven-surefire-plugin</artifactId>
+          <version>2.3.1</version>
+        </plugin>
+        <plugin>
+          <artifactId>maven-deploy-plugin</artifactId>
+          <version>2.3</version>
+        </plugin>
+        <plugin>
+          <artifactId>maven-install-plugin</artifactId>
+          <version>2.2</version>
+        </plugin>
+        <plugin>
+          <artifactId>maven-resources-plugin</artifactId>
+          <version>2.2</version>
+        </plugin>
+        <plugin>
+          <artifactId>maven-jar-plugin</artifactId>
+          <version>2.1</version>
+        </plugin>
+        <plugin>
+          <artifactId>maven-compiler-plugin</artifactId>
+          <version>2.0.2</version>
+        </plugin>
+        <plugin>
+          <artifactId>maven-plugin-plugin</artifactId>
+          <version>2.3</version>
+        </plugin>
+      </plugins>
+    </pluginManagement>
+  </build>
+</project>

maven-embedder/src/test/error-reporting-projects/missing-direct-invoke-mojo/plugin/src/main/java/org/plugin/TestPlugin.java

@@ -0,0 +1,36 @@
+package org.plugin;
+
+import org.apache.maven.plugin.Mojo;
+import org.apache.maven.plugin.MojoExecutionException;
+import org.apache.maven.plugin.MojoFailureException;
+import org.apache.maven.plugin.logging.Log;
+
+/**
+ * @goal test-unused
+ * @aggregator
+ *
+ * @author jdcasey
+ */
+public class TestPlugin
+    implements Mojo
+{
+
+    private Log log;
+
+    public void execute()
+        throws MojoExecutionException, MojoFailureException
+    {
+        throw new MojoFailureException( this, "This mojo will always fail.", "This mojo is programmed to fail at all times, to express certain error-reporting functions." );
+    }
+
+    public Log getLog()
+    {
+        return log;
+    }
+
+    public void setLog( Log log )
+    {
+        this.log = log;
+    }
+
+}

maven-embedder/src/test/java/org/apache/maven/error/ErrorReporterPointcutTest.java

@@ -203,6 +203,15 @@ public class ErrorReporterPointcutTest
     private void reportExceptions( MavenExecutionResult result,
                                    File basedir )
     {
+        reportExceptions( result, basedir, false );
+    }
+
+    private void reportExceptions( MavenExecutionResult result,
+                                   File basedir,
+                                   boolean expectExceptions )
+    {
+        assertTrue( !expectExceptions || result.hasExceptions() );
+
         StringWriter writer = new StringWriter();
         PrintWriter pWriter = new PrintWriter( writer );
 
@@ -217,7 +226,14 @@ public class ErrorReporterPointcutTest
             error.printStackTrace( pWriter );
         }
 
-        fail( writer.toString() );
+        if ( expectExceptions )
+        {
+            fail( writer.toString() );
+        }
+        else
+        {
+            System.out.println( writer.toString() );
+        }
     }
 
     public void testReportErrorResolvingExtensionDirectDependencies()
@@ -1455,4 +1471,38 @@ public class ErrorReporterPointcutTest
         reporterCtl.verify();
     }
 
+    public void testReportInvalidPluginForDirectInvocation()
+        throws IOException
+    {
+        File projectDir = prepareProjectDir( "missing-direct-invoke-mojo" );
+
+        File plugin = new File( projectDir, "plugin" );
+
+        buildTestAccessory( plugin );
+
+        Settings settings = new Settings();
+        settings.addPluginGroup( "org.apache.maven.errortest" );
+
+        reporter.reportInvalidPluginForDirectInvocation( null, null, null, null );
+        reporterCtl.setMatcher( MockControl.ALWAYS_MATCHER );
+        reporterCtl.setVoidCallable();
+
+        reporterCtl.replay();
+
+        MavenExecutionRequest request = new DefaultMavenExecutionRequest().setBaseDirectory( projectDir )
+                                                                          .setSettings( settings )
+                                                                          .setShowErrors( true )
+                                                                          .setErrorReporter( reporter )
+                                                                          .setGoals( Arrays.asList( new String[] {
+                                                                              "missing-direct-invoke-mojo:test"
+                                                                          } ) );
+
+        MavenExecutionResult result = maven.execute( request );
+
+        assertTrue( result.hasExceptions() );
+        reportExceptions( result, projectDir );
+
+        reporterCtl.verify();
+    }
+
 }