commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jerome lacoste <jerome.laco...@gmail.com>
Subject [exec] vision for the library
Date Sun, 25 Sep 2005 20:41:16 GMT
[Sorry for the long mail. I hope I was clear enough this time]

This complements/summarizes the couple of mails I sent this week-end,
trying to give a better description of the vision I have for
commons-exec.

I don't want to impose anything. A library like commons-exec is
something I have been thinking over for several months, so I am just a
little bit enthousiastic when I saw this project getting started in
commons.

Approach
-------------
My approach was the following. Brett asked me to make a small how-to.
So I wrote the little how-to, looking at the API and how it was
implemented. I compared using commons-exec as of today and what the
different alternatives (in particular SDK 5.0) had for them and how
they had to be used.

Doing so I read all the code and found some things that could be improved.

I first started by annotating the code with @todo (see #36798 [1]). I
know it's not the correct approach (and I didn't expect the patch to
be checked in), but it helps a lot to mark locally each piece of code
with a particular issue. So it was an easy way to share the problems.

Then I sent a mail with a short vision and described how I saw the
library evolving, and did that refactoring just to prove (also to me)
that those ideas could fit together. I stopped at a certain point so
the current implementation doesn't contain all my ideas yet.

I tried refactored the initial code (instead of rewriting it) to not
lose the information it had gathered around the years. Not sure if I
broke stuff as there wasn't so many unit tests :)

NOTE: I didn't take into account whether this library should do more
than what it needs today. I am mostly satisfied with the current set
of features. But.

Current issues
--------------------

There's a certain legacy in the current commons-exec and it shows a
lot. The main issue is the following: commons-exec as today was
written to solve a particular problem within ant and not as a reusable
library. In particular, reusing it forces any client to reuse many
classes that a client should not necessarily care about. In comparison
ProcessBuilder (SDK 5.0) (which solves only a small subset of what
commons-exec solves), is much cleaner in its approach. It relies much
more on SDK classes: no need for a CommandLine class, no need for an
Environment class, etc...

In commons-exec the Watchdog and the ProcessDestroyer have their own
issues. And the functionality of some things are not open enough for
reuse. For some of these issues, see the notes in my patch [2] and in
the aforementionned mails. Let me know if this lacks details.

I see commons-exec as an not only an improvement of what
Runtime.exec() brings, but as what ProcessBuilder brings. To me,
commons-exec should close the following holes let by ProcessBuilder:

- ProcessBuilder is too dependent on native calls under the hood. We
have a pure java implementation (that depends on some native calls
exposed by the SDK).
- can be used on SDK < 5.0
- can be upgraded. If I find an issue in SDK 5.0, I don't want to wait
for a point release to be able to make it work. commons-exec
functionality is so important that I don't want to depend on the SDK
for this feature.
- adds process management capabilities.
- adds streams management capabilities (ProcessBuilder only adds the
stream redirector stuff, which I think is a strange addition as that
level of the API. We can see that later on).

So commons-exec has the good functionality, it needs to be more
flexible & simpler to use.

Current commons-exec features
--------------------------------------------

