Opened 8 years ago
Closed 6 years ago
#3170 closed defect (fixed)
GRASS_BATCH_JOB does not tolerate path with spaces
Reported by: | marisn | Owned by: | wenzeslaus |
---|---|---|---|
Priority: | trivial | Milestone: | 7.8.0 |
Component: | Startup | Version: | svn-trunk |
Keywords: | grass.py init spaces exec GRASS_BATCH_JOB Popen shell=True | Cc: | |
CPU: | Unspecified | Platform: | Unspecified |
Description
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.
Change History (10)
comment:1 by , 7 years ago
Milestone: | 7.2.1 → 7.2.2 |
---|
comment:2 by , 7 years ago
Milestone: | 7.2.2 → 7.2.3 |
---|
comment:5 by , 7 years ago
Milestone: | → 7.2.4 |
---|
follow-up: 7 comment:6 by , 6 years ago
Keywords: | spaces exec GRASS_BATCH_JOB Popen shell=True added |
---|---|
Milestone: | 7.2.4 → 7.6.0 |
Owner: | changed from | to
For 7.2 and 7.4 (and maybe 7.6) the workarounds are:
- Don't use
GRASS_BATCH_JOB
, use more powerful--exec
instead. - 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:
-
lib/init/grass.py
1511 1511 batch_job_string = batch_job 1512 1512 if not isinstance(batch_job, string_types): 1513 1513 batch_job_string = ' '.join(batch_job) 1514 1514 message(_("Executing <%s> ...") % batch_job_string) 1515 1515 if isinstance(batch_job, string_types): 1516 def quote(string): 1517 if '"' in string: 1518 return "'%s'" % batch_job 1519 else: 1520 return '"%s"' % batch_job 1521 batch_job = quote(batch_job) 1516 1522 proc = Popen(batch_job, shell=True) 1517 1523 else: 1518 1524 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.
follow-up: 8 comment:7 by , 6 years ago
Replying to 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).
comment:8 by , 6 years ago
Replying to marisn:
Replying to 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.
comment:10 by , 6 years ago
Milestone: | 7.6.0 → 7.8.0 |
---|---|
Priority: | normal → trivial |
Resolution: | → fixed |
Status: | new → closed |
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 retargeted after milestone closed