/usr/lib/grass64/etc/grass-run.sh does not work properly when called by gis.m to execute in a external xterm

[frankie]

PATH=<grass>/bin:<grass>/scripts:$PATH prepending is missing: that causes not found errors for all grass commands. Of course it works correctly when called within the main terminal window because the PATH is properly initialized before.

================================================================= If you wish to resize the X monitor, do so now. Window size is locked while interactive modules are running. =================================================================

/usr/lib/grass64/etc/grass-run.sh: line 29: g.proj -p: command not found

ERROR: g.proj -p exited abnormally. Press <enter> to continue.

[hamish]

what tool? r.digit works fine for me (using etch) from the Raster->Develop map gis.m menu.

the profile tool seems to work ok. (I tried it because that runs '|g.proj -p')

if you look in grass-run.sh it has already:

# workaround for systems with xterm is setuid/setgid
#  http://grass.itc.it/pipermail/grass5/2004-September/015409.html
LD_LIBRARY_PATH=$GRASS_LD_LIBRARY_PATH
export LD_LIBRARY_PATH

what xterm do you use? is this 6.4.0rc4?

Hamish

[frankie]

I mean the Output Tk window of gis.m, where "Run (in Xterm)" can be called. That is not a problem with library search path, but binaries search path (aka PATH in bash).

[hamish]

Replying to frankie:

I mean the Output Tk window of gis.m, where "Run (in Xterm)" can be called.

ah, yes. I can reproduce the problem in 6.5svn on Lenny.

That is not a problem with library search path, but binaries search path (aka PATH in bash).

yeah my initial thought was that Sid might have introduced some new xterm child filtering which now cleansed PATH as well as LD_LIBRARY_PATH.

Hamish

[hamish]

Ok, found it. It was gis.m/grass-run.sh trying to execute the command in full (spaces, args, and all) quoted instead of breaking it down into a $cmd and $args. I added "set" to the grass-run.sh script and the PATHs are all fine, it is just that there is no ./bin/"g.proj -p" executable. The give-away was trying "ls -l" and having that fail too.

I've now added in 6.5svn a new feature where if g.gisenv's DEBUG is set >=1 it will pause before closing the xterm.

it would seem that the problem is on this line of lib/init/grass-run.src:

# run command
"$@"

which was introduced, somewhat ironically, in r20416. The point of r20416 was to allow commands with spaces like:

  # (maybe not the best example, but..)
  v.db.select roads where="label ~ 'highway'"

... to function correctly.

but it looks like it is backfiring.

not sure how to fix it in sh, in Tcl at least we can use a list of strings. some magic with "shift" perhaps?

Hamish

[hamish]

Replying to hamish:

not sure how to fix it in sh

this patch seems to work:

Index: lib/init/grass-run.src
===================================================================
--- lib/init/grass-run.src      (revision 37238)
+++ lib/init/grass-run.src      (working copy)
@@ -26,7 +26,7 @@
 echo
 
 # run command
-"$@"
+eval "$@"
 
 EXIT_VAL=$?
 if [ $EXIT_VAL -ne 0 ] ; then

exec is no good because it takes over control from the script.

?, Hamish

[glynn]

Replying to hamish:

Ok, found it. It was gis.m/grass-run.sh trying to execute the command in full (spaces, args, and all) quoted instead of breaking it down into a $cmd and $args.

That's incorrect; the shell handles "$@" specially.

it would seem that the problem is on this line of lib/init/grass-run.src:

> # run command
> "$@"

The "$@" syntax evaluates to the argument list broken into words according to argument boundaries, rather than broken by spaces. If you use $@ without quotes, the argument list is re-parsed, so any arguments which contain spaces will be split into multiple arguments.

which was introduced, somewhat ironically, in r20416. The point of r20416 was to allow commands with spaces like:

>   # (maybe not the best example, but..)
>   v.db.select roads where="label ~ 'highway'"

... to function correctly.

Yep.

but it looks like it is backfiring.

No, the script itself is correct. How is it being called?

