occasionally one will call an eclass's function without inheriting said eclass, either with a new ebuild or with an edit to an existing ebuild (for instance, moving away from eutils), and portage, either via the emerge or ebuild command, will not error out due to this. As its quite easy to miss a $PORTAGE_TMPDIR/$CATEGORY/${P}/temp/environment: line xxxx: function: command not found error, its equally easy to commit a broken ebuild or eclass to the tree. Reproducible: Always Steps to Reproduce: 1. edit sys-apps/zlib-1.2.11-r2's ebuild to call ver_cut 1-2 in src_prepare, without being EAPI 7 or inheriting eapi7-ver 2. run ebuild zlib-1.2.11-r2.ebuild clean prepare Actual Results: Appending /home/hanetzer/Projects/gentoo/gentoo to PORTDIR_OVERLAY... * zlib-1.2.11.tar.gz BLAKE2B SHA512 size ;-) ... [ ok ] >>> Unpacking source... >>> Unpacking zlib-1.2.11.tar.gz to /tmp/portage/sys-libs/zlib-1.2.11-r2/work >>> Source unpacked in /tmp/portage/sys-libs/zlib-1.2.11-r2/work >>> Preparing source in /tmp/portage/sys-libs/zlib-1.2.11-r2/work/zlib-1.2.11 ... * Applying zlib-1.2.11-fix-deflateParams-usage.patch ... [ ok ] * Applying zlib-1.2.11-minizip-drop-crypt-header.patch ... [ ok ] /tmp/portage/sys-libs/zlib-1.2.11-r2/temp/environment: line 2787: ver_cut: command not found * Running eautoreconf in '/tmp/portage/sys-libs/zlib-1.2.11-r2/work/zlib-1.2.11/contrib/minizip' ... * Running libtoolize --install --copy --force --automake ... [ ok ] * Running aclocal ... [ ok ] * Running autoconf --force ... [ ok ] * Running automake --add-missing --copy --foreign --force-missing ... [ ok ] * Running elibtoolize in: zlib-1.2.11/contrib/minizip/ * Applying portage/1.2.0 patch ... * Applying sed/1.5.6 patch ... * Applying as-needed/2.4.3 patch ... >>> Source prepared. Expected Results: Appending /home/hanetzer/Projects/gentoo/gentoo to PORTDIR_OVERLAY... * zlib-1.2.11.tar.gz BLAKE2B SHA512 size ;-) ... [ ok ] >>> Unpacking source... >>> Unpacking zlib-1.2.11.tar.gz to /tmp/portage/sys-libs/zlib-1.2.11-r2/work >>> Source unpacked in /tmp/portage/sys-libs/zlib-1.2.11-r2/work >>> Preparing source in /tmp/portage/sys-libs/zlib-1.2.11-r2/work/zlib-1.2.11 ... * Applying zlib-1.2.11-fix-deflateParams-usage.patch ... [ ok ] * Applying zlib-1.2.11-minizip-drop-crypt-header.patch ... [ ok ] /tmp/portage/sys-libs/zlib-1.2.11-r2/temp/environment: line 2787: ver_cut: command not found * ERROR: sys-libs/zlib-1.2.11-r2::gentoo failed (prepare phase): * (no error message) * * Call stack: * ebuild.sh, line 124: Called src_prepare * environment, line 2787: Called die * The specific snippet of code: * ver_cut 1-2 || die; * * If you need support, post the output of `emerge --info '=sys-libs/zlib-1.2.11-r2::gentoo'`, * the complete build log and the output of `emerge -pqv '=sys-libs/zlib-1.2.11-r2::gentoo'`. * The complete build log is located at '/var/log/portage/sys-libs:zlib-1.2.11-r2:20180807-012426.log'. * For convenience, a symlink to the build log is located at '/tmp/portage/sys-libs/zlib-1.2.11-r2/temp/build.log'. * The ebuild environment file is located at '/tmp/portage/sys-libs/zlib-1.2.11-r2/temp/environment'. * Working directory: '/tmp/portage/sys-libs/zlib-1.2.11-r2/work/zlib-1.2.11' * S: '/tmp/portage/sys-libs/zlib-1.2.11-r2/work/zlib-1.2.11' The above error occurs because I explicitly put || die after ver_cut 1-2 in the ebuild, but when someone is calling eclass code one expects it to die on its own if there is an issue (either syntactical or it just doesn't work properly) so adding || die to every function call in an ebuild/eclass is cumbersome and even then, it doesn't show why it died in this case (because the eclass the function lives in doesn't exist). A change to portage to fail when it calls a nonexistant function (in the same manner it dies when you call a non-existant external command) will greatly reduce the chance that a given ebuild or eclass submitted to the tree will be broken in some minor or major way.
At the very least, this should be able to be turned on via a portage FEATURES flag, and be stuck into the developer profile target.
I guess what you're suggesting is equivalent to 'set -e'? Wouldn't that require EAPI support, since every eclass and ebuild function would have to be well behaved for 'set -e'?
(In reply to Zac Medico from comment #2) > I guess what you're suggesting is equivalent to 'set -e'? Wouldn't that > require EAPI support, since every eclass and ebuild function would have to > be well behaved for 'set -e'? Honestly I think it may require a fundamental redoing of how portage discovers phases and such; stock ebuild which comes out of gentoo-syntax throws errors if you turn off set +e in those bin files you mentioned to me on irc.
Generally, all that matters with set -e is that anything that's supposed to return non-zero be guarded with somehow. For example, the code that discovers phases already works fine with set -e, since the declare -F call that's supposed to return non-zero is guarded with an 'if' statement. This is the code from ebuild.sh: DEFINED_PHASES= for _f in $_valid_phases ; do if declare -F $_f >/dev/null ; then _f=${_f#pkg_} DEFINED_PHASES+=" ${_f#src_}" fi done [[ -n $DEFINED_PHASES ]] || DEFINED_PHASES=-
(In reply to hanetzer from comment #0) > A change to portage to fail when it calls a nonexistant function (in the > same manner it dies when you call a non-existant external command) will > greatly reduce the chance that a given ebuild or eclass submitted to the > tree will be broken in some minor or major way. How does bash tell the difference between a non-existent external command, and a non-existent function? Since when does portage error out on the former if you don't explicitly "|| die"? ... When sourcing an ebuild, we set this: _PORTAGE_ORIG_PATH=${PATH} export PATH=/dev/null command_not_found_handle() { die "External commands disallowed while sourcing ebuild: ${*}" } This doesn't handle "non-existent external commands", just all of them no matter what. :) But it does demonstrate that it's very much possible to detect non-existent commands being run (whether they are external or functions is beside the point). It's possible to set this handle during non-sourcing runs of ebuild.sh too. But I suppose it needs to go in EAPI 9 if we do. The alternative to making this an EAPI 9 change is to make it an ewarn. You could still write and commit a broken ebuild or eclass, but it would be much, much, much easier to notice that you are doing so. (In reply to Zac Medico from comment #2) > I guess what you're suggesting is equivalent to 'set -e'? Wouldn't that > require EAPI support, since every eclass and ebuild function would have to > be well behaved for 'set -e'? set -e would be very different in behavior. It would cause an ebuild to fail if *any* command so much as exited 1, even when it exists and successfully fork+exec'd. It would be the equivalent of automatic "|| die" by default for bash pipelines. It would be interesting to do so but apparently there is not much taste for it.
