From patchwork Wed Apr 27 07:14:41 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [1,of,2,STABLE] setup: add .dll to Python library name From: Adrian Buehlmann X-Patchwork-Id: 14798 Message-Id: <572066E1.6000505@cadifra.com> To: Gregory Szorc Cc: mercurial-devel@mercurial-scm.org Date: Wed, 27 Apr 2016 09:14:41 +0200 On 2016-04-27 01:25, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1461711788 25200 > # Tue Apr 26 16:03:08 2016 -0700 > # Branch stable > # Node ID ba89075c616d56c3f1ce02c97103c6d0ff8aa006 > # Parent 87d4a6c5567e81386b8c2209d95060d5bf72e064 > setup: add .dll to Python library name > > LoadLibrary() changes behavior depending on whether the argument > passed to it contains a period. From the MSDN docs: > > If no file name extension is specified in the lpFileName parameter, > the default library extension .dll is appended. However, the file name > string can include a trailing point character (.) to indicate that the > module name has no extension. When no path is specified, the function > searches for loaded modules whose base name matches the base name of > the module to be loaded. If the name matches, the load succeeds. > Otherwise, the function searches for the file. > > As the subsequent patch will show, some environments on Windows > define their Python library as e.g. "libpython2.7.dll." The existing > code would pass "libpython2.7" into LoadLibrary(). It would assume > "7" was the file extension and look for a "libpython2.dll" to load. > > By passing ".dll" into LoadLibrary(), we force it to search for the > exact basename we want, even if it contains a period. > > diff --git a/mercurial/exewrapper.c b/mercurial/exewrapper.c > --- a/mercurial/exewrapper.c > +++ b/mercurial/exewrapper.c > @@ -95,33 +95,33 @@ int main(int argc, char *argv[]) > if (hfind != INVALID_HANDLE_VALUE) { > /* path pyhome exists, let's use it */ > FindClose(hfind); > strcpy_s(pydllfile, sizeof(pydllfile), pyhome); > strcat_s(pydllfile, sizeof(pydllfile), "\\" HGPYTHONLIB); > pydll = LoadLibrary(pydllfile); > if (pydll == NULL) { > err = "failed to load private Python DLL " > - HGPYTHONLIB ".dll"; > + HGPYTHONLIB; > goto bail; > } > Py_SetPythonHome = (void*)GetProcAddress(pydll, > "Py_SetPythonHome"); > if (Py_SetPythonHome == NULL) { > err = "failed to get Py_SetPythonHome"; > goto bail; > } > Py_SetPythonHome(pyhome); > } > } > > if (pydll == NULL) { > pydll = LoadLibrary(HGPYTHONLIB); > if (pydll == NULL) { > - err = "failed to load Python DLL " HGPYTHONLIB ".dll"; > + err = "failed to load Python DLL " HGPYTHONLIB; > goto bail; > } > } > > Py_Main = (void*)GetProcAddress(pydll, "Py_Main"); > if (Py_Main == NULL) { > err = "failed to get Py_Main"; > goto bail; > diff --git a/setup.py b/setup.py > --- a/setup.py > +++ b/setup.py > @@ -365,17 +365,17 @@ class buildhgexe(build_ext): > > 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) > + pythonlib = 'python%d%d.dll' % (hv >> 24, (hv >> 16) & 0xff) > 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, I haven't looked into what (and why) you are trying to solve here, but regarding the way *how* you are doing it, I would suggest to *not* add ".dll" on the #define in hgpythonlib.h but instead add it in exewrapper.c wherever you need it. For example (untested): It's very difficult (if not impossible..) to remove the ".dll" from a #define at compile time in case this would be needed (again). Explicitly Adding ".dll" right where you need/want it using string concatenation is much easier. And I would prefer if you would say "exewrapper:" instead of "setup:" in the subject line of your patch. diff --git a/mercurial/exewrapper.c b/mercurial/exewrapper.c --- a/mercurial/exewrapper.c +++ b/mercurial/exewrapper.c @@ -96,7 +96,7 @@ /* path pyhome exists, let's use it */ FindClose(hfind); strcpy_s(pydllfile, sizeof(pydllfile), pyhome); - strcat_s(pydllfile, sizeof(pydllfile), "\\" HGPYTHONLIB); + strcat_s(pydllfile, sizeof(pydllfile), "\\" HGPYTHONLIB ".dll"); pydll = LoadLibrary(pydllfile); if (pydll == NULL) { err = "failed to load private Python DLL "