Opened 15 years ago

Closed 14 years ago

#693 closed defect (fixed)

wxGUI menus: i.ortho.photo locks up GUI

Reported by: hamish Owned by: grass-dev@…
Priority: major Milestone: 6.4.0
Component: wxGUI Version: svn-develbranch6
Keywords: i.ortho.photo Cc:
CPU: x86-64 Platform: All

Description

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

Attachments (1)

bug693_mac.diff (1.7 KB ) - added by hamish 14 years ago.
patch to test xterm-wrapper for mac

Download all attachments as: .zip

Change History (43)

comment:1 by hamish, 15 years ago

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

comment:2 by cmbarton, 15 years ago

Platform: LinuxAll

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

comment:3 by cmbarton, 14 years ago

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

comment:4 by hamish, 14 years ago

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

comment:5 by hamish, 14 years ago

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

comment:6 by kyngchaos, 14 years ago

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

comment:7 by cmbarton, 14 years ago

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

comment:8 by kyngchaos, 14 years ago

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.

comment:9 by cmbarton, 14 years ago

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

comment:10 by kyngchaos, 14 years ago

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?

in reply to:  7 comment:11 by glynn, 14 years ago

Replying to 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.

comment:12 by cmbarton, 14 years ago

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

in reply to:  12 comment:13 by hamish, 14 years ago

Replying to 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

comment:14 by hamish, 14 years ago

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

comment:15 by hamish, 14 years ago

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?

by hamish, 14 years ago

Attachment: bug693_mac.diff added

patch to test xterm-wrapper for mac

comment:16 by hamish, 14 years ago

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

comment:17 by hamish, 14 years ago

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

comment:18 by cmbarton, 14 years ago

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

in reply to:  14 comment:19 by glynn, 14 years ago

Replying to 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.

comment:20 by cmbarton, 14 years ago

This works, but there is not much point to the change. Calling d.mon just has the Terminal start xterm. If there is no xterm package, it will fail non-gracefully.

To avoid an xterm, we need to run the grass-xterm-mac wrapper for d.mon, not the text prompt/response interface. You can try to switch it or I can try later this week.

I much appreciate your willingness to get this code working. I agree about the greyout for Windows users. I think it can be done but need to figure out how.

Michael

in reply to:  20 ; comment:21 by hamish, 14 years ago

Replying to cmbarton:

This works, but there is not much point to the change. Calling d.mon just has the Terminal start xterm. If there is no xterm package, it will fail non-gracefully.

To avoid an xterm, we need to run the grass-xterm-mac wrapper for d.mon, not the text prompt/response interface.

? d.mon doesn't go near an xterm. it just starts the x0 display window.

the xterm (or xterm-alike) is launched later. $GRASS_UI_TERM is set to avoid the curses based text window "TUI" ending up as broken rubbish in the tcl/wx module GUI output window. The question/response parser stuff is just an unfortuante byproduct of that.

You can try to switch it or I can try later this week.

I already have changed it to try the mac wrapper (in 6.5).

Q: how to get wxgui.py to detect Cygwin so that does try an xterm instead of a "start" dos box? is there a $CYGWIN enviro var or similar? os.* or sys.* smart enough to pick it up?

I agree about the greyout for Windows users. I think it can be done but need to figure out how.

worst case a patch could be applied as part of the mswindows packaging script.

Hamish

in reply to:  21 ; comment:22 by cmbarton, 14 years ago

Replying to hamish:

Replying to cmbarton:

This works, but there is not much point to the change. Calling d.mon just has the Terminal start xterm. If there is no xterm package, it will fail non-gracefully.

To avoid an xterm, we need to run the grass-xterm-mac wrapper for d.mon, not the text prompt/response interface.

? d.mon doesn't go near an xterm. it just starts the x0 display window.

The x0 display window IS an xterm window. You can see it launch x bouncing in the doc.

the xterm (or xterm-alike) is launched later. $GRASS_UI_TERM is set to avoid the curses based text window "TUI" ending up as broken rubbish in the tcl/wx module GUI output window. The question/response parser stuff is just an unfortuante byproduct of that.

You can try to switch it or I can try later this week.

I already have changed it to try the mac wrapper (in 6.5).

OK. Have to test later. Thanks

Michael

Q: how to get wxgui.py to detect Cygwin so that does try an xterm instead of a "start" dos box? is there a $CYGWIN enviro var or similar? os.* or sys.* smart enough to pick it up?

I agree about the greyout for Windows users. I think it can be done but need to figure out how.

