From 9afb2578f8ae4856b8df8d9ae4bc24581d897681 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 1 Nov 2015 17:53:53 +0100 Subject: [PATCH 1/2] Spawn: Improve handling of startup notification The ID for startup notification is transmitted to the spawned process via the DESKTOP_STARTUP_ID environment variable. Before this commit, we set this variable in the main process. This meant that if we started something "without" a startup id, then it might get the ID that was used by the last spawn and which was still saved in our env. Fix this by setting the environment variable only after fork(). Small anecdote: The above wasn't enough to make Daniel's test case succeed and at first I couldn't figure out why. Turns out that rxvt-unicode doesn't unset the DESKTOP_STARTUP_ID environment variable (I think it should, according to some spec), even though it supports startup notification. So awesome was already started with DESKTOP_STARTUP_ID set and thus all spawned processes used this ID. Fix this by explicitly unsetting DESKTOP_STARTUP_ID if we don't set any new value (even though this breaks encapsulation; we shouldn't have to care about this "implementation detail" of libstartup-notification). Signed-off-by: Uli Schlachter --- spawn.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/spawn.c b/spawn.c index 2bba63e29..51830046f 100644 --- a/spawn.c +++ b/spawn.c @@ -283,7 +283,14 @@ spawn_launchee_timeout(gpointer context) static void spawn_callback(gpointer user_data) { + SnLauncherContext *context = (SnLauncherContext *) user_data; setsid(); + + if (context) + sn_launcher_context_setup_child_process(context); + else + /* Unset in case awesome was already started with this variable set */ + unsetenv("DESKTOP_STARTUP_ID"); } /** Parse a command line. @@ -400,11 +407,10 @@ luaA_spawn(lua_State *L) /* app will have AWESOME_SPAWN_TIMEOUT seconds to complete, * or the timeout function will terminate the launch sequence anyway */ g_timeout_add_seconds(AWESOME_SPAWN_TIMEOUT, spawn_launchee_timeout, context); - sn_launcher_context_setup_child_process(context); } retval = g_spawn_async_with_pipes(NULL, argv, NULL, G_SPAWN_SEARCH_PATH, - spawn_callback, NULL, &pid, + spawn_callback, context, &pid, stdin_ptr, stdout_ptr, stderr_ptr, &error); g_strfreev(argv); if(!retval) From 339e2b0ecc7bc740c616b6ccd124fbeb71e62bc8 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Wed, 28 Oct 2015 01:00:00 +0100 Subject: [PATCH 2/2] Add failing test: c.startup_id should be nil when snid is disabled When using `spawn` without startup notification support, the `startup_id` property on the client should be nil. This adds rxvt-unicode to the Travis build, because it supports startup notifications, but xterm does not. We should replace xterm with it in the existing tests then later. --- .travis.yml | 2 +- tests/test-spawn-snid.lua | 42 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 tests/test-spawn-snid.lua diff --git a/.travis.yml b/.travis.yml index cf610b59e..c6b08ccdc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -31,7 +31,7 @@ install: - sudo apt-get install -y libcairo2-dev xmlto asciidoc libpango1.0-dev gperf luadoc libxcb-xtest0-dev libxcb-icccm4-dev libxcb-randr0-dev libxcb-keysyms1-dev libxcb-xinerama0-dev libxcb-image0-dev libev-dev libimlib2-dev libdbus-1-dev libxdg-basedir-dev libstartup-notification0-dev imagemagick libxcb1-dev libxcb-shape0-dev libxcb-util0-dev libxcursor-dev libx11-xcb-dev libxcb-cursor-dev libxcb-xkb-dev libxkbcommon-dev libxkbcommon-x11-dev # Deps for functional tests. - - sudo apt-get install -y dbus-x11 xterm xdotool xterm xvfb + - sudo apt-get install -y dbus-x11 xterm xdotool xterm xvfb rxvt-unicode # Install Lua (per env). - sudo apt-get install -y lib${LUANAME}-dev ${LUANAME} ${INSTALL_PKGS} diff --git a/tests/test-spawn-snid.lua b/tests/test-spawn-snid.lua new file mode 100644 index 000000000..87541583f --- /dev/null +++ b/tests/test-spawn-snid.lua @@ -0,0 +1,42 @@ +--- Tests for spawn's startup notifications. + +local spawn = require("awful.spawn") + +local manage_called, c_snid + +client.connect_signal("manage", function(c) + manage_called = true + c_snid = c.startup_id +end) + + +local ret, snid +local steps = { + function(count) + if count == 1 then + ret, snid = spawn('urxvt', true) + elseif manage_called then + local c = client.get()[1] + assert(ret) + assert(snid) + assert(snid == c_snid) + return true + end + end, + + -- Test that c.startup_id is nil for a client without startup notifications, + -- and especially not the one from the previous spawn. + function(count) + if count == 1 then + manage_called = false + ret, snid = spawn('urxvt', false) + elseif manage_called then + assert(ret) + assert(snid == nil) + assert(c_snid == nil, "c.startup_snid should be nil!") + return true + end + end +} + +require("_runner").run_steps(steps)