[Python-Dev] popen test case inquiry in r55735 using PCBuild8 (original) (raw)

Mark Hammond mhammond at skippinet.com.au
Wed Jun 6 05:26:33 CEST 2007

My apologies for being a day late, got working on some other things. So here's the scoop as it relates to the issue at hand:

- If you run rt.bat from the trunk as-is and place it in a path that contains an empty space, you receive the error outlined in resultwithspace.txt. - If you run rt.bat from the trunk as-is and place it in a path that does not contain an empty space, you receive no errors as outlined in resultwithoutspace.txt. - If you run rt.bat with the patch, on Windows XP, you receive no errors as outlined in resultafterpatch.txt.

In that last step, you failed to indicate if the path had a space or not. ie, on Windows XP I get that behaviour now without needing to apply the patch.

The patch is attached.

The vast majority of the patch is insignificant - it is either adding braces where they are not necessary, or changing whitespace inappropriately (the spaces you replaced are so the lines all line up regardless of the tab width.) It seems there is only one significant block in your patch, and its not clear to me what the intent of the patch is - I admit I didn't apply it and look at it in-place, but a couple of comments indicating exactly what you are trying to do would be good, especially as I'm not aware of this behaviour change from Win2K -> WinXP.

Probably my biggest question now is the use of GetVersion as opposed to GetVersionEx.

The existing code explicitly checks if it is the 9x or NT family, which your patch no longer does. It seems to me that Windows ME will also qualify - although in general the strcmp for command.com will succeed, if an alternative shell is installed on a ME box it will do the wrong thing. If you need to check anything more than the high-bit of GetVersion(), IMO it should be replaced with GetVersionEx().

Cheers,

Mark

More information about the Python-Dev mailing list