worst case a patch could be applied as part of the mswindows packaging script.

Hamish

in reply to:  21 ; comment:23 by glynn, 14 years ago

Replying to hamish:

Q: how to get wxgui.py to detect Cygwin so that does try an xterm instead of a "start" dos box? is there a $CYGWIN enviro var or similar? os.* or sys.* smart enough to pick it up?

Cygwin's Python has sys.platform == 'cygwin'.

So if you use sys.platform == win32 as the test for (native) Windows, it won't affect Cygwin.

in reply to:  22 comment:24 by glynn, 14 years ago

Replying to cmbarton:

? d.mon doesn't go near an xterm. it just starts the x0 display window.

The x0 display window IS an xterm window.

No it isn't.

You can see it launch x bouncing in the doc.

XDRIVER creates an X window.

in reply to:  22 comment:25 by hamish, 14 years ago

Hamish:

? d.mon doesn't go near an xterm. it just starts the x0 display window.

Replying to cmbarton:

The x0 display window IS an xterm window. You can see it launch x bouncing in the doc.

let me rephrase. d.mon doesn't [need to] go near an xterm. the mac specific code of a few days ago was using it to launch the program (which is why you saw it in the dock), but doing it that way is redundant so I removed it from 6.5svn in the last day or two.

Hamish

in reply to:  23 comment:26 by hamish, 14 years ago

Replying to glynn:

Cygwin's Python has sys.platform == 'cygwin'.

So if you use sys.platform == win32 as the test for (native) Windows, it won't affect Cygwin.

ok thanks. applied in 6.5svn as r40039.

comment:27 by cmbarton, 14 years ago

Back to the Mac.

I just updated wxgui.py from the develbranch_6 svn and tried it, running r.digit to test.

When I run it, an xterm is launched. When I run d.mon x0 from the command line, an xterm is launched.

Like the average Mac user, I've not set any special environmental variables in .profile or elsewhere. Typing $XDRIVER also launches an xterm, but I have not set XDRIVER anywhere.

So I don't see the advantage of this over running xterm directly.

Michael

comment:28 by kyngchaos, 14 years ago

So, do you mean an actual xterm terminal window, in addition to the x0 monitor window? This is a default action by Apple's X11. When X11 starts, it starts an xterm. In Leopard+, since X11 starts automatically, it seems like the program you are starting is the cause of the xterm.

