[GRASS-dev] [GRASS GIS] #693: wxGUI menus: i.ortho.photo locks up GUI

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------------+------------------------------------------------
Reporter: hamish | Owner: grass-dev@lists.osgeo.org
     Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Keywords: i.ortho.photo | Platform: Linux
      Cpu: x86-64 |
---------------------------+------------------------------------------------
Hi,

while running the wxGui I notice that if I select
  `Imagery -> Ortho Photo Rectification (requires Xterm)`
it brings up an empty Xmon (x0) but then the GUI windows are locked until
I close the Xmonitor window by clicking on the window manager's "x" . As
soon as I do that I see a xterm window flash on the screen then disappear
and then I get control of the GUI back.

there are no residual error messages either on the parent terminal or in
the GUI command output tab.

?

thanks,
Hamish

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

same problem with r.digit (Raster -> Develop -> Digitize [req xterm])

(todo: add r.in.poly output to wxVdigit lines/areas/points and retire
r.digit module in grass 7)

Hamish

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Changes (by cmbarton):

  * platform: Linux => All

Comment:

I started looking into this and found that I can't get the underlying bash
scripts to work correctly on my Mac even from the command line.

I am wondering if there is a pure Python way to do this better, at least
for the wxp GUI?

Michael

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/693#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by cmbarton):

I fixed this on the Mac. Is it a problem on Linux? I assume that it does
not work with Windows.

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

It still locks up on Linux.

bypassing xterm-wrapper and hardcoding xterm is not a viable option for
linux. that script is there for a reason... many distros don't ship xterm
by default (they typically use something more bloated (gnome-terminal) or
minimalistic (rxvt) or generic symlink to x-terminal-emulator)

By hardcoding it on Mac you probably will now get a more confusing error
message if X11 isn't installed than the additional one produced by the
wrapper script.

what is the error you get on the command line?

it is likely the source of this bug will cause trouble elsewhere, so
simply plastering over it ain't the best outcome..

Hamish

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

I'd add that the same thing works correctly from gis.m.
(and that IMO 6.5's wxgui crtl-q should just be closing the gui, not the
entire session)

Hamish

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by kyngchaos):

Also note that on OSX the preference may be to use Terminal instead of
xterm. That's what grass-xterm-mac is for.

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by cmbarton):

My only change so far has been to the code for Mac. On that platform, you
have something called xterm (and it can be a couple of different things)
or you don't. Good point that there needs to be error trapping for the
times when you don't. This is easy to add. ISTR that the xterm wrapper for
Mac was a port from the Linux xterm wrapper Linux. It has always been iffy
for working on the Mac, even in TclTk. The terminal won't substitute for
an xterm in the commands that require one, primarily because of need for
graphics. The terminal can open an xterm at need, but only if xterm
exists. Also, this doesn't always work right for the old commands that
expect one. Hence the need to guarantee one--and as you note, gracefully
trap the situations where Mac owners have not installed xterm.

I left all the linux stuff alone because I'm not sure what the context for
the wrapper is. But it might be better if there is an environmental
variable GRASS_XTERM that is set to a platform specific value than a
script that tries to figure it out. On the other hand, this may not be
worth the trouble for the remaining life of version 6.

Michael

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by kyngchaos):

On the Mac side, I think Terminal (capital-T, the application) should be
able to do the 'graphics', whatever that means in a terminal (small-t,
generic terminal). It's supposed to be able to operate as a standard
xterm (there's even an option starting in Leopard: declare terminal as
with xterm and xterm-color as options, among others. What it doesn't do
is accept xterm parameters on startup, and that's what the wrapper is for,
to translate at least the basics (ie the command) to pass to Terminal.

I just hate to see, even this late in the life of 6.x, forcing xterm on
OSX, so I think it's worth it to try and get any terminal usage to work in
Terminal.

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by cmbarton):

I've added graceful error checking for the lack of an xterm on the Mac.

If there is a way to make xterm GRASS modules work with the Terminal.app,
I'm happy to try and get them to work. But currently, the wrapper is just
locking up GRASS. For wxPython, at least, it may be easier to write this
in Python code than to use the existing and rather convoluted shell
script.

What does it take to get the Terminal.app to work like an xterm for a
GRASS module like r.digit? That is, what kind of command do I need to type
from the terminal to make this work?

Michael

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by kyngchaos):

The key for any OSX *application*, is that you don't want to run it with
traditional unix methods. You use the 'open' command. This accepts
parameters that the application accepts in CLI usage (usually a filename
to open). It's best to specify an app signature (you can get it from the
app's info.plist) with the -b flag. See the html_browser_mac.sh for an
example.

Terminal has an additional consideration - it doesn't inherit the shell
environment when started this way. That's why I create the dummy script
in grass_xterm_mac, so that it can be run inside the new Terminal window
to set needed env vars. It may be possible to pipe a script to open
without creating a temporary script file.

The applescript step I think was so that the new Terminal window wouldn't
immediately return after activating ('open' is basically a spawn/fork,
with intelligence to not run multiple copies of a program), thus causing
the calling process to think it was finished. Maybe this is what's
hanging your python?

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by glynn):

Replying to [comment:7 cmbarton]:

> But it might be better if there is an environmental variable GRASS_XTERM
that is set to a platform specific value than a script that tries to
figure it out.

There is such a variable; that's the whole point of grass-xterm-wrapper.
It is essentially just:
{{{
exec "$GRASS_XTERM" "$@"
}}}
except that it also tries to autodetect a suitable program if GRASS_XTERM
isn't set.

However, wxGUI's GMFrame.OnXTerm() method involves rather more than simply
running a command in a terminal.

