classification
Title: Trivial mingw compile fixes
Type: Stage:
Components: Build Versions: Python 3.3
process
Status: open Resolution:
Dependencies: 17591 17598 28269 Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, eric.araujo, jonny, kalev, loewis, martin.panter, rpetrov, schmir
Priority: normal Keywords: patch

Created on 2010-11-22 13:20 by jonny, last changed 2016-09-25 11:27 by martin.panter.

Files
File name Uploaded Description Edit
Python-2.7.diff jonny, 2010-11-22 13:20
Python-2.7.diff jonny, 2010-11-23 06:53
Messages (14)
msg122122 - (view) Author: Johann Hanne (jonny) Date: 2010-11-22 13:20
There are a number of mingw compile issues which are easily fixed
* some "_MSC_VER" #if's should be "MS_WINDOWS" instead
* for cross-compiling, windows.h should be all-lowercase
* mingw has a strcasecmp, so private implementations must not use that name
msg122141 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-11-22 16:17
What's the objective of this patch? I.e. what precisely is it supposed to achieve?
msg122150 - (view) Author: Johann Hanne (jonny) Date: 2010-11-22 17:32
Python 2.7 will currently not compile with MinGW for the outlined reasons.

* There are several "#if defined(_MSC_VER)" macros which surround Windows specific code/preprocessor fragments. But _MSC_VER is only defined with the Visual Studio compiler, not with gcc/MinGW. So the MS_WINDOWS define needs to be used for Windows specific code (and _MSC_VER only for compiler specific code).

* When cross-compiling on Unix with gcc/MinGW, the windows.h header is only found if it's written all lowercase due to Unix filesystems being case-sensitive.

* strcasecmp is a already defined by gcc/MinGW, so it must not be used for defining another function. The patch thus renames a function currently named strcasecmp to my_strcasecmp.
msg122151 - (view) Author: Johann Hanne (jonny) Date: 2010-11-22 17:33
> What's the objective of this patch? I.e. what precisely is it supposed to achieve?

So the answer is: It will fix compiling with gcc/MinGW by fixing the 3 issues described.
msg122174 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-11-22 22:42
I doubt this fix will be enough to fix compilation with mingw32.
But IMO it goes in the right direction.

With this patch, "#ifdef MS_WINDOWS" has the meaning of "linked with some version of MSVCRT".  Do we agree on this, or is another symbol necessary?
msg122175 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-11-22 23:12
> I doubt this fix will be enough to fix compilation with mingw32.

So somebody will have to verify independently. If it fails to fix
the bug completely, it's out of scope for 2.7 (it's out of scope for
2.7.1 IMO either way).

> With this patch, "#ifdef MS_WINDOWS" has the meaning of "linked with
> some version of MSVCRT". Do we agree on this, or is another symbol
> necessary?

MS_WINDOWS is defined as

MS_WINDOWS - Code specific to Windows, but all versions.

So it does *not* imply "MSVCRT". Introducing a new preprocessor symbol
is also out of scope for 2.7, so if we mean "MSC or mingw", we need
to spell that out (i.e. defined(_MSC_VER) || defined(__MINGW32__)).
msg122195 - (view) Author: Johann Hanne (jonny) Date: 2010-11-23 06:53
I've revised the patch to use "defined(_MSC_VER) || defined(__MINGW32__)" as suggested.

And no, it does not solve all mingw compilition issues, but most of them. I've tried to only address the most obvious ones, which are *very* unlikely to break anything.

I have Python 2.7 completely compiling with MinGW (and even major parts of pywin32), but that requires some small changes to MinGW, too. I've also already started to submitting patches to the MinGW project.

Please give me some starting point by applying these patches. I don't care if it's for 2.7.1 or 2.7.2. I'll try hard to get *everything* fixed, but we have to start somewhere.
msg122197 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-11-23 07:07
> Please give me some starting point by applying these patches. I don't
> care if it's for 2.7.1 or 2.7.2. I'll try hard to get *everything*
> fixed, but we have to start somewhere.

I'm skeptical that we should do that with 2.7, then. So it would be
better to target 3.2.
msg122198 - (view) Author: Johann Hanne (jonny) Date: 2010-11-23 07:23
Why exactly are you skeptical? Because it doesn't fix everything in one go? The other changes are also minimal (I'm not even sure if it requires more source changes, maybe I have just to get my #defines right). If you prefer to see a single patch which you can reproduce to fix MinGW compilation completely, I'm willing to do that.

