[pacman-dev] [PATCH 1/3] makepkg: fallback to legacy termcap capabilities

older
[pacman-dev] [PATCH 1/4] CLI args:...

Carlos Diaz

14 Oct 2010 14 Oct '10

4:32 p.m.

In some systems, such as Freebsd, the base tput isn't linked against a terminfo capable ncurses. This means that there's no portable way to set color, so instead of guessing, makepkg fallbacks on using legacy termcap output manipulation. For reference, see terminfo(5) and termcap(5) for a comparison between the abilities. The hardcoded list of terminals is taken from freebsdports/shells/tcshrc and tmux [1]. This prevents echoing gibberish in dumb and otherwise incapacitated terminals. 1: http://www.openbsd.org/cgi-bin/cvsweb/src/usr.bin/tmux/tty.c?rev=1.91;conten... Signed-off-by: Carlos Diaz <839273@gmail.com> --- scripts/makepkg.sh.in | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index bdf63ef..3d29d26 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1660,21 +1660,30 @@ 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 - # prefer terminal safe colored and bold text when tput is supported if tput setaf 0 &>/dev/null; then + # tput linked against a compliant terminfo ncurses 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)" - else + elif [[ $TERM =~ ^(dtterm|rxvt|screen|xterm) ]]; then + # no terminfo, but don't fallback to ugly reverse text just yet ALL_OFF="\033[1;0m" BOLD="\033[1;1m" BLUE="${BOLD}\033[1;34m" GREEN="${BOLD}\033[1;32m" RED="${BOLD}\033[1;31m" YELLOW="${BOLD}\033[1;33m" + elif tput me &>/dev/null; then + # rely on termcap capabilities; no portable way of using color + ALL_OFF="$(tput me)" + BOLD="$(tput md)" + BLUE="$(tput mr)" # reverse + GREEN="$(tput us)" # underline + RED="$(tput so)" # standout + YELLOW="$(tput mh)" # half-bright fi fi readonly ALL_OFF BOLD BLUE GREEN RED YELLOW -- 1.7.3.1

Show replies by date

Carlos Diaz

14 Oct 14 Oct

4:32 p.m.

New subject: [pacman-dev] [PATCH 2/3] makepkg: use portable escape sequence for terminal escapes

The escape string isn't necesarily \033; it's determined by what the particular termcap/info entry for that terminal contains. Bash uses ncurses functions to expand \e to the _correct_ terminal escape. Signed-off-by: Carlos Diaz <839273@gmail.com> --- scripts/makepkg.sh.in | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 3d29d26..6a5cff8 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1670,12 +1670,12 @@ if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]]; then YELLOW="${BOLD}$(tput setaf 3)" elif [[ $TERM =~ ^(dtterm|rxvt|screen|xterm) ]]; then # no terminfo, but don't fallback to ugly reverse text just yet - ALL_OFF="\033[1;0m" - BOLD="\033[1;1m" - BLUE="${BOLD}\033[1;34m" - GREEN="${BOLD}\033[1;32m" - RED="${BOLD}\033[1;31m" - YELLOW="${BOLD}\033[1;33m" + ALL_OFF="\e[1;0m" + BOLD="\e[1;1m" + BLUE="${BOLD}\e[1;34m" + GREEN="${BOLD}\e[1;32m" + RED="${BOLD}\e[1;31m" + YELLOW="${BOLD}\e[1;33m" elif tput me &>/dev/null; then # rely on termcap capabilities; no portable way of using color ALL_OFF="$(tput me)" -- 1.7.3.1

Carlos Diaz

4:32 p.m.

New subject: [pacman-dev] [PATCH 3/3] makepkg: do not abort when a terminal capability is unpresent

makepkg does not contain logic to react if a terminal capability is present when another one in the list dictated by the wrapping conditional is not. This commit makes it a benign condition; e.g., if the tput probing chunk determines that makepkg is to fallback to using standout and reverse capabilities, it will make the best use of what it can find, instead of abruptly ending the program. Signed-off-by: Carlos Diaz <839273@gmail.com> --- scripts/makepkg.sh.in | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 6a5cff8..866b995 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1659,6 +1659,8 @@ PACMAN=${PACMAN:-pacman} # check if messages are to be printed using color unset ALL_OFF BOLD BLUE GREEN RED YELLOW +# do not abort when a terminal capability is unpresent +set +E +e if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]]; then if tput setaf 0 &>/dev/null; then # tput linked against a compliant terminfo ncurses @@ -1686,6 +1688,7 @@ if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]]; then YELLOW="$(tput mh)" # half-bright fi fi +set -E -e readonly ALL_OFF BOLD BLUE GREEN RED YELLOW # override settings with an environment variable for batch processing -- 1.7.3.1

