fixed #5984 and improved test coverage

Lib/distutils/command/build_ext.py

@@ -136,7 +136,7 @@ class build_ext (Command):
         self.swig_opts = None
         self.user = None
 
-    def finalize_options (self):
+    def finalize_options(self):
         from distutils import sysconfig
 
         self.set_undefined_options('build',
@@ -153,15 +153,14 @@ class build_ext (Command):
 
         self.extensions = self.distribution.ext_modules
 
-
         # Make sure Python's include directories (for Python.h, pyconfig.h,
         # etc.) are in the include search path.
         py_include = sysconfig.get_python_inc()
         plat_py_include = sysconfig.get_python_inc(plat_specific=1)
         if self.include_dirs is None:
             self.include_dirs = self.distribution.include_dirs or []
-        if type(self.include_dirs) is StringType:
-            self.include_dirs = string.split(self.include_dirs, os.pathsep)
+        if isinstance(self.include_dirs, str):
+            self.include_dirs = self.include_dirs.split(os.pathsep)
 
         # Put the Python "system" include dir at the end, so that
         # any local include dirs take precedence.
@@ -169,7 +168,7 @@ class build_ext (Command):
         if plat_py_include != py_include:
             self.include_dirs.append(plat_py_include)
 
-        if type(self.libraries) is StringType:
+        if isinstance(self.libraries, str):
             self.libraries = [self.libraries]
 
         # Life is easier if we're not forever checking for None, so
@@ -260,14 +259,14 @@ class build_ext (Command):
         # symbols can be separated with commas.
 
         if self.define:
-            defines = string.split(self.define, ',')
+            defines = self.define.split(',')
             self.define = map(lambda symbol: (symbol, '1'), defines)
 
         # The option for macros to undefine is also a string from the
         # option parsing, but has to be a list.  Multiple symbols can also
         # be separated with commas here.
         if self.undef:
-            self.undef = string.split(self.undef, ',')
+            self.undef = self.undef.split(',')
 
         if self.swig_opts is None:
             self.swig_opts = []
@@ -284,11 +283,7 @@ class build_ext (Command):
                 self.library_dirs.append(user_lib)
                 self.rpath.append(user_lib)
 
-    # finalize_options ()
-
-
-    def run (self):
-
+    def run(self):
         from distutils.ccompiler import new_compiler
 
         # 'self.extensions', as supplied by setup.py, is a list of
@@ -335,7 +330,7 @@ class build_ext (Command):
             self.compiler.set_include_dirs(self.include_dirs)
         if self.define is not None:
             # 'define' option is a list of (name,value) tuples
-            for (name,value) in self.define:
+            for (name, value) in self.define:
                 self.compiler.define_macro(name, value)
         if self.undef is not None:
             for macro in self.undef:
@@ -352,10 +347,7 @@ class build_ext (Command):
         # Now actually compile and link everything.
         self.build_extensions()
 
-    # run ()
-
-
-    def check_extensions_list (self, extensions):
+    def check_extensions_list(self, extensions):
         """Ensure that the list of extensions (presumably provided as a
         command option 'extensions') is valid, i.e. it is a list of
         Extension objects.  We also support the old-style list of 2-tuples,
@@ -365,32 +357,33 @@ class build_ext (Command):
         Raise DistutilsSetupError if the structure is invalid anywhere;
         just returns otherwise.
         """
-        if type(extensions) is not ListType:
+        if not isinstance(extensions, list):
             raise DistutilsSetupError, \
                   "'ext_modules' option must be a list of Extension instances"
 
-        for i in range(len(extensions)):
-            ext = extensions[i]
+        for i, ext in enumerate(extensions):
             if isinstance(ext, Extension):
                 continue                # OK! (assume type-checking done
                                         # by Extension constructor)
 
-            (ext_name, build_info) = ext
-            log.warn(("old-style (ext_name, build_info) tuple found in "
-                      "ext_modules for extension '%s'"
-                      "-- please convert to Extension instance" % ext_name))
-            if type(ext) is not TupleType and len(ext) != 2:
+            if not isinstance(ext, tuple) or len(ext) != 2:
                 raise DistutilsSetupError, \
                       ("each element of 'ext_modules' option must be an "
                        "Extension instance or 2-tuple")
 
-            if not (type(ext_name) is StringType and
+            ext_name, build_info = ext
+
+            log.warn(("old-style (ext_name, build_info) tuple found in "
+                      "ext_modules for extension '%s'"
+                      "-- please convert to Extension instance" % ext_name))
+
+            if not (isinstance(ext_name, str) and
                     extension_name_re.match(ext_name)):
                 raise DistutilsSetupError, \
                       ("first element of each tuple in 'ext_modules' "
                        "must be the extension name (a string)")
 
-            if type(build_info) is not DictionaryType:
+            if not isinstance(build_info, dict):
                 raise DistutilsSetupError, \
                       ("second element of each tuple in 'ext_modules' "
                        "must be a dictionary (build info)")
@@ -401,11 +394,8 @@ class build_ext (Command):
 
             # Easy stuff: one-to-one mapping from dict elements to
             # instance attributes.
-            for key in ('include_dirs',
-                        'library_dirs',
-                        'libraries',
-                        'extra_objects',
-                        'extra_compile_args',
+            for key in ('include_dirs', 'library_dirs', 'libraries',
+                        'extra_objects', 'extra_compile_args',
                         'extra_link_args'):
                 val = build_info.get(key)
                 if val is not None:
@@ -424,8 +414,7 @@ class build_ext (Command):
                 ext.define_macros = []
                 ext.undef_macros = []
                 for macro in macros:
-                    if not (type(macro) is TupleType and
-                            1 <= len(macro) <= 2):
+                    if not (isinstance(macro, tuple) and len(macro) in (1, 2)):
                         raise DistutilsSetupError, \
                               ("'macros' element of build info dict "
                                "must be 1- or 2-tuple")
@@ -436,12 +425,7 @@ class build_ext (Command):
 
             extensions[i] = ext
 
-        # for extensions
-
-    # check_extensions_list ()
-
-
-    def get_source_files (self):
+    def get_source_files(self):
         self.check_extensions_list(self.extensions)
         filenames = []
 
@@ -451,9 +435,7 @@ class build_ext (Command):
 
         return filenames
 
-
-    def get_outputs (self):
-
+    def get_outputs(self):
         # Sanity check the 'extensions' list -- can't assume this is being
         # done in the same run as a 'build_extensions()' call (in fact, we
         # can probably assume that it *isn't*!).
@@ -469,8 +451,6 @@ class build_ext (Command):
                                         self.get_ext_filename(fullname)))
         return outputs
 
-    # get_outputs ()
-
     def build_extensions(self):
         # First, sanity-check the 'extensions' list
         self.check_extensions_list(self.extensions)

Lib/distutils/tests/test_build_ext.py

@@ -9,8 +9,8 @@ from distutils.command.build_ext import build_ext
 from distutils import sysconfig
 from distutils.tests import support
 from distutils.extension import Extension
-from distutils.errors import UnknownFileError
-from distutils.errors import CompileError
+from distutils.errors import (UnknownFileError, DistutilsSetupError,
+                              CompileError)
 
 import unittest
 from test import test_support
@@ -165,6 +165,130 @@ class BuildExtTestCase(support.TempdirManager,
         cmd.ensure_finalized()
         cmd.run()  # should pass
 
+    def test_finalize_options(self):
+        # Make sure Python's include directories (for Python.h, pyconfig.h,
+        # etc.) are in the include search path.
+        modules = [Extension('foo', ['xxx'], optional=False)]
+        dist = Distribution({'name': 'xx', 'ext_modules': modules})
+        cmd = build_ext(dist)
+        cmd.finalize_options()
+
+        from distutils import sysconfig
+        py_include = sysconfig.get_python_inc()
+        self.assert_(py_include in cmd.include_dirs)
+
+        plat_py_include = sysconfig.get_python_inc(plat_specific=1)
+        self.assert_(plat_py_include in cmd.include_dirs)
+
+        # make sure cmd.libraries is turned into a list
+        # if it's a string
+        cmd = build_ext(dist)
+        cmd.libraries = 'my_lib'
+        cmd.finalize_options()
+        self.assertEquals(cmd.libraries, ['my_lib'])
+
+        # make sure cmd.library_dirs is turned into a list
+        # if it's a string
+        cmd = build_ext(dist)
+        cmd.library_dirs = 'my_lib_dir'
+        cmd.finalize_options()
+        self.assertEquals(cmd.library_dirs, ['my_lib_dir'])
+
+        # make sure rpath is turned into a list
+        # if it's a list of os.pathsep's paths
+        cmd = build_ext(dist)
+        cmd.rpath = os.pathsep.join(['one', 'two'])
+        cmd.finalize_options()
+        self.assertEquals(cmd.rpath, ['one', 'two'])
+
+        # XXX more tests to perform for win32
+
+        # make sure define is turned into 2-tuples
+        # strings if they are ','-separated strings
+        cmd = build_ext(dist)
+        cmd.define = 'one,two'
+        cmd.finalize_options()
+        self.assertEquals(cmd.define, [('one', '1'), ('two', '1')])
+
+        # make sure undef is turned into a list of
+        # strings if they are ','-separated strings
+        cmd = build_ext(dist)
+        cmd.undef = 'one,two'
+        cmd.finalize_options()
+        self.assertEquals(cmd.undef, ['one', 'two'])
+
+        # make sure swig_opts is turned into a list
+        cmd = build_ext(dist)
+        cmd.swig_opts = None
+        cmd.finalize_options()
+        self.assertEquals(cmd.swig_opts, [])
+
+        cmd = build_ext(dist)
+        cmd.swig_opts = '1 2'
+        cmd.finalize_options()
+        self.assertEquals(cmd.swig_opts, ['1', '2'])
+
+    def test_check_extensions_list(self):
+        dist = Distribution()
+        cmd = build_ext(dist)
+        cmd.finalize_options()
+
+        #'extensions' option must be a list of Extension instances
+        self.assertRaises(DistutilsSetupError, cmd.check_extensions_list, 'foo')
+
+        # each element of 'ext_modules' option must be an
+        # Extension instance or 2-tuple
+        exts = [('bar', 'foo', 'bar'), 'foo']
+        self.assertRaises(DistutilsSetupError, cmd.check_extensions_list, exts)
+
+        # first element of each tuple in 'ext_modules'
+        # must be the extension name (a string) and match
+        # a python dotted-separated name
+        exts = [('foo-bar', '')]
+        self.assertRaises(DistutilsSetupError, cmd.check_extensions_list, exts)
+
+        # second element of each tuple in 'ext_modules'
+        # must be a ary (build info)
+        exts = [('foo.bar', '')]
+        self.assertRaises(DistutilsSetupError, cmd.check_extensions_list, exts)
+
+        # ok this one should pass
+        exts = [('foo.bar', {'sources': [''], 'libraries': 'foo',
+                             'some': 'bar'})]
+        cmd.check_extensions_list(exts)
+        ext = exts[0]
+        self.assert_(isinstance(ext, Extension))
+
+        # check_extensions_list adds in ext the values passed
+        # when they are in ('include_dirs', 'library_dirs', 'libraries'
+        # 'extra_objects', 'extra_compile_args', 'extra_link_args')
+        self.assertEquals(ext.libraries, 'foo')
+        self.assert_(not hasattr(ext, 'some'))
+
+        # 'macros' element of build info dict must be 1- or 2-tuple
+        exts = [('foo.bar', {'sources': [''], 'libraries': 'foo',
+                'some': 'bar', 'macros': [('1', '2', '3'), 'foo']})]
+        self.assertRaises(DistutilsSetupError, cmd.check_extensions_list, exts)
+
+        exts[0][1]['macros'] = [('1', '2'), ('3',)]
+        cmd.check_extensions_list(exts)
+        self.assertEquals(exts[0].undef_macros, ['3'])
+        self.assertEquals(exts[0].define_macros, [('1', '2')])
+
+    def test_get_source_files(self):
+        modules = [Extension('foo', ['xxx'], optional=False)]
+        dist = Distribution({'name': 'xx', 'ext_modules': modules})
+        cmd = build_ext(dist)
+        cmd.ensure_finalized()
+        self.assertEquals(cmd.get_source_files(), ['xxx'])
+
+    def test_get_outputs(self):
+        modules = [Extension('foo', ['xxx'], optional=False)]
+        dist = Distribution({'name': 'xx', 'ext_modules': modules})
+        cmd = build_ext(dist)
+        cmd.ensure_finalized()
+        self.assertEquals(len(cmd.get_outputs()), 1)
+
 def test_suite():
     src = _get_source_filename()
     if not os.path.exists(src):

Misc/NEWS

@@ -285,6 +285,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #5984: distutils.command.build_ext.check_extensions_list checks were broken
+  for old-style extensions.
+
 - Issue #5971: StreamHandler.handleError now swallows IOErrors which occur when
   trying to print a traceback.