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: Actual results: Expected results: Does this happen every time? Other information:
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] 1st revision 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] revised^2 patch With this patch version, one of the end to end autoools tests passes!
Created attachment 135558 [details] [review] revised^3 patch 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': In tests.py: - self.assertEquals(stdout, 'Hello world (distutils)\n') + self.assertEquals(stdout, 'Hello world (distutils)'+os.linesep) I'd prefer + 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. commit 16bb6ae3761592042868f11bd4ff359667e8df71 Author: Sam Thursfield <ssssam@gmail.com> Date: Thu Jun 4 11:21:26 2009 +0200 [win32] monkeypatch subprocess.Popen on Microsoft Windows (GNOME bug 583455)