feat(startwm): surface session logs when SELKIES_DEBUG is enabled (openbox + labwc)

[junkerderprovinz]

Currently both session scripts send their output to /dev/null, so SELKIES_DEBUG=true cannot tell you why an autostart entry or the desktop itself failed to come up.

This makes the existing SELKIES_DEBUG flag also cover session startup: when it is enabled, the openbox session (startwm.sh) and the labwc/wayland session (startwm_wayland.sh) route their output (autostart, app launchers, ready banners) to the container log. Default behaviour is unchanged and quiet.

No new env var, no fd rebinding, no duplicated code, just the existing redirect made conditional in the two session scripts.

[thelamer]

Three things here:

1. We can just piggyback on the existing SELKIES_DEBUG=true and use that, no need for new env var.
2. These logs are suppressed for good reason, desktop applications were not meant to run in docker, something like chromium can output millions of lines on a loop constantly when open depending on what it is doing.
3. This does not cover wayland mode.

On the fence here, while I see the value of it users can always run the applications from the command line inside their containers to see what is happening and get output there. This would only open up showing labwc/openbox logs which are pretty uninteresting.

I do not have a great solution for this that would not see a bunch of code duplication or use use stdout/stderr rebinding to 3 and 4.

I'll think about a potential solution, but anything downstream can run any startwm.sh they want that is how stuff like webtop works and this is documented.

[junkerderprovinz]

Thanks for the detailed look, all three addressed:

1. Dropped the new env var. It now piggybacks on the existing SELKIES_DEBUG, so there's no new surface; SELKIES_DEBUG=true becomes a complete switch that also surfaces the session.

2. Fair point on volume. It's gated behind SELKIES_DEBUG, so the default stays quiet, and verbosity is exactly what you expect when you flip a debug flag on. The gap it targets: today SELKIES_DEBUG can't tell you why an autostart entry or the DE itself failed to come up, because startwm's output goes to /dev/null.

3. Wayland is covered now. startwm_wayland.sh (labwc) gets the same gate, so both session paths behave the same.

No fd rebinding and no duplicated code, just the existing redirect made conditional in the two session scripts. Happy to drop it if you'd still rather keep this downstream, but making SELKIES_DEBUG actually cover session startup seemed like a small, contained win.

[junkerderprovinz]

Rebased onto current master (the branch was based on an older master, so the previous diff looked noisy) and reworked to keep it minimal: a single LOGDEST variable per script makes the existing redirect conditional, so there is no duplication and no fd rebinding. It now covers both startwm.sh (openbox) and startwm_wayland.sh (labwc). I left the one already-commented-out redirect in the PELORUS desktop path untouched, since that looks like work in progress.

[thelamer]

Did you test this? I think the duplication of logic is unavoidable honestly, abc cannot write to stdout

[thelamer]

Let's put a pin in this as I said I am on the fence, downstream containers can modify their own startwm script. As support stands right now I prefer having people run the apps in the container from console to get logs for specific apps.

[junkerderprovinz]

Thanks for weighing it, that's fair. You're right that abc can't write to the container stdout, so surfacing it there needs more than a conditional redirect, and I take the point about keeping the default support flow console-based. I'll keep this downstream in my own images instead. Appreciate the look.

[thelamer]

Just a heads up given the number of PRs and how building out all these bases works and the time that it takes to build them the other PRs you have submitted will come in one patch, if you want to be in the commit history looks like this

diff --git a/Dockerfile b/Dockerfile
index 61d364b..d6c3775 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -16,7 +16,7 @@ RUN \
     https://github.com/selkies-project/selkies.git \
     /src && \
   cd /src && \
-  git checkout -f a4aadef97ba4f74f5a89b510bad18df8c2ca8370
+  git checkout -f 348bc4f61da66198573e7e57db9a266aca1991d5
 
 RUN \
   echo "**** build shared core library ****" && \
@@ -467,7 +467,7 @@ RUN \
     | awk '/tag_name/{print $4;exit}' FS='[""]') && \
   curl -o \
     /tmp/selkies.tar.gz -L \
-    "https://github.com/selkies-project/selkies/archive/a4aadef97ba4f74f5a89b510bad18df8c2ca8370.tar.gz" && \
+    "https://github.com/selkies-project/selkies/archive/348bc4f61da66198573e7e57db9a266aca1991d5.tar.gz" && \
   cd /tmp && \
   tar xf selkies.tar.gz && \
   cd selkies-* && \
