Patchwork [2,of,2,V3] setup: detect Python DLL filename from loaded DLL

login
register
mail settings
Submitter Gregory Szorc
Date April 28, 2016, 2:44 a.m.
Message ID <ef03a7bb759fdb74eae0.1461811448@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/14820/
State Superseded
Commit ee2e4a2c369055564d9d417899b7be42bcaa6e53
Delegated to: Yuya Nishihara
Headers show

Comments

Gregory Szorc - April 28, 2016, 2:44 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1461811432 25200
#      Wed Apr 27 19:43:52 2016 -0700
# Branch stable
# Node ID ef03a7bb759fdb74eae08f00f350de3ef51eedfb
# Parent  5b7f85ccc5851871997f0383331f2982c6db1f43
setup: detect Python DLL filename from loaded DLL

Attempting to build Mercurial from source using MinGW from
msys2 on Windows produces a hg.exe that attempts to load e.g.
python27.dll. MinGW prefixes its library name with "lib" and
adds a period between the major and minor versions. e.g.
"libpython2.7.dll."

Before this patch, hg.exe files in a MinGW environment would
either fail to find a Python DLL or would attempt to load a
non-MinGW DLL, which would summarily explode. Either way,
hg.exe wouldn't work.

This patch improves the code that determines the Python DLL
filename to actually use the loaded Python DLL instead of
inferring it. Basically we take the handle of the loaded DLL
from sys.dllhandle and call a Windows API to try to resolve
that handle to a filename.
Adrian Buehlmann - April 28, 2016, 2:21 p.m.
On 2016-04-28 04:44, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1461811432 25200
> #      Wed Apr 27 19:43:52 2016 -0700
> # Branch stable
> # Node ID ef03a7bb759fdb74eae08f00f350de3ef51eedfb
> # Parent  5b7f85ccc5851871997f0383331f2982c6db1f43
> setup: detect Python DLL filename from loaded DLL

Patch 1 & 2 look good. I didn't test them though.
Yuya Nishihara - April 28, 2016, 3:47 p.m.
On Wed, 27 Apr 2016 19:44:08 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1461811432 25200
> #      Wed Apr 27 19:43:52 2016 -0700
> # Branch stable
> # Node ID ef03a7bb759fdb74eae08f00f350de3ef51eedfb
> # Parent  5b7f85ccc5851871997f0383331f2982c6db1f43
> setup: detect Python DLL filename from loaded DLL
> 
> Attempting to build Mercurial from source using MinGW from
> msys2 on Windows produces a hg.exe that attempts to load e.g.
> python27.dll. MinGW prefixes its library name with "lib" and
> adds a period between the major and minor versions. e.g.
> "libpython2.7.dll."
> 
> Before this patch, hg.exe files in a MinGW environment would
> either fail to find a Python DLL or would attempt to load a
> non-MinGW DLL, which would summarily explode. Either way,
> hg.exe wouldn't work.
> 
> This patch improves the code that determines the Python DLL
> filename to actually use the loaded Python DLL instead of
> inferring it. Basically we take the handle of the loaded DLL
> from sys.dllhandle and call a Windows API to try to resolve
> that handle to a filename.
> 
> diff --git a/setup.py b/setup.py
> --- a/setup.py
> +++ b/setup.py
> @@ -52,16 +52,17 @@ else:
>          import bz2
>          bz2.BZ2Compressor # silence unused import warning
>      except ImportError:
>          raise SystemExit(
>              "Couldn't import standard bz2 (incomplete Python install).")
>  
>  ispypy = "PyPy" in sys.version
>  
> +import ctypes
>  import os, stat, subprocess, time
>  import re
>  import shutil
>  import tempfile
>  from distutils import log
>  if 'FORCE_SETUPTOOLS' in os.environ:
>      from setuptools import setup
>  else:
> @@ -364,18 +365,44 @@ class buildhgexe(build_ext):
>      description = 'compile hg.exe from mercurial/exewrapper.c'
>  
>      def build_extensions(self):
>          if os.name != 'nt':
>              return
>          if isinstance(self.compiler, HackedMingw32CCompiler):
>              self.compiler.compiler_so = self.compiler.compiler # no -mdll
>              self.compiler.dll_libraries = [] # no -lmsrvc90
> -        hv = sys.hexversion
> -        pythonlib = 'python%d%d' % (hv >> 24, (hv >> 16) & 0xff)
> +
> +        # Different Python installs can have different Python library
> +        # names. e.g. the official CPython distribution uses pythonXY.dll
> +        # and MinGW uses libpythonX.Y.dll.
> +        _kernel32 = ctypes.windll.kernel32
> +        _kernel32.GetModuleFileNameA.argtypes = [ctypes.c_void_p,
> +                                                 ctypes.c_void_p,
> +                                                 ctypes.c_ulong]
> +        _kernel32.GetModuleFileNameA.restype = ctypes.c_ulong
> +        size = 1000
> +        buf = ctypes.create_string_buffer(size + 1)
> +        filelen = _kernel32.GetModuleFileNameA(sys.dllhandle, ctypes.byref(buf),
> +                                               size)
> +
> +        if filelen > 0 and filelen != size:
> +            dllbasename = os.path.basename(buf.value)
> +            if not dllbasename.endswith('.dll'):

