Cannot call Python scripts from Python on MS Windows

[wenzeslaus]

On MS Windows, GRASS does not ensure that its Python executable is used when calling Python script from Python script. This is a problem when some other software installed some other version of Python system-wide.

Currently, GRASS is using Python 2.7.4 and when the 3rd party software installs Python 2.7.3 this one (EXE and DDL) is used but with 2.7.4 packages which causes the famous MAXREPEAT error.

It is not clear who to blame for the issue (GRASS, MS Windows way of calling programs or 3rd party software installing old Python) but it seems that it is GRASS who has to fix the problem.

The issue may appear when:

• Python script calls Python script
• wxGUI calls Python scripts (in its own way or using same mechanism as GRASS Python scripts; example is GUI command console)
• C module calling Python scripts (not included in the ticket title, no issue reported so far, probably not tested; case of g.gui and parser starting generated module forms/dialogs)

The issue was previously discussed in:

• ​new osgeo4w-python and winGrass643RC3-standalone: error in GUI startup (​nabble)
• ​Python handling in winGRASS7 (​nabble)
• ​Handling of Python scripts on MS Windows (​nabble)
• #1941 wxGUI fails with Japanese locale --> mixed Python installs on Windows
• #2015 Missing file association dialog pops up all the time
• #2039 Startup Error

Provided patches:

• putting DLLs to bin not lib in r57639 + r57646 and for G6 r57694 (applied and works: GRASS GUI starts but calling Python scripts does not work)
• ensuring the right Python executable manually in r57910; the non-GUI part reverted in r57911 as a confusing workaround hiding the problem

It is possible to test using v.db.univar module which calls db.univar module (both are Python scripts).

The issue is not so big for G6 but it is very serious for G7 where a lot of modules are Python scripts. But it is even a bigger problem for addons and user scripts which are expected to be written mainly in Python using a lot of existing GRASS modules.

The issue is discussed at many places, let's continue the discussion here only.

[annakrat]

Can we distribute GRASS with the newest version of Python 2.7? Version 2.7.6 fixes the incompatibility issue:

• ​http://hg.python.org/cpython/raw-file/99d03261c1ba/Misc/NEWS (search for "incompatibility")
• ​http://bugs.python.org/issue18050

This wouldn't fix the real problem, it would just fix the consequences (still 2 different Python versions would be used but packages would be compatible). Sooner or later, we will use this Python version anyway.

[glynn]

Replying to wenzeslaus:

It is not clear who to blame for the issue (GRASS, MS Windows way of calling programs or 3rd party software installing old Python) but it seems that it is GRASS who has to fix the problem.

It's GRASS' problem for trying to "bundle" Python.

[hellik]

please see also ticket ​http://trac.osgeo.org/grass/ticket/2138 for some improvements related to addon python script in wingrass6. Hamis did some work in wingrass6dev, IMHO we are near a working solution for addon scripts, some issues are open.

Replying to wenzeslaus:

On MS Windows, GRASS does not ensure that its Python executable is used when calling Python script from Python script. This is a problem when some other software installed some other version of Python system-wide.

Currently, GRASS is using Python 2.7.4 and when the 3rd party software installs Python 2.7.3 this one (EXE and DDL) is used but with 2.7.4 packages which causes the famous MAXREPEAT error.

It is not clear who to blame for the issue (GRASS, MS Windows way of calling programs or 3rd party software installing old Python) but it seems that it is GRASS who has to fix the problem.

The issue may appear when:

• Python script calls Python script

please see ticket mentioned above, Python script calls Python script is working by Hamish's hack

• wxGUI calls Python scripts (in its own way or using same mechanism as GRASS Python scripts; example is GUI command console)
• C module calling Python scripts (not included in the ticket title, no issue reported so far, probably not tested; case of g.gui and parser starting generated module forms/dialogs)

The issue was previously discussed in:

