GNOME Bugzilla – Bug 631589
set_emulation does not work in the python bindings
Last modified: 2010-10-13 20:00:45 UTC
When I use set_emulation to set the terminal it has no effect, the system env TERM variable is used instead. For example I start the vte-demo.py from another terminal where TERM is set to dumb. The TERM variable in the vte-demo.py is then set to dumb, too - even so set_emulation does set it to xterm in this demo program. In 0.24 when I do the same thing, and print out the TERM variable in vte-demo.py it is set to xterm. This has an impact on the terminal activity in Sugar: http://bugs.sugarlabs.org/ticket/2394
Created attachment 171886 [details] [review] Pass the correct TERM value inside envp when spawning Previously we were removeing TERM from envp and calling setenv from vte_pty_child_setup but it had no effect because execve would rewrite the whole environment with whatever we passed as envp.
This patch is wrong, since using __vte_pty_spawn/vte_terminal_fork_command[_full] isn't required to use VtePty, it's just a convenience API.
(In reply to comment #2) > This patch is wrong, since using > __vte_pty_spawn/vte_terminal_fork_command[_full] isn't required to use VtePty, > it's just a convenience API. Thanks for looking at it, could you be more specific about which codepaths this patch breaks? This is the first time I look at vte.
It breaks if you use vte_pty_child_setup (with fork, or g_spawn_async*) directly, instead of the vte_terminal_fork_command* convenience APIs.
Created attachment 171896 [details] [review] Pass the correct TERM value inside envp when spawning So the codepaths that end up invoking execve still have the correct TERM entry.
Had a quick look at the new patch, that looks better. However, I don't quite understand where the bug actually IS; we set the TERM var in child_setup, where is it overwritten it after that?
(In reply to comment #6) > Had a quick look at the new patch, that looks better. However, I don't quite > understand where the bug actually IS; we set the TERM var in child_setup, where > is it overwritten it after that? g_spawn_async_with_pipes() will end up calling execve() which discards the existing environment: http://git.gnome.org/browse/glib/tree/glib/gspawn.c#n1103
Hmm right. So the approach of this patch looks right. + if (term_value != NULL) + g_ptr_array_add (array, g_strconcat ("TERM", "=", term_value, NULL)); I'd rather put this into the hash table after iterating over parent_environ (a few lines above this place), and keep the hash table to ptr array code as-is. And let's add a few words in the documentation for VtePty about this issue, so one knows what to do when using vte_pty directly with g_spawn_async without using the vte_terminal_fork_command* wrapper.
*** Bug 630861 has been marked as a duplicate of this bug. ***
Created attachment 171942 [details] [review] Pass the correct TERM value inside envp when spawning So the codepaths that end up invoking execve still have the correct TERM entry.
Created attachment 171943 [details] [review] Pass the correct TERM value inside envp when spawning So the codepaths that end up invoking execve still have the correct TERM entry. Clarify the effect of vte_pty_set_term().
ChPe, can we get this in if it looks good now?
Yes, this looks good now (apart from the documentation issue which I can amend later). Ok to commit to master and vte-0-26.
Attachment 171943 [details] pushed as e5fd6c3 - Pass the correct TERM value inside envp when spawning
Thanks everyone for working on this! Do you already have plans when to do a new release of the 0.26 branch? I ask because in one week there is the F14 Final Change deadline and we either try to ship a patch but a bug fix release is preferred of course.
See http://live.gnome.org/TwoPointThirtythree for the GNOME release schedule for 2.32.1 . Unless you think we should get a 2.26.0.1 out just for this fix --- Behdad?
Thanks Christian for your reply. 2.32.1 is still a bit out, indeed. Sugar was originally aligned with the GNOME schedule but we added another bug fix release ( http://wiki.sugarlabs.org/go/0.90/Roadmap#Schedule ) for this Friday ourselves to get a few more fixes into F14.
*** Bug 631317 has been marked as a duplicate of this bug. ***
(In reply to comment #16) > See http://live.gnome.org/TwoPointThirtythree for the GNOME release schedule > for 2.32.1 . > > Unless you think we should get a 2.26.0.1 out just for this fix --- Behdad? Released 2.26.1.
(In reply to comment #19) > (In reply to comment #16) > > See http://live.gnome.org/TwoPointThirtythree for the GNOME release schedule > > for 2.32.1 . > > > > Unless you think we should get a 2.26.0.1 out just for this fix --- Behdad? > > Released 2.26.1. Thanks for the bug fix release, has been found it's way into a F14 update already. Works great! Thanks as well to Tomeu for the actual code!