[GRASS-dev] [GRASS GIS] #2150: Cannot call Python scripts from Python on MS Windows

[GRASS GIS]

#2150: Cannot call Python scripts from Python on MS Windows
-------------------------------------------+--------------------------------
 Reporter:  wenzeslaus                     |       Owner:  grass-dev@…              
     Type:  defect                         |      Status:  new                      
 Priority:  blocker                        |   Milestone:  7.0.0                    
Component:  Python                         |     Version:  svn-releasebranch64      
 Keywords:  packaging, MAXREPEAT, scripts  |    Platform:  MSWindows 7              
      Cpu:  Unspecified                    |  
-------------------------------------------+--------------------------------

Comment(by 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:

 {{{
 #!diff
 --- releasebranch_7_0/lib/python/script/core.py 2014-08-18
 14:59:07.838440569 -0400
 +++ gcc_trunk/lib/python/script/core.py 2014-09-12 17:44:31.281833779
 -0400
 @@ -41,39 +43,32 @@


  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 [http://osgeo-org.1560.x6.nabble.com/Re-GRASS-
 SVN-r60679-grass-trunk-lib-python-script-td5143645i20.html one of the last
 discussions] about that topic:

 > Glynn Clements
 > Vaclav Petras wrote:
 > > Glynn Clements wrote:
 > > > kwargs['shell'] = 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.)

-- 
Ticket URL: <http://trac.osgeo.org/grass/ticket/2150#comment:17>
GRASS GIS <http://grass.osgeo.org>