Dan McGee

4:38 p.m.

New subject: [pacman-dev] [PATCH 2/3] makepkg: use portable escape sequence for terminal escapes

On Thu, Oct 14, 2010 at 11:32 AM, Carlos Diaz <839273@gmail.com> wrote:

...

The escape string isn't necesarily \033; it's determined by what the particular termcap/info entry for that terminal contains.

Bash uses ncurses functions to expand \e to the _correct_ terminal escape.

Signed-off-by: Carlos Diaz <839273@gmail.com>

How would this affect a particular bit of code in src/pacman/callback.c; can we do anything there to make sure we are doing it right? Or is \033 right in almost all situations anyway, and this patch is mainly just for pedantic purposes. -Dan

Andres Perera

4:45 p.m.

New subject: [pacman-dev] [PATCH 2/3] makepkg: use portable escape sequence for terminal escapes

On Thu, Oct 14, 2010 at 12:08 PM, Dan McGee wrote:

...

On Thu, Oct 14, 2010 at 11:32 AM, Carlos Diaz <839273@gmail.com> wrote:

...
The escape string isn't necesarily \033; it's determined by what the particular termcap/info entry for that terminal contains.

Bash uses ncurses functions to expand \e to the _correct_ terminal escape.

Signed-off-by: Carlos Diaz <839273@gmail.com>

How would this affect a particular bit of code in src/pacman/callback.c; can we do anything there to make sure we are doing it right? Or is \033 right in almost all situations anyway, and this patch is mainly just for pedantic purposes.

As long as you don't want to maintain compatibility with legacy terminals, it's pedantic. You'd have to ask yourself if being correct in this case is worth linking against ncurses. Granted I haven't looked at it yet, so I might just end recommending that pacman shouldn't be using terminal escapes in the first place.

...

-Dan

Dan McGee

5:21 p.m.

New subject: [pacman-dev] [PATCH 2/3] makepkg: use portable escape sequence for terminal escapes

On Thursday, October 14, 2010, Andres Perera wrote:

...

On Thu, Oct 14, 2010 at 12:08 PM, Dan McGee wrote:

...
On Thu, Oct 14, 2010 at 11:32 AM, Carlos Diaz <839273@gmail.com> wrote:

...
The escape string isn't necesarily \033; it's determined by what the particular termcap/info entry for that terminal contains.

Bash uses ncurses functions to expand \e to the _correct_ terminal escape.

Signed-off-by: Carlos Diaz <839273@gmail.com>

How would this affect a particular bit of code in src/pacman/callback.c; can we do anything there to make sure we are doing it right? Or is \033 right in almost all situations anyway, and this patch is mainly just for pedantic purposes.

As long as you don't want to maintain compatibility with legacy terminals, it's pedantic. You'd have to ask yourself if being correct in this case is worth linking against ncurses.

Granted I haven't looked at it yet, so I might just end recommending that pacman shouldn't be using terminal escapes in the first place.

If you read the code in question you'll see why we use them and also why it isn't a big deal. -Dan

Xavier Chantry

5:27 p.m.

New subject: [pacman-dev] [PATCH 2/3] makepkg: use portable escape sequence for terminal escapes

On Thu, Oct 14, 2010 at 7:21 PM, Dan McGee wrote:

...

If you read the code in question you'll see why we use them and also why it isn't a big deal.

Are you nuts !? This is actually the most crucial part of pacman. We definitely need this code to be 100% portable on any systems and any terminals from the last decade.

Nezmer

4:49 p.m.

New subject: [pacman-dev] [PATCH 1/3] makepkg: fallback to legacy termcap capabilities

On Thu, Oct 14, 2010 at 12:02:45PM -0430, Carlos Diaz wrote:

...

In some systems, such as Freebsd, the base tput isn't linked against a terminfo capable ncurses.

This means that there's no portable way to set color, so instead of guessing, makepkg fallbacks on using legacy termcap output manipulation.

For reference, see terminfo(5) and termcap(5) for a comparison between the abilities.

The hardcoded list of terminals is taken from freebsdports/shells/tcshrc and tmux [1]. This prevents echoing gibberish in dumb and otherwise incapacitated terminals.

1: http://www.openbsd.org/cgi-bin/cvsweb/src/usr.bin/tmux/tty.c?rev=1.91;conten...

