GNOME Bugzilla – Bug 758452
Set $PWD from vte
Last modified: 2018-02-06 18:25:37 UTC
This is a pretty core Unix feature, even built in to basic library methods such as get_current_dir_name(). It is required for preserving symlink components in the working directory for new terminals. The rest of the story (tracking OSC 7) is done by vte. So I'm wondering if this should also be done down there in vte_pty_child_setup(), rather than separately in each vte-based app. (Inspired by https://bugs.launchpad.net/terminator/+bug/1518554)
Created attachment 360997 [details] [review] vte bits: set $PWD
Created attachment 360998 [details] [review] g-t bits: don't set $PWD
vte: Env vars are set in pty.cc, at two different places: vte_pty_child_setup() and __vte_pty_merge_environ(). It's not clear to me what goes where, e.g. TERM and VTE_VERSION are set at both, and COLORTERM lives a completely different life than TERM. Maybe this could be cleaned up, whatever. I went with whichever seemed to be simpler. __vte_pty_spawn(), after constructing the array of env vars, calls vte_spawn_async_with_pipes_cancellable() first with the desired directory, and if it fails then again with NULL. Ideally we'd set PWD only for the first, and probably leave unchanged (at its original value) for the second. So we either construct two different sets of envp upfront, or bother with saving/restoring PWD (or lack thereof). Alternatively, set PWD in vtespawn.cc vte_spawn_async_with_pipes_cancellable() which is far away from setting all other env vars, and works on the totally inconvenient gchar** array rather than the convenient hash table used in __vte_pty_merge_environ(). All these approaches are truly ugly and cumbersome to implement and no fun at all. So I just went with the simplest, which is by the way what happens currently when gnome-terminal (and other vte-based frontends) manually set PWD: it's also there for the fallback spawn. I think it's good enough because PWD is ignored by bash (and other apps) if it doesn't match the actual working directory. The only role of PWD is to add the extra bit of info about the symlinks traversed to get to the cwd. g-t: There's something I'm unsure about, see the patch. I guess I'll just leave that line unchanged.
Comment on attachment 360997 [details] [review] vte bits: set $PWD Looks good, but please add a short text to the vte_terminal_spawn_sync() and spawn_async() docs that explain that the caller should have used g_get_current_dir() (from glib) or get_current_dir_name() (from glibc) to get the @working_directory (those 2 return $PWD if it's correct) instead of plain getcwd(), so that symlinks are preserved.
Comment on attachment 360998 [details] [review] g-t bits: don't set $PWD The line in terminal-client-utils just filters which env vars are forwarded to the server, so keeping PWD in the list is harmless (and since the PWD env will be unused, there's no point forwarding it). So keep that part as-is. master only, since this requires the vte change. Thanks!
Fixed.