[PATCH 1/3] makepkg: send messages to stdout rather than stderr

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/3] makepkg: send messages to stdout rather than stderr

Eli Schwartz-2
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 <[hidden email]>
---

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
--
2.18.0
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/3] message.sh: add modifications from output_format.sh

Eli Schwartz-2
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 <[hidden email]>
---

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
--
2.18.0
Reply | Threaded
Open this post in threaded view
|

[PATCH 3/3] Port scripts to use libmakepkg's messaging code.

Eli Schwartz-2
In reply to this post by Eli Schwartz-2
Remove all remnants of library/{output_format,term_colors}.sh

Signed-off-by: Eli Schwartz <[hidden email]>
---
 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
--
2.18.0
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] makepkg: send messages to stdout rather than stderr

Eli Schwartz-2
In reply to this post by Eli Schwartz-2
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. :(

--
Eli Schwartz
Bug Wrangler and Trusted User


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] makepkg: send messages to stdout rather than stderr

Allan McRae
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
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] makepkg: send messages to stdout rather than stderr

Eli Schwartz-2
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/

--
Eli Schwartz
Bug Wrangler and Trusted User


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] makepkg: send messages to stdout rather than stderr

Eli Schwartz-2
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.

--
Eli Schwartz
Bug Wrangler and Trusted User


signature.asc (849 bytes) Download Attachment