diff --git a/Dockerfile.aarch64 b/Dockerfile.aarch64
index 270012a..8b94952 100644
--- a/Dockerfile.aarch64
+++ b/Dockerfile.aarch64
@@ -16,7 +16,7 @@ RUN \
     https://github.com/selkies-project/selkies.git \
     /src && \
   cd /src && \
-  git checkout -f a4aadef97ba4f74f5a89b510bad18df8c2ca8370
+  git checkout -f 348bc4f61da66198573e7e57db9a266aca1991d5
 
 RUN \
   echo "**** build shared core library ****" && \
@@ -463,7 +463,7 @@ RUN \
     | awk '/tag_name/{print $4;exit}' FS='[""]') && \
   curl -o \
     /tmp/selkies.tar.gz -L \
-    "https://github.com/selkies-project/selkies/archive/a4aadef97ba4f74f5a89b510bad18df8c2ca8370.tar.gz" && \
+    "https://github.com/selkies-project/selkies/archive/348bc4f61da66198573e7e57db9a266aca1991d5.tar.gz" && \
   cd /tmp && \
   tar xf selkies.tar.gz && \
   cd selkies-* && \
diff --git a/root/defaults/startwm_wayland.sh b/root/defaults/startwm_wayland.sh
index 0b1cd6f..fde82bb 100755
--- a/root/defaults/startwm_wayland.sh
+++ b/root/defaults/startwm_wayland.sh
@@ -28,7 +28,7 @@ if [ "${PELORUS,,}" == "true" ]; then
       kill $ATSPI_PID
       kill $LABWC_PID
       kill $PELORUS_PID
-    ' #> /dev/null 2>&1 &
+    ' > /dev/null 2>&1
   else
     dbus-run-session bash -c '
       /usr/libexec/at-spi2-registryd &
diff --git a/root/etc/s6-overlay/s6-rc.d/init-nginx/run b/root/etc/s6-overlay/s6-rc.d/init-nginx/run
index eba905e..dbbd034 100755
--- a/root/etc/s6-overlay/s6-rc.d/init-nginx/run
+++ b/root/etc/s6-overlay/s6-rc.d/init-nginx/run
@@ -34,13 +34,13 @@ sed -i "s/CWS/$CWS/g" ${NGINX_CONFIG}
 sed -i "s|SUBFOLDER|$SFOLDER|g" ${NGINX_CONFIG}
 sed -i "s|REPLACE_DOWNLOADS_PATH|$FILE_MANAGER_PATH|g" ${NGINX_CONFIG}
 s6-setuidgid abc mkdir -p ${FILE_MANAGER_PATH}
-if [[ $SELKIES_FILE_TRANSFERS != *"download"* ]] || [[ ${HARDEN_DESKTOP,,} == "true" ]]; then
+if [[ ${SELKIES_FILE_TRANSFERS,,} != *"download"* ]] || [[ ${HARDEN_DESKTOP,,} == "true" ]]; then
   sed -i '/files {/,/^  }/d' ${NGINX_CONFIG}
 fi
-if [ ! -z ${DISABLE_IPV6+x} ]; then
+if [ -n "${DISABLE_IPV6}" ]; then
   sed -i '/listen \[::\]/d' ${NGINX_CONFIG}
 fi
-if [ ! -z ${PASSWORD+x} ]; then
+if [ -n "${PASSWORD}" ]; then
   printf "${CUSER}:$(openssl passwd -apr1 ${PASSWORD})\n" > /etc/nginx/.htpasswd
   sed -i 's/#//g' ${NGINX_CONFIG}
 fi
diff --git a/root/etc/s6-overlay/s6-rc.d/init-selkies-config/run b/root/etc/s6-overlay/s6-rc.d/init-selkies-config/run
index 27d1ea4..8b6dbc0 100755
--- a/root/etc/s6-overlay/s6-rc.d/init-selkies-config/run
+++ b/root/etc/s6-overlay/s6-rc.d/init-selkies-config/run
@@ -157,8 +157,8 @@ if [[ "${PIXELFLUX_WAYLAND,,}" == "true" ]];then
     echo "[ls.io-init] Generating labwc rc.xml from template"
     cp "$DEF_RC" "$USER_RC_XML"
     chown abc:abc "$USER_RC_XML"
