Patchwork [v2] exewrapper: prefer HackableMercurial python if availbale

login
register
mail settings
Submitter Kostia Balytskyi
Date March 13, 2017, 8:06 p.m.
Message ID <83925d3a3306e7023b4b.1489435601@devvm1416.lla2.facebook.com>
Download mbox | patch
Permalink /patch/19301/
State Accepted
Headers show

Comments

Kostia Balytskyi - March 13, 2017, 8:06 p.m.
# HG changeset patch
# User Kostia Balytskyi <ikostia@fb.com>
# Date 1489434253 25200
#      Mon Mar 13 12:44:13 2017 -0700
# Node ID 83925d3a3306e7023b4b43757812285b40fcd90b
# Parent  7548522742b5f4f9f5c0881ae4a2783ecda2f969
exewrapper: prefer HackableMercurial python if availbale

Currently hg.exe will only try to load python27.dll from hg-python
subdir if PYTHONHOME environment variable is not set. I think that
it is better to check whether 'hg-python' subdir exists and load
python from it in that case, regardless of environment. This allows
for reliable approach of distributing Mercurial with its own Python.
David Soria Parra - March 14, 2017, 2:44 a.m.
On Mon, Mar 13, 2017 at 01:06:41PM -0700, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1489434253 25200
> #      Mon Mar 13 12:44:13 2017 -0700
> # Node ID 83925d3a3306e7023b4b43757812285b40fcd90b
> # Parent  7548522742b5f4f9f5c0881ae4a2783ecda2f969
> exewrapper: prefer HackableMercurial python if availbale
> 
> Currently hg.exe will only try to load python27.dll from hg-python
> subdir if PYTHONHOME environment variable is not set. I think that
> it is better to check whether 'hg-python' subdir exists and load
> python from it in that case, regardless of environment. This allows
> for reliable approach of distributing Mercurial with its own Python.

FWIW this looks good to me
Augie Fackler - March 16, 2017, 5:17 p.m.
On Mon, Mar 13, 2017 at 01:06:41PM -0700, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1489434253 25200
> #      Mon Mar 13 12:44:13 2017 -0700
> # Node ID 83925d3a3306e7023b4b43757812285b40fcd90b
> # Parent  7548522742b5f4f9f5c0881ae4a2783ecda2f969
> exewrapper: prefer HackableMercurial python if availbale

Sure, queued. Seems like the right choice.