Can we expect the name is all in lowercase?

> +                raise SystemExit('Python DLL does not end with .dll: %s' %
> +                                 dllbasename)
> +            pythonlib = dllbasename[:-4]

I would do sys.platform == 'msys' or something like that, but your version
looks solid.

> +        else:
> +            log.warn('could not determine Python DLL filename; '
> +                     'assuming pythonX.Y')
> +
> +            hv = sys.hexversion
> +            pythonlib = 'python%d.%d' % (hv >> 24, (hv >> 16) & 0xff)

Perhaps it should be 'python%d%d' (with no dot.)
Gregory Szorc - April 28, 2016, 3:54 p.m.
On Thu, Apr 28, 2016 at 8:47 AM, Yuya Nishihara <yuya@tcha.org> wrote:

> On Wed, 27 Apr 2016 19:44:08 -0700, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc@gmail.com>
> > # Date 1461811432 25200
> > #      Wed Apr 27 19:43:52 2016 -0700
> > # Branch stable
> > # Node ID ef03a7bb759fdb74eae08f00f350de3ef51eedfb
> > # Parent  5b7f85ccc5851871997f0383331f2982c6db1f43
> > setup: detect Python DLL filename from loaded DLL
> >
> > Attempting to build Mercurial from source using MinGW from
> > msys2 on Windows produces a hg.exe that attempts to load e.g.
> > python27.dll. MinGW prefixes its library name with "lib" and
> > adds a period between the major and minor versions. e.g.
> > "libpython2.7.dll."
> >
> > Before this patch, hg.exe files in a MinGW environment would
> > either fail to find a Python DLL or would attempt to load a
> > non-MinGW DLL, which would summarily explode. Either way,
> > hg.exe wouldn't work.
> >
> > This patch improves the code that determines the Python DLL
> > filename to actually use the loaded Python DLL instead of
> > inferring it. Basically we take the handle of the loaded DLL
> > from sys.dllhandle and call a Windows API to try to resolve
> > that handle to a filename.
> >
> > diff --git a/setup.py b/setup.py
> > --- a/setup.py
> > +++ b/setup.py
> > @@ -52,16 +52,17 @@ else:
> >          import bz2
> >          bz2.BZ2Compressor # silence unused import warning
> >      except ImportError:
> >          raise SystemExit(
> >              "Couldn't import standard bz2 (incomplete Python install).")
> >
> >  ispypy = "PyPy" in sys.version
> >
> > +import ctypes
> >  import os, stat, subprocess, time
> >  import re
> >  import shutil
> >  import tempfile
> >  from distutils import log
> >  if 'FORCE_SETUPTOOLS' in os.environ:
> >      from setuptools import setup
> >  else:
> > @@ -364,18 +365,44 @@ class buildhgexe(build_ext):
> >      description = 'compile hg.exe from mercurial/exewrapper.c'
> >
> >      def build_extensions(self):
> >          if os.name != 'nt':
> >              return
> >          if isinstance(self.compiler, HackedMingw32CCompiler):
> >              self.compiler.compiler_so = self.compiler.compiler # no
> -mdll
> >              self.compiler.dll_libraries = [] # no -lmsrvc90
> > -        hv = sys.hexversion
> > -        pythonlib = 'python%d%d' % (hv >> 24, (hv >> 16) & 0xff)
> > +
> > +        # Different Python installs can have different Python library
> > +        # names. e.g. the official CPython distribution uses
> pythonXY.dll
> > +        # and MinGW uses libpythonX.Y.dll.
> > +        _kernel32 = ctypes.windll.kernel32
> > +        _kernel32.GetModuleFileNameA.argtypes = [ctypes.c_void_p,
> > +                                                 ctypes.c_void_p,
> > +                                                 ctypes.c_ulong]
> > +        _kernel32.GetModuleFileNameA.restype = ctypes.c_ulong
> > +        size = 1000
> > +        buf = ctypes.create_string_buffer(size + 1)
> > +        filelen = _kernel32.GetModuleFileNameA(sys.dllhandle,
> ctypes.byref(buf),
> > +                                               size)
> > +
> > +        if filelen > 0 and filelen != size:
> > +            dllbasename = os.path.basename(buf.value)
> > +            if not dllbasename.endswith('.dll'):
>
> Can we expect the name is all in lowercase?
>
> > +                raise SystemExit('Python DLL does not end with .dll:
> %s' %
> > +                                 dllbasename)
> > +            pythonlib = dllbasename[:-4]
>
> I would do sys.platform == 'msys' or something like that, but your version
> looks solid.
>

