Patchwork setup.py: fix C extension compilation issue with OS X 10.9 and Xcode 5.1

login
register
mail settings
Submitter Kent Frazier
Date April 15, 2014, 9:33 p.m.
Message ID <7e50af3ecb436f3dcd8f.1397597621@Phobos.local>
Download mbox | patch
Permalink /patch/4380/
State Accepted
Commit 440fbe52b4a8bf2df8eed57bb8a1145657bdaca7
Headers show

Comments

Kent Frazier - April 15, 2014, 9:33 p.m.
# HG changeset patch
# User Kent Frazier <kentfrazier@gmail.com>
# Date 1397593632 14400
#      Tue Apr 15 16:27:12 2014 -0400
# Node ID 7e50af3ecb436f3dcd8f856e6d244d458675a62c
# Parent  76f68595ff8e5616a79dd79e962a97cc9ba58713
setup.py: fix C extension compilation issue with OS X 10.9 and Xcode 5.1
Matt Mackall - April 15, 2014, 10 p.m.
On Tue, 2014-04-15 at 17:33 -0400, Kent Frazier wrote:
> # HG changeset patch
> # User Kent Frazier <kentfrazier@gmail.com>
> # Date 1397593632 14400
> #      Tue Apr 15 16:27:12 2014 -0400
> # Node ID 7e50af3ecb436f3dcd8f856e6d244d458675a62c
> # Parent  76f68595ff8e5616a79dd79e962a97cc9ba58713
> setup.py: fix C extension compilation issue with OS X 10.9 and Xcode 5.1

Queued for default, thanks. Congratulations on your first Mercurial
patch!
Mads Kiilerich - April 15, 2014, 10:18 p.m.
On 04/15/2014 11:33 PM, Kent Frazier wrote:
> # HG changeset patch
> # User Kent Frazier <kentfrazier@gmail.com>
> # Date 1397593632 14400
> #      Tue Apr 15 16:27:12 2014 -0400
> # Node ID 7e50af3ecb436f3dcd8f856e6d244d458675a62c
> # Parent  76f68595ff8e5616a79dd79e962a97cc9ba58713
> setup.py: fix C extension compilation issue with OS X 10.9 and Xcode 5.1

> +    # XCode 5.1 changes clang such that it now fails to compile if the
> +    # -mno-fused-madd flag is passed, but the version of Python shipped with
> +    # OS X 10.9 Mavericks includes this flag. This causes problems in all
> +    # C extension modules, and a bug has been filed upstream at
> +    # http://bugs.python.org/issue21244. We also need to patch this here
> +    # so Mercurial can continue to compile in the meantime.
> +    if xcode51:
> +        cflags = get_config_var('CFLAGS')
> +        if re.search(r'-mno-fused-madd\b', cflags) is not None:
> +            os.environ['CFLAGS'] = (
> +                os.environ.get('CFLAGS', '') + ' -Qunused-arguments')

Nice to have a workaround so it works out of the box - thanks.

The error message it avoids is:

clang: error: unknown argument: '-mno-fused-madd' 
[-Wunused-command-line-argument-hard-error-in-future]
clang: note: this will be a hard error (cannot be downgraded to a 
warning) in the future

Do you have any thoughts on the merits of -Qunused-arguments compared to 
the apparently more narrow 
-Wno-error=unused-command-line-argument-hard-error-in-future ?

/Mads
Kent Frazier - April 15, 2014, 10:42 p.m.
Mads,

I'm not really familiar with clang flags, but if there is a tighter-scoped
one that accomplishes the same goal, I'd be inclined to agree that it would
be better to use that. To be honest, I copy-pasted the particular flag from
StackOverflow without really researching it. I made the change to the more
restrictive flag locally, and it appears to also resolve the compilation
issue. I also found this article that discusses the underlying issue with
the new version of clang:
https://langui.sh/2014/03/10/wunused-command-line-argument-hard-error-in-future-is-a-harsh-mistress/
In it, he uses ARCHFLAGS instead of CFLAGS. Both seem to work fine, but if
ARCHFLAGS would be more appropriate, perhaps I should make that change
also. I'm not really sure of the distinction between the two.

If I were to update the patch, what would be the most appropriate way to do
that? Should it be a fresh patch on top of the upstream tip, or a
modification of my current patch?

Kent


On Tue, Apr 15, 2014 at 6:18 PM, Mads Kiilerich <mads@kiilerich.com> wrote:

> On 04/15/2014 11:33 PM, Kent Frazier wrote:
>
>> # HG changeset patch
>> # User Kent Frazier <kentfrazier@gmail.com>
>> # Date 1397593632 14400
>> #      Tue Apr 15 16:27:12 2014 -0400
>> # Node ID 7e50af3ecb436f3dcd8f856e6d244d458675a62c
>> # Parent  76f68595ff8e5616a79dd79e962a97cc9ba58713
>> setup.py: fix C extension compilation issue with OS X 10.9 and Xcode 5.1
>>
>
>  +    # XCode 5.1 changes clang such that it now fails to compile if the
>> +    # -mno-fused-madd flag is passed, but the version of Python shipped
>> with
>> +    # OS X 10.9 Mavericks includes this flag. This causes problems in all
>> +    # C extension modules, and a bug has been filed upstream at
>> +    # http://bugs.python.org/issue21244. We also need to patch this here
>> +    # so Mercurial can continue to compile in the meantime.
>> +    if xcode51:
>> +        cflags = get_config_var('CFLAGS')
>> +        if re.search(r'-mno-fused-madd\b', cflags) is not None:
>> +            os.environ['CFLAGS'] = (
>> +                os.environ.get('CFLAGS', '') + ' -Qunused-arguments')
>>
>
> Nice to have a workaround so it works out of the box - thanks.
>
> The error message it avoids is:
>
> clang: error: unknown argument: '-mno-fused-madd' [-Wunused-command-line-
> argument-hard-error-in-future]
> clang: note: this will be a hard error (cannot be downgraded to a warning)
> in the future
>
> Do you have any thoughts on the merits of -Qunused-arguments compared to
> the apparently more narrow -Wno-error=unused-command-
> line-argument-hard-error-in-future ?
>
> /Mads
>

Patch

diff -r 76f68595ff8e -r 7e50af3ecb43 setup.py
--- a/setup.py	Mon Apr 14 15:14:02 2014 -0400
+++ b/setup.py	Tue Apr 15 16:27:12 2014 -0400
@@ -61,6 +61,7 @@ 
             "Couldn't import standard bz2 (incomplete Python install).")
 
 import os, subprocess, time
+import re
 import shutil
 import tempfile
 from distutils import log
@@ -73,7 +74,7 @@ 
 from distutils.spawn import spawn, find_executable
 from distutils import cygwinccompiler
 from distutils.errors import CCompilerError, DistutilsExecError
-from distutils.sysconfig import get_python_inc
+from distutils.sysconfig import get_python_inc, get_config_var
 from distutils.version import StrictVersion
 
 convert2to3 = '--c2to3' in sys.argv
@@ -505,22 +506,36 @@ 
     setupversion = version.split('+', 1)[0]
 
 if sys.platform == 'darwin' and os.path.exists('/usr/bin/xcodebuild'):
-    # XCode 4.0 dropped support for ppc architecture, which is hardcoded in
-    # distutils.sysconfig
     version = runcmd(['/usr/bin/xcodebuild', '-version'], {})[0].splitlines()
     if version:
         version = version[0]
         xcode4 = (version.startswith('Xcode') and
                   StrictVersion(version.split()[1]) >= StrictVersion('4.0'))
+        xcode51 = re.match(r'^Xcode\s+5\.1\.', version) is not None
     else:
         # xcodebuild returns empty on OS X Lion with XCode 4.3 not
         # installed, but instead with only command-line tools. Assume
         # that only happens on >= Lion, thus no PPC support.
         xcode4 = True
+        xcode51 = False
 
+    # XCode 4.0 dropped support for ppc architecture, which is hardcoded in
+    # distutils.sysconfig
     if xcode4:
         os.environ['ARCHFLAGS'] = ''
 
+    # XCode 5.1 changes clang such that it now fails to compile if the
+    # -mno-fused-madd flag is passed, but the version of Python shipped with
+    # OS X 10.9 Mavericks includes this flag. This causes problems in all
+    # C extension modules, and a bug has been filed upstream at
+    # http://bugs.python.org/issue21244. We also need to patch this here
+    # so Mercurial can continue to compile in the meantime.
+    if xcode51:
+        cflags = get_config_var('CFLAGS')
+        if re.search(r'-mno-fused-madd\b', cflags) is not None:
+            os.environ['CFLAGS'] = (
+                os.environ.get('CFLAGS', '') + ' -Qunused-arguments')
+
 setup(name='mercurial',
       version=setupversion,
       author='Matt Mackall and many others',