• ​new osgeo4w-python and winGrass643RC3-standalone: error in GUI startup (​nabble)
• ​Python handling in winGRASS7 (​nabble)
• ​Handling of Python scripts on MS Windows (​nabble)
• #1941 wxGUI fails with Japanese locale --> mixed Python installs on Windows
• #2015 Missing file association dialog pops up all the time
• #2039 Startup Error

Provided patches:

• putting DLLs to bin not lib in r57639 + r57646 and for G6 r57694 (applied and works: GRASS GUI starts but calling Python scripts does not work)
• ensuring the right Python executable manually in r57910; the non-GUI part reverted in r57911 as a confusing workaround hiding the problem

It is possible to test using v.db.univar module which calls db.univar module (both are Python scripts).

The issue is not so big for G6

IMHO it's also important for G6 as there are a lot of python addon scripts.

but it is very serious for G7 where a lot of modules are Python scripts. But it is even a bigger problem for addons and user scripts which are expected to be written mainly in Python using a lot of existing GRASS modules.

The issue is discussed at many places, let's continue the discussion here only.

please also add important points in ticket/2138.

thanks Helmut

[wenzeslaus]

This ticket is possible duplicate of 5-year-old blocker #580 which ends with suggestions to get rid of GRASS session concept and installing Python to system.

[hellik]

Replying to wenzeslaus:

This ticket is possible duplicate of 5-year-old blocker #580 which ends with suggestions to get rid of GRASS session concept and installing Python to system.

as mentioned in #2138, IMHO we are near a working solution.

[wenzeslaus]

If I understand #2138 correctly, this would mean that we would have to create a BAT file for every Python script and also user would have to do the same for his or her own GRASS Python scripts. For sure this is better than nothing but it seems to me that it resembles r57910 in the way that it forces the right Python manually.

