* Some fixes to the command-line launcher:

* Was not exiting with non-zero exit code when the build failed.
  * Fix the error reporting so that the entire exception chain is reported.
  * Only print out stack traces when in verbose or debug mode.
* Wrap all exceptions thrown during task initialisation and execution with
  a general 'task failed' exception.
* Another <property> testcase.


git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@271802 13f79535-47bb-0310-9956-ffa450edef68

proposal/myrmidon/etc/testcases/org/apache/antlib/core/property.ant

@@ -41,4 +41,13 @@
             <property-test-type value="value 2"/>
         </property>
     </target>
+
+    <!-- Test setting the value more than once -->
+    <target name="too-many-values3">
+        <property name="some-prop">
+            <property-test-type value="value 1"/>
+            <property-test-type value="value 2"/>
+            <property-test-type value="value 3"/>
+        </property>
+    </target>
 </project>

proposal/myrmidon/src/java/org/apache/myrmidon/components/executor/AspectAwareExecutor.java

@@ -75,30 +75,41 @@ public class AspectAwareExecutor
                               final ExecutionFrame frame )
         throws TaskException
     {
-        Configuration taskModel = getAspectManager().preCreate( model );
-        taskModel = prepareAspects( taskModel );
-
-        final String taskName = taskModel.getName();
-        debug( "creating.notice", taskName );
-        final Task task = createTask( taskName, frame );
-        getAspectManager().postCreate( task );
-
-        debug( "logger.notice", taskName );
-        final Logger logger = frame.getLogger();
-        getAspectManager().preLogEnabled( logger );
-        doLogEnabled( task, taskModel, logger );
-
-        debug( "contextualizing.notice", taskName );
-        doContextualize( task, taskModel, frame.getContext() );
-
-        debug( "configuring.notice", taskName );
-        getAspectManager().preConfigure( taskModel );
-        doConfigure( task, taskModel, frame.getContext() );
-
-        debug( "executing.notice", taskName );
-        getAspectManager().preExecute();
-        doExecute( taskModel, task );
-        getAspectManager().preDestroy();
+        try
+        {
+            Configuration taskModel = getAspectManager().preCreate( model );
+            taskModel = prepareAspects( taskModel );
+
+            final String taskName = taskModel.getName();
+            debug( "creating.notice", taskName );
+            final Task task = doCreateTask( taskName, frame );
+            getAspectManager().postCreate( task );
+
+            debug( "logger.notice", taskName );
+            final Logger logger = frame.getLogger();
+            getAspectManager().preLogEnabled( logger );
+            doLogEnabled( task, taskModel, logger );
+
+            debug( "contextualizing.notice", taskName );
+            doContextualize( task, taskModel, frame.getContext() );
+
+            debug( "configuring.notice", taskName );
+            getAspectManager().preConfigure( taskModel );
+            doConfigure( task, taskModel, frame.getContext() );
+
+            debug( "executing.notice", taskName );
+            getAspectManager().preExecute();
+            doExecute( taskModel, task );
+            getAspectManager().preDestroy();
+        }
+        catch( Exception e )
+        {
+            // Wrap in generic error message
+            final String message = REZ.getString( "execute.error",
+                                                  model.getName(),
+                                                  model.getLocation() );
+            throw new TaskException( message, e );
+        }
     }
 
     protected void doExecute( final Configuration taskModel, final Task task )

proposal/myrmidon/src/java/org/apache/myrmidon/components/executor/DefaultExecutor.java

@@ -10,6 +10,7 @@ package org.apache.myrmidon.components.executor;
 import org.apache.avalon.excalibur.i18n.ResourceManager;
 import org.apache.avalon.excalibur.i18n.Resources;
 import org.apache.avalon.framework.configuration.Configuration;
+import org.apache.avalon.framework.configuration.ConfigurationException;
 import org.apache.avalon.framework.logger.AbstractLogEnabled;
 import org.apache.avalon.framework.logger.LogEnabled;
 import org.apache.avalon.framework.logger.Logger;
@@ -52,24 +53,36 @@ public class DefaultExecutor
         m_configurer = (Configurer)serviceManager.lookup( Configurer.ROLE );
     }
 
