Earnestly / sx

Start an xorg server
MIT License
231 stars 16 forks source link

Allow configuring the server command line #12

Closed prez closed 4 years ago

prez commented 4 years ago

Could be useful for setting options like ardelay, arinterval or xkbmap directly

Earnestly commented 4 years ago

I don't want to support this. If you want to override the options provide an Xorg wrapper which sets them as you want. Then sx can call the Xorg wrapper via normal PATH lookup.

prez commented 4 years ago

I've updated the documentation to reflect the changes I made on my branch in case you change your mind on this. https://github.com/seitokaichou/sx/commit/0415b4fa746530721143a460ff06e8fcf2a54390

As far as I know, this is a pretty uncontroversial way to allow the user to set his own flags. Many services in Void Linux for example use this exact mechanism. And the overhead isn't too large either.

I absolutely respect your decision to not support this if you see no use in it (I'm sure you agree with me that using a wrapper sucks), but I think this would make some users happy.

Earnestly commented 4 years ago

Yeah, I can sympathise, but it's just not something I wanted for this quite deliberately.

I have implemented some mechanisms which allow providing xserver flags in a safer away that doesn't involve any evil (eval) with another name (.) but eventually decided against it.

I don't think wrappers are bad, in both cases a new file comes into existence, and both execute arbitrary code. A wrapper gives you more control over not only the command-line but also any pre or post operations if needed. It also provides a name which execve understands and doesn't need any additional support in software to handle command-line parsing or environment substitution for their options.

prez commented 4 years ago

What mechanism free from eval did you have in mind originally, if you don't mind me asking?

Earnestly commented 4 years ago

There are two general approaches, the first is more reliable but very limited. The second uses POSIX xargs (which is fragile due to the lack of -d or -0).


The first is to use a while read loop and set for command-line building. It's not particularly pretty due to the lack of arrays but it works with the use of subshells to guard against modifying the main set of parameters ("$@"), e.g.:

trap 'DISPLAY=:$tty "${@:-$cfgdir/sxrc}"' USR1

(
    trap '' USR1

    set -- Xorg :"$tty" -keeptty vt"$tty" -noreset -auth "$XAUTHORITY"

    if [ -f "$cfgdir"/xsrvopts ]; then
        # This only handles options which potentially come with a single
        # argument and assumes that it does not contain spaces.
        while IFS= read -r opt arg; do
            [ "$opt" ] && set -- "$@" "$opt"
            [ "$arg" ] && set -- "$@" "$arg"
        done < "$cfgdir"/xsrvopts
    fi

    exec "$@"
) &

pid=$!
wait "$pid"

However this will have problems with any options which take multiple arguments themselves. You would ultimately need to implement some degree of understand for the Xserver command-line such as -multicast taking 3 potentially optional arguments and -auth taking a single argument that may contain spaces (or worse, newlines, but let's ignore that as simply broken and unsupported).


The second approach is using xargs:

trap 'DISPLAY=:$tty "${@:-$cfgdir/sxrc}"' USR1

(
    trap '' USR1
    set -- Xorg :"$tty" -keeptty vt"$tty" -noreset -auth "$XAUTHORITY"

    # Ensure xsrvopts file exists but without altering its metadata if it does.
    if [ -f "$cfgdir"/xsrvopts ]; then
        exec xargs -E '' "$@" < "$cfgdir"/xsrvopts
    else
        exec xargs -E '' "$@"
    fi
) & pid=$!

wait "$pid"

In this case xargs has the advantage of understanding quoting such that it will correctly parse the following config:

-auth '/home/directory with/spaces inside'
-ac
-listen tcp
-multicast 10.0.0.1 3 4

For example:

% xargs -E '' printf '[%s]\n' < ~/.config/sx/xsrvopts
[-auth]
[/home/directory with/spaces inside]
[-ac]
[-listen]
[tcp]
[-multicast]
[10.0.0.1]
[3]
[4]

Once again if newlines appear in here, or arguments aren't properly quoted, then it will break.

prez commented 4 years ago

Thanks a lot for elaborating on that. Yeah, I can see why you would not like to include these. Both of these solutions obviously have their drawbacks, if evaling is not an option.

How about just making use of an environment variable? $SX_XSRVOPTS perhaps? That way the change would be opt-in, and there's no need to eval a file. https://github.com/seitokaichou/sx/commit/875d23c4b75383febd39304caa080dadff098935

Earnestly commented 4 years ago

It has the same whitespace problems. Eventually you will realise that a wrapper is way more elegant and robust. :P

astier commented 3 years ago

Maybe this is obvious but I have issues creating a wrapper. What I want to execute is Xorg -ardelay 200 -arinterval 20. I created an executable shell-script Xorg and put it on my PATH. The content of the script is just a shebang referring to sh and the Xorg-command with the parameters. If I start sx from the tty it hangs ans says Xorg can not fork. How would a simple wrapper look like? Thanks.

Earnestly commented 3 years ago

There are a few options here besides resorting to a wrapper for Xorg itself.

The first and most ideal would be to provide an /etc/X11/xorg.conf.d snippet such as 00-keyboard-repeat.conf with the AutoRepeat option, e.g.

Section "InputClass"
    Identifier "keyboard"
    Driver "libinput"

    MatchIsKeyboard "on"

    Option "AutoRepeat" "200 20"
EndSection

See man 4 libinput

If this isn't possible I would instead of use xset via the sxrc, e.g.

#!/bin/sh --
xset r rate 200 20
exec wm

Failing that, I would copy sx to your local bin and just add the options directly (this is somewhat intended).

If you really must make a wrapper around Xorg itself, it might look like a bernstein chain, e.g.

#!/bin/sh --
exec Xorg "$@" -additional arguments -here

Note that "$@" is placed first. This is due to the :DISPLAY option being positional as can be seen from the SYNOPSIS in the Xorg(1) manual.

But I don't recommend this if you can avoid it.

astier commented 3 years ago

@Earnestly Thank you for the detailed response. I already tried the AutoRepeat-solution however it doesn't work. According to this article it has been deprecated since 2006 and was added back to Xorg 1.21 which is not released yet. https://www.phoronix.com/scan.php?page=news_item&px=Xorg-Input-AutoRepeat