It
- starts commands in the various platforms and SDK. Todays that's the
launcher package. It's mostly good as is.
- retrieves os environment, determining on which os we are, etc..
Today that's the OS class + environment package.
- builds a process (that's part of Execute)
- attaches stream and process operations around the execution of a
Process (still part of Execute).
- tries to provide good defaults for executing a process (hides
complexity of the Execute) (What Exec does today).
- should allow clients to make their own stream operations
(redirection, visitor-like patterns for consuming information,
etc...). Today exists but not that reusable.

In CruiseControl I use Execute but don't reuse Exec. That shows its limits.

ProcessBuilder like class
-----------------------------------

ProcessBuilder is a java.lang class. This means it's a core
functionality of the SDK. It solves a simple problem: how to prepare
and start processes in a reusable way. It's API makes use of String
and Map (which is in an interesting thing, because it's one of the
rate classes in java.lang that makes use of out of package classes,
and create a dependency loop).

Why introducing a ProcessBuilder like class in commons-exec?
+ design: Today Execute does everything. It can be split in 2 and
focus on the execution (i.e. , not on the prepartion of the execution.
It can in fact easily be extracted from the current Execute (as shown
by my partial rewrite). Clearly shows the different responsibilities
in the package which is the first rule in making a good design.
+ marketing: doing so makes it even easier to see the advantages of
using the commons-exec. Commons-exec becomes a sort of (ProcessBuilder
+ advanced features) on all SDK, all platforms.
+ flexibility: someone might want to use Process building
functionality + environment management + launching commands (as
provided by the commons-exec) without necessarily wanting the whole
Execute functionality (process and stream management).
+ testing: it makes it easy to mock and test Process creation for
users (was also one of my issues with commons-exec and #36707, which
can now be solved in a much cleaner way). It also makes it easy to
test the library itself.
+ migration: it makes it easy for people who might use the
ProcessBuilder first to migrate to commons-exec
+ a client might want to reuse the stream and process management
functionality and still use the SDK 5.0 ProcessBuilder. After all, why
not?

Should I add more?


Vision & architecture:
---------------------------

Commons-exec becomes a sort of (ProcessBuilder + advanced features) on
all SDK, all platforms.

Proposed architecture
- Launchers: exist as today
- OS functionality
- ProcessLauncher, (our ProcessBuilder)
- Execute (or Executor?): Flexible, we use it to tie together the
various concepts in the execute() call (stream management, process
management, process building, etc...)
- Exec (or ??) makes it easy to use Execute/or.

All this gives a set of classes that solve the functionality by
building on top of each other from the bottom level to a higher level.
Clients can chose what they need and can easily build new things on
top of it.


The architecture doesn't need to be very different from what it is today.
In fact I think it can be achieved by refactoring the current code. I like that.


Detailed changes:
-------------------------

This vision ended up impacting the current commons-exec classes in the
following way:

EnvironmentVariable. Too bad there's no real interface for key/value
container in SDK (apart from Map.Entry whose definition makes me want
to avoid using it outside of a Map). Otherwise I would have scrapped
it...

Environment. 1st remove Process Environment out. It's got nothing to
do in there. Environment becomes mainly practical for clients who
already use the EnvironmentVariable as for internal building of the
ProcessEnvironment class. So let's make it a Map so that users don't
need it to use the other classes. Also make it map key->value instead
of key->(key, value)

ProcessEnvironment: We don't need a real class. What is the most
needed is a getenv() call that builds an Environment (like
System.getenv()). Could maybe just return a Map. But I don't see an
issue with returning the Environment itself which maybe helps us. We
see that as implementation goes.

Commands: they are good as they are. Just don't depend on Environment
if possible. There's not much that need.

ProcessLauncher similar to ProcessBuilder. I would also probably add
an interface

Execute/or: The core value of Execute is its execute() call,
especially now that some of the functionality is moved into
ProcessBuilder. Make sure the call is flexible. Avoid complexity
(merge process destroyer and watchdog).

Exec like (I haven't though about it much yet).  allows for default
good use of the Execute class. While Execute is flexible, we need to
provide default good use for the user. Register a process killer if
there's a timeout . Would need a better name.





An important effort should be made to try to expose a minimum number
of methods and classes as public. It makes evolving the API much
harder if there's a design issue.

We regard to my other patches, they can be discarded if we go for a
rewrite (through refactorings). Trygve: in particular the serializable
patch is not critical. Was just there to maintain the functionality of
the old API, as I thought it as critical as that time I started
looking at the project.



List of things to do before 1.0.
- commons-exec defines the vision and rewrite and document the API
- CruiseControl, Plexus maven2 all make use of the new API
- I can rewrite Ant to reuse it as well (at least for testing
purposes). I would copy the classes into a new package inside ant and
make sure we expose everything that ant would need to work.
- I know other projects that need this functionality. I can contact
them for further feedback once the first new API has been defined.

I don't mind this process taking several months as long as there's a
good feedback from the potential users. I am not in a hurry to finish,
just in a hurry to get started :)

Cheers,

Jerome

[1] http://issues.apache.org/bugzilla/show_bug.cgi?id=36798
[2] http://issues.apache.org/bugzilla/attachment.cgi?id=16514


PS: I would very much appreciate feedback on the methodology I used to
raise the issues and try to solve them. If there's something I can do
better next time, let me know. I am here to learn.

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


Mime
View raw message