not sure how to fix it in sh, in Tcl at least we can use a list of strings. some magic with "shift" perhaps?

What is the exact command being passed to the script?

Add:

/path/to/args "$@"

to the beginning of the script, after compiling and installing the following program as "/path/to/args":

#include <stdio.h>
int main(int argc, char ** argv)
{
	int i;
	for (i = 0; i < argc; i++)
		printf("argv[%2d] = '%s'\n", i, argv[i]);
	return 0;
}

[glynn]

Replying to hamish:

not sure how to fix it in sh

this patch seems to work:

> -"$@"
> +eval "$@"

This is bogus; it will cause the argument list to be re-parsed.

[frankie]

Replying to glynn:

Replying to hamish:

not sure how to fix it in sh

this patch seems to work:

> -"$@"
> +eval "$@"

This is bogus; it will cause the argument list to be re-parsed.

Just for note, this issue is around in 6.2.3 too.

[hamish]

Replying to glynn:

Replying to hamish:

not sure how to fix it in sh

this patch seems to work:

-"$@"
+eval "$@"

This is bogus; it will cause the argument list to be re-parsed.

do you have a better suggestion then? That's the best I got.

FWIW the above successfully runs this via "Run in Xterm":

 v.db.select roads where="label ~ 'highway'"

(set DEBUG=1 to avoid window closing after completion; but then debug level 1 is somewhat overused so many messages to sort through)

with the above patch what's a case that fails? at leaast it's a step up from the last version which wouldn't run anything with arguments at all.

Hamish

[hamish]

Glynn:

do you have a better suggestion then? That's the best I got.

sorry, rereading the bug history I see you've already posted one and I missed it. nevemind. :)

H

[hamish]

Replying to glynn:

No, the script itself is correct. How is it being called?

​http://trac.osgeo.org/grass/browser/grass/branches/develbranch_6/gui/tcltk/gis.m/runandoutput.tcl#L227

