Calling exit directly in the relevant trap instead of using an argument
to decide when to trigger an exit in the cleanup function.
Additionally the EXIT trap is ignored in the INT trap so that when
running under bash the EXIT trap does not call cleanup again.
As `xauth remove :n' does not trigger warnings or problems for nonextant
entries it is safe to set the cleanup traps prior to using `xauth add'.
This removes a very minor potential race condition between adding an
entry and the script being interrupted before cleanup traps have been
established. Suggested by @tomty89
Finally, attempt to make the main comment a little less awkward.
This ought to be the last effort on signal handling.
Prior to this design the resending of the signals was ineffective due to
the cleanup function explicitly calling exit. As a result this would
prevent the subsequent commands in the trap from executing.
Instead a variable is employed to guard the exit on condition of not
receiving an INT signal. On UNIX systems this particular signal needs
to be resent such that the calling program knows the process was
interrupted. See https://www.cons.org/cracauer/sigint.html
This situation exists because, besides bash, all shells treat EXIT traps
as a mechanism to execute code after the program ends, as if the code it
runs was pasted at the end of the file, rather than to run it however the
program terminated.
Note that startx itself does call upon admins to improve the script for
their particular situation.
This also removes the example using DESTDIR as any package maintainer
would already be familiar with this standard and expect it to work
implicitly. For those who aren't familiar it merely serves as minor
confusion.
This commit removes the complication of trying to handle client PIDs as
doing so didn't provide any noticable gain. The main issue was that
both `wait' commands didn't wait on their respective PIDs while the
final cleanup wasn't called with both client and server PIDs and so
the server remained running due to -noreset.
The exit status of `sx' should again reflect that of the Xorg's
which may be useful for scripts.
Also slightly improves the comments on how Xorg handles USR1 signals.
Fixes https://github.com/Earnestly/sx/issues/19
Although there is apparently an update to this aspect of POSIX[0] it is
possible for an implementation[1] to induce wordsplitting during an export
assignment.
For example, with both mrsh and an older version of dash, the following
behaviour is observed:
$ var='contains spaces'
$ export ENVAR=$var
$ printf '[%s]\n' "$ENVAR"
[contains]
This commit additionally guards against file names containing leading
hyphens as is possible with user supplied inputs.
0. https://git.kernel.org/pub/scm/utils/dash/dash.git/commit/?id=cbb71a836874d176809a34e22f6b6e4e3ba8c85b
1. https://github.com/emersion/mrsh/issues/150
There appears to be no indication according to the POSIX synopsis[1] of
the ps command that it shall take a PID argument as positional. The
rest of the document doesn't suggest such a case either.
What the document does provide is the `-p' flag to specify a list of
PIDs with which ps will narrow the results.
1. https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ps.html
Sometimes people encode logic into files such as Xinitrc which absorbs
command-line arguments that typically control which window manager or
desktop environment to start.
These examples demonstrate how they might do this with sxrc and other
suggestions for reusing ~/.Xinitrc
Substring removal using the '#' form will already try to match from the
beginning of the string.
If the output from ps does not start with "tty" then nothing will be
removed.
Since the README is not intended to be installed to the system it makes
sense to provide the NOTES section in the manual itself which is.
This also expands the SYNOPSIS section to indicate that the usage of sx
should work as a typical chain command.
Correct the location mentioning the xauthority to XDG_DATA_HOME instead
of XDG_CONFIG_HOME.
Clarify why the server sets DISPLAY to 1 instead of naturally
incrementing until an unused slot is found.
Since Xorg 1.20 the location of the primary binary has changed making it
impossible to support both 1.20 and previous versions.
So instead I'm switching to the `Xorg' command directly which won't
bypass Xorg.wrap in older versions, but since Xorg 1.20, the `Xorg'
command doesn't use it.
The prior commit 666b87e would still fail if the sx program itself was
killed leaving an xorg server behind. While trap handlers aren't
typically allowed to change the exit status, if we explicitly exit with
a value then it is honoured.
This works around the subshell setting an exit status of 138 (due to
SIGUSR1) by explicitly using the return value from the last wait or the
last command if the server never started at all.
Due to the use of errexit when Xorg replied back to us with SIGUSR1 the
subshell would return an exit status of 128 + 10 (SIGUSR1)[1].
This combined with trap handlers not changing the exit value unless
explicitly done with exit $? means sx was always exiting with a status
of 138.
This change instead removes errexit and simply lets unset or erroneous
commands leak through until it hits a point where the combined errors
result in non-zero termination.
By splitting out the termination of Xorg to the end we can simply return
the return value from the last wait, which is the wait associated with
Xorg itself.
1. http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_28
Sending SIGKILL is not an appropriate response in most cases especially
after an arbitrary timeout. If signals are being ignored it's likely
due to D state. If the process is busy spinning in an infinite loop
then bug reports need to be made. Papering over significant issues is
worse for everyone in the end.
This commit also trusts the PID we get is an xorg server process. I
have no real reason to check outside of paranoia which borders on theater.
Instead I check the PID is still alive and send TERM. Then we wait
until it is done. Anything else is surely abnormal and needs manual
intervention.
This condition will always be triggered even if the Xorg server
terminates promptly. This will simply result in confusing diagnostics
being printed in the common case. As kill has no quiet option, I will
have to unfortunately silence them instead.
I was under the false assumption that -a, like &&, was used
short-circuit semantics. However the behaviour of -a is not specified
by POSIX.
At least on Linux using bash or dash the behaviour is not
short-circuiting.
As there's no reason to test the RHS if the LHS is false I've switched
back to [ ] && notation which is defined to be short-circuiting.
Turns out openssl rand would obnoxiously create a HOME/.rnd dotfile
unless told otherwise with the RANDFILE environment. Unfortunately
this neither options are compelling and neither can openssl rand be
told to not write this file.
Instead I'm going to use /dev/urandom which appears to be available
on practically every UNIX.
This commit also tidies up the timeout kill logic to better express
what I am doing.
Upon review of GNU tail I discovered that the implemention of --pid was
the same method as polling kill -0 on a sleep interval, which defaults
to 1 second unless -s is used.
This is perhaps expected due to the lack of a pwait(2) system call, I
decided to implement the same mechanic myself and drop both the
requirement on GNU tail and GNU timeout at the same time.
This commit also replaces mcookie from util-linux with openssl rand
which is used to generate a random MD5 checksum for use in the
MIT-MAGIC-COOKIE-1 value.
We need to do this in sx as neither xauth(1) or touch(1) will create the
path. Requiring the user to do this would lead to additional code on
their side to account for a detail that sx already controls. There
is no obvious benefit for omitting this functionality.
There's not a lot of point in absorbing the xserver's log file when it
will just be less intuitive for those looking for it in the standard
location. Since the log is generated by Xorg and not sx, it only seems
reasonable.
This commit also renames xauthfile to xauthority for a little more
consistency with documentation and prior art.
Xorg.wrap default system of autodetection of root rights requirement is
wrong in every case I've seen it used. The result is the Xorg server
runs as the root user when it is not necessary.
The main autodetection failure case is when the proprietary nvidia
drivers are used, incorrectly assuming Xorg needs to be run as root.