Re: [gentoo-dev] [PATCH v2 1/1] edo.eclass: add new eclass

[Florian Schmaus]

On 16/04/2022 20.14, Sam James wrote:
> Bug: https://bugs.gentoo.org/744880
> Signed-off-by: Sam James <sam@g.o>
> ---
>   eclass/edo.eclass | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 46 insertions(+)
>   create mode 100644 eclass/edo.eclass
>
> diff --git a/eclass/edo.eclass b/eclass/edo.eclass
> new file mode 100644
> index 000000000000..7b4ae04c43ab
> --- /dev/null
> +++ b/eclass/edo.eclass
> @@ -0,0 +1,46 @@
> +# Copyright 2022 Gentoo Authors
> +# Distributed under the terms of the GNU General Public License v2
> +
> +# @ECLASS: edo.class
> +# @MAINTAINER:
> +# QA Team <qa@g.o>
> +# @AUTHOR:
> +# Sam James <sam@g.o>
> +# @SUPPORTED_EAPIS: 7 8
> +# @BLURB: Convenience function to run commands verbosely and die on failure
> +# @DESCRIPTION:
> +# This eclass provides the 'edo' command, and an 'edob' variant for ebegin/eend,
> +# which dies (exits) on failure and logs the command used verbosely.
> +#
> +
> +case ${EAPI:-0} in
> +	7|8) ;;
> +	*) die "${ECLASS}: EAPI ${EAPI:-0} not supported" ;;
> +esac
> +
> +if [[ -z ${_EDO_ECLASS} ]] ; then
> + _EDO_CLASS=1

typo: s/_CLASS/_ECLASS/


> +
> +# @FUNCTION: edo
> +# @USAGE: <command> [<args>...]
> +# @DESCRIPTION:
> +# Executes 'command' with any given arguments and exits on failure unless
> +# called under 'nonfatal'.
> +edo() {
> +	elog "$@"
> +	"$@" || die -n "Failed to run command: $@ failed"
> +}
> +
> +# @FUNCTION: edob
> +# @USAGE: <command> [<args>...]
> +# @DESCRIPTION:
> +# Executes 'command' with ebegin & eend with any given arguments and exits
> +# on failure unless called under 'nonfatal'.

Maybe mention that 'command' should be "short". I do not know if we want 
or can provide a concrete value that determines shortness.


> +# Intended for single commands, otherwise regular ebegin/eend should be used.

I think it is obvious that it can be only used for single commands. If I 
am not mistaken, using multiple commands within ebegin/eend also means 
that those commands, or, at least all commands after the first, can not 
(or at least "should not") be printed/logged, as I assume that it would 
break eend()'s positioning.

With multiple commands (e.g., cmd1, cmd2, cmd3), I'd probably suggest to

edo cmd1
edo cmd2
edo cmd3

since I personally like to avoid the inflationary usage of ebegin/eend 
and instead recommend to use ebegin/eend only for a few long-running 
milestone commands in the ebuild (if any).

Hence I don't like that the description line above redirects the user to 
ebegin/eend. It can probably simply be dropped.


> +edob() {
> +	ebegin "Running $@"
> +	"$@"
> +	eend $? || die -n "$@ failed"
> +}
> +
> +fi

Like ionen, I also wondered if this could be an argument to edo, but I 
believe this would clutter edo's implementation with argument parsing 
and additional logic. Hence I believe having two separate functions, as 
in this patch, is the way to go.

Thanks Sam!

- Flow