[GRASS-dev] Exit code for Tk GUI

Hi folks,

I was looking at open issue list and noted some interesting issue #483
[1]. It seemed to be easy to fix, but after some hours of digging into
code, it started to look like unfixable.

As I'm not a good programmer, please, correct me at any place where I'm wrong.

First - I understood that current GRASS autogenerated tcl/tk GUI works
in following way (with respect to exit codes and with shortenings):
1) module (i.e. r.info) is launched and control is passed to G_parser;
2) as there are no args, G_parser builds tcl/tk description for module
and passes it to wish (via popen and fprintf);
3) when user hits RUN button in module GUI, another instance of module
gets launched with users provided args via gronsole;
4) when user closes GUI, control is returned to G_parser;
5) G_parser returns error (-1) to caller;
6) module checks G_parser return code and exits as there was an error.

Final result == module with tcl interface allways exits with error.
It's a design failure. Returning other value from G_parser if user
just ended GUI session will break all modules that use G_parser (no-no
till GRASS7);

Still there may be some way around:
1) In G_gui_tcltk (parser.c) collect whole tcl/tk wish script (file,array) and
2) launch wish via execlp(getenv("GRASS_WISH"),script);
We get rid of first module instance that created GUI and exit code is
up to gis.tcl/gronsole.

Currently such approach has some problems:
1) if user hits RUN button in GUI w/o any param's, it starts another
module window (hint - need to look how parser determines that GUI is
already running);
2) still way how to get module exit code from gronsole is required.

Both issues could be worked around and then only one political
decision has to be made:
if user just launches module and closes it without running module - is
it a fail (1) or clean (0) exit?

Today I first time read about such things like exec, popen etc. and
now I'm too tired to see solutions how to finish dealing with this
bug, but I know that out there are lots of smart GRASS coders, that
could use my hints as starting point and also spot possible problems
before they appear.

Sorry for disturbance (and poor inglish),
Maris.

Maris Nartiss wrote:

Hi folks,

I was looking at open issue list and noted some interesting issue #483
[1]. It seemed to be easy to fix, but after some hours of digging into
code, it started to look like unfixable.

As I'm not a good programmer, please, correct me at any place where I'm wrong.

First - I understood that current GRASS autogenerated tcl/tk GUI works
in following way (with respect to exit codes and with shortenings):
1) module (i.e. r.info) is launched and control is passed to G_parser;
2) as there are no args, G_parser builds tcl/tk description for module
and passes it to wish (via popen and fprintf);
3) when user hits RUN button in module GUI, another instance of module
gets launched with users provided args via gronsole;
4) when user closes GUI, control is returned to G_parser;
5) G_parser returns error (-1) to caller;
6) module checks G_parser return code and exits as there was an error.

Final result == module with tcl interface allways exits with error.
It's a design failure. Returning other value from G_parser if user
just ended GUI session will break all modules that use G_parser (no-no
till GRASS7);

Still there may be some way around:
1) In G_gui_tcltk (parser.c) collect whole tcl/tk wish script (file,array) and
2) launch wish via execlp(getenv("GRASS_WISH"),script);
We get rid of first module instance that created GUI and exit code is
up to gis.tcl/gronsole.

An alternative is to make the initial command return wish's exit code
(which is returned from pclose()), i.e.:

--- lib/gis/parser.c 8 Sep 2007 16:22:21 -0000 1.130
+++ lib/gis/parser.c 24 Sep 2007 00:05:54 -0000
@@ -1887,6 +1887,7 @@
static void G_gui_tcltk (void)
{
   FILE *fp;
+ int status;

   if (!pgm_name)
     pgm_name = G_program_name ();
@@ -1912,7 +1913,10 @@

   generate_tcl(fp);

- pclose(fp);
+ status = pclose(fp);
+ if (status < 0)
+ G_fatal_error("error executing 'wish'");
+ exit(status);
}

/* Build wxPython gui */

However, that's only part of the solution, as you note in point #2
below.

Currently such approach has some problems:
1) if user hits RUN button in GUI w/o any param's, it starts another
module window (hint - need to look how parser determines that GUI is
already running);

It doesn't.

To generate the dialogs, gis.m invokes the command with the --tcltk
switch. This causes the command to write the generated code to stdout,
rather than spawning a wish process. gis.m reads the Tcl/Tk and
executes it locally, having previously overriden some of the
procedures in gui.tcl (specifically, run_cmd and
make_{buttons,output,progress}).

2) still way how to get module exit code from gronsole is required.

