[PATCH nixpkgs] spectrumPackages.spectrum-vm: fix without Wayland
This errored because of the undefined XDG_RUNTIME_DIR or WAYLAND_DISPLAY. I'd tried to prevent that by disabling -e around this part, but it turns out I should have disabled -u instead. --- pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in b/pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in index 4fa0287a805..8d95c178689 100755 --- a/pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in +++ b/pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in @@ -55,7 +55,7 @@ do esac done -set +e +set +u if [ -n "$XDG_RUNTIME_DIR" ] then set -- -s "$XDG_RUNTIME_DIR" "$@" @@ -63,7 +63,7 @@ then then set -- --wayland-sock "$XDG_RUNTIME_DIR/$WAYLAND_DISPLAY" "$@" fi fi -set -e +set -u exec "$crosvm" run \ -p init=/sbin/init \ -- 2.27.0
Alyssa Ross <hi@alyssa.is> writes:
This errored because of the undefined XDG_RUNTIME_DIR or WAYLAND_DISPLAY. I'd tried to prevent that by disabling -e around this part, but it turns out I should have disabled -u instead. --- pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in b/pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in index 4fa0287a805..8d95c178689 100755 --- a/pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in +++ b/pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in @@ -55,7 +55,7 @@ do esac done
-set +e +set +u if [ -n "$XDG_RUNTIME_DIR" ]
Rather than using `set`, I, personally, would change this line to be: if [ -n "${XDG_RUNTIME_DIR:-}" ] (as well as the matching check for $WAYLAND_DISPLAY). Though this is just a matter of opinion, I think it is cleaner than using `set`s (but only really because I have no idea what all the `set`s do, without context).
then set -- -s "$XDG_RUNTIME_DIR" "$@" @@ -63,7 +63,7 @@ then then set -- --wayland-sock "$XDG_RUNTIME_DIR/$WAYLAND_DISPLAY" "$@" fi fi -set -e +set -u
exec "$crosvm" run \ -p init=/sbin/init \ -- 2.27.0
Cole
The surrounding "set +e"/"set -e" was an earlier attempt to fix this, but I mixed -e up with -u. But as Cole points out, it's nicer to use parameter expansion here anyway. Thanks-to: Cole Helbling <cole.e.helbling@outlook.com> --- On Tue, Aug 25, 2020 at 10:26:30AM -0700, Cole Helbling wrote:
-set +e +set +u if [ -n "$XDG_RUNTIME_DIR" ]
Rather than using `set`, I, personally, would change this line to be:
if [ -n "${XDG_RUNTIME_DIR:-}" ]
(as well as the matching check for $WAYLAND_DISPLAY).
Though this is just a matter of opinion, I think it is cleaner than using `set`s (but only really because I have no idea what all the `set`s do, without context).
then set -- -s "$XDG_RUNTIME_DIR" "$@" @@ -63,7 +63,7 @@ then then set -- --wayland-sock "$XDG_RUNTIME_DIR/$WAYLAND_DISPLAY" "$@" fi fi -set -e +set -u
pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in b/pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in index 4fa0287a805..a72c3896141 100755 --- a/pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in +++ b/pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in @@ -55,15 +55,13 @@ do esac done -set +e -if [ -n "$XDG_RUNTIME_DIR" ] +if [ -n "${XDG_RUNTIME_DIR-}" ] then set -- -s "$XDG_RUNTIME_DIR" "$@" - if [ -n "$WAYLAND_DISPLAY" ] + if [ -n "${WAYLAND_DISPLAY-}" ] then set -- --wayland-sock "$XDG_RUNTIME_DIR/$WAYLAND_DISPLAY" "$@" fi fi -set -e exec "$crosvm" run \ -p init=/sbin/init \ -- 2.30.0
On Mon Mar 15, 2021 at 6:09 PM PDT, Alyssa Ross wrote:
The surrounding "set +e"/"set -e" was an earlier attempt to fix this, but I mixed -e up with -u. But as Cole points out, it's nicer to use parameter expansion here anyway.
Thanks-to: Cole Helbling <cole.e.helbling@outlook.com> --- pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
LGTM! Reviewed-by: Cole Helbling <cole.e.helbling@outlook.com>
On Mon, Mar 15, 2021 at 06:15:37PM -0700, Cole Helbling wrote:
On Mon Mar 15, 2021 at 6:09 PM PDT, Alyssa Ross wrote:
The surrounding "set +e"/"set -e" was an earlier attempt to fix this, but I mixed -e up with -u. But as Cole points out, it's nicer to use parameter expansion here anyway.
Thanks-to: Cole Helbling <cole.e.helbling@outlook.com> --- pkgs/os-specific/linux/spectrum/spectrum-vm/spectrum-vm.in | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
LGTM!
Reviewed-by: Cole Helbling <cole.e.helbling@outlook.com>
Committed as 10ef397830d. Thanks for the review!
participants (2)
-
Alyssa Ross -
Cole Helbling