You can change this behavior - see the installed GRASS readme for the OSX application (it's in the source/macosx/pkg/resources.

comment:29 by cmbarton, 14 years ago

When d.mon x0 is run, you get the x0 monitor window running in x11.app. That is d.mon x0 starts x11.app. So if you need x11.app to run this in addition to terminal.app, what is the point of the xterm wrapper?

Michael

comment:30 by kyngchaos, 14 years ago

Which would you prefer: Terminal.app which supports drag-n-drop, tabs and text antialiasing, and appears in your Dock and application switcher (for easy access and identification), or xterm (none of those, though maybe antialiasing and tabs with the right options somewhere)?

Sorry to be cheeky there. I think Mac users generally want things to look like they're running on a Mac, so the less X11 is used, the better. Unfortunately, for whatever reason, Apple decided that starting X11 *always* needs an xterm, and disabling that behavior is not a simple option.

I suppose I could add an optional script for installation to disable this...

comment:31 by cmbarton, 14 years ago

I agree about preferring Terminal. But this is running x11.app, not terminal. So something is wrong. What happens if someone has not installed x11?

Michael

comment:32 by kyngchaos, 14 years ago

In GRASS 6.x, on OSX, X11 is a requirement for basic display. You will can't have XDRIVER displays without it. Which means you can't have those modules like r.digit that explicitly use an XDRIVER display.

comment:33 by cmbarton, 14 years ago

Right. This was my point all along. So if x11 has to be run, what is the point of the grass-xterm-mac wrapper?

Michael

comment:34 by kyngchaos, 14 years ago

Chances are GRASS was started in a Terminal (that's how I have the OSX app startup configured, and those using Macports/Fink most likely will use a Terminal also). Then an xterm opens for some commands, and now you have 2 different terminal applications, with unique looks and behaviors. A bit confusing.

in reply to:  28 comment:35 by hamish, 14 years ago

Replying to kyngchaos:

So, do you mean an actual xterm terminal window, in addition to the x0 monitor window? This is a default action by Apple's X11. When X11 starts, it starts an xterm.

ah, yes. I'd forgotten about that.

so if X11 hasn't been started yet by anything, opening a XDRIVER X-window with d.mon will automatically start x11.app.

When x11.app first starts it opens a xterm. Does x11.app close again once all x11-windows are closed or does it stay resident?

I always find the auto-xterm thing annoying and not what I want, so whenever I've installed grass or Fink on a Mac pretty much the first thing I do is

sudo su
cd /etc/X11/xinit
chmod u+w xinitrc
vi xinitrc

and comment out the "xterm &"

i.e. this behavior likely has little to do with GRASS.

In Leopard+, since X11 starts automatically, it seems like the program you are starting is the cause of the xterm.

You can change this behavior - see the installed GRASS readme for the OSX application (it's in the source/macosx/pkg/resources.

ok,

Hamish

in reply to:  20 comment:36 by hamish, 14 years ago

Replying to cmbarton:

I agree about the greyout for Windows users. I think it can be done but need to figure out how.

actually it already is.

see "XMONBASED" in imagery/Makefile.

#compile if interactive Xmons are present:
ifneq ($(USE_X11),)
    SUBDIRS += $(XMONBASED)
endif

The result is missing binaries which means greyed out menu items.


so how do r.digit and i.ortho.photo look now from the grass 6.5 wxGUI on MacOSX?

Hamish

comment:37 by hamish, 14 years ago

see also #888; still wondering how things look in MacOSX.

comment:38 by kyngchaos, 14 years ago

The only problem I see is in Leopard+, with the X11 autostart. If X11 isn't running yet when you start r.digit or i.ortho.photo, and you type the raster/group name in the Terminal before the monitor window opens (as it waits for X11 to start), it will complain about having no display. If you wait for the monitor display to open before continuing in the Terminal, it's fine except that the monitor window opens in front of the Terminal because X11 runs in the foregrond instead of the background.

Did that make sense? ;)

I suppose I could force X11 to run on GRASS startup like in earlier versions of the system, but defeats the purpose (and convience) of the X11 autostart.

in reply to:  38 comment:39 by hamish, 14 years ago

Replying to kyngchaos:

The only problem I see is in Leopard+, with the X11 autostart. If X11 isn't running yet when you start r.digit or i.ortho.photo, and you type the raster/group name in the Terminal before the monitor window opens (as it waits for X11 to start),

i.e. you get a Terminal.app (instant) not an xterm (waits for X11) for the

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

part?

it will complain about having no display. If you wait for the monitor display to open before continuing in the Terminal, it's fine

perhaps add this to the above script:

if [ "`uname`" = "Darwin" ] ; then
   echo "Please wait a moment for X11 to start and a monitor to open."
fi

except that the monitor window opens in front of the Terminal because X11 runs in the foregrond instead of the background.

I think that's minor enough and these modules are used infrequently enough that we can just ignore it and let folks move windows out of the way themselves.

Did that make sense? ;)

sure

I suppose I could force X11 to run on GRASS startup like in earlier versions of the system, but defeats the purpose (and convience) of the X11 autostart.

IMO these are used infrequently enough that the computer's resources would be put do better use doing other things.

Hamish

comment:40 by kyngchaos, 14 years ago

What about this: I noticed that in OnXTerm() it runs d.mon with gcmd.Command() with a wait=False. I tried setting it to wait=True and that worked so that OnXTerm() waited for the X11 + monitor to open before running r.digit in a Terminal. This also gets around the minor problem of the monitor opening in front of the Terminal.

Would this cause problems on other systems? conditionalize True/False on darwin. Are there other uses of OnXTerm() where this might cause problems even on OSX?

in reply to:  40 comment:41 by hamish, 14 years ago

Replying to kyngchaos:

What about this: I noticed that in OnXTerm() it runs d.mon with gcmd.Command() with a wait=False. I tried setting it to wait=True and that worked so that OnXTerm() waited for the X11 + monitor to open before running r.digit in a Terminal.

I'm not sure if that would work -- see the title & initial description of this bug report! It was hanging waiting for the monitor to finish.

But it's worth a try; lets test this round of changes in 6.4.0 and ensure that works before moving on to the next thing.

conditionalize True/False on darwin.

could do

Are there other uses of OnXTerm() where this might cause problems even on OSX?

I think g.setproj is worth testing, it needs the terminal but not the Xmon.

Hamish

comment:42 by hamish, 14 years ago

Resolution: fixed
Status: newclosed

g.setproj problems opened as #945. closing this one.

(everything's working on Mac, right?)

Note: See TracTickets for help on using tickets.