Getting the actual exit code isn't possible, AFAICT. The problem is
that Tcl's "open |..." syntax can create a pipeline consisiting of
multiple commands. In fact, the gronsole code always spawns two
commands (the second is the grocat program, used to merge stdout and
stderr).

It is possible to determine success or failure, according to whether
the "close" command throws an exception. It isn't possible to get the
actual exit code, though.

It should be feasible to have gronsole return success or failure, and
having gui.tcl indicate that through its overall exit code.

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

Hi,
just for record.

2007/9/24, Glynn Clements <glynn@gclements.plus.com>:

An alternative is to make the initial command return wish's exit code
(which is returned from pclose()), i.e.:

This was also my first idea, but, as I noted, it would break all
existing modules.
When module gets launched from console, it will create GUI via
G_parser and then use G_parser return code to exit or continue running
(assuming all required options are set, what isn't true if G_parser
was using GUI), thus getting back module fail/sucess to first running
module instance via G_parser is impossible (w/o fixing all modules).
Example from r.info:
if (G_parser(argc, argv))
  exit(EXIT_FAILURE);

Thus only way how to get back module exit code to console is by
replacing already running module with it's GUI via exec().

However, that's only part of the solution, as you note in point #2
below.

> Currently such approach has some problems:
> 1) if user hits RUN button in GUI w/o any param's, it starts another
> module window (hint - need to look how parser determines that GUI is
> already running);

It doesn't.

It does. I have to look more closely to G_parser how to distinguish
when module is launched from GUI and then not exec() new wish
instance.

To generate the dialogs, gis.m invokes the command with the --tcltk
switch. This causes the command to write the generated code to stdout,
rather than spawning a wish process. gis.m reads the Tcl/Tk and
executes it locally, having previously overriden some of the
procedures in gui.tcl (specifically, run_cmd and
make_{buttons,output,progress}).

Original bugreport was referring to running module from CLI (bash) and
checking exit code via echo $?.

> 2) still way how to get module exit code from gronsole is required.

Getting the actual exit code isn't possible, AFAICT. The problem is
that Tcl's "open |..." syntax can create a pipeline consisiting of
multiple commands. In fact, the gronsole code always spawns two
commands (the second is the grocat program, used to merge stdout and
stderr).

This is biggest bummer :frowning:
If somebody doesn't come up with some solution to this problem, then
we have to just document permanent GRASS bug - module with GUI allways
exits with error.

Maris.

Maris Nartiss wrote:

> An alternative is to make the initial command return wish's exit code
> (which is returned from pclose()), i.e.:

This was also my first idea, but, as I noted, it would break all
existing modules.
When module gets launched from console, it will create GUI via
G_parser and then use G_parser return code to exit or continue running
(assuming all required options are set, what isn't true if G_parser
was using GUI), thus getting back module fail/sucess to first running
module instance via G_parser is impossible (w/o fixing all modules).
Example from r.info:
if (G_parser(argc, argv))
  exit(EXIT_FAILURE);

Thus only way how to get back module exit code to console is by
replacing already running module with it's GUI via exec().

G_gui() can call exit() with the value returned from pclose(); it
doesn't have to return to the caller. See my patch.

> However, that's only part of the solution, as you note in point #2
> below.
>
> > Currently such approach has some problems:
> > 1) if user hits RUN button in GUI w/o any param's, it starts another
> > module window (hint - need to look how parser determines that GUI is
> > already running);
>
> It doesn't.

It does. I have to look more closely to G_parser how to distinguish
when module is launched from GUI and then not exec() new wish
instance.

Maybe my reply wasn't clear; I should probably have snipped the text
more aggressively to:

> > (hint - need to look how parser determines that GUI is
> > already running);
>
> It doesn't.

IOW, the parser makes no attempt to determine whether the GUI is
running; it behaves the same regardless.

