GNOME Bugzilla – Bug 583455
[win32 fixes] jhbuild uses subprocess.Popen incompatibly with win32
Last modified: 2009-06-04 09:26:19 UTC
Please describe the problem:
Attached is a patch which:
* Moves subprocess.Popen calls into cmds.get_subprocess, and fixes get_output on Win32.
* Adds jhprocess to utils/cmds.py: this class (mostly by John Stowers) fixes path separators, shell execution, certain environment prefixes and output piping/capture including emulation of close_fds.
Steps to reproduce:
Does this happen every time?
Created attachment 135106 [details] [review]
fixes win32 subprocess issues
This definitely needs work, for example:
- splitting commands on spaces to get a list will break in many cases (the
most obvious one is paths with spaces).
- HACK_PREFIX = 'C:\\msys\\' (already marked with FIXME)
- HACK_ENV_VARS = ['PKG_CONFIG_PATH','CFLAGS','LDFLAGS'], why couldn't those
environment variables be set in a form acceptable by Windows in config.py,
instead of doing the job everytime a command needs to be run?
- other buildscripts also have a execute() method
Thinking about it today, I wonder if this part couldn't be made in a win32kludge.py module that would monkeypatch subprocess.Popen, not changing a single line to frontends/*.py.
Would you adapt the patch that way?
Created attachment 135321 [details] [review]
Good idea. I've reimplemented from scratch - this misses out various parts of the original patch which so far haven't proved necessary. However there's some weird errors I still need to investigate so I might need to post some more patches.
However, it is possible to build some of the simpler modules with just git HEAD and this patch (and various parts of MSYS :)
This still needs some more work actually. I'll post a 2nd revised patch soon.
Created attachment 135552 [details] [review]
With this patch version, one of the end to end autoools tests passes!
Created attachment 135558 [details] [review]
Sorry for the patch spam. Fixed 2 whitespace errors I hadn't noticed, and some other dumb mistakes, and fixed accidentally running .exes through sh needlessly.
+ if envvar in [ 'PATH' ]:
You could just do if envvar == 'PATH':
- self.assertEquals(stdout, 'Hello world (distutils)\n')
+ self.assertEquals(stdout, 'Hello world (distutils)'+os.linesep)
+ self.assertEquals(stdout.strip(), 'Hello world (distutils)')
(there are several occurences)
Otherwise it looks good, thanks for working on this, please attach a fixed patch and I'll commit it.
Created attachment 135911 [details] [review]
final (i hope) patch
all done hopefully! thanks for your support!
I reformatted some long comments to fit in 80 columns, and removed the print statement that was subprocess_win32.py.
Author: Sam Thursfield <email@example.com>
Date: Thu Jun 4 11:21:26 2009 +0200
[win32] monkeypatch subprocess.Popen on Microsoft Windows (GNOME bug 583455)