More refactoring of java app execution:

* Added ExecuteJava.setIgnoreReturnCode() and executeForked() methods.

* Converted the remaining tasks from CommandlineJava to ExecuteJava.

* Removed CommandlineJava.

* Added convenience methods to Commandline and EnvironmentData.

* Made SysProperties into a static util class.  It now longer extends
  EnvironmentData, and can now handle Map (and Properties) as well as
  EnvironmentData.


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

proposal/myrmidon/build.xml

@@ -509,7 +509,7 @@ Legal:
     </target>
 
     <!-- Compiles and runs the unit tests -->
-    <target name="run-tests" depends="dist-lite" if="junit.present" description="Runs the unit tests">
+    <target name="run-tests" depends="dist-lite" if="junit.present">
         <!-- Compile the unit tests -->
         <mkdir dir="${test.classes}"/>
         <javac srcdir="src/testcases"

proposal/myrmidon/src/todo/org/apache/tools/todo/taskdefs/ANTLR.java

@@ -13,10 +13,7 @@ import java.io.FileReader;
 import java.net.URL;
 import org.apache.myrmidon.api.AbstractTask;
 import org.apache.myrmidon.api.TaskException;
-import org.apache.myrmidon.framework.Execute;
 import org.apache.tools.todo.types.Argument;
-import org.apache.tools.todo.types.Commandline;
-import org.apache.tools.todo.types.CommandlineJava;
 import org.apache.tools.todo.types.Path;
 
 /**
@@ -25,88 +22,63 @@ import org.apache.tools.todo.types.Path;
  * @author <a href="mailto:emeade@geekfarm.org">Erik Meade</a>
  * @author <a href="mailto:sbailliez@apache.org>Stephane Bailliez</a>
  */
-public class ANTLR extends AbstractTask
+public class ANTLR
+    extends AbstractTask
 {
-
-    private CommandlineJava commandline = new CommandlineJava();
-
-    /**
-     * should fork ?
-     */
-    private boolean fork = false;
-
-    /**
-     * working directory
-     */
-    private File workingdir = null;
+    private final ExecuteJava m_exe = new ExecuteJava();
 
     /**
      * where to output the result
      */
-    private File outputDirectory;
+    private File m_outputDirectory;
 
     /**
      * the file to process
      */
-    private File target;
-
-    public ANTLR()
-    {
-        commandline.setVm( "java" );
-        commandline.setClassname( "antlr.Tool" );
-    }
+    private File m_target;
 
     /**
      * The working directory of the process
      *
-     * @param d The new Dir value
+     * @param dir The new Dir value
      */
-    public void setDir( File d )
+    public void setDir( final File dir )
     {
-        this.workingdir = d;
+        m_exe.setWorkingDirectory( dir );
     }
 
-    public void setFork( boolean s )
+    public void setFork( final boolean fork )
     {
-        this.fork = s;
+        m_exe.setFork( fork );
     }
 
-    public void setOutputdirectory( File outputDirectory )
+    public void setOutputdirectory( final File outputDirectory )
     {
-        getContext().debug( "Setting output directory to: " + outputDirectory.toString() );
-        this.outputDirectory = outputDirectory;
+        m_outputDirectory = outputDirectory;
     }
 
-    public void setTarget( File target )
+    public void setTarget( final File target )
     {
-        getContext().debug( "Setting target to: " + target.toString() );
-        this.target = target;
+        m_target = target;
     }
 
     /**
      * <code>&lt;classpath&gt;</code> allows classpath to be set because a
      * directory might be given for Antlr debug...
-     *
-     * @return Description of the Returned Value
      */
-    public Path createClasspath()
+    public void addClasspath( final Path path )
     {
-        Path path1 = commandline.createClasspath();
-        final Path path = new Path();
-        path1.addPath( path );
-        return path;
+        m_exe.getClassPath().addPath( path );
     }
 
     /**
      * Create a new JVM argument. Ignored if no JVM is forked.
      *
-     * @return create a new JVM argument so that any argument can be passed to
-     *      the JVM.
      * @see #setFork(boolean)
      */
     public void addJvmarg( final Argument argument )
     {
-        commandline.addVmArgument( argument );
+        m_exe.getVmArguments().addArgument( argument );
     }
 
     public void execute()
@@ -119,24 +91,18 @@ public class ANTLR extends AbstractTask
         validateAttributes();
 
         //TODO: use ANTLR to parse the grammer file to do this.
-        if( target.lastModified() > getGeneratedFile().lastModified() )
+        if( m_target.lastModified() <= getGeneratedFile().lastModified() )
         {
-            commandline.addArgument( "-o" );
-            commandline.addArgument( outputDirectory.toString() );
-            commandline.addArgument( target.toString() );
-
-            if( fork )
-            {
-                run( commandline );
-            }
-            else
-            {
-                ExecuteJava exe = new ExecuteJava();
-                exe.setJavaCommand( commandline.getJavaCommand() );
-                exe.setClasspath( commandline.getClasspath() );
-                exe.execute();
-            }
+            return;
         }
+
+        m_exe.setClassName( "antlr.Tool" );
+
+        m_exe.getArguments().addArgument( "-o" );
+        m_exe.getArguments().addArgument( m_outputDirectory );
+        m_exe.getArguments().addArgument( m_target );
+
+        m_exe.execute( getContext() );
     }
 
     /**
@@ -148,7 +114,7 @@ public class ANTLR extends AbstractTask
      *
      * @param resource The feature to be added to the ClasspathEntry attribute
      */
