Looks awesome. Thanks for taking care of these.
Properly escape execute commands and handle environment variables cross-platform
Review Request #515 — Created Aug. 24, 2008 and submitted
|Review Board SVN (deprecated)|
|423, 543, 582, 594|
We were relying on some really bad assumptions in post-review. First, as of r1415, we were assuming all parameters to diff should be quoted, which broke post-review with Perforce when it expected a value that could be cast to an integer. We now represent all commands as lists so that we can use subprocess's support for auto-escaping/quoting of a command and parameters. This should remove the hacks we use and prevent future problems. Second, we were assuming that we could call "env" to set an environment variable for Subversion. This of course doesn't work on Windows. We should have instead used subprocess.Popen's native support for setting the environment.
Couldn't test other SCMTools due to not having a setup here, but I used this code to post this review request.