+    /**
+     * Executes a task.
+     */
     public void execute( final Configuration taskModel, final ExecutionFrame frame )
         throws TaskException
     {
         final String taskName = taskModel.getName();
-        debug( "creating.notice", taskName );
-        final Task task = createTask( taskName, frame );
+        try
+        {
+            debug( "creating.notice", taskName );
+            final Task task = doCreateTask( taskName, frame );
 
-        debug( "logger.notice", taskName );
-        doLogEnabled( task, taskModel, frame.getLogger() );
+            debug( "logger.notice", taskName );
+            doLogEnabled( task, taskModel, frame.getLogger() );
 
-        debug( "contextualizing.notice", taskName );
-        doContextualize( task, taskModel, frame.getContext() );
+            debug( "contextualizing.notice", taskName );
+            doContextualize( task, taskModel, frame.getContext() );
 
-        debug( "configuring.notice", taskName );
-        doConfigure( task, taskModel, frame.getContext() );
+            debug( "configuring.notice", taskName );
+            doConfigure( task, taskModel, frame.getContext() );
 
-        debug( "executing.notice", taskName );
-        task.execute();
+            debug( "executing.notice", taskName );
+            task.execute();
+        }
+        catch( Exception e )
+        {
+            // Wrap in generic error message
+            final String message = REZ.getString( "execute.error", taskName, taskModel.getLocation() );
+            throw new TaskException( message, e );
+        }
     }
 
     protected final void debug( final String key, final String taskName )
@@ -84,7 +97,7 @@ public class DefaultExecutor
     /**
      * Creates a task instance.
      */
-    protected final Task createTask( final String name, final ExecutionFrame frame )
+    protected final Task doCreateTask( final String name, final ExecutionFrame frame )
         throws TaskException
     {
         try
@@ -94,7 +107,7 @@ public class DefaultExecutor
         }
         catch( final TypeException te )
         {
-            final String message = REZ.getString( "no-create.error", name );
+            final String message = REZ.getString( "create.error", name );
             throw new TaskException( message, te );
         }
     }
@@ -105,20 +118,9 @@ public class DefaultExecutor
     protected final void doConfigure( final Task task,
                                       final Configuration taskModel,
                                       final TaskContext taskContext )
