Re: Refactoring spawning feature

[Jesse van den Kieboom <jesse icecrew nl>]

Op 1 jun 2009, om 23:06 heeft Guilhem Bonnefille het volgende  
geschreven:

> Hi,
>
> In order to complete my work on adding a way to set the author of a
> commit (different than the committer), I have to introduce a way to
> set environment variable for the commit command. Gitg internal is
> quite unfriendly for this: uses both a GitgRunner class and many
> gitg_repository_command_xxx functions. Jesse agreed that this is not
> very nice and not easy to extend.
>
> In order to refactor this part, I did a small survey on how spawning
> feature is designed in other framework I know. Here are the elements.
>
> Glib
> A single command with all possible arguments.
>
> Java
> A Process class.
> This class hosts method to access output and input stream. It allows
> to kill the process (destroy), wait for its termination (waitFor) and
> collect the exit status (exitValue).
> This class is built from an other class, via a Runtime.exec(...) call.
> This call sets the command line and the environment variables.
>
> Qt
> A QProcess class.
> This class offers ALL features: creation of process (start),
> environment settings (setEnvironment, setWorkingDirectory), wait for
> the termination (wait), exit code collect (exitStatus), pid access,
> and termination. It also offers a way to handle process's streams.
>
> KDE
> A KProcess class. Refined in KProcIO (when you need to handle
> input/output) and KShellProcess for process activated through a shell.
> The base class offers an IO managment based on signals (qt's signals,
> similar to GObject system). Like QProcess, it offers all method to
> create and manage a process.
>
> ACE
> Two classes ACE_Process and ACE_Process_Options.
> The ACE_Process is the dynamic "vue" of the process: start, terminate,
> kill, wait.
> The ACE_Process_Options is the static "vue" of the process:
> environment, args, working_dir...
>
>
> CONCLUSION
> This survey is far from perfect or exhaustive.
> It seems to exist two main vision to represents a process:
> - a single huge class
> - two classes (or a class and a factory)
>
> In gitg we currently use a scheme where we have two main access
> points: GitgRunner for the dynamic part and
> gitg_repository_command_xxx for the static part (settings of the
> command). Following this scheme I can easily imagine a new class:
> GitgCommand following the same design as ACE_Process_Options.
> But this can be quite overkill. So we can also imagine to extend
> anything to GitgRunner class.
>
> For the details, I imagine that all settings are GObject properties.
> So, working directory is a property. For environment, we can imagine
> an helper function that will append a new couple (name,value) to an
> array. A typical call will becomes:
>
> GitgCommand *command = gitg_command_new_with_args("name", "arg0",
> "arg1", "arg2", ..., NULL);
> gitg_command_set_working_dir(command, "working_dir");
> gitg_command_set_env(command, envp); // or
> gitg_command_append_env(command, "varname=value");
> GitgRunner *runner = gitg_runner_new(10000);
> gitg_runner_run(runner, command);

This seems quite good to me, I like the separation of the command (the  
information to execute the command) and the runner (what executes the  
command and does IO). If you would like to give a go at implementing  
this, don't hesitate. For GitgRepository, I think less specific  
functions might be better. We could have a look at what is actually  
used, and what not. For instance, with regard to the functions that  
take a runner, we could also make a function that constructs a  
GitgCommand from the repository, and let the caller use that with his  
own runner.


Jesse

> Of course, we have to keep existing facade (current
> gitg_repository_command_xxx functions).
>
>
> What is your opinion?
> --
> Guilhem BONNEFILLE
> -=- JID: guyou im apinc org MSN: guilhem_bonnefille hotmail com
> -=- mailto:guilhem bonnefille gmail com
> -=- http://nathguil.free.fr/
> _______________________________________________
> gitg-list mailing list
> gitg-list gnome org
> http://mail.gnome.org/mailman/listinfo/gitg-list