[Fusionforge-general] RFC : better error checks on system() and exec() in cronjobs using exceptions

Sylvain Beucler - Inria sylvain.beucler at inria.fr
Mon Apr 14 10:50:28 CEST 2014


Hi,

Le 11/04/2014 19:06, Olivier Berger a écrit :
> I've tried to add a few functions to utils.php that could act as
> replacements for all the calls to system() or exec() where no return
> code checks are made, using an exception catching strategy.
>
> Currently, a lot of subsequent system() invocations are made, and the
> cronjobs output will then only report stdout + stderr without giving any
> clue on what command what actually run, which doesn't help trace the
> execution (where a set -x in a shell script would help).
>
> I think that using something like the following could help :
> try {
>      my_system();
>      my_system():
>      ...
> } catch (Exception $e) {
>      echo'Caught exception: ',  $e->getMessage(), "\n";
> }
> where my_system's throw would tell which command actually failed.
That sounds good. If there's an error, the series of system() is stopped 
and there's an error message.
It's more readable than checking the return code every time.
> I've committed and pushed such util functions to [0] on my repo's
> 'utilscmd' branch.
>
> An example of the impact on GitPlugin can be see in this commit [1].
>
> Now... is it allowed to use exceptions in FusionForge's PHP ?
>
> Remember that this code is supposed to run as a cronjob AFAIK.
On Friday, we'll discuss how to improve the system replication, in 
particular run tasks immediately rather than through a cron job.
So I'd suggest not to sweat too much on this right now.
> This could be a first improvement until we eventually use some proper
> shell scripts some day instead of all these system() invocations (see my
> previous posts of today).
I don't like the idea, because of the perfs (each chmod = 1 fork&exec in 
shell, but only 1 simple syscall in PHP), and because interfacing bash 
scripts from PHP code with lots of parameters to pass, subcalls to 
forge_get_config, etc. doesn't appeal to me.
> Any comments on that use exceptions in a system()/exec() wrapper ?
Quickly:
- Naming : cmd_<php_func_name> confusingly looks like a real PHP 
builtin, and doesn't convey that it adds exceptions. I'd suggest ffexec, 
ffsystem, etc.
- Naming : a replacement for "exec" should use "exec" IMHO, not 
"proc_open". It difficult enough to understand the differences between 
exec/shell/passthrough/etc. already.
- Naming : I don't understand "full" in cmd_exec_full, and I don't 
really understand the difference with cmd_exec.
- Double negation : instead of $no_exception=null, how about 
$exception=true ?
- Existing libraries : I found no existing library for wrapping shell 
calls in PHP, so indeed let's code our own (unless I missed one). I'm 
wondering if it would make sense to publish a proper PEAR module or 
something, this could benefit others.
- Discarding input : redirect stdout/stderr to /dev/null rather than 
creatning /tmp/xxx/in and /tmp/xxx/out for _each_ syscall.
- Exceptions : in the commit you link [0], 1 function out of 3 adds 
exceptions, not sure if that's unfinished code. If not, we don't need 
the other functions IMHO.
> Thanks in advance.
>
> [0] https://fusionforge.org/plugins/scmgit/cgi-bin/gitweb.cgi?p=fusionforge/users/olberger.git;a=commit;h=1729a5f79691b576c60a4e6bb01e1ab91dc9c34b
> [1] https://fusionforge.org/plugins/scmgit/cgi-bin/gitweb.cgi?p=fusionforge/users/olberger.git;a=commit;h=68e0c85af74490c1aa21545b0be6c6f196236d87
-- 
Sylvain



More information about the Fusionforge-general mailing list