-    protected void addClasspathEntry( String resource )
+    protected void addClasspathEntry( final String resource )
     {
         URL url = getClass().getResource( resource );
         if( url != null )
@@ -159,14 +125,14 @@ public class ANTLR extends AbstractTask
                 int pling = u.indexOf( "!" );
                 String jarName = u.substring( 9, pling );
                 getContext().debug( "Implicitly adding " + jarName + " to classpath" );
-                createClasspath().setLocation( new File( ( new File( jarName ) ).getAbsolutePath() ) );
+                m_exe.getClassPath().addLocation( new File( jarName ) );
             }
             else if( u.startsWith( "file:" ) )
             {
                 int tail = u.indexOf( resource );
                 String dirName = u.substring( 5, tail );
                 getContext().debug( "Implicitly adding " + dirName + " to classpath" );
-                createClasspath().setLocation( new File( ( new File( dirName ) ).getAbsolutePath() ) );
+                m_exe.getClassPath().addLocation( new File( dirName ) );
             }
             else
             {
@@ -185,7 +151,7 @@ public class ANTLR extends AbstractTask
         String generatedFileName = null;
         try
         {
-            BufferedReader in = new BufferedReader( new FileReader( target ) );
+            BufferedReader in = new BufferedReader( new FileReader( m_target ) );
             String line;
             while( ( line = in.readLine() ) != null )
             {
@@ -206,40 +172,25 @@ public class ANTLR extends AbstractTask
         {
             throw new TaskException( "Unable to determine generated class" );
         }
-        return new File( outputDirectory, generatedFileName + ".java" );
-    }
-
-    /**
-     * execute in a forked VM
-     *
-     * @param command Description of Parameter
-     * @exception org.apache.myrmidon.api.TaskException Description of Exception
-     */
-    private void run( final Commandline command )
-        throws TaskException
-    {
-        final Execute exe = new Execute();
-        exe.setWorkingDirectory( workingdir );
-        exe.setCommandline( command );
-        exe.execute( getContext() );
+        return new File( m_outputDirectory, generatedFileName + ".java" );
     }
 
     private void validateAttributes()
         throws TaskException
     {
-        if( target == null || !target.isFile() )
+        if( m_target == null || !m_target.isFile() )
         {
-            throw new TaskException( "Invalid target: " + target );
+            throw new TaskException( "Invalid target: " + m_target );
         }
 
         // if no output directory is specified, used the target's directory
-        if( outputDirectory == null )
+        if( m_outputDirectory == null )
         {
-            setOutputdirectory( new File( target.getParent() ) );
+            m_outputDirectory = m_target.getParentFile();
         }
-        if( !outputDirectory.isDirectory() )
+        if( !m_outputDirectory.isDirectory() )
         {
-            throw new TaskException( "Invalid output directory: " + outputDirectory );
+            throw new TaskException( "Invalid output directory: " + m_outputDirectory );
         }
     }
 }

proposal/myrmidon/src/todo/org/apache/tools/todo/taskdefs/ExecuteJava.java

@@ -18,6 +18,7 @@ import org.apache.myrmidon.framework.Execute;
 import org.apache.tools.todo.types.Commandline;
 import org.apache.tools.todo.types.Path;
 import org.apache.tools.todo.types.PathUtil;
+import org.apache.tools.todo.types.EnvironmentData;
 import org.apache.tools.todo.types.SysProperties;
 import org.apache.tools.todo.util.FileUtils;
 import org.apache.aut.nativelib.Os;
@@ -32,7 +33,7 @@ import org.apache.aut.nativelib.Os;
 public class ExecuteJava
 {
     private final Path m_classPath = new Path();
-    private final SysProperties m_sysProperties = new SysProperties();
+    private final EnvironmentData m_sysProperties = new EnvironmentData();
     private final Commandline m_args = new Commandline();
     private final Commandline m_vmArgs = new Commandline();
     private boolean m_fork;
@@ -41,6 +42,7 @@ public class ExecuteJava
     private String m_jvm;
     private String m_className;
     private String m_maxMemory;
+    private boolean m_ignoreReturnCode;
 
     public void setClassName( final String className )
     {
@@ -57,6 +59,11 @@ public class ExecuteJava
         m_fork = fork;
     }
 
+    /**
+     * Sets the max memory to use when running the application in a forked JVM.
+     *
+     * @param maxMemory the maximum memory, or null for the default.
+     */
     public void setMaxMemory( final String maxMemory )
     {
         m_maxMemory = maxMemory;
@@ -67,6 +74,17 @@ public class ExecuteJava
         m_workingDirectory = workingDirectory;
     }
 
+    public void setIgnoreReturnCode( boolean ignore )
+    {
+        m_ignoreReturnCode = ignore;
+    }
+
+    /**
+     * Sets the JVM executable to use to run the application in a forked JVM.
+     *
+     * @param jvm the path to the JVM executable, or null to use the default
+     *            JVM executable.
+     */
     public void setJvm( final String jvm )
     {
         m_jvm = jvm;
@@ -77,7 +95,7 @@ public class ExecuteJava
         return m_classPath;
     }
 
-    public SysProperties getSysProperties()
+    public EnvironmentData getSysProperties()
     {
         return m_sysProperties;
     }
@@ -92,54 +110,49 @@ public class ExecuteJava
         return m_vmArgs;
     }
 
+    /**
+     * Executes the application.
+     */
     public void execute( final TaskContext context )
         throws TaskException
     {
-        // Validate
-        if( m_className != null && m_jar != null )
-        {
-            throw new TaskException( "Only one of Classname and Jar can be set." );
-        }
-        else if( m_className == null && m_jar == null )
-        {
-            throw new TaskException( "Classname must not be null." );
-        }
-        if( ! m_fork )
-        {
-            if( m_jar != null )
-            {
-                throw new TaskException( "Cannot execute a jar in non-forked mode." );
-            }
-            if( m_vmArgs.size() > 0 )
-            {
-                context.warn( "JVM args ignored when same JVM is used." );
-            }
-            if( m_workingDirectory != null )
-            {
-                context.warn( "Working directory ignored when same JVM is used." );
-            }
-            if( m_sysProperties.size() > 0 )
-            {
-                context.warn( "System properties ignored when same JVM is used." );
-            }
-        }
-
         if( m_fork )
         {
-            execForked( context );
+            executeForked( context );
         }
         else
         {
-            execNonForked( context );
+            executeNonForked( context );
         }
     }
 
     /**
-     * Executes the app in this JVM.
+     * Executes the application in this JVM.
      */
-    private void execNonForked( final TaskContext context )
+    public void executeNonForked( final TaskContext context )
         throws TaskException
     {
+        if( m_className == null )
+        {
+            throw new TaskException( "Classname must not be null." );
+        }
+        if( m_jar != null )
+        {
+            throw new TaskException( "Cannot execute a jar in non-forked mode." );
+        }
+        if( m_vmArgs.size() > 0 )
+        {
+            context.warn( "JVM args ignored when same JVM is used." );
+        }
+        if( m_workingDirectory != null )
+        {
+            context.warn( "Working directory ignored when same JVM is used." );
+        }
+        if( m_sysProperties.size() > 0 )
+        {
+            context.warn( "System properties ignored when same JVM is used." );
+        }
+
         final String[] args = m_args.getArguments();
         context.debug( "Running in same VM: " + m_className + " " + FileUtils.formatCommandLine( args ) );
 
@@ -182,13 +195,24 @@ public class ExecuteJava
     }
 
     /**
-     * Executes the given classname with the given arguments in a separate VM.
+     * Executes the application in a separate JVM.
      */
-    private void execForked( final TaskContext context )
+    public int executeForked( final TaskContext context )
         throws TaskException
     {
+        // Validate
+        if( m_className != null && m_jar != null )
+        {
+            throw new TaskException( "Only one of Classname and Jar can be set." );
+        }
+        else if( m_className == null && m_jar == null )
+        {
+            throw new TaskException( "Classname must not be null." );
+        }
+
         final Execute exe = new Execute();
         exe.setWorkingDirectory( m_workingDirectory );
+        exe.setIgnoreReturnCode( m_ignoreReturnCode );
 
         // Setup the command line
         final Commandline command = exe.getCommandline();
@@ -204,8 +228,7 @@ public class ExecuteJava
         }
 
         // JVM arguments
-        final String[] vmArgs = m_vmArgs.getArguments();
-        command.addArguments( vmArgs );
+        command.addArguments( m_vmArgs );
 
         // Max memory size
         if( m_maxMemory != null )
@@ -214,7 +237,7 @@ public class ExecuteJava
         }
 
         // System properties
-        final String[] props = m_sysProperties.getJavaVariables();
+        final String[] props = SysProperties.getJavaVariables( m_sysProperties );
         command.addArguments( props );
 
         // Classpath
@@ -236,11 +259,10 @@ public class ExecuteJava
         }
 
         // Java app arguments
-        final String[] args = m_args.getArguments();
-        command.addArguments( args );
+        command.addArguments( m_args );
 
         // Execute