(hmm, I see there is a mingw case to check there as well)

 if {[catch {eval [list exec -- $env(GISBASE)/etc/grass-xterm-wrapper -name xterm-grass -e $env(GISBASE)/etc/grass-run.sh $cmd] $args &} error]} {

Hamish

[hamish]

The "Run (in Xterm)" button is actually called here:

​http://trac.osgeo.org/grass/browser/grass/branches/develbranch_6/gui/tcltk/gis.m/runandoutput.tcl#L125

Hamish

[glynn]

Replying to hamish:

Replying to glynn:

No, the script itself is correct. How is it being called?

​http://trac.osgeo.org/grass/browser/grass/branches/develbranch_6/gui/tcltk/gis.m/runandoutput.tcl#L227

Okay; it appears to be a bug in Gronsole::run_xterm (lib/gtcltk/gronsole.tcl):

    exec -- $env(GISBASE)/etc/grass-xterm-wrapper -name xterm-grass -e env(GISBASE)/etc/grass-run.sh $cmd &

The command is being treated as a string, and passed as a single argument. It should be a list, and each element should be a separate argument to "exec". See the "term" procedure in runandoutput.tcl for an example of calling "exec" with a list of arguments.

[hamish]

Replying to glynn:

Okay; it appears to be a bug in Gronsole::run_xterm (lib/gtcltk/gronsole.tcl):

    exec -- $env(GISBASE)/etc/grass-xterm-wrapper -name xterm-grass -e env(GISBASE)/etc/grass-run.sh $cmd &

The command is being treated as a string, and passed as a single argument. It should be a list, and each element should be a separate argument to "exec". See the "term" procedure in runandoutput.tcl for an example of calling "exec" with a list of arguments.

ok, thanks. but more fundamentally the $cmd string is passed from gis.m/runandoutput.tcl as a whole concatenated string, and so run_term in gtcltk/gronsole.tcl has no $args list to work with.

and how can runandoutput.tcl know how to parse apart the "Run (in Xterm)" command string into a series of without extra quoting parsing and logic? As a "document it and walk away" solution I tried replacing "" with {} for the quoting in my input command, but that didn't help at all.

fwiw1: all modules in the gmmenu.tcl menu called with "term" are single element, ie just the name of the module. so this is just a problem with the "Run (in Xterm)" button in the Output window.

fwiw2: on native WinGRASS all 4 "Run" buttons fail for the v.db.select example listed earlier in this bug report, but pass for 'g.proj -p'. aside: I notice gronsole.tcl uses "&" at the end of the cmd.exe run_xterm. does that work on windows?

Hamish

[glynn]

Replying to hamish:

The command is being treated as a string, and passed as a single argument. It should be a list, and each element should be a separate argument to "exec". See the "term" procedure in runandoutput.tcl for an example of calling "exec" with a list of arguments.

ok, thanks. but more fundamentally the $cmd string is passed from gis.m/runandoutput.tcl as a whole concatenated string, and so run_term in gtcltk/gronsole.tcl has no $args list to work with.

In Tcl, everything is a string, including lists.

and how can runandoutput.tcl know how to parse apart the "Run (in Xterm)" command string into a series of without extra quoting parsing and logic? As a "document it and walk away" solution I tried replacing "" with {} for the quoting in my input command, but that didn't help at all.

If you change Gronsole::run_xterm to treat $cmd as a list, then the string will be interpreted according to Tcl list syntax, i.e. spaces separate arguments, but you can provide an argument which contains spaces by using braces or backslashes.

aside: I notice gronsole.tcl uses "&" at the end of the cmd.exe run_xterm. does that work on windows?

It should do. The "&" is part of the syntax of Tcl's "exec" command. The shell isn't involved in any way.

[hamish]

Replying to hamish:

and how can runandoutput.tcl know how to parse apart the "Run (in Xterm)" command string into a series of without extra quoting parsing and logic? As a "document it and walk away" solution I tried replacing "" with {} for the quoting in my input command, but that didn't help at all.

Replying to glynn:

If you change Gronsole::run_xterm to treat $cmd as a list, then the string will be interpreted according to Tcl list syntax, i.e. spaces separate arguments, but you can provide an argument which contains spaces by using braces or backslashes.

hmmm, I had tried that, but it didn't seem to work. (again, I'm no tcl expert).

As I see it we have two choices:

1) use eval "$@" which may be technically wrong but it seems to work for all cases and means that the quoting works as the user expects using DOS or Bourne syntax. i.e. just quietly walk away.

2) treat it as a tcl list, figure out how to implement that, and document somewhere that the user has to use {} instead of quotes.

Hamish

[glynn]

If you change Gronsole::run_xterm to treat $cmd as a list, then the string will be interpreted according to Tcl list syntax, i.e. spaces separate arguments, but you can provide an argument which contains spaces by using braces or backslashes.

hmmm, I had tried that, but it didn't seem to work. (again, I'm no tcl expert).

As I see it we have two choices:

1) use eval "$@" which may be technically wrong but it seems to work for all cases and means that the quoting works as the user expects using DOS or Bourne syntax. i.e. just quietly walk away.

2) treat it as a tcl list, figure out how to implement that, and document somewhere that the user has to use {} instead of quotes.

grass-xterm-wrapper is supposed to be invoked in the same manner as xterm, i.e. the -e switch is followed by a command and its arguments, not a command string. The bug is in Gronsole::run_xterm; this should be fixed, rather than introducing an equal and opposite bug elsewhere.

If you were to use option 1) above, any Tcl code which needed to invoke a command with arguments via grass-xterm-wrapper would need to construct the command according to Bourne shell syntax.

Tcl doesn't have a function to do this, and doing it manually is error prone. The obvious risk is the the code just concatenates the arguments with spaces in between, which breaks if any of the arguments contain any shell metacharacters, e.g. spaces.

OTOH, Tcl has several functions to construct Tcl lists from individual elements and/or lists, as well as invoking a command with a list of arguments.

