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
_______________________________________________
Fusionforge-general mailing list
Fusionforge-general@lists.fusionforge.org
http://lists.fusionforge.org/cgi-bin/mailman/listinfo/fusionforge-general