-        exe.execute( context );
+        return exe.execute( context );
     }
 
     /**

proposal/myrmidon/src/todo/org/apache/tools/todo/taskdefs/javacc/JJTree.java

@@ -10,13 +10,9 @@ package org.apache.tools.todo.taskdefs.javacc;
 import java.io.File;
 import java.util.Enumeration;
 import java.util.Hashtable;
-import org.apache.aut.nativelib.ExecManager;
 import org.apache.myrmidon.api.AbstractTask;
 import org.apache.myrmidon.api.TaskException;
-import org.apache.myrmidon.api.TaskContext;
-import org.apache.myrmidon.framework.Execute;
-import org.apache.tools.todo.types.Commandline;
-import org.apache.tools.todo.types.CommandlineJava;
+import org.apache.tools.todo.taskdefs.ExecuteJava;
 import org.apache.tools.todo.types.Path;
 import org.apache.tools.todo.types.PathUtil;
 
@@ -51,14 +47,6 @@ public class JJTree
     private File target = null;
     private File javaccHome = null;
 
-    private CommandlineJava cmdl = new CommandlineJava();
-
-    public JJTree()
-    {
-        cmdl.setVm( "java" );
-        cmdl.setClassname( "COM.sun.labs.jjtree.Main" );
-    }
-
     public void setBuildnodefiles( boolean buildNodeFiles )
     {
         optionalAttrs.put( BUILD_NODE_FILES, new Boolean( buildNodeFiles ) );
@@ -132,6 +120,8 @@ public class JJTree
     public void execute()
         throws TaskException
     {
+        final ExecuteJava exe = new ExecuteJava();
+        exe.setClassName( "COM.sun.labs.jjtree.Main" );
 
         // load command line with optional attributes
         Enumeration iter = optionalAttrs.keys();
@@ -139,7 +129,7 @@ public class JJTree
         {
             String name = (String)iter.nextElement();
             Object value = optionalAttrs.get( name );
-            cmdl.addArgument( "-" + name + ":" + value.toString() );
+            exe.getArguments().addArgument( "-" + name + ":" + value.toString() );
         }
 
         if( target == null || !target.isFile() )
@@ -150,15 +140,16 @@ public class JJTree
         // use the directory containing the target as the output directory
         if( outputDirectory == null )
         {
-            outputDirectory = new File( target.getParent() );
+            outputDirectory = target.getParentFile();
         }
         if( !outputDirectory.isDirectory() )
         {
             throw new TaskException( "'outputdirectory' " + outputDirectory + " is not a directory." );
         }
+
         // convert backslashes to slashes, otherwise jjtree will put this as
         // comments and this seems to confuse javacc
-        cmdl.addArgument( "-OUTPUT_DIRECTORY:" + outputDirectory.getAbsolutePath().replace( '\\', '/' ) );
+        exe.getArguments().addArgument( "-OUTPUT_DIRECTORY:" + outputDirectory.getAbsolutePath().replace( '\\', '/' ) );
 
         String targetName = target.getName();
         final File javaFile = new File( outputDirectory,
@@ -168,22 +159,19 @@ public class JJTree
             getContext().info( "Target is already built - skipping (" + target + ")" );
             return;
         }
-        cmdl.addArgument( target.getAbsolutePath() );
+        exe.getArguments().addArgument( target );
 
         if( javaccHome == null || !javaccHome.isDirectory() )
         {
             throw new TaskException( "Javacchome not set." );
         }
-        final Path classpath = cmdl.createClasspath();
+        final Path classpath = exe.getClassPath();
         classpath.addLocation( new File( javaccHome, "JavaCC.zip" ) );
         PathUtil.addJavaRuntime( classpath );
 
-        cmdl.addVmArgument( "-mx140M" );
-        cmdl.addVmArgument( "-Dinstall.root=" + javaccHome.getAbsolutePath() );
+        exe.setMaxMemory( "140M" );
+        exe.getSysProperties().addVariable( "install.root", javaccHome.getAbsolutePath() );
 
-        final Execute exe = new Execute();
-        getContext().debug( cmdl.toString() );
-        exe.setCommandline( new Commandline( cmdl.getCommandline() ) );
-        exe.execute( getContext() );
+        exe.executeForked( getContext() );
     }
 }

proposal/myrmidon/src/todo/org/apache/tools/todo/taskdefs/javacc/JavaCC.java

@@ -10,13 +10,9 @@ package org.apache.tools.todo.taskdefs.javacc;
 import java.io.File;
 import java.util.Enumeration;
 import java.util.Hashtable;
-import org.apache.aut.nativelib.ExecManager;
 import org.apache.myrmidon.api.AbstractTask;
 import org.apache.myrmidon.api.TaskException;
-import org.apache.myrmidon.api.TaskContext;
-import org.apache.myrmidon.framework.Execute;
-import org.apache.tools.todo.types.Commandline;
-import org.apache.tools.todo.types.CommandlineJava;
+import org.apache.tools.todo.taskdefs.ExecuteJava;
 import org.apache.tools.todo.types.Path;
 import org.apache.tools.todo.types.PathUtil;
 
@@ -61,14 +57,6 @@ public class JavaCC
     private File target = null;
     private File javaccHome = null;
 
-    private CommandlineJava cmdl = new CommandlineJava();
-
-    public JavaCC()
-    {
-        cmdl.setVm( "java" );
-        cmdl.setClassname( "COM.sun.labs.javacc.Main" );
-    }
-
     public void setBuildparser( boolean buildParser )
     {
         optionalAttrs.put( BUILD_PARSER, new Boolean( buildParser ) );
@@ -187,16 +175,6 @@ public class JavaCC
     public void execute()
         throws TaskException
     {
-
-        // load command line with optional attributes
-        Enumeration iter = optionalAttrs.keys();
-        while( iter.hasMoreElements() )
-        {
-            String name = (String)iter.nextElement();
-            Object value = optionalAttrs.get( name );
-            cmdl.addArgument( "-" + name + ":" + value.toString() );
-        }
-
         // check the target is a file
         if( target == null || !target.isFile() )
         {
@@ -206,13 +184,17 @@ public class JavaCC
         // use the directory containing the target as the output directory
         if( outputDirectory == null )
         {
-            outputDirectory = new File( target.getParent() );
+            outputDirectory = target.getParentFile();
         }
-        else if( !outputDirectory.isDirectory() )
+        if( !outputDirectory.isDirectory() )
         {
             throw new TaskException( "Outputdir not a directory." );
         }
-        cmdl.addArgument( "-OUTPUT_DIRECTORY:" + outputDirectory.getAbsolutePath() );
+
+        if( javaccHome == null || !javaccHome.isDirectory() )
+        {
+            throw new TaskException( "Javacchome not set." );
+        }
 
         // determine if the generated java file is up-to-date
         final File javaFile = getOutputJavaFile( outputDirectory, target );
@@ -221,30 +203,31 @@ public class JavaCC
             getContext().debug( "Target is already built - skipping (" + target + ")" );
             return;
         }
-        cmdl.addArgument( target.getAbsolutePath() );
 
-        if( javaccHome == null || !javaccHome.isDirectory() )
+        ExecuteJava exe = new ExecuteJava();
+        exe.setClassName( "COM.sun.labs.javacc.Main" );
+
+        // load command line with optional attributes
+        Enumeration iter = optionalAttrs.keys();
+        while( iter.hasMoreElements() )
         {
-            throw new TaskException( "Javacchome not set." );
+            String name = (String)iter.nextElement();
+            Object value = optionalAttrs.get( name );
+            exe.getArguments().addArgument( "-" + name + ":" + value.toString() );
         }
-        final Path classpath = cmdl.createClasspath();
+
+        exe.getArguments().addArgument( "-OUTPUT_DIRECTORY:" + outputDirectory.getAbsolutePath() );
+
+        exe.getArguments().addArgument( target );
+
+        final Path classpath = exe.getClassPath();
         classpath.addLocation( new File( javaccHome, "JavaCC.zip" ) );
         PathUtil.addJavaRuntime( classpath );
 
-        cmdl.addVmArgument( "-mx140M" );
-        cmdl.addVmArgument( "-Dinstall.root=" + javaccHome.getAbsolutePath() );
+        exe.setMaxMemory( "140M" );
+        exe.getSysProperties().addVariable( "install.root", javaccHome.getAbsolutePath() );
 
-        runCommand( cmdl );
-    }
-
-    private void runCommand( final CommandlineJava cmdline )
-        throws TaskException
-    {
-        getContext().debug( cmdline.toString() );
-        final Execute exe = new Execute();
-        final String[] commandline = cmdline.getCommandline();
-        exe.setCommandline( new Commandline( commandline ) );
-        exe.execute( getContext() );
+        exe.executeForked( getContext() );
     }
 
     /**

proposal/myrmidon/src/todo/org/apache/tools/todo/taskdefs/jdepend/JDependTask.java

@@ -13,12 +13,8 @@ import java.io.IOException;
 import java.io.PrintWriter;
 import org.apache.myrmidon.api.AbstractTask;
 import org.apache.myrmidon.api.TaskException;
-import org.apache.myrmidon.framework.Execute;
-import org.apache.tools.todo.types.Commandline;
-import org.apache.tools.todo.types.CommandlineJava;
+import org.apache.tools.todo.taskdefs.ExecuteJava;
 import org.apache.tools.todo.types.Path;
-import org.apache.tools.todo.types.PathUtil;
-import org.apache.tools.todo.util.FileUtils;
 
 /**
  * Ant task to run JDepend tests. <p>
@@ -38,7 +34,7 @@ public class JDependTask
     private boolean m_fork;
     private String m_jvm;
     private String m_format = "text";
-    private Path m_compileClasspath;
+    private Path m_compileClasspath = new Path();
     private File m_dir;
     private File m_outputFile;
     private Path m_sourcesPath;
@@ -49,14 +45,7 @@ public class JDependTask
     public void setClasspath( final Path classpath )
         throws TaskException
     {
-        if( m_compileClasspath == null )
-        {
-            m_compileClasspath = classpath;
-        }
-        else
-        {
-            m_compileClasspath.addPath( classpath );
-        }
+        addClasspath( classpath );
     }
 
     /**
@@ -112,20 +101,11 @@ public class JDependTask
     }
 
     /**
-     * Maybe creates a nested classpath element.
-     *
-     * @return Description of the Returned Value
+     * Adds a nested classpath element.
      */
-    public Path createClasspath()
+    public void addClasspath( final Path path )
     {
-        if( m_compileClasspath == null )
-        {
-            m_compileClasspath = new Path();
-        }
-        Path path1 = m_compileClasspath;
-        final Path path = new Path();
-        path1.addPath( path );
-        return path;
+        m_compileClasspath.addPath( path );
     }
 
     /**
@@ -146,22 +126,6 @@ public class JDependTask
     public void execute()
         throws TaskException
     {
-        final CommandlineJava commandline = new CommandlineJava();
-
-        if( "text".equals( m_format ) )
-        {
-            commandline.setClassname( "jdepend.textui.JDepend" );
-        }
-        else if( "xml".equals( m_format ) )
-        {
-            commandline.setClassname( "jdepend.xmlui.JDepend" );
-        }
-
-        if( m_jvm != null )
-        {
-            commandline.setVm( m_jvm );
-        }
-
         if( m_sourcesPath == null )
         {
             throw new TaskException( "Missing Sourcepath required argument" );
@@ -170,11 +134,11 @@ public class JDependTask
         // execute the test and get the return code
         if( !m_fork )
         {
-            executeInVM( commandline );
+            executeInVM();
         }
         else
         {
-            executeAsForked( commandline );
+            executeAsForked();
         }
     }
 
@@ -185,28 +149,35 @@ public class JDependTask
      * killedProcess()</tt> method of the watchdog class.
      */
     // JL: comment extracted from JUnitTask (and slightly modified)
