Skip to content

Commit

Permalink
core: better logging when spawn() fails
Browse files Browse the repository at this point in the history
If someone sends us an invalid thing to run, right now we just silently
swallow it, because the logging module doesn't work after we've forked.

Instead, let's just keep around what we're going to exec (being smart about
shell=True invocations so we don't just do a lookup of "/bin/sh") and do a
path lookup on it. If it doesn't exist, we can fail without doing all the
forking and give the user a real error message.

While arguably not logging anything is fine (someone typed an invalid
thing), I've seen a lot of confusion more recently about spawn not working
because of typos and such. Hopefully this will fix things.

Signed-off-by: Tycho Andersen <[email protected]>
  • Loading branch information
tych0 authored and ramnes committed Mar 14, 2021
1 parent 4caa61c commit abc29a2
Showing 1 changed file with 14 additions and 7 deletions.
21 changes: 14 additions & 7 deletions libqtile/core/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import os
import pickle
import shlex
import shutil
import signal
import subprocess
import tempfile
Expand Down Expand Up @@ -1138,14 +1139,19 @@ def cmd_spawn(self, cmd, shell=False):
spawn(["xterm", "-T", "Temporary terminal"])
"""
if shell:
if not isinstance(cmd, str):
cmd = subprocess.list2cmdline(cmd)
args = ["/bin/sh", "-c", cmd]
elif isinstance(cmd, str):
if isinstance(cmd, str):
args = shlex.split(cmd)
else:
args = list(cmd)
cmd = subprocess.list2cmdline(args)

to_lookup = args[0]
if shell:
args = ["/bin/sh", "-c", cmd]

if shutil.which(to_lookup) is None:
logger.error("couldn't find `{}`".format(to_lookup))
return -1

r, w = os.pipe()
pid = os.fork()
Expand Down Expand Up @@ -1197,8 +1203,9 @@ def cmd_spawn(self, cmd, shell=False):

try:
os.execvp(args[0], args)
except OSError as e:
logger.error("failed spawn: \"{0}\"\n{1}".format(cmd, e))
except OSError:
# can't log here since we forked :(
pass

os._exit(1)
else:
Expand Down

0 comments on commit abc29a2

Please sign in to comment.