I think always checking the actually loaded filename is the proper way to
determine the library name.


>
> > +        else:
> > +            log.warn('could not determine Python DLL filename; '
> > +                     'assuming pythonX.Y')
> > +
> > +            hv = sys.hexversion
> > +            pythonlib = 'python%d.%d' % (hv >> 24, (hv >> 16) & 0xff)
>
> Perhaps it should be 'python%d%d' (with no dot.)
>

Yup. I plan to remove this branch in the 3.9 cycle. I only left it in out
of an abundance of caution in case the new code doesn't work somewhere.

Patch

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -52,16 +52,17 @@  else:
         import bz2
         bz2.BZ2Compressor # silence unused import warning
     except ImportError:
         raise SystemExit(
             "Couldn't import standard bz2 (incomplete Python install).")
 
 ispypy = "PyPy" in sys.version
 
+import ctypes
 import os, stat, subprocess, time
 import re
 import shutil
 import tempfile
 from distutils import log
 if 'FORCE_SETUPTOOLS' in os.environ:
     from setuptools import setup
 else:
@@ -364,18 +365,44 @@  class buildhgexe(build_ext):
     description = 'compile hg.exe from mercurial/exewrapper.c'
 
     def build_extensions(self):
         if os.name != 'nt':
             return
         if isinstance(self.compiler, HackedMingw32CCompiler):
             self.compiler.compiler_so = self.compiler.compiler # no -mdll
             self.compiler.dll_libraries = [] # no -lmsrvc90
-        hv = sys.hexversion
-        pythonlib = 'python%d%d' % (hv >> 24, (hv >> 16) & 0xff)
+
+        # Different Python installs can have different Python library
+        # names. e.g. the official CPython distribution uses pythonXY.dll
+        # and MinGW uses libpythonX.Y.dll.
+        _kernel32 = ctypes.windll.kernel32
+        _kernel32.GetModuleFileNameA.argtypes = [ctypes.c_void_p,
+                                                 ctypes.c_void_p,
+                                                 ctypes.c_ulong]
+        _kernel32.GetModuleFileNameA.restype = ctypes.c_ulong
+        size = 1000
+        buf = ctypes.create_string_buffer(size + 1)
+        filelen = _kernel32.GetModuleFileNameA(sys.dllhandle, ctypes.byref(buf),
+                                               size)
+
+        if filelen > 0 and filelen != size:
+            dllbasename = os.path.basename(buf.value)
+            if not dllbasename.endswith('.dll'):
+                raise SystemExit('Python DLL does not end with .dll: %s' %
+                                 dllbasename)
+            pythonlib = dllbasename[:-4]
+        else:
+            log.warn('could not determine Python DLL filename; '
+                     'assuming pythonX.Y')
+
+            hv = sys.hexversion
+            pythonlib = 'python%d.%d' % (hv >> 24, (hv >> 16) & 0xff)
+
+        log.info('using %s as Python library name' % pythonlib)
         with open('mercurial/hgpythonlib.h', 'wb') as f:
             f.write('/* this file is autogenerated by setup.py */\n')
             f.write('#define HGPYTHONLIB "%s"\n' % pythonlib)
         objects = self.compiler.compile(['mercurial/exewrapper.c'],
                                          output_dir=self.build_temp)
         dir = os.path.dirname(self.get_ext_fullpath('dummy'))
         target = os.path.join(dir, 'hg')
         self.compiler.link_executable(objects, target,