Closed 52e384d2-8525-4f37-9cb0-4e820faf100e closed 11 years ago
Oscar Benjamin wrote:
Oscar Benjamin added the comment:
I have written a function that can be used to determine if the gcc that distutils will use is from Cygwin or MinGW:
def is_cygwingcc(): '''Try to determine if the gcc that would be used is from cygwin.''' out = Popen(['gcc', '-dumpmachine'], shell=True, stdout=PIPE).stdout try: out_string = out.read() finally: out.close()
out_string is the target triplet cpu-vendor-os
# Cygwin's gcc sets the os to 'cygwin' return out_string.strip().endswith('cygwin')
The idea is that 'gcc -dumpmachine' emits a string that always ends in 'cygwin' for the Cygwin gcc (please let me know if I'm wrong about that). Earnie Boyd at mingw-users described this method for distinguishing MinGW and Cygwin gcc as not being a bad idea: http://permalink.gmane.org/gmane.comp.gnu.mingw.user/42137
With this the Mingw32CCompiler.__init__ method can be modified to do:
if self.gcc_version \< '4' or is_cygwingcc():
It seems to me you try to find another method to detect support of some options. Where is written that compiler is gcc ? Yes this is current distutils code but please review my set of patches
no_cygwin = ' -mno-cygwin'
else: no_cygwin = ''
self.set_executables(compiler='gcc%s -O -Wall' % no_cygwin, compiler_so='gcc%s -mdll -O -Wall' % no_cygwin, compiler_cxx='g++%s -O -Wall' % no_cygwin, linker_exe='gcc%s' % no_cygwin, linker_so='%s%s %s %s' % (self.linker_dll, no_cygwin, shared_option, entry_point))
This will not work in new cygwin (1.7) environment with true cross-compilers. Reason is simple - executable is not gcc.
This will fix the problem for MinGW, should not break existing no-cygwin/gcc 3.x setups and preserves the error message currently seen for no-cygwin with gcc 4.x. In other words it should satisfy users in all three groups A, B and C referred to above. In particular the is_cygwingcc() function hopefully addresses Martin's concern for users in group C.
Is this approach acceptable? It is not enough.
Thanks, Oscar
----------
Roumen
On 23 May 2013 20:11, Roumen Petrov \report@bugs.python.org\ wrote:
> Is this approach acceptable? It is not enough.
Support for compilers other than gcc (including cross-compilers) is a separate issue, and one which is much less likely to get accepted.
I must note that GCC 4.x *does* support -mno-cygwin, at least until 4.4, and at least the MinGW version. I have used it myself for building Pidgin under Windows, which requires that option. See [1] where a Pidgin developer confirms that.
[1] http://pidgin.im/pipermail/support/2011-December/011159.html
Renato Silva added the comment:
I must note that GCC 4.x *does* support -mno-cygwin, at least until 4.4, and at least the MinGW version.
MinGW has never "supported" the -mno-cygwin option. It has simply tolerated it. The option never did anything useful and at some point it became an error to even supply it. I'm not sure exactly when but some time after 4.4 sounds reasonable to me.
The option was only ever meaningful in cygwin's gcc 3.x and was always an error in 4.x.
I have used it myself for building Pidgin under Windows, which requires that option. See [1] where a Pidgin developer confirms that.
No the developer does not confirm that the -mno-cygin option is required for MinGW. Also from what I've seen I would say that the error message that the OP shows there comes from Cygwin's gcc not MinGW.
MinGW has never "supported" the -mno-cygwin option. It has simply tolerated it. The option never did anything useful and at some point it became an error to even supply it. I'm not sure exactly when but some time after 4.4 sounds reasonable to me.
Hi Oscar! Sorry, I just meant to correct this information: "in gcc 4.x it produces an error preventing build". Even if it doesn't do anything useful, still GCC 4.4 does accept that option normally. If MinGW didn't touch anything relevant, then what Cygwin folks said about 4.x [1] clearly did not come to reality.
No the developer does not confirm that the -mno-cygin option is required for MinGW.
Not for MinGW, but for building Pidgin. I have just checked it, and -mno-cygwin actually is no longer necessary since 2.10.7 [1], but it was at the time of that message. Even though it didn't do anything meaningful, a GCC like 4.6 would cause build to fail.
Also from what I've seen I would say that the error message that the OP shows there comes from Cygwin's gcc not MinGW.
No, you can use either Cygwin or MinGW MSYS as environment, but the compiler must be MinGW [2].
[1] https://hg.pidgin.im/pidgin/main/rev/c959cde2a7bd [2] https://developer.pidgin.im/wiki/BuildingWinPidgin#Setupyourbuildenvironment
ERRATA.
Where you see: "[1] clearly did not come to reality." Please read: "[0] clearly did not come to reality."
Oscar Benjamin wrote:
[SNIP]The option was only ever meaningful in cygwin's gcc 3.x and was always an error in 4.x. May be . It seems to me flag was removed in GCC 4.5 .
Since Renata's ERRATA was unclear to whether or not this was *actually removed (please point to a changeset if the option was removed): If the option is no longer required by Pidgin and that was the original reason to have it in the first, there's really no reason *not to remove this, no?
Eg this code
int main() { return 0; } Fails to compile on MingW gcc 4.6 and 4.7 when passing -mno-cygwin to gcc. The solution that distutils does not work with current versions of MingW (and GCC) for good is unacceptable so the only working solution is removal of no-cygwin. Just that GCC 4.4 would support the flag does not change this bug one way or the other.
ERRATA Misunderstood the meaning of ERRATA, please ignore my last post.
Roumen Petrov wrote:
Where is written that compiler is gcc ? Yes this is current distutils code but please review my set of patches I kinda like the using --target-help for finding out if -mno-cygwin is supported assuming --target-help is supported by all GCC's. I disliked the version-based approach since back from 2011 since it seems fragile to me. I'm noo sure if it falls into the category of lightweight checks Eric asked for in #msg146499 but at least more lightweight than my proposal of trying to compile a sample program with the compiler.
On 25 May 2013 04:43, Renato Silva \report@bugs.python.org\ wrote:
Renato Silva added the comment:
Hi Oscar! Sorry, I just meant to correct this information: "in gcc 4.x it produces an error preventing build". Even if it doesn't do anything useful, still GCC 4.4 does accept that option normally. If MinGW didn't touch anything relevant, then what Cygwin folks said about 4.x [1] clearly did not come to reality.
In context it should be clear that the statement "in gcc 4.x it produces an error preventing build" refers to Cygwin's gcc and not MinGW's. Which gcc are you referring to?
> No the developer does not confirm that the -mno-cygin option is > required for MinGW.
Not for MinGW, but for building Pidgin. I have just checked it, and -mno-cygwin actually is no longer necessary since 2.10.7 [1], but it was at the time of that message. Even though it didn't do anything meaningful, a GCC like 4.6 would cause build to fail.
Yes gcc 4.6 would fail because it won't accept the -mno-cygwin option. That does not mean that any other MinGW gcc ever *required* the -mno-cygwin option for anything. The MinGW devs have repeatedly and explicitly stated that the -mno-cygwin option never did anything useful when used with MinGW:
http://permalink.gmane.org/gmane.comp.gnu.mingw.user/42097 http://permalink.gmane.org/gmane.comp.gnu.mingw.user/42101 http://permalink.gmane.org/gmane.comp.gnu.mingw.user/42104
> Also from what I've seen I would say that the error message that > the OP shows there comes from Cygwin's gcc not MinGW.
No, you can use either Cygwin or MinGW MSYS as environment, but the compiler must be MinGW [2].
Yes but that particular error message is coming from Cygwin's gcc not MinGW. As stated by the Pidgin dev in that message the OP does not know which compiler they are using: http://pidgin.im/pipermail/support/2011-December/011159.html
Oscar, 10x for info
I know how to find information for this particular case .
So you last post just confrim what I wrote before two years ( 2011-08-03 http://bugs.python.org/issue12641#msg141614 )
Go ahead and just remove flag.
Roumen
In context it should be clear that the statement "in gcc 4.x it produces an error preventing build" refers to Cygwin's gcc and not MinGW's. Which gcc are you referring to?
If it refers to Cygwin only, sorry then. However, didn't folks said in MinGW thread that they didn't touch anything about such flag there? If so, then how can it have been removed later in 4.5 or 4.6 instead of 4.0 like in Cygwin?
Yes gcc 4.6 would fail because it won't accept the -mno-cygwin option. That does not mean that any other MinGW gcc ever *required* the -mno-cygwin option for anything.
Again, I was not saying it was required *for MinGW, but *for building Pidgin. Note that I'm not saying either that their use of such option was ever meaningful in the build process (in fact, they have removed such flag while still using 4.4, indicating that it was really useless).
Yes but that particular error message is coming from Cygwin's gcc not MinGW. As stated by the Pidgin dev in that message the OP does not know which compiler they are using: http://pidgin.im/pipermail/support/2011-December/011159.html
We actually cannot confirm whether the GCC was from Cygwin, MSYS or Pidgin build box (the correct), and which version. It could be from Cygwin, but still my point stands, that the Pidgin developer does confirm that GCC 4.4 from MinGW *does* accept that flag.
It's cool that you guys are discussing semantics of who said what and how, but that still doesn't fix this very simple issue that breaks compiling for everyone on Windows who uses MinGW. -mno-cygwin, was, as far as I know, only ever required to build from cygwin as host to mingw as target. It is very unlikely that anyone who'd upgrade python still needs this option and is running a gcc cygwin 3.x version. And if it breaks for them it's still better than the current state, which is "broken for everyone else".
Sorry if this sounds snarky, but the fact that this issue has been open for almost 2 years is quite ridiculous.
...the fact that this issue has been open for almost 2 years is quite ridiculous.
I thought that I'd add a little statistic for everyone that might put this bug into perspective. Since this bug was opened, the MinGW installer has been downloaded about 32,000,000 (32 million) times per sf.net:
http://sourceforge.net/projects/mingw/files/Installer/stats/timeline?dates=2011-07-26+to+2013-06-23
If we take a naive approach, that's 32 million compiler installations that can't build Python extensions without manually modifying distutils first.
That statistic doesn't include the multitude of people installing other builds of GCC for Windows (including MinGW-W64, a whole other unsupported version of GCC, but that's a different bug).
On 24.06.2013 00:00, Jeffrey Armstrong wrote:
Jeffrey Armstrong added the comment:
> ...the fact that this issue has been open for almost 2 years is quite ridiculous.
I thought that I'd add a little statistic for everyone that might put this bug into perspective. Since this bug was opened, the MinGW installer has been downloaded about 32,000,000 (32 million) times per sf.net:
http://sourceforge.net/projects/mingw/files/Installer/stats/timeline?dates=2011-07-26+to+2013-06-23
If we take a naive approach, that's 32 million compiler installations that can't build Python extensions without manually modifying distutils first.
That statistic doesn't include the multitude of people installing other builds of GCC for Windows (including MinGW-W64, a whole other unsupported version of GCC, but that's a different bug).
Could someone perhaps produce a single final patch file which can be applied to Python 2.7 and 3.2+ ?
It is not clear at the moment which of all those patches on the ticket should be applied.
On 24 June 2013 09:07, Marc-Andre Lemburg \report@bugs.python.org\ wrote:
Could someone perhaps produce a single final patch file which can be applied to Python 2.7 and 3.2+ ?
I've attached two patches "check_mno_cywin_py27.patch" for Python 2.7 and "check_mno_cywin_py3.patch" for Python 3.2 and 3.3. The changes are identical but the 2.7 patch didn't apply cleanly against 3.x. I'll upload the files used to test the patches in "test_mno_cygwin.tar.gz".
The patches are as I described previously and check the output of 'gcc -dumpmachine' to see if the gcc on PATH is from cygwin. With the patch '-mno-cygwin' will be passed if gcc version \< 4 or the gcc is from cygwin. Otherwise it will not be passed.
I've tested with versions: Python 2.7.5, 3.2.5 and 3.3.2 MinGW gcc 4.7.2 Cygwin gcc 3.4.4 and 4.5.3
The results of the patch are the same for all versions of Python tested: Cygwin gcc 3.x - still works Cygwin gcc 4.x - still doesn't work (same error message) MinGW gcc 4.7 - fixed after the patch
This patch does not attempt to add support for the newer (gcc 4.x) Cygwin cross-compilers. I have experimented with what it would take to have those work and it is something like:
if is_cygwingcc() and version >= 4:
platform = platform_map[get_platform()]
use platform + '-pc-cygwin-gcc' as gcc
use platform + '-pc-cygwin-g++' as g++
etc.
Then there would also need to modifications to the linker settings to fix the problem that Martin mentioned (a long way above) that it would link against the wrong MSVC runtime. I started writing the patch to do these things as well as fix MinGW support and it became more and more of a mess. I don't think that distutils should be trying to guess whether or not people intended to use the Cygwin cross-compilers. If these are to be supported then they should have a new --compiler=cygwin-cross and a separate subclass of CygwinCCompiler to avoid more issues like this one arising in the future.
Oscar
On 24 June 2013 12:53, Oscar Benjamin \report@bugs.python.org\ wrote:
The changes are identical but the 2.7 patch didn't apply cleanly against 3.x. I'll upload the files used to test the patches in "test_mno_cygwin.tar.gz".
Correction: the patches are not quite identical as the py3 patch decodes the output of the subprocess as ascii.
I'm attaching one more patch "check_mno_cywin_py34.patch". This is my preferred patch for Python 3.4 (default). It fixes building with MinGW and removes all support for using Cygwin gcc with --compiler=mingw32. The user would see the following error message:
''' Q:\current\testing\hello>testbuild q:\tools\cygwin\bin -3.3 running build_ext error: Cygwin gcc cannot be used with --compiler=mingw32 '''
I think that this is reasonable as '-mno-cygwin' is a previously experimental and now long deprecated, discouraged and discontinued feature of Cygwin's gcc. Removing support for it in future Pythons would make problems involving MinGW build (like this one) much easier to solve in future: there would be no need to consider anything other than the behaviour of MinGW's gcc.
The is_cygwingcc() function can be simplified a lot with subprocess.check_output().
On 9 July 2013 16:25, Christian Heimes \report@bugs.python.org\ wrote:
The is_cygwingcc() function can be simplified a lot with subprocess.check_output().
My initial thought was to do that but then I based it on _find_exe_version which for whatever reason uses Popen directly [1]. I'm happy to make that change and retest the patches although I can't do it right now.
Can someone first accept or reject the general idea of the patches though? I'm happy to answer any questions about them but it takes time to get the diffs right and test against all compilers and Python versions and I don't really want to do it if the patches will just be rejected.
Also I may soon lose access to the machine that I used to write and test these patches. If it is desired for me to change and retest them it may not be possible after two weeks or so.
[1] http://hg.python.org/cpython/file/3f3cbfd52f94/Lib/distutils/cygwinccompiler.py#l368
Don’t forget that distutils is used during CPython’s build process to compile extension modules: subprocess may not be importable then.
On 9 July 2013 17:36, Éric Araujo \report@bugs.python.org\ wrote:
Don’t forget that distutils is used during CPython’s build process to compile extension modules: subprocess may not be importable then.
Subprocess is imported at at the top of the module in 3.x [1]. The whole distutils.cygwinccompiler module is an ImportError if subprocess is not importable.
Or did you mean for 2.7 only (where get_versions() uses os.popen)?
[1] http://hg.python.org/cpython/file/3f3cbfd52f94/Lib/distutils/cygwinccompiler.py#l51
I'm attaching three new patches following on from Eric and Christian's suggestions:
check_mno_cywin_py27_1.patch (for Python 2.7) check_mno_cywin_py3_1.patch (for Python 3.2 and 3.3) check_mno_cywin_py34_1.patch (for Python 3.4)
The py27 patch now uses os.popen to avoid importing subprocess as suggested by Eric. The other two patches are changed to use check_output as suggested by Christian (subprocess is already imported in 3.x).
I've retested the patches using the same setup as before and the results are unchanged for all gcc and Python versions tested.
*bump*.
This is a critical bugfix that prevents I bet 90%+ of Python users on Windows compiling C extensions. It has been open for 2 years and it's a great disservice to people having to compile stuff on Windows.
Oscar has been doing a terrific job of providing man patches. May I ask that a core dev finally takes some responsibility here, signs of on the patch, and get it applied?
I just noticed today that the fix that implemented by these patches (only providing -mno-cygwin if gcc_ver \< 4) is also used by numpy's distutils. You can see the relevant code here:
https://github.com/numpy/numpy/blob/master/numpy/distutils/mingw32ccompiler.py#L117
The relevant commit was three years ago:
https://github.com/numpy/numpy/commit/9dd7c7b8ad826beefbbc0c10ff457c62f1be223d
They haven't bothered with checking for cygwin gcc (my patches only do this to try and show a helpful error message).
Oscar, thanks for the patches. Two things:
Thanks for looking at this Antoine.
I've attached an updated patch for Python 2.7 called check_mno_cywin_py27_2.patch. This explicitly closes the popen object in the same way as the get_versions() function immediately above.
I've just signed an electronic contributor's agreement.
On 30 September 2013 12:08, Oscar Benjamin \report@bugs.python.org\ wrote:
I've attached an updated patch for Python 2.7 called check_mno_cywin_py27_2.patch.
To be clear: I retested this patch (using the setup described above) and the results are unchanged.
New changeset 7d9a1aa8d95e by Antoine Pitrou in branch '2.7': Issue bpo-12641: Avoid passing "-mno-cygwin" to the mingw32 compiler, except when necessary. http://hg.python.org/cpython/rev/7d9a1aa8d95e
New changeset 6b89176f1be5 by Antoine Pitrou in branch '3.3': Issue bpo-12641: Avoid passing "-mno-cygwin" to the mingw32 compiler, except when necessary. http://hg.python.org/cpython/rev/6b89176f1be5
New changeset 8e180b2067e4 by Antoine Pitrou in branch 'default': Issue bpo-12641: Avoid passing "-mno-cygwin" to the mingw32 compiler, except when necessary. http://hg.python.org/cpython/rev/8e180b2067e4
Thank you Oscar! This issue can endly be fixed.
Thanks Antoine!
This is broken for Python 2.7.4. Is it fixed in 2.7.6? gcc deprecated -mno-cygwin. It's not there any more. There shouldn't be a discussion. This is an incredibly poor example of how fragmentation and poor process result in poor quality open source software--in the corners, even when the overall code is excellent. Over 2 years on something this trivial. Hundreds of lines of discussion. There wasn't really any reason for this to have persisted.
This is broken for Python 2.7.4. Is it fixed in 2.7.6?
The fix is in 2.7.6, yes.
I'm seeing this bug in 2.7.9. The reason seems to be that the version detection doesn't work...
This snippet:
out = os.popen(gcc_exe + ' -dumpversion', 'r')
out_string = out.read()
returns an empty out_string, causing gcc_version = None \< '4'
Maybe the \< '4' check could be restructured to see None as "probably modern" instead of "probably very out of date" ?
Michael, this issue is closed and the changes have long since been released. Comments here will likely be ignored. Please open a new issue describing the problem you are seeing and under what environment, with exact steps to reproduce it.
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields: ```python assignee = 'https://github.com/pitrou' closed_at =
created_at =
labels = ['type-bug', 'library']
title = 'Remove -mno-cygwin from distutils'
updated_at =
user = 'https://bugs.python.org/jonforums'
```
bugs.python.org fields:
```python
activity =
actor = 'ned.deily'
assignee = 'pitrou'
closed = True
closed_date =
closer = 'pitrou'
components = ['Distutils', 'Distutils2']
creation =
creator = 'jonforums'
dependencies = []
files = ['22775', '29030', '29031', '29032', '29103', '30681', '30682', '30683', '30698', '30888', '30889', '30890', '31919']
hgrepos = []
issue_num = 12641
keywords = ['patch']
message_count = 86.0
messages = ['141172', '141173', '141230', '141231', '141265', '141271', '141277', '141296', '141301', '141613', '141614', '141661', '141662', '146384', '146499', '161514', '165178', '165181', '165182', '165184', '165210', '167620', '179175', '179176', '181830', '181831', '181834', '181865', '182090', '182277', '182726', '182737', '183032', '183048', '183077', '183081', '183213', '183224', '183227', '183240', '183241', '189748', '189756', '189769', '189770', '189786', '189805', '189813', '189814', '189871', '189874', '189878', '189887', '189906', '189941', '189943', '189946', '189947', '189948', '189958', '189962', '190060', '191714', '191735', '191749', '191753', '191755', '191845', '192758', '192763', '192764', '192766', '192877', '195576', '195809', '198650', '198690', '198691', '198732', '198733', '198734', '198763', '207498', '207499', '238062', '238067']
nosy_count = 35.0
nosy_names = ['lemburg', 'loewis', 'doko', 'paul.moore', 'pje', 'geertj', 'pitrou', 'christian.heimes', 'schmir', 'tarek', 'jkloth', 'jwilk', 'ned.deily', 'eric.araujo', 'rpetrov', 'cmcqueen1975', 'rubenvb', 'santoso.wijaya', 'alexis', 'python-dev', 'Seppo.Yli-Olli', 'jonforums', 'RubyTuesdayDONO', 'Jeffrey.Armstrong', 'danmbox', 'Martin.Fiers', 'oscarbenjamin', 'Pete.Forman', 'fratti', 'rivy', 'gaudyallure52', 'honorableinvasi', 'stakingrainbow2', 'lewisl', 'Michael.Clerx']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue12641'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']
```