commit 57ba9ec58adf39b441325855d359dd75951b10c9 Author: Gregory M. Turner Date: Mon Oct 22 17:47:50 2012 -0700 Fix bugs in __helpers_die() and nonfatal This implements the following changes: bin/isolated-functions.sh: __helpers_die(): rewire to only use die() when actually die()ing; use annotated ewarn instead of unceremoneously dumping to stderr when deciding not to die after all bin/isolated-functions.sh: die(): remove code preventing death from actually occuring when [[ ${PORTAGE_NONFATAL} -eq 1 ]] bin/isolated-functions.sh: __assert_sigpipe_ok(): use __helpers_die instead of die; also, rename to __helpers_assert_sigpipe_ok bin/helper-functinos.sh: __redirect_alloc_fd(): use __helpers_die instead of die bin/isolated-functions.sh: nonfatal: don't die when nonfatal command is issued without arguments; instead emit an error and return 1 bin/phase-helpers.sh: unpack_tar(): use the new name __helpers_assert_sigpipe_ok instead of __assert_sigpipe_ok bin/phase-helpers.sh: unpack(): likewise bin/save-ebuild-env.sh: __save_ebuild_env(): likewise doc/package/ebuild/eapi/4.docbook: Clarify some things about how nonfatal, die, assert, and helper functions relate to each other. In particular, make it clear that nonfatal is not designed to be applied to constructs other than helper function invocations. Give an example of how to use nonfatal. Document explicitly that the die() and assert() helpers are not affected by nonfatal. These changes are an attempt to address the issues identified in this thread: http://thread.gmane.org/gmane.linux.gentoo.devel/80780 In addition to the issues and solutions agreed upon in that thread, the patch makes one additional "change": it allows assert() to continue to die() regardless of PORTAGE_NONFATAL. Since assert() was already calling die(), no change to the source was neccesary to achieve this; but it is worth noting, because this is nevertheless a novel semantic change I have decided to make without consulting anybody. I am pretty darn confident that it's the right change to make, given the consensus from the aformentioned thread, however, I suppose I'm not 100% sure and some SME should approve this change -- it may require some kind of update to the PMS. (a bit off topic: IMHO, the PMS content pertaining to this entire matter could really use some editing, but I'll leave that to others to fix, since I don't very well understand the process by which PMS changes are made. If someone would like to clue me in on that, drop me a line at the address to be found at http://gmturner.com/contact; I'd be happy to take a crack at it, now that I feel like I understand the intent behind the existing language.) Signed-off-by: Gregory M. Turner diff --git a/bin/helper-functions.sh b/bin/helper-functions.sh index 65f41f6..d9b6f56 100644 --- a/bin/helper-functions.sh +++ b/bin/helper-functions.sh @@ -82,7 +82,7 @@ __redirect_alloc_fd() { if [[ ! -e /dev/fd/${fd} ]] && [[ ! -L /dev/fd/${fd} ]] ; then eval "exec ${fd}${redir}'${file}'" && break fi - [[ ${fd} -gt 1024 ]] && die "__redirect_alloc_fd failed" + [[ ${fd} -gt 1024 ]] && __helpers_die "__redirect_alloc_fd failed" : $(( ++fd )) done : $(( ${var} = fd )) diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh index 11efdb7..eb7b356 100644 --- a/bin/isolated-functions.sh +++ b/bin/isolated-functions.sh @@ -17,7 +17,7 @@ assert() { done } -__assert_sigpipe_ok() { +__helpers_assert_sigpipe_ok() { # When extracting a tar file like this: # # bzip2 -dc foo.tar.bz2 | tar xof - @@ -36,11 +36,11 @@ __assert_sigpipe_ok() { local x pipestatus=${PIPESTATUS[*]} for x in $pipestatus ; do # Allow SIGPIPE through (128 + 13) - [[ $x -ne 0 && $x -ne ${PORTAGE_SIGPIPE_STATUS:-141} ]] && die "$@" + [[ $x -ne 0 && $x -ne ${PORTAGE_SIGPIPE_STATUS:-141} ]] && __helpers_die "$@" done # Require normal success for the last process (tar). - [[ $x -eq 0 ]] || die "$@" + [[ $x -eq 0 ]] || __helpers_die "$@" } shopt -s extdebug @@ -92,26 +92,33 @@ nonfatal() { die "$FUNCNAME() not supported in this EAPI" fi if [[ $# -lt 1 ]]; then - die "$FUNCNAME(): Missing argument" + # Well they said nonfatal and forgot to finish their sentence. + # Something ain't right, to be sure, but die()ing here seems a bit + # pendantic and mean-spirited :) + local phase_str= + [[ -n ${EBUILD_PHASE} ]] && phase_str=" (${EBUILD_PHASE} phase)" + eerror "ERROR: ${CATEGORY}/${PF}${phase_str}:" + eerror " ${FUNCNAME}: Missing argument" + eerror + return 1 fi PORTAGE_NONFATAL=1 "$@" } __helpers_die() { - if ___eapi_helpers_can_die; then + if ___eapi_helpers_can_die && [[ "${PORTAGE_NONFATAL:-0}" != 1 ]]; then die "$@" else - echo -e "$@" >&2 + local phase_str= + [[ -n ${EBUILD_PHASE} ]] && phase_str=" (${EBUILD_PHASE} phase)" + ewarn "WARNING: ${CATEGORY}/${PF}${phase_str}:" + ewarn " ${FUNCNAME}: ${*:-(no error message)}" + ewarn fi } die() { - if [[ $PORTAGE_NONFATAL -eq 1 ]]; then - echo -e " $WARN*$NORMAL ${FUNCNAME[1]}: WARNING: $@" >&2 - return 1 - fi - set +e if [ -n "${QA_INTERCEPTORS}" ] ; then # die was called from inside inherit. We need to clean up diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh index be0f641..5318cfa 100644 --- a/bin/phase-helpers.sh +++ b/bin/phase-helpers.sh @@ -299,7 +299,7 @@ unpack() { __unpack_tar() { if [ "${y}" == "tar" ]; then $1 -c -- "$srcdir$x" | tar xof - - __assert_sigpipe_ok "$myfail" + __helpers_assert_sigpipe_ok "$myfail" else local cwd_dest=${x##*/} cwd_dest=${cwd_dest%.*} @@ -317,7 +317,7 @@ unpack() { ;; tbz|tbz2) ${PORTAGE_BUNZIP2_COMMAND:-${PORTAGE_BZIP2_COMMAND} -d} -c -- "$srcdir$x" | tar xof - - __assert_sigpipe_ok "$myfail" + __helpers_assert_sigpipe_ok "$myfail" ;; ZIP|zip|jar) # unzip will interactively prompt under some error conditions, diff --git a/bin/save-ebuild-env.sh b/bin/save-ebuild-env.sh index 69d4525..0c599ea 100644 --- a/bin/save-ebuild-env.sh +++ b/bin/save-ebuild-env.sh @@ -46,7 +46,7 @@ __save_ebuild_env() { done unset x - unset -f assert __assert_sigpipe_ok \ + unset -f assert __helpers_assert_sigpipe_ok \ __dump_trace die \ __quiet_mode __vecho __elog_base eqawarn elog \ einfo einfon ewarn eerror ebegin __eend eend KV_major \ diff --git a/doc/package/ebuild/eapi/4.docbook b/doc/package/ebuild/eapi/4.docbook index 8dd0f14..0b49635 100644 --- a/doc/package/ebuild/eapi/4.docbook +++ b/doc/package/ebuild/eapi/4.docbook @@ -11,9 +11,14 @@ The dohard and dosed helpers from previous EAPIs are no longer available.
All helpers die on failure -All helpers now die automatically whenever some sort of error occurs. -Helper calls may be prefixed with the 'nonfatal' helper in order -to prevent errors from being fatal. +All helpers now die automatically whenever any meaningful error occurs. +The 'nonfatal' helper may be used to effect the error-handling behavior +of EAPIs 0-3, in which recoverable errors emit a warning and return +nonzero exit-codes to invokees. It is used by prepending 'nonfatal' to any helper +invocation; for example, by replacing 'dodoc /foo.txt' with 'nonfatal dodoc /foo.txt'. +The 'nonfatal' helper should not be applied to +statements other than helper invocations, and has no effect on the 'die' +and 'assert' helpers.