Op Dinsdag 2007-08-21 skryf lars:
> Hi,
> the attached patch improves the currently shell based command execution in
> versioncontrol.py.
> 
> Reasoning: 
> Currently versioncontrol.py uses the shell for all commands. This is quite
> insecure and can be improved by using the cross-platform compatible
> "subprocess" module. It allows direct command execution without shell
> interaction for all platforms. (the currently used "popen2" module does it 
> only
> for unix)
> Sadly the "subprocess" module requires python 2.4. Thus we cannot use it for
> now, since pootle requires only python 2.3.
> 
> Changes of the patch:
> - replace all shell string commands with arrays of strings
> - improve consistency of error messages
> - use exitcodes instead of stderr to check for failures
> - use python instead of the shell for "cd", "rm", "mv", ...
> 
> Result:
> - a future switching to the "subprocess" module requires only the change of 
> one
> line
> - consistent error detection and more informative output
> 
> I could only test the patch with subversion update/commit on linux.
> Maybe someone else could verify it on windows and for the other version 
> control
> systems?
> 
> What do you think about the patch?
> 
> regards,
> Lars

Hi Lars

I didn't look in detail at everything, but allow me a few comments:
 * It looks good and I think it is a big improvement, especially the
platform independence and the better comments / errors.

 *
- raise IOError("Git: unexpected path names: '%s' and '%s'" \
+ raise IOError("[BZR] unexpected path names: '%s' and '%s'" \

Shame on all of us for not seeing this before :-)  Well done to see this
now :-)

 * It would be nice to do the proper subprocess handling. Perhaps we can
do that by importing subprocess, and catching the import error, and then
falling back to the behaviour in your patch? It should be two lines or a
function that needs to be declared in the import block, and gives us all
the benefits of your proposal. See line 45 of pootle.py for an example.
If I understood everything correctly I believe this can work.

 * You sometimes use double # signs for comments. I assume single ones
are ok?

Thanks again for the contribution.

Friedel


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
Translate-pootle mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/translate-pootle

Reply via email to