Re: [gentoo-dev] [PATCH 8/8] virtualx.eclass: Simplify API into single virtx()

[Davide Pesavento <pesa@gentoo.org>]

On Sat, Nov 28, 2015 at 2:24 PM, Justin Lecher <jlec@gentoo.org> wrote:
> The new API runs all specified arguments to virtx() inside an XFVB,
> instead of defining VIRTUALX_COMMAND and running that in virtualmake.
>
> Xemake and Xeconf should be replaced by "virtx emake" and "virtx econf".
>
> Signed-off-by: Justin Lecher <jlec@gentoo.org>
> ---
>  eclass/virtualx.eclass | 42 ++++++++++++++++++++++++++++++++----------
>  1 file changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/eclass/virtualx.eclass b/eclass/virtualx.eclass
> index f9fa879..ca6a1f9 100644
> --- a/eclass/virtualx.eclass
> +++ b/eclass/virtualx.eclass
> @@ -90,22 +90,38 @@ esac
>  virtualmake() {
>         debug-print-function ${FUNCNAME} "$@"
>
> -       local i=0
> -       local retval=0
> -       local OLD_SANDBOX_ON="${SANDBOX_ON}"
> -       local XVFB XHOST XDISPLAY
> -       local xvfbargs="-screen 0 1280x1024x24"
> -       XVFB=$(type -p Xvfb) || die
> -       XHOST=$(type -p xhost) || die
> +       [[ ${EAPI} == [2345] ]] || die "${FUNCNAME} is unsupported in EAPI > 5, please use virtx"
>
>         # backcompat for maketype
>         if [[ -n ${maketype} ]]; then
> +               [[ ${EAPI} == [2345] ]] || die "maketype is banned in EAPI > 5"
>                 eqawarn "ebuild is exporting \$maketype=${maketype}"
>                 eqawarn "Ebuild should be migrated to use VIRTUALX_COMMAND=${maketype} instead."
>                 eqawarn "Setting VIRTUALX_COMMAND to \$maketype conveniently for now."
>                 VIRTUALX_COMMAND=${maketype}
>         fi
>
> +       virtx "${VIRTUALX_COMMAND}" "${@}"
> +}
> +
> +
> +# @FUNCTION: virtx
> +# @USAGE: <command> [comman arguments]

typo comman -> command

> +# @DESCRIPTION:
> +# Function which start new Xvfb session where the command gets executed.

I'd drop "Function which" from the description.

> +virtx() {
> +       debug-print-function ${FUNCNAME} "$@"
> +
> +       [[ $# -lt 1 ]] && die "${FUNCNAME} needs at least one argument"
> +
> +       local i=0
> +       local retval=0
> +       local OLD_SANDBOX_ON="${SANDBOX_ON}"
> +       local XVFB XHOST XDISPLAY
> +       local xvfbargs="-screen 0 1280x1024x24"
> +       XVFB=$(type -p Xvfb) || die
> +       XHOST=$(type -p xhost) || die
> +
>         debug-print "${FUNCNAME}: running Xvfb hack"
>         export XAUTHORITY=
>         # The following is derived from Mandrake's hack to allow
> @@ -155,10 +171,10 @@ virtualmake() {
>         # to kill Xvfb
>         debug-print "${FUNCNAME}: ${VIRTUALX_COMMAND} \"$@\""
>         if has "${EAPI}" 2 3; then
> -               ${VIRTUALX_COMMAND} "$@"
> +               "$@"
>                 retval=$?
>         else
> -               nonfatal ${VIRTUALX_COMMAND} "$@"
> +               nonfatal "$@"

Please take the opportunity to clean this up, possibly only in EAPI=6
if you don't want to risk breaking existing ebuilds. See bug 517976
for details.

>                 retval=$?
>         fi
>
> @@ -179,7 +195,7 @@ Xmake() {
>         debug-print-function ${FUNCNAME} "$@"
>
>         [[ ${EAPI} == [2345] ]] \
> -               || die "${FUNCNAME} is removed in EAPI > 5; use Xemake -j1 instead"
> +               || die "${FUNCNAME} is unsupported in EAPI > 5, please use 'virtx emake -j1 ....'"
>
>         eqawarn "you should not execute make directly"
>         eqawarn "rather execute Xemake -j1 if you have issues with parallel make"
> @@ -192,6 +208,9 @@ Xmake() {
>  Xemake() {
>         debug-print-function ${FUNCNAME} "$@"
>
> +       [[ ${EAPI} == [2345] ]] \
> +               || die "${FUNCNAME} is unsupported in EAPI > 5, please use 'virtx emake ....'"
> +
>         VIRTUALX_COMMAND="emake" virtualmake "$@"
>  }
>
> @@ -201,6 +220,9 @@ Xemake() {
>  Xeconf() {
>         debug-print-function ${FUNCNAME} "$@"
>
> +       [[ ${EAPI} == [2345] ]] \
> +               || die "${FUNCNAME} is unsupported in EAPI > 5, please use 'virtx econf ....'"
> +
>         VIRTUALX_COMMAND="econf" virtualmake "$@"
>  }
>
> --
> 2.6.3
>
>