chjj / compton

A compositor for X11.
Other
2.25k stars 498 forks source link

compton-trans assumes /bin/sh == /usr/local/bin/bash #311

Open Parakleta opened 9 years ago

Parakleta commented 9 years ago

This is a particularly frustrating linuxism... Bourne Shell is not Bourne Again Shell

Your #! line for the compton-trans script needs to use /usr/bin/env bash or similar rather than incorrectly assuming that /bin/sh is bash.

richardgv commented 9 years ago

Thanks for the report, firstly.

If you know the reason compton-trans fails on your sh, and there is an easy fix, I am pleased to apply it. Otherwise, we probably won't fix it or change the shebang:

compton-trans aims to be POSIX-compliant. It works on dash (Debian Almquist Shell, a small POSIX-compliant shell), and mostly on busybox sh (ash) and posh (Debian Policy-compliant Ordinary SHell).

But if your sh is not POSIX-compliant, we are probably not going to support it.

As we support most POSIX-compliant sh, it is reasonable to use the #!/bin/sh shebang, and we are not interested in forcing everybody to use bash, even though sh does not mean POSIX-compliance and it is actually a wrong shebang. (Indeed a very Linuxism choice...) If some particular OS uses a non-POSIX-compliant sh, the maintainer of the package for the OS should modify the shebang.

Parakleta commented 9 years ago

If your intention is to target posix compliance then I can investigate further to find the cause of the failure on my system. I was thrown by the fact that the script opens with #!/bin/sh and then 4 lines later claims to be a bash script. Being a BSD user it is a frustratingly frequent problem to encounter very bash scripts trying to launch off sh.

I'll experiment tomorrow and update you with the exact failure.

Parakleta commented 9 years ago

It turns out the error is not due to the shell, I jumped to conclusions from the comment on line 5 that it was a bash script. The issue is actually that the sed commands are using extended regular expressions which are not POSIX compliant, notably the addition of \+, \?, \| and \b.

There are two possible solutions. The simplest is to use the -r switch which is accepted on both GNU and FreeBSD to activate extended regular expressions (although FreeBSD prefers -E to match grep, GNU doesn't recognise this option). I don't know how portable these switches are beyond those two systems.

The other is to generally replace \+ with \{1,\} and \? with \{0,1\}. The \| and \b are only used in the expression on line 187 which I would propse be changed to:

sed -n 's/^\(.*[^_[:alnum:]]\)\{0,1\}\([0-9]\{1,\}\).*$/\2/p'

This is the closest direct translation of the existing regular expression with the word boundary token (although note that while GNU defines a word character as [_[:alnum:]], FreeBSD defines it as [[:alnum:]]). There may be simpler ways to define the boundary condition but I'm not familiar with the expected output grammar of xprop. Note the -n and p combination as the replacement for \|^.*$.