exec statements
| Project: | Version Control API -- Subversion backend |
| Version: | 5.x-1.1 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Jump to:
I found 2 problems in svnlib.inc that have to do with the exec commands (used 3 times). One of these problems occurs when the Subversion repository is accessed over https. This is also discussed in:
http://drupal.org/node/220882
http://drupal.org/node/198639 (bottom of issue text)
The solution is to use
<?php
$return_code = 0;
exec(SUBVERSION_WEB_SERVER_SET_HOME_DIR . implode(' ', $cmd), $output, $return_code);
?>instead of
<?php
$return_code = 0;
exec(implode(' ', $cmd), $output, $return_code);
?>where
define('SUBVERSION_WEB_SERVER_SET_HOME_DIR', 'export HOME="/home/apache";');
and /home/apache is the home of the webserver user, where it can read and write to store the accepted certificate. Note that accepting the certificate or storing a file with the accepted certificate should be done manually on the server.
Of course /home/apache should be a configurable parameter.
Another problem is that on my server all exec commands return an empty array in $output. I have no idea why it happens, but it can be solved by adding:
<?php
$cmd[] = ' 2>&1';
$return_code = 0;
exec(SUBVERSION_WEB_SERVER_SET_HOME_DIR . implode(' ', $cmd), $output, $return_code);
?>
#1
2>&1redirects STDERR to STDOUT, and I can't do that because svnlib relies on the output being valid XML, and that's not the case with STDERR. Would2> $TEMP/versioncontrol_svn_stderr.txt(and deleting that file after the invocation) be a workable alternative?Also, the SUBVERSION_WEB_SERVER_SET_HOME_DIR is a hack and shouldn't need to be configured at all. Ideally, I would want svn to ignore the config dir altogether, but as there's obviously no option for that, I'll just import an appropriately suited .subversion directory into the module and ask svn to use that one by passing the --config-dir option.
Sounds like better solutions?
#2
Sounds great.
There is one thing I would like to comment on: As is discussed in #220860: messages for subversion errors, it would be convenient to have some feedback when something goes wrong. For example, if the repository is accessed over a secure connection and user interaction is needed to accept the certificate, or if there is a problem with the certificate, it would be convenient to know this. Now, a message is shown that the repository does not exist, if such a problem occurs. I think this might confuse the user. Showing the actual Subversion error message could be very informative. This should of course only be done in administrative pages for security reasons.
So using
2> $TEMP/versioncontrol_svn_stderr.txtis a great solution, which can become even better when the content of$TEMP/versioncontrol_svn_stderr.txtis shown to the user when necessary (only administrative page), or in watchdog.#3
Note: $TEMP/versioncontrol_svn_stderr.txt is probably a bad name, because multiple Apache/lighttpd processes or threads might access that file at the same time. So we need some distinguishing unique id, maybe a mixture of the current time(stamp) and the process id.
#4
Just as I predicted; the first one is reporting here:
http://drupal.org/node/227560
#5
Ok, error reporting with tempfile STDERR piping works with commit 103629 (respectively 103631, which is the backport to DRUPAL-5). Next up: fixing the config dir.
#6
...and commit 103638 (respectively 103639) should make that
/root/.subversionwarning disappear. Please try out the brand new 5.x-1.2-rc1 test release in order to see if it actually does :)#7
Automatically closed -- issue fixed for two weeks with no activity.