When a module requires one or more arguments and is run with none,
G_parser() either calls interactive() (if GRASS_UI_TERM is set) or
G_gui() (if GRASS_UI_TERM isn't set.

If the --ui switch is used, G_gui() is always called, even if other
arguments are given or if GRASS_UI_TERM is set.

If you want to prevent the GUI from invoking a command which simply
creates another GUI, change the GUI code so that it never runs a
command without arguments. The GUI will never be displayed
automatically (without --ui) if any arguments are given.

Alternatively, we could change G_parser() to only go interactive if no
arguments are given *and* there is at least one "required" option.
Typically, modules with no required options (i.e. where all options
have a default value) skip calling G_parser() altogether if no
arguments are given (argc <= 1); e.g. d.erase:

  if (argc > 1 && G_parser(argc, argv))
    exit(1);

> To generate the dialogs, gis.m invokes the command with the --tcltk
> switch. This causes the command to write the generated code to stdout,
> rather than spawning a wish process. gis.m reads the Tcl/Tk and
> executes it locally, having previously overriden some of the
> procedures in gui.tcl (specifically, run_cmd and
> make_{buttons,output,progress}).

Original bugreport was referring to running module from CLI (bash) and
checking exit code via echo $?.

Yes, the (implied) --ui case.

> > 2) still way how to get module exit code from gronsole is required.
>
> Getting the actual exit code isn't possible, AFAICT. The problem is
> that Tcl's "open |..." syntax can create a pipeline consisiting of
> multiple commands. In fact, the gronsole code always spawns two
> commands (the second is the grocat program, used to merge stdout and
> stderr).

This is biggest bummer :frowning:
If somebody doesn't come up with some solution to this problem, then
we have to just document permanent GRASS bug - module with GUI allways
exits with error.

If you're running the command manually from the command line, you
probably aren't paying any attention to exit codes. If you're running
it from a script, you're likely to be providing arguments.

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

2007/9/24, Glynn Clements <glynn@gclements.plus.com>:

G_gui() can call exit() with the value returned from pclose(); it
doesn't have to return to the caller. See my patch.

Sorry. I really misread Your patch.
Just for record if somebody is going to use it - correct way is like this:

--- lib/gis/parser.c 8 Sep 2007 16:22:21 -0000 1.130
+++ lib/gis/parser.c 24 Sep 2007 13:20:36 -0000
@@ -81,6 +81,8 @@
#include <ctype.h>
#include <unistd.h>
#include <stdarg.h>
+#include <sys/types.h>
+#include <sys/wait.h>
#include <grass/gis.h>
#include <grass/glocale.h>
#include <grass/spawn.h>
@@ -1887,6 +1889,7 @@
static void G_gui_tcltk (void)
{
   FILE *fp;
+ int status;

   if (!pgm_name)
     pgm_name = G_program_name ();
@@ -1912,7 +1915,10 @@

   generate_tcl(fp);

- pclose(fp);
+ status = pclose(fp);
+ if (status < 0)
+ G_fatal_error("error while executing 'wish'");
+ exit(WEXITSTATUS(status));
}

/* Build wxPython gui */

If you're running the command manually from the command line, you
probably aren't paying any attention to exit codes. If you're running
it from a script, you're likely to be providing arguments.

Probably reporter has some use case where GUI's returned exit code has
some use otherwise there would be no bugreport about this.

TCL's close{} provides error code from launched module in "proc
Gronsole::readeof" IF in "proc Gronsole::execbg " pipeline is not
changed to be non-blocking (remove "fconfigure $fh -blocking 0").

wbr,
Maris.

Maris Nartiss wrote:

TCL's close{} provides error code from launched module in "proc
Gronsole::readeof" IF in "proc Gronsole::execbg " pipeline is not
changed to be non-blocking (remove "fconfigure $fh -blocking 0").

Both Gronsole::destroy_command and Gronsole::execout close the stream
without recording the status (both silently "catch" any exception).

However: I'm not sure if it's meaningful for the GUI to report the
command's exit status. Once the GUI is created, you can run the
command multiple times (with different options each time).

What does "success" mean? That all of the runs succeeded? Or at least
one run? Or the last run?

I suspect that either:

1. G_gui() should simply exit(EXIT_SUCCESS) if it managed to spawn the
GUI and exit(EXIT_FAILURE) if it didn't, or:

2. G_gui() should return wish's exit code, and wish should report
success if it was terminated by the user (failure would imply that
wish crashed).

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

Glynn Clements wrote:

Maris Nartiss wrote:

TCL's close{} provides error code from launched module in "proc
Gronsole::readeof" IF in "proc Gronsole::execbg " pipeline is not
changed to be non-blocking (remove "fconfigure $fh -blocking 0").

Both Gronsole::destroy_command and Gronsole::execout close the stream
without recording the status (both silently "catch" any exception).

However: I'm not sure if it's meaningful for the GUI to report the
command's exit status. Once the GUI is created, you can run the
command multiple times (with different options each time).

What does "success" mean? That all of the runs succeeded? Or at least
one run? Or the last run?

I believe the success should indicate that the GUI was
closed cleanly (user pressing "Close" button) and failure
should indicate that the window crashed.

Maciek