[GRASS-dev] [GRASS GIS] #3170: GRASS_BATCH_JOB does not tolerate path with spaces

#3170: GRASS_BATCH_JOB does not tolerate path with spaces
----------------------------+-------------------------
Reporter: marisn | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.2.1
Component: Startup | Version: svn-trunk
Keywords: grass.py, init | CPU: Unspecified
Platform: Unspecified |
----------------------------+-------------------------
It is not possible to execute any GRASS_BATCH_JOB script form a path
containing spaces.
{{{
Executing </home/maris/my way/gol_batch.sh>....
/bin/sh: /home/maris/my: No such file or directory
}}}

Quoting or shell escaping also doesn't work due to
get_batch_job_from_env_variable checking file presence with os.access()

Workaround is to change line 1351 of grass.py to read:
{{{
proc = Popen('"' + batch_job + '"', shell=True)
}}}

As quoting of path is a delicate issue and there have been fights in other
parts of GRASS with Popen, I'll better leave for more experienced GRASS
Pythonists to decide if such workaround can be considered safe or more
serious changes are needed.

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

#3170: GRASS_BATCH_JOB does not tolerate path with spaces
--------------------------+----------------------------
  Reporter: marisn | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.2.3
Component: Startup | Version: svn-trunk
Resolution: | Keywords: grass.py, init
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------

Comment (by neteler):

Is the suggested Python workaround a viable way to solve this ticket?

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

#3170: GRASS_BATCH_JOB does not tolerate path with spaces
-------------------------+-------------------------------------------------
  Reporter: marisn | Owner: wenzeslaus
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.0
Component: Startup | Version: svn-trunk
Resolution: | Keywords: grass.py init spaces exec
       CPU: | GRASS_BATCH_JOB Popen shell=True
  Unspecified | Platform: Unspecified
-------------------------+-------------------------------------------------
Changes (by wenzeslaus):

* owner: grass-dev@… => wenzeslaus
* keywords: grass.py, init => grass.py init spaces exec GRASS_BATCH_JOB
     Popen shell=True
* milestone: 7.2.4 => 7.6.0

Comment:

For 7.2 and 7.4 (and maybe 7.6) the workarounds are:

1. Don't use `GRASS_BATCH_JOB`, use more powerful `--exec` instead.
2. Don't use spaces in the path. (And please let us know why do you prefer
`GRASS_BATCH_JOB` rather over `--exec`.)

The spaces-in-path support can be implemented like this:

{{{
#!diff
Index: lib/init/grass.py

--- lib/init/grass.py (revision 73229)
+++ lib/init/grass.py (working copy)
@@ -1511,9 +1511,17 @@
      batch_job_string = batch_job
      if not isinstance(batch_job, string_types):
          batch_job_string = ' '.join(batch_job)
      message(_("Executing <%s> ...") % batch_job_string)
      if isinstance(batch_job, string_types):
+ def quote(string):
+ if '"' in string:
+ return "'%s'" % batch_job
+ else:
+ return '"%s"' % batch_job
+ batch_job = quote(batch_job)
          proc = Popen(batch_job, shell=True)
      else:
          try:
}}}

Here `shell=True` (which is splitting the command using spaces) is used
because it was used with `GRASS_BATCH_JOB` in the past. `--exec` is not
using `shell=True` and allows spaces. Note that for 8.0 there might be
changes to `GRASS_BATCH_JOB` in order to make it more aligned with
`--exec` (see wiki:Grass8Planning).

''Milestone is 7.6 or 7.8.''

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

#3170: GRASS_BATCH_JOB does not tolerate path with spaces
-------------------------+-------------------------------------------------
  Reporter: marisn | Owner: wenzeslaus
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.0
Component: Startup | Version: svn-trunk
Resolution: | Keywords: grass.py init spaces exec
       CPU: | GRASS_BATCH_JOB Popen shell=True
  Unspecified | Platform: Unspecified
-------------------------+-------------------------------------------------

Comment (by marisn):

Replying to [comment:6 wenzeslaus]:
> And please let us know why do you prefer `GRASS_BATCH_JOB` rather over
`--exec`.

To be honest – I do not remember what was the reason of using
GRASS_BATCH_JOB and not --exec. May be just experimenting and stumbled on
a bug (as it is a bug per se).

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3170#comment:7&gt;
GRASS GIS <https://grass.osgeo.org>

#3170: GRASS_BATCH_JOB does not tolerate path with spaces
-------------------------+-------------------------------------------------
  Reporter: marisn | Owner: wenzeslaus
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.0
Component: Startup | Version: svn-trunk
Resolution: | Keywords: grass.py init spaces exec
       CPU: | GRASS_BATCH_JOB Popen shell=True
  Unspecified | Platform: Unspecified
-------------------------+-------------------------------------------------

Comment (by wenzeslaus):

Replying to [comment:7 marisn]:
> Replying to [comment:6 wenzeslaus]:
> > And please let us know why do you prefer `GRASS_BATCH_JOB` rather over
`--exec`.
>
> To be honest – I do not remember what was the reason of using
GRASS_BATCH_JOB and not --exec. May be just experimenting and stumbled on
a bug (as it is a bug per se).

Thanks, makes sense.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3170#comment:8&gt;
GRASS GIS <https://grass.osgeo.org>

#3170: GRASS_BATCH_JOB does not tolerate path with spaces
-------------------------+-------------------------------------------------
  Reporter: marisn | Owner: wenzeslaus
      Type: defect | Status: new
  Priority: normal | Milestone: 7.6.0
Component: Startup | Version: svn-trunk
Resolution: | Keywords: grass.py init spaces exec
       CPU: | GRASS_BATCH_JOB Popen shell=True
  Unspecified | Platform: Unspecified
-------------------------+-------------------------------------------------

Comment (by wenzeslaus):

In [changeset:"73261" 73261]:
{{{
#!CommitTicketReference repository="" revision="73261"
init: support spaces in path for GRASS_BATCH_JOB (see #3170)
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3170#comment:9&gt;
GRASS GIS <https://grass.osgeo.org>

#3170: GRASS_BATCH_JOB does not tolerate path with spaces
-------------------------+-------------------------------------------------
  Reporter: marisn | Owner: wenzeslaus
      Type: defect | Status: closed
  Priority: trivial | Milestone: 7.8.0
Component: Startup | Version: svn-trunk
Resolution: fixed | Keywords: grass.py init spaces exec
       CPU: | GRASS_BATCH_JOB Popen shell=True
  Unspecified | Platform: Unspecified
-------------------------+-------------------------------------------------
Changes (by wenzeslaus):

* priority: normal => trivial
* status: new => closed
* resolution: => fixed
* milestone: 7.6.0 => 7.8.0

Comment:

Since there is `--exec` (see above) and spaces are unlikely here,
backporting this is more trouble than it is worth. Let's have this more
tested and shipped in (or around) 7.8. ''(Thanks for creating that
milestone, BTW.)''

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3170#comment:10&gt;
GRASS GIS <https://grass.osgeo.org>