-    private void executeAsForked( final CommandlineJava commandline )
+    private void executeAsForked()
         throws TaskException
     {
-        // if not set, auto-create the ClassPath from the project
-        createClasspath();
+        final ExecuteJava exe = new ExecuteJava();
+        exe.setWorkingDirectory( m_dir );
 
-        // not sure whether this test is needed but cost nothing to put.
-        // hope it will be reviewed by anybody competent
-        final String compileClasspath = PathUtil.formatPath( m_compileClasspath );
-        if( compileClasspath.length() > 0 )
+        if( "text".equals( m_format ) )
+        {
+            exe.setClassName( "jdepend.textui.JDepend" );
+        }
+        else
+        {
+            exe.setClassName( "jdepend.xmlui.JDepend" );
+        }
+
+        if( m_jvm != null )
         {
-            commandline.addVmArgument( "-classpath" );
-            commandline.addVmArgument( compileClasspath );
+            exe.setJvm( m_jvm );
         }
 
+        exe.getClassPath().addPath( m_compileClasspath );
+
         if( m_outputFile != null )
         {
             // having a space between the file and its path causes commandline to add quotes "
             // around the argument thus making JDepend not taking it into account. Thus we split it in two
-            commandline.addArgument( "-file" );
-            commandline.addArgument( m_outputFile.getPath() );
-            // we have to find a cleaner way to put this output
+            exe.getArguments().addArgument( "-file" );
+            exe.getArguments().addArgument( m_outputFile );
+            getContext().info( "Output to be stored in " + m_outputFile.getPath() );
         }
 
         final String[] elements = m_sourcesPath.list();
@@ -219,20 +190,10 @@ public class JDependTask
             {
                 throw new TaskException( "\"" + f.getPath() + "\" does not represent a valid directory. JDepend would fail." );
             }
-            commandline.addArgument( f.getPath() );
+            exe.getArguments().addArgument( f );
         }
 
-        final Execute exe = new Execute();
-
-        final String[] commandline1 = commandline.getCommandline();
-        exe.setCommandline( new Commandline( commandline1 ) );
-        exe.setWorkingDirectory( m_dir );
-
-        if( m_outputFile != null )
-        {
-            getContext().info( "Output to be stored in " + m_outputFile.getPath() );
-        }
-        exe.execute( getContext() );
+        exe.executeForked( getContext() );
     }
 
 
@@ -243,11 +204,8 @@ public class JDependTask
 
     /**
      * Execute inside VM.
-     *
-     * @param commandline Description of Parameter
-     * @exception TaskException Description of Exception
      */
-    private void executeInVM( final CommandlineJava commandline )
+    private void executeInVM()
         throws TaskException
     {
         jdepend.textui.JDepend jdepend;

proposal/myrmidon/src/todo/org/apache/tools/todo/taskdefs/jsp/WLJspc.java

@@ -218,11 +218,10 @@ public class WLJspc extends MatchingTask
             args[ j + 2 ] = sourceDirectory + File.separator + (String)filesToDo.get( i );
 
             ExecuteJava helperTask = new ExecuteJava();
-            helperTask.setFork( true );
             helperTask.setClassName( "weblogic.jspc" );
             helperTask.getArguments().addArguments( args );
             helperTask.getClassPath().addPath( compileClasspath );
-            helperTask.execute( getContext() );
+            helperTask.executeForked( getContext() );
         }
     }
 

proposal/myrmidon/src/todo/org/apache/tools/todo/taskdefs/junit/JUnitTask.java

@@ -14,22 +14,20 @@ import java.net.URL;
 import java.net.URLClassLoader;
 import java.util.ArrayList;
 import java.util.Iterator;
-import java.util.Map;
 import java.util.Properties;
 import java.util.Random;
-import org.apache.aut.nativelib.ExecManager;
 import org.apache.myrmidon.api.AbstractTask;
 import org.apache.myrmidon.api.TaskContext;
 import org.apache.myrmidon.api.TaskException;
-import org.apache.myrmidon.framework.Execute;
+import org.apache.tools.todo.taskdefs.ExecuteJava;
 import org.apache.tools.todo.types.Argument;
-import org.apache.tools.todo.types.Commandline;
-import org.apache.tools.todo.types.CommandlineJava;
 import org.apache.tools.todo.types.EnumeratedAttribute;
 import org.apache.tools.todo.types.EnvironmentVariable;
 import org.apache.tools.todo.types.Path;
 import org.apache.tools.todo.types.PathUtil;
 import org.apache.tools.todo.types.SysProperties;
