Author Ray.Donnelly
Recipients BreamoreBoy, LRN, Ray.Donnelly, WhiteTiger, alesko, amaury.forgeotdarc, davidfraser, doko, eric.araujo, georg.brandl, giampaolo.rodola, jhuntley, kalev, lkcl, rpetrov, rschoon.old, schmir, scott.tsai, tarek, tshepang
Date 2013-01-26.11:54:03
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1359201245.65.0.673480512673.issue3871@psf.upfronthosting.co.za>
In-reply-to
Content
When I say “our patches” I mean mine and Alexey Pavlov’s jointly maintained patch-set.

I hope you don’t mind that I find you saying:

"I tried some of these patches, but they aren't very organinzed. I really need some docemntaiton to better understand what each patch does and which pairs go together and which don't."

...and then continuing to present a single uber-patch to be mildly funny/ironic? You've already accepted that you should split them up and remove the needless formatting changes to make them more reviewable so I won't labour on this point.

"ctypes
 bz2
 multiprocessing
 sqlite3
 ssl
 pyexpat
 zlib"

Likewise for ours, but also curses, curses_panel, readline, TKinter, IDLE (and probably more besides) for both Win32, Win64, Mac OS X 32bit and 64bit.

The main idea behind our patches is (mostly in common with all patches) that they:
1. Are as atomic as possible in their operation (which as few interdependencies as necessary).
2. Are applied in an obvious ordering.
3. Are named sensibly according to what they do.
4. Build upon - and can reuse updates to - Roumen's uber-patch (and also clean it up a bit; let’s face it Roumen did a lot of the hard work and everyone else is trying to finish it off so we can build releases from it).
5. Enable comprehensive cross-compilation.

Given the name of each patch and the fact that they do one thing (the novel ones at least) and that by-and-large they fit on a few screens, I had hoped that they were usefully descriptive.

To explain which ones go together:
The numbering skips 5 each time for each independent bug-fix or feature - to allow space for later, related modifications. Therefore, where the numbering changes monotonically, the patches are related. This only happens twice, once to address the problem caused by Roumen providing an uber-patch and once due to a block of fixes all related to curses (ncurses and PDCurses are both supported). I assume my addressing of Roumen's uber-patch is what led you to write:

"However, I'm noticing a ton of descrepancies with these patches. Some are removing pthreads, others are adding them back."

There is logic to what you describe as a ton of discrepancies:
Roumen's patch is included in it's entirety as 0030-py3k-20121004-MINGW.patch, then I remove one feature of that patch that needed fixing (threading changes) with 0031-py3k-20121004-MINGW-removal-of-pthread-patch.patch, then I add a patch that provides my newer fix, 0032-py3k-20121004-MINGW-ntthreads.patch. The exact same thing happens with the libffi changes. The pthreads stuff needs fixing due to libwinpthread in MinGW-w64 and libffi is broken when targetting a 64bit Windows host.

