Mailing-List: contact commons-dev-help@jakarta.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Jakarta Commons Developers List" <commons-dev@jakarta.apache.org>
Received-SPF: pass (asf.osuosl.org: domain of jerome.lacoste@gmail.com
 designates 72.14.204.207 as permitted sender)
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
        s=beta; d=gmail.com;
        h=received:message-id:date:from:reply-to:to:subject:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references;
        b=KnzVOQZvgFP/fuHE2866veCaKr5IvkqCC/FbJIpzXNTgJ13cYFbeENc3sM/LzQwh/F88+eof0SMf5dXy6oOPtzTNZe0QOKgjlv3SdmwgwV3ZDkM2U+e6twte5j6K0IFWb91w1VcrDqru0R5rFw+rGhxBVLCx9MIjHzj1MozeUlM=
Message-ID: <5a2cf1f605092305314ba4a139@mail.gmail.com>
Date: Fri, 23 Sep 2005 14:31:20 +0200
From: jerome lacoste <jerome.lacoste@gmail.com>
Reply-To: jerome lacoste <jerome.lacoste@gmail.com>
To: commons-dev@jakarta.apache.org
Subject: Re: [exec] API, implementation various notes
In-Reply-To: <5a2cf1f605092303325075fc39@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
References: <5a2cf1f605092303325075fc39@mail.gmail.com>

> I am also voluntary to produce documentation for the package (as said
> to Brett and Trygve yesterday), help redesign things if these small
> redesigns are allowed, making sure if will be used fully in
> CruiseControl and why not help ant migration when it comes.

Some notes below on that regard:

Evaluation of the current commons-exec design.

Process + Runtime.exec (SDK < 5.0)
----------------------------------

     String[] cmdarray =3D new String[] { "myCommand", "myArg1", "myArg2"};
     String[] envp =3D new String[] {"VAR1=3DmyValue"};
     File workinDir =3D new File("/tmp");
     Process p =3D Runtime.exec(cmdarray, envp, workingDir);

Limitations/drawbacks:
- no difference between command and arguments
- when envp =3D=3D null, current environment inherited fully. Not possible
to remove one variable.
- lack handling of streams (because not consuming them properly can
lead to hangs).
- lack handling of processes
- ugly & confusing Process API (e.g. getInputStream() returns an input
stream that represents the data piped from the output stream. Yuckk)
- nothing makes it really reusable.

ProcessBuilder (SDK >=3D 5.0)
---------------------------

     ProcessBuilder pb =3D new ProcessBuilder("myCommand", "myArg1", "myArg=
2");
     Map<String, String> env =3D pb.environment();
     env.put("VAR1", "myValue");
     env.remove("OTHERVAR");
     env.put("VAR2", env.get("VAR1") + "suffix");
     pb.directory("myDir");
     Process p =3D pb.start();

     // must handle streams, kill process etc...

Most of the issues fixed.

Limitations/drawbacks
- streams and Processes still not handled by implementation
- API still ugly (see directory() and command() methods)
- requires SDK 5.0...


commons-exec
------------

current API taken from Ant. Can be reused in several projects (Plexus,
CruiseControl etc...).
Provide beans for easy XML -> Java mapping.

Low level code centered around Execute, CommandLine and Environment.
Similar in one way to what ProcessBuilder tries to do.

     String[] arguments =3D new String[] {"myArg1", "myArg2"};
     String[] envp =3D new String[] {"VAR1=3DmyValue"};

     ExecuteStreamHandler streamHandler =3D new PumpStreamHandler();
     ExecuteWatchdog watchdog =3D new ExecuteWatchdog();
     Execute execute =3D new Execute();
     CommandLine commandLine =3D new CommandLineImpl(cmdarray)
     commandLine.setExecutable("myCommand");
     commandLine.setArguments(arguments)
     execute.setCommandLine(commandLine);
     Environment environment =3D Environment.getProcEnvironment();
     environment.remove("OTHERVAR");
     execute.setEnvironment(environment);
     int result =3D execute.execute();

Exec: code is broken at best. Tries to hide the complexity of using Execute
Because I find it clumsy I prefered to work directly on top of Execute
when integrating it with CruiseControl.

     Exec exec =3D new Exec();
     CommandLine commandLine =3D new CommandLineImpl(cmdarray)
     commandLine.setExecutable("myCommand");
     commandLine.setArguments(arguments)
     // exec.setSpawn(true);
     exec.setTimeout(10000);
     // exec.setNewenvironment(true); // overrides proc env
     // exec.setResolveExecutable(true); //
     exec.setDir(new File());
     // exec.execute(commandLine); // a simpler version (provides
defaults for unspecified parameters/properties. Cover most cases?)
     // but most flexible is
     exec.execute(commandLine, environment, OutputStream,
OutputStream); // hide type of failure


Key benefits
+ Execute.execute() does both process management (kill) and stream manageme=
nt.
  But what complexity needed to use it! Can't use simple things like
String[] and Map. gaaaaa
+ Process is hidden, but could be obtained if we were to add some listeners=
.
+ Environment.getProcEnvironment(). System.getenv() is SDK 5.0.
+ launchers. But are these used at all? See my TODO_comments.diff patch

Limitations/Drawbacks
- execute make it complex by having both watchdog & process destroyer inter=
faces
- not enough interfaces (ExecuteWatchdog)
- Lacks the concept of builder for reusability?
- do not have the flexibility ProcessBuilder has with Environment
- Exec: executable resolving (broken)
- Exec: converts failures, timeouts to ExecuteException (sub
IOException) inconditionaly.
- forces inheritance to do something else with the streams (which we
definitively want in CruiseControl). Yuck.


Proposal
--------

to me commons-exec is slightly over-engineered. As is, clients will
have to resort to strange things and rewrite somewhat complex helper
classes to do simple things.

I propose to change the API.

Vision: Make commons-exec a reusable Library that is available to all
SDK, that not only solves the problems solved by ProcessBuilder but
also add functionality for Process and Stream management, allowing
flexible reuse of this functionality.

For that I need to look at the forces in Action, in particular look at
how this can be improved without requiring too much change in Ant. I
believe it's possible.

Some ideas to make it simpler:
- make Execute less dependent on classes from commons-exec package,
more like a ProcessBuilder for SDK < 5.0.
- is there a need for a package exception. What's wrong with
IOException? Especially as Exec.execute() 'hides' it again (esposes
IOException)
- is there a need for an Environment class? environment is now a Map
(I've made a patch) so it should be mostly compatible with
pb.environment().
For the sake of simplicity I would consider removing the Environment
class (what does it had? type safety?), provide the System.getenv()
compatible functionality currently implemented in
Environment.getProcEnvironment() outside of the Environment class
- ProcessDestroyer / Watchdog. All that can be cleaned up / merged.
- what about merging stdout and stderr as done in ProcessBuilder

See also http://moca.dynalias.com/~jerome/projects/commons-exec/patches/
For some more notes on design issues.

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org