Given that 2.7 is supposed to be a long time support version, I really think people should have a chance to use MinGW. The patch is trivial, but figuring out everything from the compiler error messages can be brain bending.
msg122199 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-11-23 07:46
Since the patch does not completely fix the mingw32 build, I suggest to apply it only on 3.2, and continue to work on mingw32 support there.

Only after we will be able to discuss whether all the changes can be backported to 2.7, provided that they fix the mingw32 build.

2.7 is in maintenance mode, and should not accept a change that only gives progress to a larger development process.
msg122204 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-11-23 08:16
> Why exactly are you skeptical? Because it doesn't fix everything in
> one go? The other changes are also minimal (I'm not even sure if it
> requires more source changes, maybe I have just to get my #defines
> right). If you prefer to see a single patch which you can reproduce
> to fix MinGW compilation completely, I'm willing to do that.

Only bug fixes are acceptable for the 2.7 branch. If this *doesn't*
actually fix the bug (i.e. allows building Python with mingw), then
it's not a bug fix. Mere "improvements" are not acceptable for that
branch.

We have a long tradition of people proposing patches "we need this
and that code to support this and that platform". Then, right after
applying the patch, they come back with more patches, and after
that with more patches. I can accept this for the trunk if it doesn't
break anything, but not for a maintenance branch. The objective should
be to have the minimal necessary amount of changes that just address
the goal of the maintenance branch (i.e. fixes). Work in progress
doesn't belong here (some will argue that work in progress doesn't
even belong to the trunk, but should be carried out in a separate
branch).

> Given that 2.7 is supposed to be a long time support version, I
> really think people should have a chance to use MinGW. The patch is
> trivial, but figuring out everything from the compiler error messages
> can be brain bending.

If this bug (which, unfortunately, hasn't been identified clearly,
either) ever gets fixed, I'm open for backporting it (with the
release manager's approval - it could also be considered as a new
feature - port to mingw - in which case it also would be out of
scope).
msg122207 - (view) Author: Johann Hanne (jonny) Date: 2010-11-23 08:47
Well... ok. Although I already regard the patch as a strict bugfix (it fixes compilation of some C modules on MinGW), I'll go forward and create a patch for Python 3.2 which fixes compilation of all C modules on MinGW (all which are supposed to work on Win32 that is).

I assume once I've shown that the patch still mostly consists of some preprocessor fixes, it will have a fair chance of being applied in 2.7. If somebody knows that there is *no* chance, please speak *now* so I can just stop trying to make something that already works for me available to everybody.
msg122212 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-11-23 12:05
But a "strict bugfix" should fix something.  Is there something that did not work before, and will work after this patch? IOW, how do you compile posixmodule.c with MinGW and does it produce a working module?

Now, I *am* interested in a working MinGW build.  Please share what you have done.  Your patch will certainly be merged into the py3k branch.
But it won't be merged into 2.7 until it gives a tangible benefit to the maintenance branch.
msg277366 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-09-25 11:27
The patch seems to share some changes to Modules/posixmodule.c with parts of Issue 17598’s patch (and one common part has already been applied).

Issue 17591 already fixed <windows.h> to lowercase.

Just now Issue 28269 has been opened about strcasecmp().
History
Date User Action Args
2016-09-25 11:27:58martin.pantersetnosy: + martin.panter
dependencies: + mingw: use header in lowercase, mingw: init system calls, [MinGW] Can't compile Python/dynload_win.c due to static strcasecmp
messages: + msg277366
2011-08-21 11:58:56eric.araujosetnosy: + eric.araujo

versions: + Python 3.3, - Python 2.7
2011-08-19 09:31:42kalevsetnosy: + kalev
2011-02-15 23:08:10schmirsetnosy: + schmir
2010-11-23 12:05:35amaury.forgeotdarcsetmessages: + msg122212
2010-11-23 08:47:21jonnysetmessages: + msg122207
2010-11-23 08:16:02loewissetmessages: + msg122204
2010-11-23 07:46:24amaury.forgeotdarcsetmessages: + msg122199
2010-11-23 07:23:26jonnysetmessages: + msg122198
2010-11-23 07:07:53loewissetmessages: + msg122197
2010-11-23 06:53:48jonnysetfiles: + Python-2.7.diff

messages: + msg122195
2010-11-22 23:12:54loewissetmessages: + msg122175
2010-11-22 22:42:33amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg122174
2010-11-22 22:16:18rpetrovsetnosy: + rpetrov
2010-11-22 17:33:46jonnysetmessages: + msg122151
2010-11-22 17:32:01jonnysetmessages: + msg122150
2010-11-22 16:17:26loewissetnosy: + loewis
messages: + msg122141
2010-11-22 13:20:18jonnycreate