-        throws TaskException
+        throws ConfigurationException
     {
-        try
-        {
-            m_configurer.configure( task, taskModel, taskContext );
-        }
-        catch( final Throwable throwable )
-        {
-            final String message =
-                REZ.getString( "config.error",
-                               taskModel.getName(),
-                               taskModel.getLocation() );
-            throw new TaskException( message, throwable );
-        }
+        m_configurer.configure( task, taskModel, taskContext );
     }
 
     /**
@@ -136,9 +138,7 @@ public class DefaultExecutor
         catch( final Throwable throwable )
         {
             final String message =
-                REZ.getString( "contextualize.error",
-                               taskModel.getName(),
-                               taskModel.getLocation() );
+                REZ.getString( "contextualize.error", taskModel.getName() );
             throw new TaskException( message, throwable );
         }
     }
@@ -160,9 +160,7 @@ public class DefaultExecutor
             catch( final Throwable throwable )
             {
                 final String message =
-                    REZ.getString( "logger.error",
-                                   taskModel.getName(),
-                                   taskModel.getLocation() );
+                    REZ.getString( "logger.error", taskModel.getName() );
                 throw new TaskException( message, throwable );
             }
         }

proposal/myrmidon/src/java/org/apache/myrmidon/components/executor/Resources.properties

@@ -4,10 +4,10 @@ contextualizing.notice=Contextualizing {0}.
 configuring.notice=Configuring {0}.
 executing.notice=Executing {0}.
 
-no-create.error=Could not create task "{0}".
-config.error={1}: Could not configure task "{0}".
-contextualize.error={1}: Could not set the context for task "{0}".
-logger.error={1}: Could not set the logger for task "{0}".
+create.error=Could not create task <{0}>.
+contextualize.error=Could not set the context for task <{0}>.
+logger.error=Could not set the logger for task <{0}>.
+execute.error={1}: Could not execute task <{0}>.
 
 unused-settings.error=Unused aspect settings for namespace {0} (parameterCount={1} elementCount={2}).
 dispatch-settings.notice=Dispatching Aspect Settings to namespace {0} (parameterCount={1} elementCount={2}).

proposal/myrmidon/src/java/org/apache/myrmidon/frontends/CLIMain.java

@@ -18,6 +18,7 @@ import org.apache.avalon.excalibur.cli.CLOptionDescriptor;
 import org.apache.avalon.excalibur.cli.CLUtil;
 import org.apache.avalon.excalibur.i18n.ResourceManager;
 import org.apache.avalon.excalibur.i18n.Resources;
+import org.apache.avalon.framework.CascadingException;
 import org.apache.avalon.framework.ExceptionUtil;
 import org.apache.avalon.framework.logger.AbstractLogEnabled;
 import org.apache.avalon.framework.logger.LogKitLogger;
@@ -105,6 +106,9 @@ public class CLIMain
     ///Determine whether tasks are actually executed
     private boolean m_dryRun = false;
 
+    ///Log level to use
+    private static Priority m_priority = Priority.WARN;
+
     /**
      * Main entry point called to run standard Myrmidon.
      *
@@ -120,9 +124,7 @@ public class CLIMain
         }
         catch( final Throwable throwable )
         {
-            final String message =
-                REZ.getString( "error-message", ExceptionUtil.printStackTrace( throwable ) );
-            System.err.println( message );
+            main.reportError( throwable );
             exitCode = -1;
         }
         finally
@@ -219,6 +221,7 @@ public class CLIMain
     }
 
     private boolean parseCommandLineOptions( final String[] args )
+        throws Exception
     {
         final CLOptionDescriptor[] options = createCLOptions();
         final CLArgsParser parser = new CLArgsParser( args, options );
@@ -226,8 +229,7 @@ public class CLIMain
         if( null != parser.getErrorString() )
         {
             final String message = REZ.getString( "error-message", parser.getErrorString() );
-            System.err.println( message );
-            return false;
+            throw new Exception( message );
         }
 
         final List clOptions = parser.getArguments();
@@ -254,13 +256,13 @@ public class CLIMain
                     break;
 
                 case LOG_LEVEL_OPT:
-                    m_parameters.setParameter( "log.level", option.getArgument() );
+                    m_priority = mapLogLevel( option.getArgument() );
                     break;
                 case VERBOSE_OPT:
-                    m_parameters.setParameter( "log.level", "INFO" );
+                    m_priority = Priority.INFO;
                     break;
                 case QUIET_OPT:
-                    m_parameters.setParameter( "log.level", "ERROR" );
+                    m_priority = Priority.ERROR;
                     break;
 
                 case INCREMENTAL_OPT:
@@ -322,7 +324,7 @@ public class CLIMain
 
         prepareLogging();
 
-        final File homeDir = getHomeDir();
+        checkHomeDir();
         final File buildFile = getBuildFile();
 
         //getLogger().debug( "Ant Bin Directory: " + m_binDir );
@@ -336,24 +338,35 @@ public class CLIMain
         }
 
         final Embeddor embeddor = prepareEmbeddor();
-        final ProjectListener listener = prepareListener( embeddor );
 
-        //create the project
-        final Project project =
-            embeddor.createProject( buildFile.toString(), null, m_builderParameters );
+        try
+        {
+            final ProjectListener listener = prepareListener( embeddor );
 
-        //loop over build if we are in incremental mode..
-        final boolean incremental = m_parameters.getParameterAsBoolean( "incremental", false );
-        if( !incremental )
+            //create the project
+            final Project project =
+                embeddor.createProject( buildFile.toString(), null, m_builderParameters );
+
+            //loop over build if we are in incremental mode..
+            final boolean incremental = m_parameters.getParameterAsBoolean( "incremental", false );
+            if( !incremental )
+            {
+                executeBuild( embeddor, project, listener );
+            }
+            else
+            {
+                executeIncrementalBuild( embeddor, project, listener );
+            }
+        }
+        catch( final Exception e )
         {
-            executeBuild( embeddor, project, listener );
+            final String message = REZ.getString( "build-failed.error" );
+            throw new CascadingException( message, e );
         }
-        else
+        finally
         {
-            executeIncrementalBuild( embeddor, project, listener );
+            shutdownEmbeddor( embeddor );
         }
-
-        shutdownEmbeddor( embeddor );
     }
 
     private void executeIncrementalBuild( final Embeddor embeddor,
@@ -365,7 +378,14 @@ public class CLIMain
 
         while( true )
         {
-            executeBuild( embeddor, project, listener );
+            try
+            {
+                executeBuild( embeddor, project, listener );
+            }
+            catch( final TaskException te )
+            {
+                reportError( te );
+            }
 
             final String message = REZ.getString( "repeat.notice" );
             System.out.println( message );
@@ -385,6 +405,44 @@ public class CLIMain
         }
     }
 
+    /**
+     * Builds the error message for an exception
+     */
+    private void reportError( final Throwable throwable )
+    {
+        // Build the message
+        final String message;
+        if( m_priority.isLowerOrEqual( Priority.INFO ) )
+        {
+            // Verbose mode - include the stack traces
+            message = ExceptionUtil.printStackTrace( throwable, 5, true, true );
+        }
+        else
+        {
+            // Build the message
+            final StringBuffer buffer = new StringBuffer();
+            buffer.append( throwable.getMessage() );
+            for( Throwable current = ExceptionUtil.getCause( throwable, true );
+                 current != null;
+                 current = ExceptionUtil.getCause( current, true ) )
+            {
+                final String causeMessage = REZ.getString( "cause.error", current.getMessage() );
+                buffer.append( causeMessage );
+            }
+            message = buffer.toString();
+        }
+
+        // Write the message out
+        if( getLogger() == null )
+        {
+            System.err.println( message );
+        }
+        else
+        {
+            getLogger().error( message );
+        }
+    }
+
     private void executeBuild( final Embeddor embeddor,
                                final Project project,
                                final ProjectListener listener )
