[GRASS-dev] [GRASS GIS] #2189: PyGRASS Module does not work on Windows

GRASS GIS GRASS GIS developer
1

#2189: PyGRASS Module does not work on Windows
-------------------------+--------------------------------------------------
Reporter: annakrat | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-trunk
Keywords: pygrass | Platform: MSWindows 8
      Cpu: Unspecified |
-------------------------+--------------------------------------------------
PyGRASS Module class is not working on Windows. It can't find the command:

{{{
import grass.pygrass.modules as pymod
region = pymod.Module("g.region")

GrassError: 'Error running: `g.region --interface-description`.'
}}}

If I enter the entire path of the command, it works:

{{{
egion =
pymod.Module(r"C:\OSGeo4W_dev\apps\grass\grass-7.0.svn\bin\g.region")
region.description
Manages the boundary definitions for the geographic region.
}}}

This ticket is related to #2067 and #2188.

Maybe PyGRASS could be a separate component in trac?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2189&gt;
GRASS GIS <http://grass.osgeo.org>

2

#2189: PyGRASS Module does not work on Windows
-------------------------+--------------------------------------------------
Reporter: annakrat | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-trunk
Keywords: pygrass | Platform: MSWindows 8
      Cpu: Unspecified |
-------------------------+--------------------------------------------------

Comment(by zarch):

Replying to [ticket:2189 annakrat]:
> PyGRASS Module class is not working on Windows. It can't find the
command:
>
> {{{
> import grass.pygrass.modules as pymod
> region = pymod.Module("g.region")
>
> GrassError: 'Error running: `g.region --interface-description`.'
> }}}
>
> If I enter the entire path of the command, it works:
[[BR]]

May be it is only a PATH problem? are you able to run the command from the
terminal?, I'm just using:

(http://trac.osgeo.org/grass/browser/grass/trunk/lib/python/pygrass/modules/interface/module.py#L243)
{{{
# call the command with --interface-description
get_cmd_xml = subprocess.Popen([cmd, "--interface-description"],
                                stdout=subprocess.PIPE)
}}}
[[BR]]

So from my understanding also:

{{{
core.run_command("g.region", "p")
}}}

Should not find the command. Someone has an idea why this is happening?
Both are using the same python class, and both avoid to set any special
environmental variable...

[[BR]]

> Maybe PyGRASS could be a separate component in trac?

For me it's fine.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2189#comment:1&gt;
GRASS GIS <http://grass.osgeo.org>

3

#2189: PyGRASS Module does not work on Windows
-------------------------+--------------------------------------------------
Reporter: annakrat | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-trunk
Keywords: pygrass | Platform: MSWindows 8
      Cpu: Unspecified |
-------------------------+--------------------------------------------------

Comment(by annakrat):

Replying to [comment:1 zarch]:
> Replying to [ticket:2189 annakrat]:
> > PyGRASS Module class is not working on Windows. It can't find the
command:
> >
> > {{{
> > import grass.pygrass.modules as pymod
> > region = pymod.Module("g.region")
> >
> > GrassError: 'Error running: `g.region --interface-description`.'
> > }}}
> >
> > If I enter the entire path of the command, it works:
> [[BR]]
>
> May be it is only a PATH problem? are you able to run the command from
the terminal?, I'm just using:
>
>
(http://trac.osgeo.org/grass/browser/grass/trunk/lib/python/pygrass/modules/interface/module.py#L243)
> {{{
> # call the command with --interface-description
> get_cmd_xml = subprocess.Popen([cmd, "--interface-description"],
> stdout=subprocess.PIPE)
> }}}
> [[BR]]
>
> So from my understanding also:
>
> {{{
> core.run_command("g.region", "p")
> }}}
>
> Should not find the command. Someone has an idea why this is happening?
> Both are using the same python class, and both avoid to set any special
environmental variable...

run_command works. The difference is passing shell=True in Popen
([http://trac.osgeo.org/grass/browser/grass/trunk/lib/python/script/core.py#L51
as here]) and adding this helps. Python documentation warns against using
this but we are using it anyway in script.py. I added it in r58902. I
haven't checked if there are other places in pygrass where it is needed.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2189#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

4

#2189: PyGRASS Module does not work on Windows
-------------------------+--------------------------------------------------
Reporter: annakrat | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-trunk
Keywords: pygrass | Platform: MSWindows 8
      Cpu: Unspecified |
-------------------------+--------------------------------------------------

Comment(by zarch):

Replying to [comment:2 annakrat]:
> > Should not find the command. Someone has an idea why this is
happening?
> > Both are using the same python class, and both avoid to set any
special
> > environmental variable...
>
> run_command works. The difference is passing shell=True in Popen

Thanks, I didn't notice that we were using a modify version of Popen!

> I added it in r58902. I haven't checked if there are other places
> in pygrass where it is needed.

May be I should just use the grass.script.core.Popen class instead. I
would like to reduce duplicated code, especially these tricky parts that
are platform dependent, do you mind If I revert the commit r58902, and
apply the attached changes?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2189#comment:3&gt;
GRASS GIS <http://grass.osgeo.org>

5

#2189: PyGRASS Module does not work on Windows
-------------------------+--------------------------------------------------
Reporter: annakrat | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-trunk
Keywords: pygrass | Platform: MSWindows 8
      Cpu: Unspecified |
-------------------------+--------------------------------------------------

Comment(by annakrat):

Replying to [comment:3 zarch]:
> Replying to [comment:2 annakrat]:
> > > Should not find the command. Someone has an idea why this is
happening?
> > > Both are using the same python class, and both avoid to set any
special
> > > environmental variable...
> >
> > run_command works. The difference is passing shell=True in Popen
>
>
> Thanks, I didn't notice that we were using a modify version of Popen!
>
>
>
> > I added it in r58902. I haven't checked if there are other places
> > in pygrass where it is needed.
>
>
> May be I should just use the grass.script.core.Popen class instead. I
would like to reduce duplicated code, especially these tricky parts that
are platform dependent, do you mind If I revert the commit r58902, and
apply the attached changes?

Sure, that's probably a good idea.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2189#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>

6

#2189: PyGRASS Module does not work on Windows
--------------------------+-------------------------------------------------
  Reporter: annakrat | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-trunk
Resolution: fixed | Keywords: pygrass
  Platform: MSWindows 8 | Cpu: Unspecified
--------------------------+-------------------------------------------------
Changes (by zarch):

  * status: new => closed
  * resolution: => fixed

Comment:

Replying to [comment:4 annakrat]:
> Replying to [comment:3 zarch]:
> > May be I should just use the grass.script.core.Popen
> > class instead. I would like to reduce duplicated code,
> > especially these tricky parts that are platform
> > dependent, do you mind If I revert the commit r58902,
> > and apply the attached changes?
>
> Sure, that's probably a good idea.

[[BR]]

ok, done in r58932. Anyway thank you for your help, you've found the
solution of the problem! :slight_smile:

I've tested on Win7 and it seems to work properly to me, so I'm closing
this ticket as fixed.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2189#comment:5&gt;
GRASS GIS <http://grass.osgeo.org>

7

#2189: PyGRASS Module does not work on Windows
--------------------------+-------------------------------------------------
  Reporter: annakrat | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-trunk
Resolution: fixed | Keywords: pygrass
  Platform: MSWindows 8 | Cpu: Unspecified
--------------------------+-------------------------------------------------

Comment(by glynn):

Replying to [comment:2 annakrat]:

> Python documentation warns against using this but we are using it anyway
in script.py.

The warning in the Python documentation is misleading. It's not that
shell=True is a problem by itself. The problem is shell=True in
combination with passing the command as a string rather than a list, where
the string is dynamically generated.

On Unix, passing the command as a string practically requires shell=True
(passing a string with shell=False will treat the entire string as the
path to an executable, which will be executed without arguments).

As the documentation notes, this combination risks an injection attack. If
a chunk of text which is supposed to be a single argument or part of an
argument contains shell metcharacters (spaces, quotes, etc), it can end up
supplying additional arguments or even additional commands to be executed.
Avoiding the shell and passing the command as a list avoids this problem.

On Windows, the fundamental execution primitive (!CreateProcess) takes a
string rather than a list of strings. A list passed to subprocess.Popen()
is always converted to a string by "inverting" the rules by which MSVCRT
parses the string back to an argument list.

There, the only difference between shell=False and shell=True is that the
latter prepends "cmd.exe /c " to the resulting string (actually, if the
COMSPEC environment variable is set, its value is used in place of
"cmd.exe"). This is the same behaviour as MSVCRT's system() function. The
end result is that shell=True allows you to execute scripts (including
batch files), documents, etc, whereas shell=False only works for binary
executables (execution of scripts is implemented by the shell, rather than
by the kernel as is the case on Unix).

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2189#comment:6&gt;
GRASS GIS <http://grass.osgeo.org>