Signed-off-by: Carlos Diaz <839273@gmail.com> --- scripts/makepkg.sh.in | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index bdf63ef..3d29d26 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1660,21 +1660,30 @@ 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 - # prefer terminal safe colored and bold text when tput is supported if tput setaf 0 &>/dev/null; then + # tput linked against a compliant terminfo ncurses 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)" - else + elif [[ $TERM =~ ^(dtterm|rxvt|screen|xterm) ]]; then + # no terminfo, but don't fallback to ugly reverse text just yet ALL_OFF="\033[1;0m" BOLD="\033[1;1m" BLUE="${BOLD}\033[1;34m" GREEN="${BOLD}\033[1;32m" RED="${BOLD}\033[1;31m" YELLOW="${BOLD}\033[1;33m" + elif tput me &>/dev/null; then + # rely on termcap capabilities; no portable way of using color + ALL_OFF="$(tput me)" + BOLD="$(tput md)" + BLUE="$(tput mr)" # reverse + GREEN="$(tput us)" # underline + RED="$(tput so)" # standout + YELLOW="$(tput mh)" # half-bright fi fi readonly ALL_OFF BOLD BLUE GREEN RED YELLOW -- 1.7.3.1

This is already fixed in master!

Andres Perera

5:07 p.m.

On Thu, Oct 14, 2010 at 12:19 PM, Nezmer wrote:

...

This is already fixed in master!

Err, this was done against master 2010-10-13 23:42 de5f438aef1 And what as I read from [1], you're still using terminfo capability names and hardcoded shell escapes. 1: http://gitorious.org/pacman-bsd/pacman-bsd/blobs/master/scripts/makepkg.sh.i...

Nezmer

5:11 p.m.

New subject: [pacman-dev] [PATCH 1/3] makepkg: fallback to legacy termcap capabilities

On Thu, Oct 14, 2010 at 07:49:07PM +0300, Nezmer wrote:

...

On Thu, Oct 14, 2010 at 12:02:45PM -0430, Carlos Diaz wrote:

...
In some systems, such as Freebsd, the base tput isn't linked against a terminfo capable ncurses.

This means that there's no portable way to set color, so instead of guessing, makepkg fallbacks on using legacy termcap output manipulation.

For reference, see terminfo(5) and termcap(5) for a comparison between the abilities.

The hardcoded list of terminals is taken from freebsdports/shells/tcshrc and tmux [1]. This prevents echoing gibberish in dumb and otherwise incapacitated terminals.

1: http://www.openbsd.org/cgi-bin/cvsweb/src/usr.bin/tmux/tty.c?rev=1.91;conten...

Signed-off-by: Carlos Diaz <839273@gmail.com> --- scripts/makepkg.sh.in | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index bdf63ef..3d29d26 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1660,21 +1660,30 @@ 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 - # prefer terminal safe colored and bold text when tput is supported if tput setaf 0 &>/dev/null; then + # tput linked against a compliant terminfo ncurses 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)" - else + elif [[ $TERM =~ ^(dtterm|rxvt|screen|xterm) ]]; then + # no terminfo, but don't fallback to ugly reverse text just yet ALL_OFF="\033[1;0m" BOLD="\033[1;1m" BLUE="${BOLD}\033[1;34m" GREEN="${BOLD}\033[1;32m" RED="${BOLD}\033[1;31m" YELLOW="${BOLD}\033[1;33m" + elif tput me &>/dev/null; then + # rely on termcap capabilities; no portable way of using color + ALL_OFF="$(tput me)" + BOLD="$(tput md)" + BLUE="$(tput mr)" # reverse + GREEN="$(tput us)" # underline + RED="$(tput so)" # standout + YELLOW="$(tput mh)" # half-bright fi fi readonly ALL_OFF BOLD BLUE GREEN RED YELLOW -- 1.7.3.1

This is already fixed in master!

If you're running FreeBSD. The default $TERM in ttyv<int> is cons25(1) and it can handle the colours just fine. So I don't know how the hard-coded list is relevant. (1) I heard this vintage default will change soon.

Andres Perera

5:17 p.m.

On Thu, Oct 14, 2010 at 12:41 PM, Nezmer wrote:

...

If you're running FreeBSD. The default $TERM in ttyv<int> is cons25(1) and it can handle the colours just fine. So I don't know how the hard-coded list is relevant.

(1) I heard this vintage default will change soon.

There's really no stopping at that point; the default terminal in OpenBSD is "vt220" and it can handle colors, so can Linux's "linux" terminal. So it's screwing some terminals up vs. replicating ncurses in Bash -- since that's exactly what ncurses does, it looks at the TERM string and compares it to a list. Once this "vintage" changes, I guess makepkg is supposed to be updated, if the second option is any good. Are you proposing that it should check for all these terminals or just grant these bonuses to those who link against terminfo?

4953

Age (days ago)

4953

Last active (days ago)

List overview

Download

10 comments

5 participants

participants (5)

Andres Perera
Carlos Diaz
Dan McGee
Nezmer
Xavier Chantry