[PATCH 1 of 2 STABLE] setup: add .dll to Python library name

[Adrian Buehlmann]

On 2016-04-27 01:25, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # 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):

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 "


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.