[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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
