neutrinolabs / xorgxrdp

Xorg drivers for xrdp
Other
428 stars 108 forks source link

guess the xorg path in tests #308

Closed jat001 closed 2 months ago

metalefty commented 2 months ago

Good catch.

It is also mentioned here: https://github.com/neutrinolabs/xrdp/blob/cc7d5ef921938ff4a04e6efe5d0228712abaf471/sesman/sesman.ini.in#L138

jat001 commented 2 months ago

I wrote a loop to guess the xorg path

matt335672 commented 2 months ago

@jat001 - very sorry. The positional parameters are used for XORG_ARGS later in the file. I didn't check carefully enough.

I can give you a couple of options:- 1) Move the new code below the line XORG_ARGS="$@" 2) Try this patch which solves the problem a different way:-

--- a/tests/xorg-test-run.sh
+++ b/tests/xorg-test-run.sh
@@ -2,8 +2,8 @@

 # Test that Xorg can load the compiled modules

-# X server to run
-if [ -z "$XORG" ]; then
+FindXorg()
+{
     set -- \
         /usr/local/libexec/Xorg \
         /usr/libexec/Xorg \
@@ -13,14 +13,17 @@ if [ -z "$XORG" ]; then

     for path in "$@"; do
         if [ -x "$path" ]; then
-            XORG=$path
-            break
+            echo "$path"
+            exit
         fi
     done

-    if [ -z "$XORG" ]; then
-        XORG=Xorg
-    fi
+    echo "Xorg"
+}
+
+# X server to run
+if [ -z "$XORG" ]; then
+    XORG=$(FindXorg)
 fi

 # Client to connect to Xorg

Use whatever work best for you.

jat001 commented 2 months ago

@matt335672 that's why I don't like posix sh LOL

matt335672 commented 2 months ago

Thanks @jat001. I really shouldn't propose changes without testing them.

sh is a bit of a pig to get right, but it's always there. It's made easier with tools like shellcheck (which we haven't used for this script). Only having one array available, and no associative arrays does make it only suitable for very simple tasks though.