If r58680 (fix for #2138) and r57910 do the same thing I would say that r57910 is easier to implement and creates less noise. Anyway, can r58680 be ported to trunk in some way? Or is somebody preparing some other patch for this issue (#2150)?

By the way, when we can expect Python version 2.7.6 (which will probably hide this issue) to be packaged in GRASS for MS Windows?

[martinl]

Replying to wenzeslaus:

If I understand #2138 correctly, this would mean that we would have to create a BAT file for every Python script and also user would have to do the same for his or her own GRASS Python scripts. For sure this is better than nothing but it seems to me that it resembles r57910 in the way that it forces the right Python manually.

My opinion is that requiring BAT file for Python scripts is much more worse solution that r57910.

If r58680 (fix for #2138) and r57910 do the same thing I would say that r57910 is easier to implement and creates less noise. Anyway, can r58680 be ported to trunk in some way? Or is somebody preparing some other patch for this issue (#2150)?

I guess nobody is really working on this issue. And if so his work can be reverted again without providing any better solution;-)

By the way, when we can expect Python version 2.7.6 (which will probably hide this issue) to be packaged in GRASS for MS Windows?

Please ask on osgeo4w mailing list.

[glynn]

Replying to martinl:

My opinion is that requiring BAT file for Python scripts is much more worse solution that r57910.

Also, note that there's nothing special about .bat (or .cmd). The mechanism used for "executing" anything other than binary executables is the same, whether it's .bat or .py. If .bat works when .py doesn't, that indicates a fault in the Python installation.

[wenzeslaus]

Replying to martinl:

Please ask on osgeo4w mailing list.

​Update of Python to version 2.7.6 (​nabble, ​gmane)

[martinl]

Replying to glynn:

My opinion is that requiring BAT file for Python scripts is much more worse solution that r57910.

Also, note that there's nothing special about .bat (or .cmd). The mechanism used for "executing" anything other than binary executables is the same, whether it's .bat or .py. If .bat works when .py doesn't, that indicates a fault in the Python installation.

Update: as a result of never ending discussion I took liberty to add generation of bat files in r61136. The missing part is how to solve user python scripts.

[martinl]

Replying to martinl:

Update: as a result of never ending discussion I took liberty to add generation of bat files in r61136. The missing part is how to solve user python scripts.

Update*: calling Python modules from addons (eg. r.basin) is not currently working.

[glynn]

Replying to martinl:

Update*: calling Python modules from addons (eg. r.basin) is not currently working.

Presumably this is because the addon scripts aren't being installed to $GISBASE/scripts.

In which case, we may want something like attachment:pybatch.diff​ (untested), so that SCRIPT_DIR can be overridden by addons.

[martinl]

Replying to glynn:

In which case, we may want something like attachment:pybatch.diff​ (untested), so that SCRIPT_DIR can be overridden by addons.

That's right, the addon scripts are installed by default to $GRASS_ADDON_BASE\scripts. I took liberty to apply your patch in r61715. So let's wait for the next build.

[martinl]

Replying to martinl:

That's right, the addon scripts are installed by default to $GRASS_ADDON_BASE\scripts. I took liberty to apply your patch in r61715. So let's wait for the next build.

this issue is solved. Please note that these changes haven't been backported to relbr7 yet.

[glynn]

Replying to martinl:

I took liberty to apply your patch in r61715. So let's wait for the next build.

Note that the patch is only one part of the solution. It will be necessary for add-ons to set e.g.

SCRIPT_DIR=%GRASS_ADDON_BASE%/scripts

I don't know enough about building add-ons to know where to do this. g.extension? The Makefile for each add-on?

Or maybe we can modify the standard *.make scripts to auto-detect if the module is an add-on and set SCRIPT_DIR accordingly.

[martinl]

Replying to martinl:

Replying to martinl:

That's right, the addon scripts are installed by default to $GRASS_ADDON_BASE\scripts. I took liberty to apply your patch in r61715. So let's wait for the next build.

this issue is solved. Please note that these changes haven't been backported to relbr7 yet.

Done in r61869.

[wenzeslaus]

I'm not sure what is the status now, to be honest.

Here is the current (r61885) diff of grass.script.core.Popen class:

lib/python/script/core.py

@@ -41 +43 @@
 
 
 class Popen(subprocess.Popen):
+    _builtin_exts = set(['.com', '.exe', '.bat', '.cmd'])
 
-    def __init__(self, args, bufsize=0, executable=None,
-                 stdin=None, stdout=None, stderr=None,
-                 preexec_fn=None, close_fds=False, shell=None,
-                 cwd=None, env=None, universal_newlines=False,
-                 startupinfo=None, creationflags=0):
-
-        if shell == None:
-            shell = (sys.platform == "win32")
-        if sys.platform == "win32":
-            # get full path including file extension for scripts
-            fcmd = get_real_command(args[0]) 
-            if fcmd.endswith('.py'):
-                args[0] = fcmd
-                args.insert(0, sys.executable)
-        
-        subprocess.Popen.__init__(self, args, bufsize, executable,
-                                  stdin, stdout, stderr,
-                                  preexec_fn, close_fds, shell,
-                                  cwd, env, universal_newlines,
-                                  startupinfo, creationflags)
+    @staticmethod
+    def _escape_for_shell(arg):
+        # TODO: what are cmd.exe's parsing rules?
+        return arg
+
+    def __init__(self, args, **kwargs):
+        if ( sys.platform == 'win32'
+             and isinstance(args, list)
+             and not kwargs.get('shell', False)
+             and kwargs.get('executable') is None ):
+            cmd = shutil_which(args[0])
+            if cmd is None:
+                raise OSError
+            args = [cmd] + args[1:]
+            name, ext = os.path.splitext(cmd)
+            if ext.lower() not in self._builtin_exts:
+                kwargs['shell'] = True
+                args = [self._escape_for_shell(arg) for arg in args]
+        subprocess.Popen.__init__(self, args, **kwargs)

The 7.0 release branch contains the code which adds sys.executable before executing. The trunk contains the code which finds the the exact executable but I'm not sure about the rest. Especially why it is better then the solution in 7.0 branch and if BAT files are even required since all the work must be done "manually" in Popen class anyway (which seems to bring us back to r57910).

One of the last emails in ​one of the last discussions about that topic:

Glynn Clements Vaclav Petras wrote:

Glynn Clements wrote:

kwargsshell = True args = [self._escape_for_shell(arg) for arg in args]

Considering security issues connected to shell=True* and uncertainty of escaping for MS Windows, wouldn't be better to avoid shell=True and try to use the right interpreter? This can work at least for the most common (and probably only important) case which is Python.

That's an option. Although if we use .bat files to execute Python scripts, shutil_which() will find the .bat file rather than the script itself. If we hard-code the handling of Python scripts, it should only be done for those which are part of GRASS (i.e. where the script is located in a subdirectory of $GISBASE). We would still need to fall back to using the shell for other extensions.

Related blocker tickets (some can be probably considered duplicates):

• #580: WinGRASS: $GISBASE/etc/gui/scripts/ require something like $(EXE) to run
• #1871: winGRASS 7: solve python distribution
• #2333: choose python interpreter during the GRASS installation on windows

Whatever is the solution it should be used in GUI and for user scripts (e.g. I suggested to provide BAT files using g.extension for Python user scripts on MS Windows in the same way g.extension can be used for C modules on unix, see #1652.)

[martinl]

Replying to wenzeslaus:

I'm not sure what is the status now, to be honest.

Here is the current (r61885) diff of grass.script.core.Popen class:

right, I have changed Popen class according to trunk in r61890 and will check the next WinGRASS build.

The 7.0 release branch contains the code which adds sys.executable before executing. The trunk contains the code which finds the the exact executable but I'm not sure about the rest. Especially why it is better then the solution in 7.0 branch and if BAT files are even required since all the work must be done "manually" in Popen class anyway (which seems to bring us back to r57910).

[...]

Whatever is the solution it should be used in GUI and for user scripts (e.g. I suggested to provide BAT files using g.extension for Python user scripts on MS Windows in the same way g.extension can be used for C modules on unix, see #1652.)

They are provided automatically in zip files (1). The bat files are created by building system. The missing part speaking about BAT files are probably user scripts which are not installed via G7:g.extension. The way could be to extent wxGUI code source:grass/trunk/gui/wxpython/lmgr/frame.py#L829​ to generate BAT files automatically or to design a new module which could allow to create BAT files also from terminal (which will be probably very rare situation when speaking about Windows users, so probably not needed).

(1) ​http://wingrass.fsv.cvut.cz/grass71/addons/grass-7.1.svn/logs/d.mon2.log

sed -e "s#SCRIPT_NAME#d.mon2#" -e "s#SCRIPT_DIR#%GISBASE%/scripts#" /c/osgeo4w/usr/src/grass_trunk/scripts/windows_launch.bat > /c/Users/landa/grass_packager/grass71/addons/d.mon2/bin/d.mon2.bat
unix2dos /c/Users/landa/grass_packager/grass71/addons/d.mon2/bin/d.mon2.bat
unix2dos: converting file /c/Users/landa/grass_packager/grass71/addons/d.mon2/bin/d.mon2.bat to DOS format ...

[annakrat]

Glynn, what about r60870? Should it be backported?

[martinl]

Replying to martinl:

this issue is solved. Please note that these changes haven't been backported to relbr7 yet.

Done in r61869.

to make bat files working also from command line I changed interpret from SH to CMD in r61976. But it still doesn't work since forms.py requires to have a module in the path, see attachment:launch-bat.png​ and debug info

g.extension
: filename = C:\OSGEO4~1\apps\grass\grass-7.0.0svn/scripts/g.extension.py
: G_file_name(): path = C:\Users\landa\Documents\GIS DataBase/arccr500-sjtsk
eler
: G_recreate_command()
: win_spawn: args = C:\Windows\system32\cmd.exe /c "C:\OSGEO4~1\bin\python.e
:\OSGEO4~1\apps\grass\grass-7.0.0svn/gui/wxpython/gui_core/forms.py g.extens
py"
: grass.script.core.start_command(): g.gisenv -n

[glynn]

Replying to annakrat:

Glynn, what about r60870? Should it be backported?

Yes.

[martinl]

Replying to glynn:

Replying to annakrat:

Glynn, what about r60870? Should it be backported?

Yes.

done in r61992.

[neteler]

What is the state of this ticket? Can it be closed?

[martinl]

Replying to martinl:

to make bat files working also from command line I changed interpret from SH to CMD in r61976. But it still doesn't work since forms.py requires to have a module in the path, see attachment:launch-bat.png​ and debug info

> g.extension
> : filename = C:\OSGEO4~1\apps\grass\grass-7.0.0svn/scripts/g.extension.py
> : G_file_name(): path = C:\Users\landa\Documents\GIS DataBase/arccr500-sjtsk
> eler
> : G_recreate_command()
> : win_spawn: args = C:\Windows\system32\cmd.exe /c "C:\OSGEO4~1\bin\python.e
> :\OSGEO4~1\apps\grass\grass-7.0.0svn/gui/wxpython/gui_core/forms.py g.extens
> py"
> : grass.script.core.start_command(): g.gisenv -n

this issue is still not solved...

[glynn]

Replying to martinl:

to make bat files working also from command line I changed interpret from SH to CMD in r61976. But it still doesn't work since forms.py requires to have a module in the path

Why does forms.py need the Python script (rather than the batch file) in the path?

[wenzeslaus]

Replying to glynn:

Replying to martinl:

to make bat files working also from command line I changed interpret from SH to CMD in r61976. But it still doesn't work since forms.py requires to have a module in the path

Why does forms.py need the Python script (rather than the batch file) in the path?

Without really checking it now, I think it is because of #2133 (g.parser does call the form.py with full path).

[glynn]

Replying to wenzeslaus:

Why does forms.py need the Python script (rather than the batch file) in the path?

Without really checking it now, I think it is because of #2133 (g.parser does call the form.py with full path).

Okay; the actual issue there is that:

1. The parser doesn't include the path. It doesn't actually know the path, just the name passed to G_set_program_name() (typically by G_gisinit()).

2. It includes the extension, which for a script will be ".py" rather than ".bat".

I note that G_set_program_name() removes any ".exe" suffix. On Windows, it should probably remove ".py" or also (on Unix, scripts are installed without the .py suffix). Or perhaps g.parser should do this. Or even forms.py for that matter.

Actually, this might be a good time to re-consider whether the forms.py behaviour of re-executing the script with --interface-description is wise, or whether it should work the same way as the Tcl/Tk version, i.e. have it fed the description via stdin. OTOH, it's going to have to invoke the script eventually, so may be this doesn't really matter.

[martinl]

Replying to glynn:

I note that G_set_program_name() removes any ".exe" suffix. On Windows, it should probably remove ".py" or also (on Unix, scripts are installed without the .py suffix). Or perhaps g.parser should do this. Or even forms.py for that matter.

done in r62904 (trunk). Let's check the next build on Windows.

[martinl]

Replying to martinl:

Replying to glynn:

I note that G_set_program_name() removes any ".exe" suffix. On Windows, it should probably remove ".py" or also (on Unix, scripts are installed without the .py suffix). Or perhaps g.parser should do this. Or even forms.py for that matter.

done in r62904 (trunk). Let's check the next build on Windows.

that fixes the problem. Backported to relbr70 in r63446.

[martinl]

Replying to martinl:

done in r62904 (trunk). Let's check the next build on Windows.

that fixes the problem. Backported to relbr70 in r63446.

tested, it works. From my point of view it was last issue of this long ticket. Closing, please reopen if needed.