@@ -416,9 +474,9 @@ public class CLIMain
         return buildFile;
     }
 
-    private File getHomeDir() throws Exception
+    private void checkHomeDir() throws Exception
     {
-        final String home = m_parameters.getParameter( "myrmidon.home", null );
+        final String home = m_parameters.getParameter( "myrmidon.home" );
         final File homeDir = ( new File( home ) ).getAbsoluteFile();
         if( !homeDir.isDirectory() )
         {
@@ -431,15 +489,12 @@ public class CLIMain
             final String message = REZ.getString( "homedir.notice", homeDir );
             getLogger().info( message );
         }
-
-        return homeDir;
     }
 
     private void prepareLogging() throws Exception
     {
         //handle logging...
-        final String logLevel = m_parameters.getParameter( "log.level", null );
-        enableLogging( new LogKitLogger( createLogger( logLevel ) ) );
+        enableLogging( new LogKitLogger( createLogger( m_priority ) ) );
     }
 
     private void shutdownEmbeddor( final Embeddor embeddor )
@@ -486,51 +541,49 @@ public class CLIMain
     private void doBuild( final Workspace workspace,
                           final Project project,
                           final ArrayList targets )
+        throws TaskException
     {
-        try
-        {
-            final int targetCount = targets.size();
+        final int targetCount = targets.size();
 
-            //if we didn't specify a target on CLI then choose default
-            if( 0 == targetCount )
-            {
-                workspace.executeProject( project, project.getDefaultTargetName() );
-            }
-            else
-            {
-                for( int i = 0; i < targetCount; i++ )
-                {
-                    workspace.executeProject( project, (String)targets.get( i ) );
-                }
-            }
+        //if we didn't specify a target on CLI then choose default
+        if( 0 == targetCount )
+        {
+            workspace.executeProject( project, project.getDefaultTargetName() );
         }
-        catch( final TaskException ae )
+        else
         {
-            final String message =
-                REZ.getString( "build-failed.error", ExceptionUtil.printStackTrace( ae, 5, true ) );
-            getLogger().error( message );
+            for( int i = 0; i < targetCount; i++ )
+            {
+                workspace.executeProject( project, (String)targets.get( i ) );
+            }
         }
     }
 
     /**
-     * Create Logger of appropriate log-level.
-     *
-     * @param logLevel the log-level
-     * @return the logger
-     * @exception Exception if an error occurs
+     * Sets the log level.
      */
-    private Logger createLogger( final String logLevel )
-        throws Exception
+    private Priority mapLogLevel( final String logLevel ) throws Exception
     {
         final String logLevelCapitalized = logLevel.toUpperCase();
         final Priority priority = Priority.getPriorityForName( logLevelCapitalized );
-
         if( !priority.getName().equals( logLevelCapitalized ) )
         {
             final String message = REZ.getString( "bad-loglevel.error", logLevel );
             throw new Exception( message );
         }
+        return priority;
+    }
 
+    /**
+     * Create Logger of appropriate log-level.
+     *
+     * @param priority the log-level
+     * @return the logger
+     * @exception Exception if an error occurs
+     */
+    private Logger createLogger( final Priority priority )
+        throws Exception
+    {
         final Logger logger = Hierarchy.getDefaultHierarchy().getLoggerFor( "myrmidon" );
 
         final PatternFormatter formatter = new PatternFormatter( PATTERN );

proposal/myrmidon/src/java/org/apache/myrmidon/frontends/Resources.properties

@@ -18,7 +18,8 @@ dry-run.opt=Do not execute tasks - just print them out.
 home-not-dir.error=myrmidon-home ({0}) is not a directory.
 bad-file.error=File {0} is not a file or doesn't exist.
 bad-loglevel.error=Unknown log level - {0}.
-build-failed.error=BUILD FAILED\nReason:\n{0}
+build-failed.error=BUILD FAILED.
+cause.error=\nReason: {0}
 
 repeat.notice=Continue ? (Enter no to stop)
 

proposal/myrmidon/src/test/org/apache/antlib/core/IfTest.java

@@ -83,11 +83,14 @@ public class IfTest
         final File projectFile = getTestResource( "if.ant" );
 
         // Check for missing condition
-        String message = REZ.getString( "if.no-condition.error" );
-        executeTargetExpectError( projectFile, "no-condition", message );
+        String[] messages = {
+            null,
+            REZ.getString( "if.no-condition.error" )
+        };
+        executeTargetExpectError( projectFile, "no-condition", messages );
 
         // Check for too many conditions
-        String[] messages =
+        messages = new String[]
         {
             null,
             null,

proposal/myrmidon/src/test/org/apache/antlib/core/PropertyTest.java

@@ -64,21 +64,31 @@ public class PropertyTest
         final File projectFile = getTestResource( "property.ant" );
 
         // Missing name
-        String message = REZ.getString( "property.no-name.error" );
-        executeTargetExpectError( projectFile, "missing-name", message );
+        String[] messages =
+        {
+            null,
+            REZ.getString( "property.no-name.error" )
+        };
+        executeTargetExpectError( projectFile, "missing-name", messages );
 
         // Missing value
-        message = REZ.getString( "property.no-value.error" );
-        executeTargetExpectError( projectFile, "missing-value", message );
+        messages = new String[]
+        {
+            null,
+            REZ.getString( "property.no-value.error" )
+        };
+        executeTargetExpectError( projectFile, "missing-value", messages );
 
         // Too many values
-        String[] messages = {
+        messages = new String[]
+        {
             null,
             null,
             REZ.getString( "property.multi-set.error" )
         };
         executeTargetExpectError( projectFile, "too-many-values1", messages );
         executeTargetExpectError( projectFile, "too-many-values2", messages );
+        executeTargetExpectError( projectFile, "too-many-values3", messages );
     }
 
 }

proposal/myrmidon/src/testcases/org/apache/antlib/core/IfTest.java

@@ -83,11 +83,14 @@ public class IfTest
         final File projectFile = getTestResource( "if.ant" );
 
         // Check for missing condition
-        String message = REZ.getString( "if.no-condition.error" );
-        executeTargetExpectError( projectFile, "no-condition", message );
+        String[] messages = {
+            null,
+            REZ.getString( "if.no-condition.error" )
+        };
+        executeTargetExpectError( projectFile, "no-condition", messages );
 
         // Check for too many conditions
-        String[] messages =
+        messages = new String[]
         {
             null,
             null,

proposal/myrmidon/src/testcases/org/apache/antlib/core/PropertyTest.java

@@ -64,21 +64,31 @@ public class PropertyTest
         final File projectFile = getTestResource( "property.ant" );
 
         // Missing name
-        String message = REZ.getString( "property.no-name.error" );
-        executeTargetExpectError( projectFile, "missing-name", message );
+        String[] messages =
+        {
+            null,
+            REZ.getString( "property.no-name.error" )
+        };
+        executeTargetExpectError( projectFile, "missing-name", messages );
 
         // Missing value
-        message = REZ.getString( "property.no-value.error" );
-        executeTargetExpectError( projectFile, "missing-value", message );
+        messages = new String[]
+        {
+            null,
+            REZ.getString( "property.no-value.error" )
+        };
+        executeTargetExpectError( projectFile, "missing-value", messages );
 
         // Too many values
-        String[] messages = {
+        messages = new String[]
+        {
             null,
             null,
             REZ.getString( "property.multi-set.error" )
         };
         executeTargetExpectError( projectFile, "too-many-values1", messages );
         executeTargetExpectError( projectFile, "too-many-values2", messages );
+        executeTargetExpectError( projectFile, "too-many-values3", messages );
     }
 
 }