This behavior is confusing, since it means absolutely everything goes to stderr and makepkg itself is a quiet program that produces no expected output???
The only situation where messages should go to stderr rather than stdout, is with --geninteg which is meant to return the checksums on stdout (but we don't want to totally get rid of status messages when redirecting the results elsewhere, or, worse, redirect status messages to a PKGBUILD). For this specific case, redirect message output to stderr in the --geninteg callers directly.
Implements FS#17173
Signed-off-by: Eli Schwartz eschwartz@archlinux.org ---
It took us 9 years, but we got there...
scripts/libmakepkg/integrity/generate_checksum.sh.in | 2 +- scripts/libmakepkg/util/message.sh.in | 6 +++--- scripts/makepkg.sh.in | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/scripts/libmakepkg/integrity/generate_checksum.sh.in b/scripts/libmakepkg/integrity/generate_checksum.sh.in index eb9b74fc..8edc48d3 100644 --- a/scripts/libmakepkg/integrity/generate_checksum.sh.in +++ b/scripts/libmakepkg/integrity/generate_checksum.sh.in @@ -78,7 +78,7 @@ generate_one_checksum() { }
generate_checksums() { - msg "$(gettext "Generating checksums for source files...")" + msg "$(gettext "Generating checksums for source files...")" >&2
local integlist if (( $# == 0 )); then diff --git a/scripts/libmakepkg/util/message.sh.in b/scripts/libmakepkg/util/message.sh.in index 0746b677..36790c26 100644 --- a/scripts/libmakepkg/util/message.sh.in +++ b/scripts/libmakepkg/util/message.sh.in @@ -45,17 +45,17 @@ colorize() {
plain() { local mesg=$1; shift - printf "${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 + printf "${BOLD} ${mesg}${ALL_OFF}\n" "$@" }
msg() { local mesg=$1; shift - printf "${GREEN}==>${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 + printf "${GREEN}==>${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" }
msg2() { local mesg=$1; shift - printf "${BLUE} ->${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 + printf "${BLUE} ->${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" }
warning() { diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 32423262..650b8494 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1469,7 +1469,7 @@ if (( GENINTEG )); then mkdir -p "$srcdir" chmod a-s "$srcdir" cd_safe "$srcdir" - download_sources novcs allarch + download_sources novcs allarch >&2 generate_checksums exit $E_OK fi
In the spirit of making libmakepkg more useful as a library, and, critically, *using* that library for additional pacman scripts, we should include all of output_format.sh and term_colors.sh directly in libmakepkg and hopefully stop having to embed additional copies in e.g. repo-add via m4 macros.
Signed-off-by: Eli Schwartz eschwartz@archlinux.org ---
This has been sitting in my queue for a bit, but I cleaned it up with the preceding and following patches. It's a double win, because we can delete duplicated code from our own scripts as well as rely on this for pacman-contrib. At least now that we consistently use stdout in both.
scripts/libmakepkg/util/message.sh.in | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/scripts/libmakepkg/util/message.sh.in b/scripts/libmakepkg/util/message.sh.in index 36790c26..097da2b2 100644 --- a/scripts/libmakepkg/util/message.sh.in +++ b/scripts/libmakepkg/util/message.sh.in @@ -44,20 +44,28 @@ colorize() { }
plain() { + (( QUIET )) && return local mesg=$1; shift printf "${BOLD} ${mesg}${ALL_OFF}\n" "$@" }
msg() { + (( QUIET )) && return local mesg=$1; shift printf "${GREEN}==>${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" }
msg2() { + (( QUIET )) && return local mesg=$1; shift printf "${BLUE} ->${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" }
+ask() { + local mesg=$1; shift + printf "${BLUE}::${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}" "$@" +} + warning() { local mesg=$1; shift printf "${YELLOW}==> $(gettext "WARNING:")${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2
Remove all remnants of library/{output_format,term_colors}.sh
Signed-off-by: Eli Schwartz eschwartz@archlinux.org --- scripts/Makefile.am | 20 +++++--------------- scripts/library/README | 10 ---------- scripts/library/output_format.sh | 32 -------------------------------- scripts/library/term_colors.sh | 21 --------------------- scripts/pacman-db-upgrade.sh.in | 12 ++++++++---- scripts/pacman-key.sh.in | 12 ++++++++---- scripts/pkgdelta.sh.in | 12 ++++++++---- scripts/po/POTFILES.in | 2 -- scripts/repo-add.sh.in | 14 +++++++++++--- 9 files changed, 40 insertions(+), 95 deletions(-) delete mode 100644 scripts/library/output_format.sh delete mode 100644 scripts/library/term_colors.sh
diff --git a/scripts/Makefile.am b/scripts/Makefile.am index f759e149..66f8a7c9 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -35,10 +35,8 @@ EXTRA_DIST = \ $(LIBMAKEPKG_DIST)
LIBRARY = \ - library/output_format.sh \ library/human_to_size.sh \ - library/size_to_human.sh \ - library/term_colors.sh + library/size_to_human.sh
libmakepkgdir = $(datarootdir)/makepkg
@@ -206,21 +204,13 @@ makepkg-template: \ $(AM_V_GEN)$(edit) $< > $@ $(AM_V_at)chmod +x,a-w $@
-pacman-db-upgrade: \ - $(srcdir)/pacman-db-upgrade.sh.in \ - $(srcdir)/library/output_format.sh +pacman-db-upgrade: $(srcdir)/pacman-db-upgrade.sh.in
-pacman-key: \ - $(srcdir)/pacman-key.sh.in \ - $(srcdir)/library/output_format.sh +pacman-key: $(srcdir)/pacman-key.sh.in
-pkgdelta: \ - $(srcdir)/pkgdelta.sh.in \ - $(srcdir)/library/output_format.sh +pkgdelta: $(srcdir)/pkgdelta.sh.in
-repo-add: \ - $(srcdir)/repo-add.sh.in \ - $(srcdir)/library/output_format.sh +repo-add: $(srcdir)/repo-add.sh.in
repo-remove: $(srcdir)/repo-add.sh.in $(AM_V_at)$(RM) repo-remove diff --git a/scripts/library/README b/scripts/library/README index a9d15f1e..2b3a97bc 100644 --- a/scripts/library/README +++ b/scripts/library/README @@ -1,13 +1,6 @@ This directory contains code snippets that can be reused by multiple scripts. A brief description of each file follows.
-output_format.sh: -Provides basic output formatting functions with levels 'plain', 'msg', -'msg2', 'warning' and 'error'. The 'msg' amd 'msg2' functions print to -stdout and can be silenced by defining 'QUIET'. The 'warning' and 'error' -functions print to stderr with the appropriate prefix added to the -message. - human_to_size.sh: A function to convert human readable sizes (such as "5.3 GiB") to raw byte equivalents. base10 and base2 suffixes are supported, case sensitively. If @@ -19,6 +12,3 @@ as mawk or busybox awk. size_to_human.sh: The reverse of human_to_size, this function takes an integer byte size and prints its in human readable format, with SI prefixes (e.g. MiB, TiB). - -term_colors.sh: -Contains some common color settings for output_format.sh. diff --git a/scripts/library/output_format.sh b/scripts/library/output_format.sh deleted file mode 100644 index 9f02c00b..00000000 --- a/scripts/library/output_format.sh +++ /dev/null @@ -1,32 +0,0 @@ -plain() { - (( QUIET )) && return - local mesg=$1; shift - printf "${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&1 -} - -msg() { - (( QUIET )) && return - local mesg=$1; shift - printf "${GREEN}==>${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&1 -} - -msg2() { - (( QUIET )) && return - local mesg=$1; shift - printf "${BLUE} ->${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&1 -} - -ask() { - local mesg=$1; shift - printf "${BLUE}::${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}" "$@" >&1 -} - -warning() { - local mesg=$1; shift - printf "${YELLOW}==> $(gettext "WARNING:")${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 -} - -error() { - local mesg=$1; shift - printf "${RED}==> $(gettext "ERROR:")${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 -} diff --git a/scripts/library/term_colors.sh b/scripts/library/term_colors.sh deleted file mode 100644 index a675247c..00000000 --- a/scripts/library/term_colors.sh +++ /dev/null @@ -1,21 +0,0 @@ -# check if messages are to be printed using color -unset ALL_OFF BOLD BLUE GREEN RED YELLOW -if [[ -t 2 && ! $USE_COLOR = "n" ]]; then - # prefer terminal safe colored and bold text when tput is supported - if tput setaf 0 &>/dev/null; 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)" - else - 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" - fi -fi -readonly ALL_OFF BOLD BLUE GREEN RED YELLOW diff --git a/scripts/pacman-db-upgrade.sh.in b/scripts/pacman-db-upgrade.sh.in index e9e995bd..73b1b225 100644 --- a/scripts/pacman-db-upgrade.sh.in +++ b/scripts/pacman-db-upgrade.sh.in @@ -28,11 +28,10 @@ export TEXTDOMAINDIR='@localedir@'
declare -r myver='@PACKAGE_VERSION@'
-m4_include(library/output_format.sh) - LIBRARY=${LIBRARY:-'@libmakepkgdir@'}
-# Import parseopts.sh +# Import libmakepkg +source "$LIBRARY"/util/message.sh source "$LIBRARY"/util/parseopts.sh
usage() { @@ -112,7 +111,12 @@ conffile=${conffile:-@sysconfdir@/pacman.conf} [[ -z $pacroot ]] && pacroot=$(pacman-conf --config="$conffile" rootdir) [[ -z $dbroot ]] && dbroot=$(pacman-conf --config="$conffile" --rootdir="$pacroot" dbpath)
-m4_include(library/term_colors.sh) +# check if messages are to be printed using color +if [[ -t 2 && $USE_COLOR != "n" ]]; then + colorize +else + unset ALL_OFF BOLD BLUE GREEN RED YELLOW +fi
if [[ ! -d $dbroot ]]; then die "$(gettext "%s does not exist or is not a directory.")" "$dbroot" diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 5db4ea7a..bc32c5eb 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -28,7 +28,8 @@ declare -r myver="@PACKAGE_VERSION@"
LIBRARY=${LIBRARY:-'@libmakepkgdir@'}
-# Import parseopts.sh +# Import libmakepkg +source "$LIBRARY"/util/message.sh source "$LIBRARY"/util/parseopts.sh
# Options @@ -51,8 +52,6 @@ UPDATEDB=0 USE_COLOR='y' VERIFY=0
-m4_include(library/output_format.sh) - usage() { printf "pacman-key (pacman) %s\n" ${myver} echo @@ -562,7 +561,12 @@ while (( $# )); do shift done
-m4_include(library/term_colors.sh) +# check if messages are to be printed using color +if [[ -t 2 && $USE_COLOR != "n" ]]; then + colorize +else + unset ALL_OFF BOLD BLUE GREEN RED YELLOW +fi
if ! type -p gpg >/dev/null; then error "$(gettext "Cannot find the %s binary required for all %s operations.")" "gpg" "pacman-key" diff --git a/scripts/pkgdelta.sh.in b/scripts/pkgdelta.sh.in index f9b108da..cedb82f1 100644 --- a/scripts/pkgdelta.sh.in +++ b/scripts/pkgdelta.sh.in @@ -30,7 +30,8 @@ declare -r myver='@PACKAGE_VERSION@'
LIBRARY=${LIBRARY:-'@libmakepkgdir@'}
-# Import parseopts.sh +# Import libmakepkg +source "$LIBRARY"/util/message.sh source "$LIBRARY"/util/parseopts.sh
# Options @@ -47,8 +48,6 @@ max_delta_size=70 # ensure we have a sane umask set umask 0022
-m4_include(library/output_format.sh) - # print usage instructions usage() { printf "pkgdelta (pacman) %s\n" "${myver}" @@ -207,7 +206,12 @@ while :; do shift done
-m4_include(library/term_colors.sh) +# check if messages are to be printed using color +if [[ -t 2 && $USE_COLOR != "n" ]]; then + colorize +else + unset ALL_OFF BOLD BLUE GREEN RED YELLOW +fi
if (( $# != 2 )); then usage diff --git a/scripts/po/POTFILES.in b/scripts/po/POTFILES.in index 46d2af7f..37afc3b2 100644 --- a/scripts/po/POTFILES.in +++ b/scripts/po/POTFILES.in @@ -67,6 +67,4 @@ scripts/libmakepkg/util/pkgbuild.sh.in scripts/libmakepkg/util/source.sh.in scripts/libmakepkg/util/util.sh.in scripts/library/human_to_size.sh -scripts/library/output_format.sh scripts/library/size_to_human.sh -scripts/library/term_colors.sh diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index e80e1278..a21f8622 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -28,6 +28,8 @@ export TEXTDOMAINDIR='@localedir@' declare -r myver='@PACKAGE_VERSION@' declare -r confdir='@sysconfdir@'
+LIBRARY=${LIBRARY:-'@libmakepkgdir@'} + QUIET=0 DELTA=0 ONLYADDNEW=0 @@ -42,11 +44,12 @@ LOCKFILE= CLEAN_LOCK=0 USE_COLOR='y'
+# Import libmakepkg +source "$LIBRARY"/util/message.sh + # ensure we have a sane umask set umask 0022
-m4_include(library/output_format.sh) - # print usage instructions usage() { cmd=${0##*/} @@ -772,7 +775,12 @@ while (( $# )); do shift done
-m4_include(library/term_colors.sh) +# check if messages are to be printed using color +if [[ -t 2 && $USE_COLOR != "n" ]]; then + colorize +else + unset ALL_OFF BOLD BLUE GREEN RED YELLOW +fi
REPO_DB_FILE=${args[0]} if [[ -z $REPO_DB_FILE ]]; then
On 6/28/18 1:19 PM, Eli Schwartz wrote:
This behavior is confusing, since it means absolutely everything goes to stderr and makepkg itself is a quiet program that produces no expected output???
The only situation where messages should go to stderr rather than stdout, is with --geninteg which is meant to return the checksums on stdout (but we don't want to totally get rid of status messages when redirecting the results elsewhere, or, worse, redirect status messages to a PKGBUILD). For this specific case, redirect message output to stderr in the --geninteg callers directly.
Implements FS#17173
Don't use this.
Actually the pkgver() function saves the stdout of run_function_safe to a variable, and this patch would ensure the variable contains some decidedly not-pkgver content. :(
On 14/08/18 11:25, Eli Schwartz wrote:
On 6/28/18 1:19 PM, Eli Schwartz wrote:
This behavior is confusing, since it means absolutely everything goes to stderr and makepkg itself is a quiet program that produces no expected output???
The only situation where messages should go to stderr rather than stdout, is with --geninteg which is meant to return the checksums on stdout (but we don't want to totally get rid of status messages when redirecting the results elsewhere, or, worse, redirect status messages to a PKGBUILD). For this specific case, redirect message output to stderr in the --geninteg callers directly.
Implements FS#17173
Don't use this.
Actually the pkgver() function saves the stdout of run_function_safe to a variable, and this patch would ensure the variable contains some decidedly not-pkgver content. :(
Have not had time to look into this, but is there an easy/obvious way to adjust pkgver() capturing to stop this? Do we need to special case run_function?
A
On 8/29/18 12:37 AM, Allan McRae wrote:
On 14/08/18 11:25, Eli Schwartz wrote:
On 6/28/18 1:19 PM, Eli Schwartz wrote:
This behavior is confusing, since it means absolutely everything goes to stderr and makepkg itself is a quiet program that produces no expected output???
The only situation where messages should go to stderr rather than stdout, is with --geninteg which is meant to return the checksums on stdout (but we don't want to totally get rid of status messages when redirecting the results elsewhere, or, worse, redirect status messages to a PKGBUILD). For this specific case, redirect message output to stderr in the --geninteg callers directly.
Implements FS#17173
Don't use this.
Actually the pkgver() function saves the stdout of run_function_safe to a variable, and this patch would ensure the variable contains some decidedly not-pkgver content. :(
Have not had time to look into this, but is there an easy/obvious way to adjust pkgver() capturing to stop this? Do we need to special case run_function?
I was actually thinking, we should maybe special-case run_function to not emit this when run in a subshell.
Subshells aren't something we usually do, and when we do, it's probably to capture output, as in fact it is here.
So, we could then handle this surrounding the subshell, the same way I did for https://patchwork.archlinux.org/patch/736/
On 8/29/18 12:54 AM, Eli Schwartz wrote:
On 8/29/18 12:37 AM, Allan McRae wrote:
On 14/08/18 11:25, Eli Schwartz wrote:
On 6/28/18 1:19 PM, Eli Schwartz wrote:
This behavior is confusing, since it means absolutely everything goes to stderr and makepkg itself is a quiet program that produces no expected output???
The only situation where messages should go to stderr rather than stdout, is with --geninteg which is meant to return the checksums on stdout (but we don't want to totally get rid of status messages when redirecting the results elsewhere, or, worse, redirect status messages to a PKGBUILD). For this specific case, redirect message output to stderr in the --geninteg callers directly.
Implements FS#17173
Don't use this.
Actually the pkgver() function saves the stdout of run_function_safe to a variable, and this patch would ensure the variable contains some decidedly not-pkgver content. :(
Have not had time to look into this, but is there an easy/obvious way to adjust pkgver() capturing to stop this? Do we need to special case run_function?
I was actually thinking, we should maybe special-case run_function to not emit this when run in a subshell.
Subshells aren't something we usually do, and when we do, it's probably to capture output, as in fact it is here.
So, we could then handle this surrounding the subshell, the same way I did for https://patchwork.archlinux.org/patch/736/
Adding https://lists.archlinux.org/pipermail/pacman-dev/2018-August/022793.html on top of my branch with this patchset, makes pkgver() work again while sending messages to stdout.
So this patchset is now back on the table.
pacman-dev@lists.archlinux.org