[pacman-dev] [PATCH] makepkg: check explicitly if the terminal supports colors
Currently, makepkg aborts when colors are enabled (the default) and it runs in a terminal that does not support it, because tput returns a non-zero exit code. --- Another solution would be appending "|| true" to each tput line or disabling errexit in this section, so we get at least bold. Not sure if this is a real improvement, when the color option makes text bold but not colored. scripts/makepkg.sh.in | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index dbc4047..1795b54 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1620,8 +1620,11 @@ fi PACMAN=${PACMAN:-pacman} # check if messages are to be printed using color +# It is needed to explicitly check if the terminal supports colors, because +# otherwise, tput will return a non-zero exit code that stops makepkg due to +# bash's errexit option. unset ALL_OFF BOLD BLUE GREEN RED YELLOW -if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]]; then +if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]] && tput setaf 0 &>/dev/null; then ALL_OFF="$(tput sgr0)" BOLD="$(tput bold)" BLUE="${BOLD}$(tput setaf 4)" -- 1.7.1
On Thu, Jun 24, 2010 at 12:01:52PM +0200, Cedric Staniewski wrote:
Currently, makepkg aborts when colors are enabled (the default) and it runs in a terminal that does not support it, because tput returns a non-zero exit code. ---
Another solution would be appending "|| true" to each tput line or disabling errexit in this section, so we get at least bold. Not sure if this is a real improvement, when the color option makes text bold but not colored.
On that subject, that was the 1st or 2nd issue I fixed in makepkg to make it work in(or on, which one is correct?) FreeBSD. The thing is, tput is not fully portable as advertised. I actually fixed this by failing back to the old method of using colours! This is a patch I wrote months ago and applied it in a personal branch: diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index a8402f9..39f7865 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1600,12 +1600,12 @@ PACMAN=${PACMAN:-pacman} # check if messages are to be printed using color unset ALL_OFF BOLD BLUE GREEN RED YELLOW if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]]; then - ALL_OFF="$(tput sgr0)" - BOLD="$(tput bold)" - BLUE="${BOLD}$(tput setaf 4)" - GREEN="${BOLD}$(tput setaf 2)" - RED="${BOLD}$(tput setaf 1)" - YELLOW="${BOLD}$(tput setaf 3)" + ALL_OFF="$(tput sgr0)" || ALL_OFF="\033[1;0m" + BOLD="$(tput bold)" || BOLD="\033[1;1m" + BLUE="${BOLD}$(tput setaf 4)" || BLUE="${BOLD}\033[1;34m" + GREEN="${BOLD}$(tput setaf 2)" || GREEN="${BOLD}\033[1;32m" + RED="${BOLD}$(tput setaf 1)" || RED="${BOLD}\033[1;31m" + YELLOW="${BOLD}$(tput setaf 3)" || YELLOW="${BOLD}\033[1;33m" fi readonly ALL_OFF BOLD BLUE GREEN RED YELLOW
scripts/makepkg.sh.in | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index dbc4047..1795b54 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1620,8 +1620,11 @@ fi PACMAN=${PACMAN:-pacman}
# check if messages are to be printed using color +# It is needed to explicitly check if the terminal supports colors, because +# otherwise, tput will return a non-zero exit code that stops makepkg due to +# bash's errexit option. unset ALL_OFF BOLD BLUE GREEN RED YELLOW -if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]]; then +if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]] && tput setaf 0 &>/dev/null; then ALL_OFF="$(tput sgr0)" BOLD="$(tput bold)" BLUE="${BOLD}$(tput setaf 4)" -- 1.7.1
On 25/06/10 01:27, Nezmer wrote:
On that subject, that was the 1st or 2nd issue I fixed in makepkg to make it work in(or on, which one is correct?) FreeBSD.
The thing is, tput is not fully portable as advertised. I actually fixed this by failing back to the old method of using colours!
This is a patch I wrote months ago and applied it in a personal branch:
<snip> As an aside, we do try to keep compatible with BSD, OSX, Cygwin & Hurd where we know pacman is used so it is good to report breakages to us so that they can get fixed.
On 24/06/10 20:01, Cedric Staniewski wrote:
Currently, makepkg aborts when colors are enabled (the default) and it runs in a terminal that does not support it, because tput returns a non-zero exit code. ---
Another solution would be appending "|| true" to each tput line or disabling errexit in this section, so we get at least bold. Not sure if this is a real improvement, when the color option makes text bold but not colored.
From your comment it seems there terminals that do not support colour but do support bold. It seems to me that keeping bold text would be best in that situation. OK... here comes a potentially stupid question... Are there terminals that do not support colour and do not support bold. Would that still give an error here (seems so...)? And do all terminals supporting bold but not colours support "tput sgr0"? Once that is cleared up, this is a good candidate for maint.
scripts/makepkg.sh.in | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index dbc4047..1795b54 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1620,8 +1620,11 @@ fi PACMAN=${PACMAN:-pacman}
# check if messages are to be printed using color +# It is needed to explicitly check if the terminal supports colors, because +# otherwise, tput will return a non-zero exit code that stops makepkg due to +# bash's errexit option. unset ALL_OFF BOLD BLUE GREEN RED YELLOW -if [[ -t 2&& ! $USE_COLOR = "n"&& $(check_buildenv color) = "y" ]]; then +if [[ -t 2&& ! $USE_COLOR = "n"&& $(check_buildenv color) = "y" ]]&& tput setaf 0&>/dev/null; then ALL_OFF="$(tput sgr0)" BOLD="$(tput bold)" BLUE="${BOLD}$(tput setaf 4)"
On Thu, Jun 24, 2010 at 5:01 AM, Cedric Staniewski <cedric@gmx.ca> wrote:
Currently, makepkg aborts when colors are enabled (the default) and it runs in a terminal that does not support it, because tput returns a non-zero exit code. ---
Another solution would be appending "|| true" to each tput line or disabling errexit in this section, so we get at least bold. Not sure if this is a real improvement, when the color option makes text bold but not colored.
Easy way to get a non-color terminal for testing: TERM=xterm-bold I'd definitely prefer a graceful fallback; Nezmer's patch isn't the prettiest but it gets close to that ideal. Alternatively, would it be awful to fall out of -e mode here and just reset it after we get our colors straightened out? Otherwise I think || true is our best option.
scripts/makepkg.sh.in | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index dbc4047..1795b54 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1620,8 +1620,11 @@ fi PACMAN=${PACMAN:-pacman}
# check if messages are to be printed using color +# It is needed to explicitly check if the terminal supports colors, because +# otherwise, tput will return a non-zero exit code that stops makepkg due to +# bash's errexit option. unset ALL_OFF BOLD BLUE GREEN RED YELLOW -if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]]; then +if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]] && tput setaf 0 &>/dev/null; then ALL_OFF="$(tput sgr0)" BOLD="$(tput bold)" BLUE="${BOLD}$(tput setaf 4)" -- 1.7.1
participants (4)
-
Allan McRae
-
Cedric Staniewski
-
Dan McGee
-
Nezmer