D8318: setup: build C extensions with -Werror=declaration-after-statement

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

D8318: setup: build C extensions with -Werror=declaration-after-statement

martinvonz (Martin von Zweigbergk)
mharbison72 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  MSVC 2008 still needs declarations at the top of the scope.  I added it to the
  3rd party code too in case somebody vendors a new version with a problem-
  they'll get an early warning.  Clang seems to ignore this (at least on 10.14
  with Xcode 10), and gcc 7.4 will error out as desired on Ubuntu 18.04.  Thanks
  to Yuya for remembering the name of the option.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8318

AFFECTED FILES
  setup.py

CHANGE DETAILS

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -1268,6 +1268,11 @@
 ]
 common_include_dirs = ['mercurial']
 
+common_cflags = []
+
+if os.name != 'nt':
+    common_cflags = ['-Werror=declaration-after-statement']
+
 osutil_cflags = []
 osutil_ldflags = []
 
@@ -1441,18 +1446,21 @@
         'mercurial.cext.base85',
         ['mercurial/cext/base85.c'],
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends,
     ),
     Extension(
         'mercurial.cext.bdiff',
         ['mercurial/bdiff.c', 'mercurial/cext/bdiff.c'] + xdiff_srcs,
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends + ['mercurial/bdiff.h'] + xdiff_headers,
     ),
     Extension(
         'mercurial.cext.mpatch',
         ['mercurial/mpatch.c', 'mercurial/cext/mpatch.c'],
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends,
     ),
     Extension(
@@ -1466,6 +1474,7 @@
             'mercurial/cext/revlog.c',
         ],
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends
         + ['mercurial/cext/charencode.h', 'mercurial/cext/revlog.h',],
     ),
@@ -1473,7 +1482,7 @@
         'mercurial.cext.osutil',
         ['mercurial/cext/osutil.c'],
         include_dirs=common_include_dirs,
-        extra_compile_args=osutil_cflags,
+        extra_compile_args=common_cflags + osutil_cflags,
         extra_link_args=osutil_ldflags,
         depends=common_depends,
     ),
@@ -1482,6 +1491,7 @@
         [
             'mercurial/thirdparty/zope/interface/_zope_interface_coptimizations.c',
         ],
+        extra_compile_args=common_cflags,
     ),
     Extension(
         'mercurial.thirdparty.sha1dc',
@@ -1490,9 +1500,12 @@
             'mercurial/thirdparty/sha1dc/lib/sha1.c',
             'mercurial/thirdparty/sha1dc/lib/ubc_check.c',
         ],
+        extra_compile_args=common_cflags,
     ),
     Extension(
-        'hgext.fsmonitor.pywatchman.bser', ['hgext/fsmonitor/pywatchman/bser.c']
+        'hgext.fsmonitor.pywatchman.bser',
+        ['hgext/fsmonitor/pywatchman/bser.c'],
+        extra_compile_args=common_cflags,
     ),
     RustStandaloneExtension(
         'mercurial.rustext', 'hg-cpython', 'librusthg', py3_features='python3'
@@ -1503,11 +1516,11 @@
 sys.path.insert(0, 'contrib/python-zstandard')
 import setup_zstd
 
-extmodules.append(
-    setup_zstd.get_c_extension(
-        name='mercurial.zstd', root=os.path.abspath(os.path.dirname(__file__))
-    )
+zstd = setup_zstd.get_c_extension(
+    name='mercurial.zstd', root=os.path.abspath(os.path.dirname(__file__))
 )
+zstd.extra_compile_args += common_cflags
+extmodules.append(zstd)
 
 try:
     from distutils import cygwinccompiler



To: mharbison72, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: D8318: setup: build C extensions with -Werror=declaration-after-statement

Yuya Nishihara
>   MSVC 2008 still needs declarations at the top of the scope.

Can you copy this to the code so we can recall why we enable this weird
warning option?

> +if os.name != 'nt':
> +    common_cflags = ['-Werror=declaration-after-statement']
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8318: setup: build C extensions with -Werror=declaration-after-statement

martinvonz (Martin von Zweigbergk)
yuja added a comment.


  >   MSVC 2008 still needs declarations at the top of the scope.
 
  Can you copy this to the code so we can recall why we enable this weird
  warning option?
 
  > +if os.name != 'nt':
  > +    common_cflags = ['-Werror=declaration-after-statement']

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8318/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8318

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8318: setup: build C extensions with -Werror=declaration-after-statement

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
mharbison72 updated this revision to Diff 20860.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8318?vs=20859&id=20860

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8318/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8318

AFFECTED FILES
  setup.py

CHANGE DETAILS

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -1268,6 +1268,12 @@
 ]
 common_include_dirs = ['mercurial']
 
+common_cflags = []
+
+# MSVC 2008 still needs declarations at the top of the scope.
+if os.name != 'nt':
+    common_cflags = ['-Werror=declaration-after-statement']
+
 osutil_cflags = []
 osutil_ldflags = []
 
@@ -1441,18 +1447,21 @@
         'mercurial.cext.base85',
         ['mercurial/cext/base85.c'],
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends,
     ),
     Extension(
         'mercurial.cext.bdiff',
         ['mercurial/bdiff.c', 'mercurial/cext/bdiff.c'] + xdiff_srcs,
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends + ['mercurial/bdiff.h'] + xdiff_headers,
     ),
     Extension(
         'mercurial.cext.mpatch',
         ['mercurial/mpatch.c', 'mercurial/cext/mpatch.c'],
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends,
     ),
     Extension(
@@ -1466,6 +1475,7 @@
             'mercurial/cext/revlog.c',
         ],
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends
         + ['mercurial/cext/charencode.h', 'mercurial/cext/revlog.h',],
     ),