-    
-    if [[ -n "${DISABLE_CLOSE_BUTTON}" ]]; then
+
+    if [[ "${DISABLE_CLOSE_BUTTON,,}" == "true" ]]; then 
       echo "[ls.io-init] Disabling close button"
       sed -i 's/close//' "$USER_RC_XML"
     fi
@@ -187,7 +187,7 @@ if [[ "${PIXELFLUX_WAYLAND,,}" == "true" ]];then
             sed -i "/<keybind key=\"${key}\"/I,/<\/keybind>/{s/^/    <!-- /;s/$/ -->/}" "$USER_RC_XML"
         done
     fi
-    
+
     # lock file if hardened
     if [[ ${DISABLE_MOUSE_BUTTONS,,} == "true" || ${HARDEN_KEYBINDS,,} == "true" ]]; then
       echo "[ls.io-init] Locking labwc rc.xml to prevent security overrides"
@@ -204,7 +204,7 @@ else
     cp "$SYS_RC_XML" "$SYS_RC_BAK"
   fi
   cp "$SYS_RC_BAK" "$SYS_RC_XML"
-  if [[ -n "${DISABLE_CLOSE_BUTTON}" ]]; then
+  if [[ "${DISABLE_CLOSE_BUTTON,,}" == "true" ]]; then
     echo "[ls.io-init] Disabling close button"
     sed -i '/<titleLayout>/s/C//' "$SYS_RC_XML"
   fi
diff --git a/root/etc/s6-overlay/s6-rc.d/svc-dbus/run b/root/etc/s6-overlay/s6-rc.d/svc-dbus/run
index d37632a..5736efe 100755
--- a/root/etc/s6-overlay/s6-rc.d/svc-dbus/run
+++ b/root/etc/s6-overlay/s6-rc.d/svc-dbus/run
@@ -10,4 +10,4 @@ exec s6-setuidgid abc \
   dbus-daemon \
     --system \
     --nofork \
-    --nosyslog 
+    --nosyslog > /dev/null 2>&1 
diff --git a/root/etc/s6-overlay/s6-rc.d/svc-xorg/run b/root/etc/s6-overlay/s6-rc.d/svc-xorg/run
index 58f4e9c..f2797fa 100755
--- a/root/etc/s6-overlay/s6-rc.d/svc-xorg/run
+++ b/root/etc/s6-overlay/s6-rc.d/svc-xorg/run
@@ -13,7 +13,7 @@ VFBCOMMAND=""
 if ! which nvidia-smi && [ -e "/dev/dri/renderD128" ]; then
   VFBCOMMAND="-vfbdevice /dev/dri/renderD128"
 fi
-if [ ! -z ${DRINODE+x} ]; then
+if [ -n "${DRINODE}" ]; then
   VFBCOMMAND="-vfbdevice ${DRINODE}"
 fi
 if [ "${DISABLE_DRI3,,}" != "false" ]; then
@@ -22,7 +22,7 @@ fi
 
 # Clamp virtual screen max size based on env
 DEFAULT_RES="15360x8640"
-if [ ! -z ${MAX_RES+x} ]; then
+if [ -n "${MAX_RES}" ]; then
   DEFAULT_RES="${MAX_RES}"
 fi
 if [ -n "${SELKIES_MANUAL_HEIGHT}" ] || [ -n "${SELKIES_MANUAL_WIDTH}" ]; then

If you don't care I am directly committing these tomorrow after full testing.

[junkerderprovinz]

Sounds good, no objection at all, please go ahead and commit them, and yes I'd be glad to be in the commit history. Thanks for folding them in.

One thing that got dropped from the empty-PASSWORD change (#179): the batch keeps the guard on the if line but not the quoting on the line below it. openssl passwd -apr1 ${PASSWORD} is still unquoted, so a password containing a space or a shell glob word-splits before openssl sees it (openssl passwd -apr1 foo bar prints two hash lines), which writes a malformed .htpasswd and silently locks the user out. Worth folding in too:

printf '%s:%s\n' "${CUSER}" "$(openssl passwd -apr1 "${PASSWORD}")" > /etc/nginx/.htpasswd

That also stops treating ${CUSER} as a printf format string. Everything else in the patch looks right to me.