First, it invokes the command via grass-run.{sh,bat}. The primary
rationale for that script is to restore LD_LIBRARY_PATH in case the loader
resets it due to xterm being setuid/setgid. But in typical form, the
grass-run.sh script has been "enhanced" with other functionality. The
grass-run.src file '''should''' be nothing more than:
{{{
LD_LIBRARY_PATH_VAR=$GRASS_LD_LIBRARY_PATH
export LD_LIBRARY_PATH_VAR
exec "$@"
}}}
Everything else is gratuitous, and possibly harmful.

Second, GMFrame.OnXTerm() starts (and selects) another XDRIVER monitor (I
don't see where (or even if) it stops this monitor, and I'm fairly sure
that it doesn't re-select the previous monitor).

Ah. This is where it goes beserk. It constructs the command as:
{{{
command = shlex.split(str(command))
...
cmdlist = [xtermwrapper, '-e "%s"' % grassrun, command]
}}}
which will result in cmdlist looking something like:
{{{
['/usr/local/src/grass/svn/dist.i686-pc-linux-gnu/etc/grass-xterm-
wrapper',
'-e "/usr/local/src/grass/svn/dist.i686-pc-linux-gnu/etc/grass-
run.sh"',['i.ortho.photo']]
}}}
How many things are wrong with this? Apart from the last argument being a
Python list (which gcmd.Command() '''might''' handle), the -e switch is
merged into the same argument as its value. And its value is quoted,
whereas xterm's -e switch signifies that all remaining arguments are to be
treated as the command and its arguments (unlike e.g. /bin/sh's -c switch,
which expects a single argument).

Does GMFrame.OnXTerm() work at all for anything on any platform? I'm hard
pressed to see how it can, unless gcmd.Command() is doing a '''lot''' of
scary magic on the mess it gets passed.

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by cmbarton):

Well, it didn't work for the Mac and I don't think it works for Windows
(though that might be for other reasons). This is the reason that I
dropped the code you mention for the simpler solution I put in for Mac.
Given William's description, I'm not sure how realistic it is to try and
make Terminal.app substitute for an xterm. But I'm willing to explore it.

Michael

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

Replying to [comment:12 cmbarton]:
> I don't think it works for Windows

right. this is for launching a X11-dependent xterm + Xmonitor. These menu
items should be greyed-out on MS Windows, unless you are using a Cygwin
build (which has X11).

> Given William's description, I'm not sure how realistic it is
> to try and make Terminal.app substitute for an xterm.

because it needs X11 for the Xmonitors anyway, there's not too much point
in cross platform heroics. The module needs X11 which if installed on a
Mac means xterm is always available. I wouldn't bother trying to get
Terminal.app to work with it.

Hamish

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

it still locked. the problem was that it was waiting for d.mon to return
before launching into grass-run.src

I've now combined Glynn's fix/explanation with Michael's Mac fix in r40019
for devbr6. I haven't backported to 6.4 yet pending testing.

highlights: instead of gcmd.RunCommand() use gcmd.Command([list],
wait=False). wait=False was needed allow the GUI to continue.

I'm not sure of the RENDER_IMMEDIATE directly after. I assume it just has
to be set for the launch of the command, and as soon as it is forked it
can be set back.

no idea if the WinNT version works or is even meaningful, as d.mon will
have failed there ? Or does Cygwin will call its os.environ as Windows_NT
so for that it is needed? I guess we could catch the d.mon call but really
r.digit, i.ortho.photo, etc should be greyed out on native WinGrass before
then.

Hamish

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

I guess one final thing we can do is to use William's nice grass-xterm-mac
for xterm-wrapper. Michael, could you try the attached patch?

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

I notice in menudata.xml that g.setproj is set to use self.OnXTerm(). this
seems wrong, as it just needs a regular terminal or dosbox, not
specifically a xterm (+xmon that goes with it). ?

maybe OnXTerm() needs to have a need_xmon=True switch added to it to
differentiate in these cases?

fwiw all other OnXTerm()s in menudata.xml really do need a xmon+xterm.

Hamish

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

enhanced patch committed in 40021. let me know if it breaks.

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by cmbarton):

I'll try it out.

IIUC, the only reason for William's grass-xterm-mac wrapper is if we want
to substitute Terminal.app for an xterm. If this is not essential (and we
can debate this), then the wrapper is not needed. It also might be
possible to re-create this wrapper in Python code more nicely than in
bash.

Michael

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

#693: wxGUI menus: i.ortho.photo locks up GUI
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: i.ortho.photo
  Platform: All | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by glynn):

Replying to [comment:14 hamish]:

> I'm not sure of the RENDER_IMMEDIATE directly after. I assume it just
has to be set for the launch of the command, and as soon as it is forked
it can be set back.

Yes. Although modifying os.environ is really the wrong approach. It's
preferable to pass a modified version via env=, e.g.:
{{{
tmpenv = os.environ.copy()
tmpenv['GRASS_RENDER_IMMEDIATE'] = 'FALSE'
p = Popen(..., env = tmpenv)
}}}
Then you don't have to worry about os.environ not getting restored if an
exception is raised.

> no idea if the WinNT version works or is even meaningful, as d.mon will
have failed there ? Or does Cygwin will call its os.environ as Windows_NT
so for that it is needed? I guess we could catch the d.mon call but really
r.digit, i.ortho.photo, etc should be greyed out on native WinGrass before
then.

Cygwin will leave OS=Windows_NT alone. But Cygwin should be using the
xterm version not the cmd.exe version.

But I think that this is an issue of not distinguishing between xterm+xmon
and just xterm.

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