If, for some reason, Tcl needs to execute a "command" which is already using Bourne shell syntax, the correct mechanism is exec /bin/sh -c $cmd. Similarly, to execute a command using DOS syntax use exec $env(COMSPEC) /c $cmd.

[hamish]

I've checked in r37289 which calls $cmd as a list, but it's still not right. I could get a little closer by using

  [lindex $cmd 0] [split [lrange $cmd 1 end]]

instead of $cmd, but then it treats $* as a single argv string:

argv[ 0] = 'print_args'
argv[ 1] = 'v.db.select'
argv[ 2] = 'roads where=\"label ~ 'highway'\"'

=================================================================
If you wish to resize the X monitor, do so now. Window size is
locked while interactive modules are running.
=================================================================

Sorry, <roads where> is not a valid parameter

ERROR: Required parameter <map> not set:
    (Name of input vector map).

or if I quote with {} instead of "":

argv[ 0] = 'print_args'
argv[ 1] = 'v.db.select'
argv[ 2] = 'roads where=\{label ~ 'highway'\}'

and if I replace $cmd with

 ... [lindex $cmd 0] [lindex $cmd 1] [lrange $cmd 2 end] ] &

I get

argv[ 0] = 'print_args'
argv[ 1] = 'v.db.select'
argv[ 2] = 'roads'
argv[ 3] = 'where=\{label ~ 'highway'\}'

there was an idea was to write the command to a g.tempfile then /bin/sh that, but I'd rather keep everything in memory.

Hamish

[hamish]

ok, with r37291 something finally works-

it splits this up by spaces:

v.db.select roads where={label ~ 'highway'}

but correctly parses:

v.db.select roads {where=label ~ 'highway'}
...
cat|label
2|primary highway, hard surface
3|secondary highway, hard surface

Hamish

[glynn]

Replying to hamish:

ok, with r37291 something finally works-

Okay, that's essentially the code which "term" uses. But you can simplify it a bit, as there's no need to split $cmd into $cmd_exe and $args; this should work just as well:

eval [list exec -- $env(GISBASE)/etc/grass-xterm-wrapper -name xterm-grass -e $env(GISBASE)/etc/grass-run.sh] $cmd &

After all, the command name is just the first argument to grass-run.sh.

it splits this up by spaces:

v.db.select roads where={label ~ 'highway'}

Yep; a brace-delimited string must have the braces at the beginning and end; you can't put them in the middle like with quotes in bash.

[hamish]

Replying to glynn:

Okay, that's essentially the code which "term" uses.

right, was trying to get back to known solid ground.

But you can simplify it a bit, as there's no need to split $cmd into $cmd_exe and $args;

done.

Yep; a brace-delimited string must have the braces at the beginning and end; you can't put them in the middle like with quotes in bash.

ok, gis.m docs updated to note that. It wasn't the root of the problem but I'm guessing it didn't help any in my debugging attempts.

I've also now committed the equivalent fix for DOS in 6.5svn.

if no objections I'll start backporting to the 6.4 branch.

Hamish

[hamish]

... the next challenge is to get the main wxGUI Cmd > command-line prompt to successfully run this command on both UNIX & DOS versions.

  v.db.select roads where="label ~ 'highway'"

Hamish

[hamish]

run in xterm fixes merged into relbr64 in r37328.

closing bug as the tcl/tk run-in-xterm stuff is all working now, but as per previous comment in this bug report please note that args containing spaces still fail from the wxPy GUI's Cmd> prompt. Maybe that's just me not knowing how to quote stuff in python though.

Hamish

[glynn]

Replying to hamish:

please note that args containing spaces still fail from the wxPy GUI's Cmd> prompt. Maybe that's just me not knowing how to quote stuff in python though.

No, it's a bug in the wx GUI; GMConsole.RunCmd in goutput.py does:

        try:
            cmdlist = command.strip().split(' ')
        except:
            cmdlist = command

Try shlex.split(). See also bug #528.

[hamish]

non-functional patch using shlex.split() instead of split(' ')