Hi, Thanks for the patch. Let's inspect this patch.
I need your feed back before applying this patch. On Sun, Mar 22, 2015 at 10:02:51AM +0800, Aron Xu wrote: > Package: src:im-config > Severity: wishlist > > The upstart user session job that makes im-config works under it. > Upstart is still used as user session init on Ubuntu, even though the > system init has been changed to systemd recently. There's still no > intention of removing upstart from Debian, so this could be still > useful. > > The attached job file works even when the display server isn't X > (hence Xsession stuff aren't run). The file needs to be installed to > /usr/share/upstart/sessions/im-config.conf > > Thanks, > Aron This looks interesting though I have no idea about the non-X display server starting mechanism used on Ubuntu. This script may need some touch up to be perfect and this discussion may be useful for the upcoming Wayland support. Please read the following first to be sure. The sysV init Xsession starting mechanism does 2 things. * PHASE=1 (70im-config_launch) Export environment variables to the child processes so they can find appropriate input methods: * XMODIFIERS sets it for plain XIM (via X IPC to IM daemon) * GTK*_IM_MODULE sets it for GTK* (via library) * QT*_IM_MODULE sets it for QT* (via library) * PHASE=2 (im-launch) Start the IM daemon including GUI indicator apps at the end of Xsession startup process. The IM daemon starting hook im-launch is specified in the STARTUP shell variable. The Xsession startup sequence updates the STARTUP shell variable as: STARTUP="$<NEW_PROGRAM_NAME> $STARTUP" so $<NEW_PROGRAM_NAME> is parent of previously defined $STARTUP . This ensures dbus started by 75dbus_dbus-launch etc. are ready to be used to avoid some race conditions previously reported for GUI indicator apps started too early. Basically, this $STARTUP string is used to start programs in the reverse order of Xsession.d sequence. Let me make comments on proposed patch code (prefixed with | ): | start on starting xsession-init and started dbus OK, So you want this after dbus when starting xsession-init. As I discuss below, GPG-agent and SSH-agent may need to be waited if they exist. What is your thought? | pre-start script | . /etc/X11/Xsession.d/70im-config_launch Since IM_CONFIG_PHASE should have been undefined, this should define it as 1 while exporting environment variables if no other script touched on them in advance. | if [ "$IM_CONFIG_PHASE" = 1 ]; then Can I reorder your script as follows so it is more logical to me. (If there is some reason why it was placed at the end, let me know) I assume Upstart does not honor export unless specifically told to export via "initctl set-env ...". So you added these for Upstart. By putting within this "if ..." segment, you wisely ensured no other script touched on them in advance. | initctl set-env -gr XMODIFIERS=$XMODIFIERS | initctl set-env -gr GTK_IM_MODULE=$GTK_IM_MODULE | initctl set-env -gr QT_IM_MODULE=$QT_IM_MODULE | initctl set-env -gr QT4_IM_MODULE=$QT4_IM_MODULE | initctl set-env -gr CLUTTER_IM_MODULE=$CLUTTER_IM_MODULE Since STARTUP variable is not used on Ubuntu Upstart case, I guess you pasted im-launch as below to be run as a part of this initialization to start daemons. Question is is dbus enough to be waited? GPG and SSH agent also mangles STARTUP. | IM_CONFIG_PHASE=2 | # initialize all im-config common functions and parameters | . /usr/share/im-config/xinputrc.common Can I update this script to prevent the leaky environment variables by adding the followings (See https://bugs.debian.org/767483 and 0.29-1): > unset TEXTDOMAIN > unset TEXTDOMAINDIR | # source the first found configuration file | if [ -r "$IM_CONFIG_XINPUTRC_USR" ]; then | . $IM_CONFIG_XINPUTRC_USR | elif [ -r "$IM_CONFIG_XINPUTRC_SYS" ]; then | . $IM_CONFIG_XINPUTRC_SYS | fi | | fi | end script Regards, Osamu -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org