My reason for doing this is so that I can directly drop updated versions from Roumen in over 0030 and it will (in theory at least) still work. He can continue to use a uber-patch if he wishes (though I don't think he does this for "new" patches). Also by taking this approach, my patch for the threading issues has no dependencies at all on Roumen’s patch, since a previous patch “handled” the dependency by removing that part of it.

Everything you said about build environment and the test-suite I agree with. My crucifixion-freedom project includes a .vbs to setup the most basic MSYS and MinGW-w64 environment: https://raw.github.com/mingwandroid/crucifixion-freedom/master/scripts/windows/BootstrapMinGW64.vbs
...this is essential to prevent spending time chasing down issues due to discrepancies between build environments.

However, we must go further and add that the patches *cannot* break any other native or cross-compilation, which - as I think Matthias is alluding to - is probably not the case with your patch. This issue is called "cross and native build of python for mingw* hosts", so that your patch possibly breaks builds on build-machine is unfortunate (especially so as a build-machine Python is required to run setup.py as part of the cross-build procedure). Even if it doesn't break building for the build-machine it breaks building the cross-MinGW-w64. Each occurrence of "/mingw" in your patch is probably a part of this problem (you can’t expect cross build systems to be be able to be rooted to /mingw, often you’ll find ~/mingw or /tmp/mingw)

You said, "It seems some are also compiling and using the mingw version outside of the msys shell, which I'm not certain of the advantages there when u can just use the mscv version", I’m not entirely sure what you mean here; if compiling MinGW Python on a Windows system with my patches, MSYS is 100% a requirement (cmd.exe will never run the configure script). I can only guess this relates to the mingw_posix2win_path stuff in your patch. Forgive me if I’m wrong, but it seems that you are conflating MSYS and MinGW when they are not related (except by frequent proximity). Without wanting to point out the obvious, MinGW-w64 is a sysroot and compiler toolchain for making native Windows programs and MSYS is a minimal Posix shell capable of running GNU Autotools. I think a result of this conflation is that a Python built using your patch is not properly usable outside of the MSYS shell (i.e. it will incorrectly transform paths), whereas using our patch-set, the built Python is fully compatible with the MSYS shell and cmd.exe (and whatever else; Wine I guess?).

You likely already know this, but there are two types programs that run on the MSYS shell, MSYS executables built with msys-gcc that link to the msys dll (which then does file path translation when you call any file IO functions) and native Windows executables (for which, when run under MSYS, MSYS’s bash.exe transforms the input arguments before calling the executable when it detects that msys dll is *not* used by that executable). A MinGW/MSYS environment consists of a mix of both, but programs built *with* it should be native Windows executables only (unless you are involved in hacking on the MSYS/MSYS2 projects that is). Your patch makes something in-between. I think this confusion is the reason why you wrote "u can just use the mscv version", please read again the name of this issue. I fail to see how adding a restriction so that MinGW Python can only work properly under MSYS can be argued as anything other than a bug (if you want to build an MSYS Python then that’s a different issue altogether - you’ll end up wrestling with GCC 3.3). Also, with regards to MSVC, given a choice, for philosophical reasons I'll always use software built predominantly with FLOSS software. For sure, as you say, our patches also allow building MinGW Python outside of the MSYS shell, specifically, you can use a Linux or Mac OS X bourne-compatible shell (good luck trying to get recent MinGW cross-compilers for Darwin though). Again, this is a feature and not a problem.

IMHO, updating these patches to track the latest Python is a pointless goal. The only reason I can see to do this would be to make it easy for a reviewer to merge it with the CPython mercurial repository, and realistically, these patches/patch-sets are not going to be merged wholesale. As I said before, the only hope is that bit-by-bit, some small fixes get merged and the patch-set is reduced over time and made more manageable [1]. Instead, I think it makes more sense to target the latest released versions from each branch as that way we’re not chasing a moving target (in particular, we’d only have to update it every-so-often, and if we enforced a disciplined usage of the test-suite then regressions caused by our patches would not be intermixed with ones caused by changes to CPython repository).

To recap, the features of our patch-set are:
1. Readable and reviewable.
2. Doesn't break builds for Cygwin or (likely) any other platform.
3. Fixes cross compilation (for me this is hugely important).
4. Can build 64bit MinGW Python.
5. Works outside of MSYS without any path assumptions or hacks.
6. Attempts to minimize impact to distutils (monkey patching instead of direct modification where possible - this is the most contentious part of Python, patches-wise).
7. Can co-exist with Windows Python due to different dll naming.
8. Can be installed to any prefix, using a Posix installation layout rather than the Windows one (“$prefix/bin”, “$prefix/include”, “$prefix/lib” and “$prefix/share”)
9. Can subsequently be relocated to anywhere the user wishes.
10. Is well tested. Python's built with the 2.7.x version of this patch-set have been in use by many people for well over a year now - to provide pretty-printer support for GDB, without any reported issues.
11. As well as cross/MSYS compiling to/on Windows, these patches allow Darwin Python to be cross-compiled.

I hope you will consider collaborating. If so then I will set aside some time to add descriptions to each of the patch files; otherwise, if you just split your patches up into a series of atomic fixes instead then I’ll examine those patches to see if there’s anything I can use.

As usual, for anyone who wants to get involved in this effort, you can send patches and/or suggestions to me at mingw.android at gmail.com or reply to this issue.

[1] http://hg.python.org/cpython/rev/c0370730b364
History
Date User Action Args
2013-01-26 11:54:06Ray.Donnellysetrecipients: + Ray.Donnelly, georg.brandl, doko, lkcl, amaury.forgeotdarc, davidfraser, giampaolo.rodola, schmir, scott.tsai, tarek, eric.araujo, rpetrov, rschoon.old, WhiteTiger, BreamoreBoy, LRN, alesko, tshepang, kalev, jhuntley
2013-01-26 11:54:05Ray.Donnellysetmessageid: <1359201245.65.0.673480512673.issue3871@psf.upfronthosting.co.za>
2013-01-26 11:54:05Ray.Donnellylinkissue3871 messages
2013-01-26 11:54:03Ray.Donnellycreate