+import org.apache.tools.todo.types.Commandline;
+import org.apache.tools.todo.types.EnvironmentData;
 
 /**
  * Ant task to run JUnit tests. <p>
@@ -102,28 +100,20 @@ import org.apache.tools.todo.types.SysProperties;
  */
 public class JUnitTask extends AbstractTask
 {
-
-    private CommandlineJava commandline = new CommandlineJava();
     private ArrayList tests = new ArrayList();
     private ArrayList batchTests = new ArrayList();
     private ArrayList formatters = new ArrayList();
-    private File dir = null;
 
     private Integer timeout = null;
     private boolean summary = false;
     private String summaryValue = "";
     private JUnitTestRunner runner = null;
-
-    /**
-     * Creates a new JUnitRunner and enables fork of a new Java VM.
-     *
-     * @exception Exception Description of Exception
-     */
-    public JUnitTask()
-        throws Exception
-    {
-        commandline.setClassname( "org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner" );
-    }
+    private File dir;
+    private String jvm;
+    private String maxMem;
+    private EnvironmentData sysProperties = new EnvironmentData();
+    private Path classPath = new Path();
+    private Commandline vmArgs = new Commandline();
 
     /**
      * The directory to invoke the VM in. Ignored if no JVM is forked.
@@ -131,7 +121,7 @@ public class JUnitTask extends AbstractTask
      * @param dir the directory to invoke the JVM from.
      * @see #setFork(boolean)
      */
-    public void setDir( File dir )
+    public void setDir( final File dir )
     {
         this.dir = dir;
     }
@@ -252,9 +242,9 @@ public class JUnitTask extends AbstractTask
      * @param value the new VM to use instead of <tt>java</tt>
      * @see #setFork(boolean)
      */
-    public void setJvm( String value )
+    public void setJvm( final String value )
     {
-        commandline.setVm( value );
+        jvm = value;
     }
 
     /**
@@ -263,9 +253,9 @@ public class JUnitTask extends AbstractTask
      * @param max the value as defined by <tt>-mx</tt> or <tt>-Xmx</tt> in the
      *      java command line options.
      */
-    public void setMaxmemory( String max )
+    public void setMaxmemory( final String max )
     {
-        commandline.addVmArgument( "-Xmx" + max );
+        maxMem = max;
     }
 
     /**
@@ -313,7 +303,7 @@ public class JUnitTask extends AbstractTask
      */
     public void addSysproperty( EnvironmentVariable sysp )
     {
-        commandline.addSysproperty( sysp );
+        sysProperties.addVariable( sysp );
     }
 
     /**
@@ -330,28 +320,18 @@ public class JUnitTask extends AbstractTask
     /**
      * Create a new set of testcases (also called ..batchtest) and add it to the
      * list.
-     *
-     * @return a new instance of a batch test.
-     * @see BatchTest
      */
-    public BatchTest createBatchTest()
+    public void addBatchTest( final BatchTest test )
     {
-        BatchTest test = new BatchTest();
         batchTests.add( test );
-        return test;
     }
 
     /**
      * <code>&lt;classpath&gt;</code> allows classpath to be set for tests.
-     *
-     * @return Description of the Returned Value
      */
-    public Path createClasspath()
+    public void addClasspath( final Path path )
     {
-        Path path1 = commandline.createClasspath();
-        final Path path = new Path();
-        path1.addPath( path );
-        return path;
+        classPath.addPath( path );
     }
 
     /**
@@ -361,7 +341,7 @@ public class JUnitTask extends AbstractTask
      */
     public void addJvmarg( final Argument argument )
     {
-        commandline.addVmArgument( argument );
+        vmArgs.addArgument( argument );
     }
 
     /**
@@ -372,6 +352,7 @@ public class JUnitTask extends AbstractTask
     public void execute()
         throws TaskException
     {
+
         /*
          * Adds the jars or directories containing Ant, this task and JUnit to the
          * classpath - this should make the forked JVM work without having to
@@ -452,14 +433,14 @@ public class JUnitTask extends AbstractTask
                 int pling = u.indexOf( "!" );
                 String jarName = u.substring( 9, pling );
                 getContext().debug( "Implicitly adding " + jarName + " to classpath" );
-                createClasspath().addLocation( new File( jarName ) );
+                classPath.addLocation( new File( jarName ) );
             }
             else if( u.startsWith( "file:" ) )
             {
                 int tail = u.indexOf( resource );
                 String dirName = u.substring( 5, tail );
                 getContext().debug( "Implicitly adding " + dirName + " to classpath" );
-                createClasspath().addLocation( new File( dirName ) );
+                classPath.addLocation( new File( dirName ) );
             }
             else
             {
@@ -484,7 +465,7 @@ public class JUnitTask extends AbstractTask
      * @param test Description of Parameter
      * @exception TaskException Description of Exception
      */
-    protected void execute( JUnitTest test )
+    protected void execute( final JUnitTest test )
         throws TaskException
     {
         // set the default values if not specified
@@ -581,17 +562,24 @@ public class JUnitTask extends AbstractTask
     private int executeAsForked( JUnitTest test )
         throws TaskException
     {
-        CommandlineJava cmd = commandline;//(CommandlineJava)commandline.clone();
-
-        cmd.setClassname( "org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner" );
-        cmd.addArgument( test.getName() );
-        cmd.addArgument( "filtertrace=" + test.getFiltertrace() );
-        cmd.addArgument( "haltOnError=" + test.getHaltonerror() );
-        cmd.addArgument( "haltOnFailure=" + test.getHaltonfailure() );
+        ExecuteJava cmd = new ExecuteJava();
+        cmd.setJvm( jvm );
+        cmd.setIgnoreReturnCode( true );
+        cmd.setWorkingDirectory( dir );
+        cmd.setMaxMemory( maxMem );
+        cmd.getClassPath().addPath( classPath );
+        cmd.getVmArguments().addArguments( vmArgs );
+        cmd.getSysProperties().addVariables( sysProperties );
+
+        cmd.setClassName( "org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner" );
+        cmd.getArguments().addArgument( test.getName() );
+        cmd.getArguments().addArgument( "filtertrace=" + test.getFiltertrace() );
+        cmd.getArguments().addArgument( "haltOnError=" + test.getHaltonerror() );
+        cmd.getArguments().addArgument( "haltOnFailure=" + test.getHaltonfailure() );
         if( summary )
         {
             getContext().info( "Running " + test.getName() );
-            cmd.addArgument( "formatter=org.apache.tools.ant.taskdefs.optional.junit.SummaryJUnitResultFormatter" );
+            cmd.getArguments().addArgument( "formatter=org.apache.tools.ant.taskdefs.optional.junit.SummaryJUnitResultFormatter" );
         }
 
         StringBuffer formatterArg = new StringBuffer( 128 );
@@ -607,20 +595,15 @@ public class JUnitTask extends AbstractTask
                 formatterArg.append( "," );
                 formatterArg.append( outFile );
             }
-            cmd.addArgument( formatterArg.toString() );
+            cmd.getArguments().addArgument( formatterArg.toString() );
             formatterArg.setLength( 0 );
         }
 
         // Create a temporary file to pass the Ant properties to the forked test
         File propsFile = new File( "junit" + ( new Random( System.currentTimeMillis() ) ).nextLong() + ".properties" );
-        cmd.addArgument( "propsfile=" + propsFile.getAbsolutePath() );
-        Map p = getContext().getProperties();
+        cmd.getArguments().addArgument( "propsfile=" + propsFile.getAbsolutePath() );
         Properties props = new Properties();
-        for( Iterator enum = p.keySet().iterator(); enum.hasNext(); )
-        {
-            final Object key = enum.next();
-            props.put( key, p.get( key ) );
-        }
+        props.putAll( getContext().getProperties() );
         try
         {
             final FileOutputStream outstream = new FileOutputStream( propsFile );
@@ -632,14 +615,9 @@ public class JUnitTask extends AbstractTask
             throw new TaskException( "Error creating temporary properties file.", ioe );
         }
 
-        final Execute exe = new Execute();
-        exe.setIgnoreReturnCode( true );
-        exe.setCommandline( new Commandline( cmd.getCommandline() ) );
-        exe.setWorkingDirectory( dir );
-
         try
         {
-            return exe.execute( getContext() );
+            return cmd.executeForked( getContext() );
         }
         finally
         {
@@ -657,25 +635,16 @@ public class JUnitTask extends AbstractTask
         throws TaskException
     {
         test.setProperties( getContext().getProperties() );
-        if( dir != null )
-        {
-            getContext().warn( "dir attribute ignored if running in the same VM" );
-        }
 
-        SysProperties sysProperties = commandline.getSystemProperties();
-        if( sysProperties != null )
-        {
-            sysProperties.setSystem();
-        }
+        SysProperties.setSystem( sysProperties );
+
         try
         {
             getContext().debug( "Using System properties " + System.getProperties() );
             ClassLoader classLoader = null;
-            Path classpath = commandline.getClasspath();
-            if( classpath != null )
+            final URL[] urls = PathUtil.toURLs( classPath );
+            if( urls.length > 0 )
             {
-                getContext().debug( "Using CLASSPATH " + classpath );
-                final URL[] urls = PathUtil.toURLs( classpath );
                 classLoader = new URLClassLoader( urls );
             }
             runner = new JUnitTestRunner( test,
@@ -715,10 +684,7 @@ public class JUnitTask extends AbstractTask
         }
         finally
         {
-            if( sysProperties != null )
-            {
-                sysProperties.restoreSystem();
-            }
+            SysProperties.restoreSystem();
         }
     }
 

proposal/myrmidon/src/todo/org/apache/tools/todo/taskdefs/metamata/AbstractMetamataTask.java

@@ -17,10 +17,8 @@ import java.util.Iterator;
 import java.util.Random;
 import org.apache.myrmidon.api.AbstractTask;
 import org.apache.myrmidon.api.TaskException;
-import org.apache.myrmidon.framework.Execute;
+import org.apache.tools.todo.taskdefs.ExecuteJava;
 import org.apache.tools.todo.types.Argument;
-import org.apache.tools.todo.types.Commandline;
-import org.apache.tools.todo.types.CommandlineJava;
 import org.apache.tools.todo.types.DirectoryScanner;
 import org.apache.tools.todo.types.FileSet;
 import org.apache.tools.todo.types.Path;
@@ -36,17 +34,10 @@ import org.apache.tools.todo.types.ScannerUtil;
 public abstract class AbstractMetamataTask
     extends AbstractTask
 {
-    /**
-     * The user classpath to be provided. It matches the -classpath of the
-     * command line. The classpath must includes both the <tt>.class</tt> and
-     * the <tt>.java</tt> files for accurate audit.
-     */
-    private Path m_classPath = new Path();
-
     /**
      * the path to the source file
      */
-    private Path m_sourcePath;
+    private Path m_sourcePath = new Path();
 
     /**
      * Metamata home directory. It will be passed as a <tt>metamata.home</tt>
@@ -58,7 +49,7 @@ public abstract class AbstractMetamataTask
     /**
      * the command line used to run MAudit
      */
-    private CommandlineJava m_cmdl = new CommandlineJava();
+    private ExecuteJava m_exe = new ExecuteJava();
 
     /**
      * the set of files to be audited
@@ -74,19 +65,14 @@ public abstract class AbstractMetamataTask
     // be set when calling scanFileSets();
     private Hashtable m_includedFiles;
 
-    public AbstractMetamataTask()
-    {
-    }
-
     /**
      * initialize the task with the classname of the task to run
      *
      * @param className Description of Parameter
      */
-    protected AbstractMetamataTask( String className )
+    protected AbstractMetamataTask( final String className )
     {
-        m_cmdl.setVm( "java" );
-        m_cmdl.setClassname( className );
+        m_exe.setClassName( className );
     }
 
     /**
@@ -116,9 +102,9 @@ public abstract class AbstractMetamataTask
      *
      * @param max The new Maxmemory value
      */
-    public void setMaxmemory( String max )
+    public void setMaxmemory( final String max )
     {
-        m_cmdl.addVmArgument( "-Xmx" + max );
+        m_exe.setMaxMemory( max );
     }
 
     /**
@@ -126,7 +112,7 @@ public abstract class AbstractMetamataTask
      */
     public void setMetamatahome( final File metamataHome )
     {
-        this.m_metamataHome = metamataHome;
+        m_metamataHome = metamataHome;
     }
 
     /**
@@ -142,7 +128,7 @@ public abstract class AbstractMetamataTask
      */
     public void addClasspath( final Path path )
     {
-        m_classPath.addPath( path );
+        m_exe.getClassPath().addPath( path );
     }
 
     /**
@@ -150,19 +136,15 @@ public abstract class AbstractMetamataTask
      */
     public void addJvmarg( final Argument argument )
     {
-        m_cmdl.addVmArgument( argument );
+        m_exe.getVmArguments().addArgument( argument );
     }
 
     /**
      * create the source path for this task
      */
-    public Path createSourcepath()
+    public void addSourcepath( final Path path )
     {
-        if( m_sourcePath == null )
-        {
-            m_sourcePath = new Path();
-        }
-        return m_sourcePath;
+        m_sourcePath.addPath( path );
     }
 
     /**
@@ -194,11 +176,11 @@ public abstract class AbstractMetamataTask
 
         // set the classpath as the jar file
         File jar = getMetamataJar( m_metamataHome );
-        final Path classPath = m_cmdl.createClasspath();
+        final Path classPath = m_exe.getClassPath();
         classPath.addLocation( jar );
 
         // set the metamata.home property
-        m_cmdl.addVmArgument( "-Dmetamata.home=" + m_metamataHome.getAbsolutePath() );
+        m_exe.getSysProperties().addVariable( "metamata.home",  m_metamataHome.getAbsolutePath() );
 
         // retrieve all the files we want to scan
         m_includedFiles = scanFileSets();
@@ -208,8 +190,8 @@ public abstract class AbstractMetamataTask
         ArrayList options = getOptions();
         m_optionsFile = createTmpFile();
         generateOptionsFile( m_optionsFile, options );
-        m_cmdl.addArgument( "-arguments " );
-        m_cmdl.addArgument( m_optionsFile.getAbsolutePath() );
+        m_exe.getArguments().addArgument( "-arguments" );
+        m_exe.getArguments().addArgument( m_optionsFile );
     }
 
     /**
@@ -271,11 +253,7 @@ public abstract class AbstractMetamataTask
     protected void execute0()
         throws TaskException
     {
-        final Execute exe = new Execute();
-        getContext().debug( m_cmdl.toString() );
-        final String[] commandline = m_cmdl.getCommandline();
-        exe.setCommandline( new Commandline( commandline ) );
-        exe.execute( getContext() );
+        m_exe.executeForked( getContext() );
     }
 
     protected void generateOptionsFile( File tofile, ArrayList options )
@@ -355,17 +333,11 @@ public abstract class AbstractMetamataTask
 
     protected Path getClassPath()
     {
-        return m_classPath;
+        return m_exe.getClassPath();
     }
 
     protected Path getSourcePath()
     {
         return m_sourcePath;
     }
-
-    protected void setSourcePath( Path sourcePath )
-    {
-        m_sourcePath = sourcePath;
-    }
-
 }

proposal/myrmidon/src/todo/org/apache/tools/todo/taskdefs/metamata/MAudit.java

@@ -124,12 +124,7 @@ public class MAudit
         // there is a bug in Metamata 2.0 build 37. The sourcepath argument does
         // not work. So we will use the sourcepath prepended to classpath. (order
         // is important since Metamata looks at .class and .java)
-        final Path sourcePath = getSourcePath();
-        if( sourcePath != null )
-        {
-            classpath.addPath( sourcePath );
-            setSourcePath( null );// prevent from using -sourcepath
-        }
+        classpath.addPath( getSourcePath() );
 
         // don't forget to modify the pattern if you change the options reporting
         classpath.addPath( getClassPath() );
@@ -157,11 +152,12 @@ public class MAudit
         {
             options.add( "-list" );
         }
-        if( getSourcePath() != null )
-        {
-            options.add( "-sourcepath" );
-            options.add( PathUtil.formatPath( getSourcePath() ) );
-        }
+
+        //if( getSourcePath() != null )
+        //{
+        //    options.add( "-sourcepath" );
+        //    options.add( PathUtil.formatPath( getSourcePath() ) );
+        //}
 
         if( m_unused )
         {

proposal/myrmidon/src/todo/org/apache/tools/todo/taskdefs/metamata/MMetrics.java

@@ -127,11 +127,7 @@ public class MMetrics extends AbstractMetamataTask
         // there is a bug in Metamata 2.0 build 37. The sourcepath argument does
         // not work. So we will use the sourcepath prepended to classpath. (order
         // is important since Metamata looks at .class and .java)
-        if( getSourcePath() != null )
-        {
-            classpath.addPath( getSourcePath() );
-            setSourcePath( null );// prevent from using -sourcepath
-        }
+        classpath.addPath( getSourcePath() );
 
         // don't forget to modify the pattern if you change the options reporting
         classpath.addPath( getClassPath() );

proposal/myrmidon/src/todo/org/apache/tools/todo/taskdefs/metamata/MParse.java

@@ -16,10 +16,8 @@ import java.util.Random;
 import org.apache.avalon.excalibur.io.IOUtil;
 import org.apache.myrmidon.api.AbstractTask;
 import org.apache.myrmidon.api.TaskException;
-import org.apache.myrmidon.framework.Execute;
+import org.apache.tools.todo.taskdefs.ExecuteJava;
 import org.apache.tools.todo.types.Argument;
-import org.apache.tools.todo.types.Commandline;
-import org.apache.tools.todo.types.CommandlineJava;
 import org.apache.tools.todo.types.Path;
 import org.apache.tools.todo.types.PathUtil;
 
@@ -34,23 +32,17 @@ import org.apache.tools.todo.types.PathUtil;
 public class MParse
     extends AbstractTask
 {
-    private Path m_classpath;
-    private Path m_sourcepath;
+    private Path m_classpath = new Path();
+    private Path m_sourcepath = new Path();
     private File m_metahome;
     private File m_target;
     private boolean m_verbose;
     private boolean m_debugparser;
     private boolean m_debugscanner;
     private boolean m_cleanup;
-    private CommandlineJava m_cmdl = new CommandlineJava();
+    private ExecuteJava m_exe = new ExecuteJava();
     private File m_optionsFile;
 
-    public MParse()
-    {
-        m_cmdl.setVm( "java" );
-        m_cmdl.setClassname( "com.metamata.jj.MParse" );
-    }
-
     /**
      * create a temporary file in the current directory
      *
@@ -101,7 +93,7 @@ public class MParse
      */
     public void setMaxmemory( String max )
     {
-        m_cmdl.addVmArgument( "-Xmx" + max );
+        m_exe.setMaxMemory( max );
     }
 
     /**
@@ -111,7 +103,7 @@ public class MParse
      */
     public void setMetamatahome( File metamatahome )
     {
-        this.m_metahome = metamatahome;
+        m_metahome = metamatahome;
     }
 
     /**
@@ -121,7 +113,7 @@ public class MParse
      */
     public void setTarget( File target )
     {
-        this.m_target = target;
+        m_target = target;
     }
 
     /**
@@ -137,15 +129,10 @@ public class MParse
     /**
      * create a classpath entry
      *
-     * @return Description of the Returned Value
      */
-    public Path createClasspath()
+    public void addClasspath( final Path path )
     {
-        if( m_classpath == null )
-        {
-            m_classpath = new Path();
-        }
-        return m_classpath;
+        m_classpath.addPath( path );
     }
 
     /**
@@ -153,21 +140,15 @@ public class MParse
      */
     public void addJvmarg( final Argument argument )
     {
-        m_cmdl.addVmArgument( argument );
+        m_exe.getVmArguments().addArgument( argument );
     }
 
     /**
      * creates a sourcepath entry
-     *
-     * @return Description of the Returned Value
      */
-    public Path createSourcepath()
+    public void addSourcepath( final Path path )
     {
-        if( m_sourcepath == null )
-        {
-            m_sourcepath = new Path();
-        }
-        return m_sourcepath;
+        m_sourcepath.addPath( path );
     }
 
     /**
@@ -201,21 +182,21 @@ public class MParse
 
         // set the classpath as the jar files
         File[] jars = getMetamataLibs();
-        final Path classPath = m_cmdl.createClasspath();
+        final Path classPath = m_exe.getClassPath();
         for( int i = 0; i < jars.length; i++ )
         {
             classPath.addLocation( jars[ i ] );
         }
 
         // set the metamata.home property
-        m_cmdl.addVmArgument( "-Dmetamata.home=" + m_metahome.getAbsolutePath() );
+        m_exe.getSysProperties().addVariable( "metamata.home",  m_metahome.getAbsolutePath() );
 
         // write all the options to a temp file and use it ro run the process
         String[] options = getOptions();
         m_optionsFile = createTmpFile();
         generateOptionsFile( m_optionsFile, options );
-        m_cmdl.addArgument( "-arguments" );
-        m_cmdl.addArgument( m_optionsFile.getAbsolutePath() );
+        m_exe.getArguments().addArgument( "-arguments" );
+        m_exe.getArguments().addArgument( m_optionsFile );
     }
 
     /**
@@ -254,12 +235,12 @@ public class MParse
         {
             options.add( "-dp" );
         }
-        if( m_classpath != null )
+        if( ! m_classpath.isEmpty() )
         {
             options.add( "-classpath" );
             options.add( PathUtil.formatPath( m_classpath ) );
         }
-        if( m_sourcepath != null )
+        if( ! m_sourcepath.isEmpty() )
         {
             options.add( "-sourcepath" );
             options.add( PathUtil.formatPath( m_sourcepath ) );
@@ -287,11 +268,8 @@ public class MParse
             return;
         }
 
-        final Execute exe = new Execute();
-        getContext().debug( m_cmdl.toString() );
-        final String[] commandline = m_cmdl.getCommandline();
-        exe.setCommandline( new Commandline( commandline ) );
-        exe.execute( getContext() );
+        m_exe.setClassName( "com.metamata.jj.MParse" );
+        m_exe.executeForked( getContext() );
     }
 
     /**

proposal/myrmidon/src/todo/org/apache/tools/todo/taskdefs/sitraka/Coverage.java

@@ -18,7 +18,6 @@ import org.apache.myrmidon.api.TaskException;
 import org.apache.myrmidon.framework.Execute;
 import org.apache.tools.todo.types.Argument;
 import org.apache.tools.todo.types.Commandline;
-import org.apache.tools.todo.types.CommandlineJava;
 import org.apache.tools.todo.types.FileSet;
 import org.apache.tools.todo.types.Path;
 import org.apache.tools.todo.types.PathUtil;
@@ -37,9 +36,6 @@ import org.apache.tools.todo.types.PathUtil;
 public class Coverage
     extends AbstractTask
 {
-    protected Commandline cmdl = new Commandline();
-    protected CommandlineJava cmdlJava = new CommandlineJava();
-
     /**
      * this is a somewhat annoying thing, set it to never
      */
@@ -60,6 +56,10 @@ public class Coverage
     private Triggers m_triggers;
     private String m_vm;
     private File m_workingDir;
+    private String m_className;
+    private Commandline m_args = new Commandline();
+    private Path m_classpath = new Path();
+    private Commandline m_vmArgs = new Commandline();
 
     /**
      * classname to run as standalone or runner for filesets
@@ -68,7 +68,7 @@ public class Coverage
      */
     public void setClassname( String value )
     {
-        cmdlJava.setClassname( value );
+        m_className = value;
     }
 
     /**
@@ -176,20 +176,15 @@ public class Coverage
      */
     public void addArg( final Argument argument )
     {
-        cmdlJava.addArgument( argument );
+        m_args.addArgument( argument );
     }
 
     /**
      * classpath to run the files
-     *
-     * @return Description of the Returned Value
      */
-    public Path createClasspath()
+    public void setClasspath( final Path path )
     {
-        Path path1 = cmdlJava.createClasspath();
-        final Path path = new Path();
-        path1.addPath( path );
-        return path;
+        m_classpath.addPath( path );
     }
 
     public Filters createFilters()
@@ -202,7 +197,7 @@ public class Coverage
      */
     public void addJvmarg( final Argument argument )
     {
-        cmdlJava.addVmArgument( argument );
+        m_vmArgs.addArgument( argument );
     }
 
     public Socket createSocket()
@@ -245,11 +240,12 @@ public class Coverage
         try
         {
             // we need to run Coverage from his directory due to dll/jar issues
+            final Execute exe = new Execute();
+            final Commandline cmdl = exe.getCommandline();
             cmdl.setExecutable( new File( m_home, "jplauncher" ).getAbsolutePath() );
             cmdl.addArgument( "-jp_input=" + paramfile.getAbsolutePath() );
 
             // use the custom handler for stdin issues
-            final Execute exe = new Execute();
             exe.setCommandline( cmdl );
             exe.execute( getContext() );
         }
@@ -273,62 +269,55 @@ public class Coverage
     protected String[] getParameters()
         throws TaskException
     {
-        ArrayList params = new ArrayList();
-        params.add( "-jp_function=coverage" );
+        Commandline params = new Commandline();
+        params.addArgument( "-jp_function=coverage" );
         if( m_vm != null )
         {
-            params.add( "-jp_vm=" + m_vm );
+            params.addArgument( "-jp_vm=" + m_vm );
         }
         if( m_javaExe != null )
         {
-            params.add( "-jp_java_exe=" + getContext().resolveFile( m_javaExe.getPath() ) );
+            params.addArgument( "-jp_java_exe=" + m_javaExe.getPath() );
         }
-        params.add( "-jp_working_dir=" + m_workingDir.getPath() );
-        params.add( "-jp_snapshot_dir=" + m_snapshotDir.getPath() );
-        params.add( "-jp_record_from_start=" + m_recordFromStart );
-        params.add( "-jp_warn=" + m_warnLevel );
+        params.addArgument( "-jp_working_dir=" + m_workingDir.getPath() );
+        params.addArgument( "-jp_snapshot_dir=" + m_snapshotDir.getPath() );
+        params.addArgument( "-jp_record_from_start=" + m_recordFromStart );
+        params.addArgument( "-jp_warn=" + m_warnLevel );
         if( m_seedName != null )
         {
-            params.add( "-jp_output_file=" + m_seedName );
+            params.addArgument( "-jp_output_file=" + m_seedName );
         }
-        params.add( "-jp_filter=" + m_filters.toString() );
+        params.addArgument( "-jp_filter=" + m_filters.toString() );
         if( m_triggers != null )
         {
-            params.add( "-jp_trigger=" + m_triggers.toString() );
+            params.addArgument( "-jp_trigger=" + m_triggers.toString() );
         }
         if( m_finalSnapshot != null )
         {
-            params.add( "-jp_final_snapshot=" + m_finalSnapshot );
+            params.addArgument( "-jp_final_snapshot=" + m_finalSnapshot );
         }
-        params.add( "-jp_exit_prompt=" + m_exitPrompt );
+        params.addArgument( "-jp_exit_prompt=" + m_exitPrompt );
         //params.add("-jp_append=" + append);
-        params.add( "-jp_track_natives=" + m_trackNatives );
+        params.addArgument( "-jp_track_natives=" + m_trackNatives );
         //.... now the jvm
         // arguments
-        String[] vmargs = cmdlJava.getVmCommand().getArguments();
-        for( int i = 0; i < vmargs.length; i++ )
-        {
-            params.add( vmargs[ i ] );
-        }
+        params.addArguments( m_vmArgs );
+
         // classpath
-        Path classpath = cmdlJava.getClasspath();
-        if( classpath != null && ! classpath.isEmpty() )
+        if( ! m_classpath.isEmpty() )
         {
-            params.add( "-classpath " + PathUtil.formatPath( classpath ) );
+            params.addArgument( "-classpath" );
+            params.addArgument( PathUtil.formatPath( m_classpath ) );
         }
         // classname (runner or standalone)
-        if( cmdlJava.getClassname() != null )
+        if( m_className != null )
         {
-            params.add( cmdlJava.getClassname() );
+            params.addArgument( m_className );
         }
         // arguments for classname
-        String[] args = cmdlJava.getJavaCommand().getArguments();
-        for( int i = 0; i < args.length; i++ )
-        {
-            params.add( args[ i ] );
-        }
+        params.addArguments( m_args );
 
-        return (String[])params.toArray( new String[ params.size() ] );
+        return params.getArguments();
     }
 
     /**

proposal/myrmidon/src/todo/org/apache/tools/todo/types/Commandline.java

@@ -123,6 +123,11 @@ public class Commandline
         }
     }
 
+    public void addArguments( final Commandline cmdline )
+    {
+        addArguments( cmdline.getArguments() );
+    }
+
     public void addArgument( final File argument )
     {
         addArgument( new Argument( argument ) );

proposal/myrmidon/src/todo/org/apache/tools/todo/types/CommandlineJava.java

@@ -1,263 +0,0 @@
-/*
- * Copyright (C) The Apache Software Foundation. All rights reserved.
- *
- * This software is published under the terms of the Apache Software License
- * version 1.1, a copy of which has been included with this distribution in
- * the LICENSE.txt file.
- */
-package org.apache.tools.todo.types;
-
-import org.apache.avalon.excalibur.util.StringUtil;
-import org.apache.myrmidon.api.TaskException;
-import org.apache.tools.todo.types.Argument;
-import org.apache.tools.todo.types.Commandline;
-import org.apache.tools.todo.taskdefs.ExecuteJava;
-
-/**
- * A representation of a Java command line that is nothing more than a composite
- * of 2 <tt>Commandline</tt> . 1 for the vm/options and 1 for the
- * classname/arguments. It provides specific methods for a java command line.
- *
- * @author thomas.haas@softwired-inc.com
- * @author <a href="sbailliez@apache.org">Stephane Bailliez</a>
- */
-public class CommandlineJava
-    implements Cloneable
-{
-    private Commandline m_vmCommand = new Commandline();
-    private Commandline m_javaCommand = new Commandline();
-    private SysProperties m_sysProperties = new SysProperties();
-    private Path m_classpath;
-    private String m_maxMemory;
-
-    /**
-     * Indicate whether it will execute a jar file or not, in this case the
-     * first vm option must be a -jar and the 'executable' is a jar file.
-     */
-    private boolean executeJar;
-
-    public CommandlineJava()
-    {
-        setVm( ExecuteJava.getJavaExecutableName() );
-    }
-
-    /**
-     * set the classname to execute
-     *
-     * @param classname the fully qualified classname.
-     */
-    public void setClassname( String classname )
-    {
-        m_javaCommand.setExecutable( classname );
-        executeJar = false;
-    }
-
-    /**
-     * set a jar file to execute via the -jar option.
-     *
-     * @param jarpathname The new Jar value
-     */
-    public void setJar( String jarpathname )
-    {
-        m_javaCommand.setExecutable( jarpathname );
-        executeJar = true;
-    }
-
-    /**
-     * -mx or -Xmx depending on VM version
-     *
-     * @param max The new Maxmemory value
-     */
-    public void setMaxmemory( String max )
-    {
-        m_maxMemory = max;
-    }
-
-    public void setSystemProperties()
-        throws TaskException
-    {
-        m_sysProperties.setSystem();
-    }
-
-    public void setVm( String vm )
-    {
-        m_vmCommand.setExecutable( vm );
-    }
-
-    /**
-     * @return the name of the class to run or <tt>null</tt> if there is no
-     *      class.
-     * @see #getJar()
-     */
-    public String getClassname()
-    {
-        if( !executeJar )
-        {
-            return m_javaCommand.getExecutable();
-        }
-        return null;
-    }
-
-    public Path getClasspath()
-    {
-        return m_classpath;
-    }
-
-    /**
-     * get the command line to run a java vm.
-     *
-     * @return the list of all arguments necessary to run the vm.
-     */
-    public String[] getCommandline()
-        throws TaskException
-    {
-        String[] result = new String[ size() ];
-        int pos = 0;
-        String[] vmArgs = getActualVMCommand().getCommandline();
-        // first argument is the java.exe path...
-        result[ pos++ ] = vmArgs[ 0 ];
-
-        // -jar must be the first option in the command line.
-        if( executeJar )
-        {
-            result[ pos++ ] = "-jar";
-        }
-        // next follows the vm options
-        System.arraycopy( vmArgs, 1, result, pos, vmArgs.length - 1 );
-        pos += vmArgs.length - 1;
-        // properties are part of the vm options...
-        if( m_sysProperties.size() > 0 )
-        {
-            System.arraycopy( m_sysProperties.getJavaVariables(), 0,
-                              result, pos, m_sysProperties.size() );
-            pos += m_sysProperties.size();
-        }
-        // classpath is a vm option too..
-        if( m_classpath != null && ! m_classpath.isEmpty() )
-        {
-            result[ pos++ ] = "-classpath";
-            result[ pos++ ] = PathUtil.formatPath( m_classpath );
-        }
-        // this is the classname to run as well as its arguments.
-        // in case of 'executeJar', the executable is a jar file.
-        System.arraycopy( m_javaCommand.getCommandline(), 0,
-                          result, pos, m_javaCommand.size() );
-        return result;
-    }
-
-    /**
-     * @return the pathname of the jar file to run via -jar option or <tt>null
-     *      </tt> if there is no jar to run.
-     * @see #getClassname()
-     */
-    public String getJar()
-    {
-        if( executeJar )
-        {
-            return m_javaCommand.getExecutable();
-        }
-        return null;
-    }
-
-    public Commandline getJavaCommand()
-    {
-        return m_javaCommand;
-    }
-
-    public SysProperties getSystemProperties()
-    {
-        return m_sysProperties;
-    }
-
-    public Commandline getVmCommand()
-    {
-        return getActualVMCommand();
-    }
-
-    public void addSysproperty( EnvironmentVariable sysp )
-    {
-        m_sysProperties.addVariable( sysp );
-    }
-
-    public void addArgument( final String argument )
-    {
-        m_javaCommand.addArgument( argument );
-    }
-
-    public void addArgument( final Argument argument )
-    {
-        m_javaCommand.addArgument( argument );
-    }
-
-    public Path createClasspath()
-    {
-        if( m_classpath == null )
-        {
-            m_classpath = new Path();
-        }
-        return m_classpath;
-    }
-
-    public void addVmArgument( final String argument )
-    {
-        m_vmCommand.addArgument( argument );
-    }
-
-    public void addVmArgument( final Argument argument )
-    {
-        m_vmCommand.addArgument( argument );
-    }
-
-    public void restoreSystemProperties()
-        throws TaskException
-    {
-        m_sysProperties.restoreSystem();
-    }
-
-    /**
-     * The size of the java command line.
-     *
-     * @return the total number of arguments in the java command line.
-     * @see #getCommandline()
-     */
-    public int size()
-        throws TaskException
-    {
-        int size = getActualVMCommand().size() + m_javaCommand.size() + m_sysProperties.size();
-        // classpath is "-classpath <classpath>" -> 2 args
-        if( m_classpath != null && ! m_classpath.isEmpty() )
-        {
-            size += 2;
-        }
-        // jar execution requires an additional -jar option
-        if( executeJar )
-        {
-            size++;
-        }
-        return size;
-    }
-
-    public String toString()
-    {
-        try
-        {
-            final String[] line = getCommandline();
-            return StringUtil.join( line, " " );
-        }
-        catch( TaskException e )
-        {
-            return e.toString();
-        }
-    }
-
-    private Commandline getActualVMCommand()
-    {
-        Commandline actualVMCommand = new Commandline();
-        //(Commandline)vmCommand.clone();
-        if( m_maxMemory != null )
-        {
-            actualVMCommand.addArgument( "-Xmx" + m_maxMemory );
-        }
-        return actualVMCommand;
-    }
-}

proposal/myrmidon/src/todo/org/apache/tools/todo/types/EnvironmentData.java

@@ -21,7 +21,6 @@ public class EnvironmentData
     protected final ArrayList m_variables = new ArrayList();
 
     public Properties getVariables()
-        throws TaskException
     {
         final Properties environment = new Properties();
         final int size = m_variables.size();
@@ -37,4 +36,22 @@ public class EnvironmentData
     {
         m_variables.add( var );
     }
+
+    public void addVariable( String key, String value )
+    {
+        final EnvironmentVariable var = new EnvironmentVariable();
+        var.setKey( key );
+        var.setValue( value );
+        addVariable( var );
+    }
+
+    public void addVariables( EnvironmentData properties )
+    {
+        m_variables.addAll( properties.m_variables );
+    }
+
+    public int size()
+    {
+        return m_variables.size();
+    }
 }

proposal/myrmidon/src/todo/org/apache/tools/todo/types/SysProperties.java

@@ -10,74 +10,74 @@ package org.apache.tools.todo.types;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.Properties;
-import org.apache.aut.nativelib.ExecException;
+import java.util.Map;
 import org.apache.myrmidon.api.TaskException;
-import org.apache.tools.todo.types.EnvironmentData;
-import org.apache.tools.todo.types.EnvironmentVariable;
 
 /**
- * Specialized EnvironmentData class for System properties
+ * A utility class for handling System properties
+ *
+ * @todo move this to AUT
  */
-public class SysProperties
-    extends EnvironmentData
-    implements Cloneable
+final public class SysProperties
 {
-    private Properties m_system;
+    private static Properties m_system;
 
-    public void setSystem()
-        throws TaskException
+    private SysProperties()
     {
-        try
-        {
-            Properties p = new Properties( m_system = System.getProperties() );
+    }
 
-            for( Iterator e = m_variables.iterator(); e.hasNext(); )
-            {
-                EnvironmentVariable v = (EnvironmentVariable)e.next();
-                p.put( v.getKey(), v.getValue() );
-            }
-            System.setProperties( p );
-        }
-        catch( SecurityException e )
-        {
-            throw new TaskException( "Cannot modify system properties", e );
-        }
+    /**
+     * Sets system properties.  The current set of system properties can be
+     * restored using {@link #restoreSystem}.
+     */
+    public static void setSystem( final EnvironmentData properties )
+        throws TaskException
+    {
+        setSystem( properties.getVariables() );
     }
 
     /**
-     * @todo move this to AUT
+     * Sets system properties.  The current set of system properties can be
+     * restored using {@link #restoreSystem}.
      */
-    public String[] getJavaVariables()
+    public synchronized static void setSystem( final Map properties )
         throws TaskException
     {
-        String props[] = toNativeFormat( super.getVariables() );
-        for( int i = 0; i < props.length; i++ )
+        if( properties.size() == 0 )
         {
-            props[ i ] = "-D" + props[ i ];
+            return;
+        }
+        if( m_system != null )
+        {
+            throw new TaskException( "System properties have not been restored." );
         }
-        return props;
-    }
 
-    public Object clone()
-    {
+        final Properties sysProps;
         try
         {
-            SysProperties c = (SysProperties)super.clone();
-            c.m_variables.addAll( (ArrayList)m_variables.clone() );
-            return c;
+            sysProps = System.getProperties();
+            Properties allProps = new Properties( sysProps );
+            allProps.putAll( properties );
+            System.setProperties( allProps );
         }
-        catch( CloneNotSupportedException e )
+        catch( final SecurityException e )
         {
-            return null;
+            throw new TaskException( "Cannot modify system properties.", e );
         }
+
+        m_system = sysProps;
     }
 
-    public void restoreSystem()
+    /**
+     * Restores the system properties to what they were before the last
+     * call to {@link #setSystem}.
+     */
+    public static synchronized void restoreSystem()
         throws TaskException
     {
         if( m_system == null )
         {
-            throw new TaskException( "Unbalanced nesting of SysProperties" );
+            return;
         }
 
         try
@@ -85,29 +85,35 @@ public class SysProperties
             System.setProperties( m_system );
             m_system = null;
         }
-        catch( SecurityException e )
+        catch( final SecurityException e )
         {
-            throw new TaskException( "Cannot modify system properties", e );
+            throw new TaskException( "Cannot modify system properties.", e );
         }
     }
 
-    public int size()
+    /**
+     * Converts a set of properties to their -D command-line equivalent.
+     */
+    public static String[] getJavaVariables( final EnvironmentData environment )
     {
-        return m_variables.size();
+        return getJavaVariables( environment.getVariables() );
     }
 
-    private String[] toNativeFormat( final Properties environment )
+    /**
+     * Converts a set of properties to their -D command-line equivalent.
+     */
+    public static String[] getJavaVariables( final Map environment )
     {
-        final ArrayList newEnvironment = new ArrayList();
+        final ArrayList vars = new ArrayList();
 
         final Iterator keys = environment.keySet().iterator();
         while( keys.hasNext() )
         {
             final String key = (String)keys.next();
-            final String value = environment.getProperty( key );
-            newEnvironment.add( key + '=' + value );
+            final Object value = environment.get( key );
+            vars.add( "-D" + key + '=' + value );
         }
 
-        return (String[])newEnvironment.toArray( new String[ newEnvironment.size() ] );
+        return (String[])vars.toArray( new String[ vars.size() ] );
     }
 }