[Python-Dev] popen test case inquiry in r55735 using PCBuild8

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