>
> Currently hg.exe will only try to load python27.dll from hg-python
> subdir if PYTHONHOME environment variable is not set. I think that
> it is better to check whether 'hg-python' subdir exists and load
> python from it in that case, regardless of environment. This allows
> for reliable approach of distributing Mercurial with its own Python.
>
> diff --git a/mercurial/exewrapper.c b/mercurial/exewrapper.c
> --- a/mercurial/exewrapper.c
> +++ b/mercurial/exewrapper.c
> @@ -67,51 +67,35 @@ int main(int argc, char *argv[])
>       }
>
>       pydll = NULL;
> -	/*
> -	We first check, that environment variable PYTHONHOME is *not* set.
> -	This just mimicks the behavior of the regular python.exe, which uses
> -	PYTHONHOME to find its installation directory (if it has been set).
> -	Note: Users of HackableMercurial are expected to *not* set PYTHONHOME!
> -	*/
> -	if (GetEnvironmentVariable("PYTHONHOME", envpyhome,
> -                                sizeof(envpyhome)) == 0)
> -	{
> -		/*
> -		Environment var PYTHONHOME is *not* set. Let's see if we are
> -		running inside a HackableMercurial.
> -		*/
> +
> +	p = strrchr(pyhome, '\\');
> +	if (p == NULL) {
> +		err = "can't find backslash in module filename";
> +		goto bail;
> +	}
> +	*p = 0; /* cut at directory */
> +
> +	/* check for private Python of HackableMercurial */
> +	strcat_s(pyhome, sizeof(pyhome), "\\hg-python");
>
> -		p = strrchr(pyhome, '\\');
> -		if (p == NULL) {
> -			err = "can't find backslash in module filename";
> +	hfind = FindFirstFile(pyhome, &fdata);
> +	if (hfind != INVALID_HANDLE_VALUE) {
> +		/* Path .\hg-python exists. We are probably in HackableMercurial
> +		scenario, so let's load python dll from this dir. */
> +		FindClose(hfind);
> +		strcpy_s(pydllfile, sizeof(pydllfile), pyhome);
> +		strcat_s(pydllfile, sizeof(pydllfile), "\\" HGPYTHONLIB ".dll");
> +		pydll = LoadLibrary(pydllfile);
> +		if (pydll == NULL) {
> +			err = "failed to load private Python DLL " HGPYTHONLIB ".dll";
>                       goto bail;
>               }
> -		*p = 0; /* cut at directory */
> -
> -		/* check for private Python of HackableMercurial */
> -		strcat_s(pyhome, sizeof(pyhome), "\\hg-python");
> -
> -		hfind = FindFirstFile(pyhome, &fdata);
> -		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 ".dll");
> -			pydll = LoadLibrary(pydllfile);
> -			if (pydll == NULL) {
> -				err = "failed to load private Python DLL "
> -                                   HGPYTHONLIB ".dll";
> -				goto bail;
> -			}
> -			Py_SetPythonHome = (void*)GetProcAddress(pydll,
> -							"Py_SetPythonHome");
> -			if (Py_SetPythonHome == NULL) {
> -				err = "failed to get Py_SetPythonHome";
> -				goto bail;
> -			}
> -			Py_SetPythonHome(pyhome);
> +		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) {
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/exewrapper.c b/mercurial/exewrapper.c
--- a/mercurial/exewrapper.c
+++ b/mercurial/exewrapper.c
@@ -67,51 +67,35 @@  int main(int argc, char *argv[])
 	}
 
 	pydll = NULL;
-	/*
-	We first check, that environment variable PYTHONHOME is *not* set.
-	This just mimicks the behavior of the regular python.exe, which uses
-	PYTHONHOME to find its installation directory (if it has been set).
-	Note: Users of HackableMercurial are expected to *not* set PYTHONHOME!
-	*/
-	if (GetEnvironmentVariable("PYTHONHOME", envpyhome,
-				   sizeof(envpyhome)) == 0)
-	{
-		/*
-		Environment var PYTHONHOME is *not* set. Let's see if we are
-		running inside a HackableMercurial.
-		*/
+
+	p = strrchr(pyhome, '\\');
+	if (p == NULL) {
+		err = "can't find backslash in module filename";
+		goto bail;
+	}
+	*p = 0; /* cut at directory */
+
+	/* check for private Python of HackableMercurial */
+	strcat_s(pyhome, sizeof(pyhome), "\\hg-python");
 
-		p = strrchr(pyhome, '\\');
-		if (p == NULL) {
-			err = "can't find backslash in module filename";
+	hfind = FindFirstFile(pyhome, &fdata);
+	if (hfind != INVALID_HANDLE_VALUE) {
+		/* Path .\hg-python exists. We are probably in HackableMercurial
+		scenario, so let's load python dll from this dir. */
+		FindClose(hfind);
+		strcpy_s(pydllfile, sizeof(pydllfile), pyhome);
+		strcat_s(pydllfile, sizeof(pydllfile), "\\" HGPYTHONLIB ".dll");
+		pydll = LoadLibrary(pydllfile);
+		if (pydll == NULL) {
+			err = "failed to load private Python DLL " HGPYTHONLIB ".dll";
 			goto bail;
 		}
-		*p = 0; /* cut at directory */
-
-		/* check for private Python of HackableMercurial */
-		strcat_s(pyhome, sizeof(pyhome), "\\hg-python");
-
-		hfind = FindFirstFile(pyhome, &fdata);
-		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 ".dll");
-			pydll = LoadLibrary(pydllfile);
-			if (pydll == NULL) {
-				err = "failed to load private Python DLL "
-				      HGPYTHONLIB ".dll";
-				goto bail;
-			}
-			Py_SetPythonHome = (void*)GetProcAddress(pydll,
-							"Py_SetPythonHome");
-			if (Py_SetPythonHome == NULL) {
-				err = "failed to get Py_SetPythonHome";
-				goto bail;
-			}
-			Py_SetPythonHome(pyhome);
+		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) {