[GRASS-dev] New wiki page summarising GRASS APIs

[Glynn Clements]

Moritz Lennert wrote:

> >> For now, I've added a warning that you should know what you are doing if
> >> you want to use these calls.
> >
> > Why not just use suitable examples, e.g. using subprocess.Popen()
> > (without shell=True), with the caveat that on Windows it won't work
> > for scripts, only compiled executables.
> 
> But what exactly is the problem with using subprocess.call with 
> shell=True ?

It requires you to construct the correct string. If the command string
is a single literal string with all argument values consisting of
alphanumerics plus "safe" punctuation (i.e. characters which have no
meaning to any shell), that's simple enough.

But if any of the arguments are variable and *could* contain
characters which are meaningful to a shell, constructing the correct
string (the one which results in the called program's argv[] having
the intended values) isn't straightforward (and the exact rules vary
between platforms).

It's much simpler (and safer) to just remove the shell from equation
altogether and pass the individual arguments directly, rather than
trying to construct a string which, when deconstructed by the shell,
will produce the correct result.

> Security issues ? Difficulties in calling shell scripts ?

Security would be a problem, but if you're dealing with potentially
malicious input, it's the least of your problems compared to the rest
of GRASS. Calling scripts on Windows is a different problem (setting
shell=True "solves" one problem while introducing more).

> AFAICT, it's just a wrapper around Popen.wait(), or ?

subprocess.call() is a wrapper() around Popen() and Popen.wait(). 
Exactly the same issue applies to Popen() itself (it applies to
.call() *because* it applies to Popen() itself).

> I've now replaced this with:
> 
>   subprocess.Popen(['r.watershed', 'elevation=elevation', 
> 'threshold=10000', 'stream=raster_streams'])
> 
> or would it be better with .wait():
> 
>   import subprocess
>   subprocess.Popen(['r.watershed', 'elevation=elevation', 
> 'threshold=10000', 'stream=raster_streams']).wait()

subprocess.call() is fine. It's using shell=True which is the problem.

For simple examples, I'd suggest using subprocess.call, and referring
them to the python.org documentation for the (2.x) subprocess module
for anything else.

> And even though within GRASS Python it should always be Popen(), maybe 
> there are situations out there where calling a module via call() is 
> justifiable...

Well, depending upon what you mean by "GRASS Python", it should
arguably be grass.script.run_command() or similar.

Even if the wiki page isn't the right place to introduce that, it's
probably worth mentioning that it exists before users end up
recreating the wheel (e.g. if they care about being able to run both
executables and script and doing so on both Unix/MacOSX and Windows,
they'll end up having to learn the same lessons we have).

-- 
Glynn Clements <glynn at gclements.plus.com>