@@ -1473,7 +1483,7 @@
         'mercurial.cext.osutil',
         ['mercurial/cext/osutil.c'],
         include_dirs=common_include_dirs,
-        extra_compile_args=osutil_cflags,
+        extra_compile_args=common_cflags + osutil_cflags,
         extra_link_args=osutil_ldflags,
         depends=common_depends,
     ),
@@ -1482,6 +1492,7 @@
         [
             'mercurial/thirdparty/zope/interface/_zope_interface_coptimizations.c',
         ],
+        extra_compile_args=common_cflags,
     ),
     Extension(
         'mercurial.thirdparty.sha1dc',
@@ -1490,9 +1501,12 @@
             'mercurial/thirdparty/sha1dc/lib/sha1.c',
             'mercurial/thirdparty/sha1dc/lib/ubc_check.c',
         ],
+        extra_compile_args=common_cflags,
     ),
     Extension(
-        'hgext.fsmonitor.pywatchman.bser', ['hgext/fsmonitor/pywatchman/bser.c']
+        'hgext.fsmonitor.pywatchman.bser',
+        ['hgext/fsmonitor/pywatchman/bser.c'],
+        extra_compile_args=common_cflags,
     ),
     RustStandaloneExtension(
         'mercurial.rustext', 'hg-cpython', 'librusthg', py3_features='python3'
@@ -1503,11 +1517,11 @@
 sys.path.insert(0, 'contrib/python-zstandard')
 import setup_zstd
 
-extmodules.append(
-    setup_zstd.get_c_extension(
-        name='mercurial.zstd', root=os.path.abspath(os.path.dirname(__file__))
-    )
+zstd = setup_zstd.get_c_extension(
+    name='mercurial.zstd', root=os.path.abspath(os.path.dirname(__file__))
 )
+zstd.extra_compile_args += common_cflags
+extmodules.append(zstd)
 
 try:
     from distutils import cygwinccompiler



To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8318: setup: build C extensions with -Werror=declaration-after-statement

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
Closed by commit rHG2a98b0cd4995: setup: build C extensions with -Werror=declaration-after-statement (authored by mharbison72).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs Review".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8318?vs=20860&id=20866

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8318/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8318

AFFECTED FILES
  setup.py

CHANGE DETAILS

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -1268,6 +1268,12 @@
 ]
 common_include_dirs = ['mercurial']
 
+common_cflags = []
+
+# MSVC 2008 still needs declarations at the top of the scope.
+if os.name != 'nt':
+    common_cflags = ['-Werror=declaration-after-statement']
+
 osutil_cflags = []
 osutil_ldflags = []
 
@@ -1441,18 +1447,21 @@
         'mercurial.cext.base85',
         ['mercurial/cext/base85.c'],
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends,
     ),
     Extension(
         'mercurial.cext.bdiff',
         ['mercurial/bdiff.c', 'mercurial/cext/bdiff.c'] + xdiff_srcs,
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends + ['mercurial/bdiff.h'] + xdiff_headers,
     ),
     Extension(
         'mercurial.cext.mpatch',
         ['mercurial/mpatch.c', 'mercurial/cext/mpatch.c'],
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends,
     ),
     Extension(
@@ -1466,6 +1475,7 @@
             'mercurial/cext/revlog.c',
         ],
         include_dirs=common_include_dirs,
+        extra_compile_args=common_cflags,
         depends=common_depends
         + ['mercurial/cext/charencode.h', 'mercurial/cext/revlog.h',],
     ),
@@ -1473,7 +1483,7 @@
         'mercurial.cext.osutil',
         ['mercurial/cext/osutil.c'],
         include_dirs=common_include_dirs,
-        extra_compile_args=osutil_cflags,
+        extra_compile_args=common_cflags + osutil_cflags,
         extra_link_args=osutil_ldflags,
         depends=common_depends,
     ),
@@ -1482,6 +1492,7 @@
         [
             'mercurial/thirdparty/zope/interface/_zope_interface_coptimizations.c',
         ],
+        extra_compile_args=common_cflags,
     ),
     Extension(
         'mercurial.thirdparty.sha1dc',
@@ -1490,9 +1501,12 @@
             'mercurial/thirdparty/sha1dc/lib/sha1.c',
             'mercurial/thirdparty/sha1dc/lib/ubc_check.c',
         ],
+        extra_compile_args=common_cflags,
     ),
     Extension(
-        'hgext.fsmonitor.pywatchman.bser', ['hgext/fsmonitor/pywatchman/bser.c']
+        'hgext.fsmonitor.pywatchman.bser',
+        ['hgext/fsmonitor/pywatchman/bser.c'],
+        extra_compile_args=common_cflags,
     ),
     RustStandaloneExtension(
         'mercurial.rustext', 'hg-cpython', 'librusthg', py3_features='python3'
@@ -1503,11 +1517,11 @@
 sys.path.insert(0, 'contrib/python-zstandard')
 import setup_zstd
 
-extmodules.append(
-    setup_zstd.get_c_extension(
-        name='mercurial.zstd', root=os.path.abspath(os.path.dirname(__file__))
-    )
+zstd = setup_zstd.get_c_extension(
+    name='mercurial.zstd', root=os.path.abspath(os.path.dirname(__file__))
 )
+zstd.extra_compile_args += common_cflags
+extmodules.append(zstd)
 
 try:
     from distutils import cygwinccompiler



To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel