[gentoo-dev] [RFC/PATCH] A cleaner API for virtualx.eclass

[gentoo-dev] [RFC/PATCH] A cleaner API for virtualx.eclass

[Michał Górny]

Hello, fellow developers,

The current virtualx.eclass API is a bit insane. It seems a bit like
stacking of a few next APIs, mostly designed to quickly run 'make
check', then extended to general functions.

For example running a function 'run_tests' with parameter '--foo' would
look like:

	VIRTUALX_COMMAND=run_tests virtualmake --foo

which is really awful, considering that '--foo' is a parameter to
'run_tests' and not virtualmake.

My patches introduce a single wrapper with argv-as-parameter syntax.
That is, the fore-mentioned example would look like:

	virtualx run_tests --foo

Depending on the maintainer decisions and your feedback, I believe that
even all the X* short-hand functions could be deprecated. They are a bit
shorter:

	Xemake check

vs:

	virtualx emake check

but I don't think that's much of a difference.

What are your thoughts?

[gentoo-dev] [PATCH virtualx.eclass 1/5] Introduce a cleaner alternative to VIRTUALX_COMMAND="" virtualmake.

[Michał Górny]

Let's get this straight:

	VIRTUALX_COMMAND="foo" virtualmake --bar --baz

is just ugly. Instead, introduce a function which can be used as:

	virtualx foo --bar -baz
---
 gx86/eclass/virtualx.eclass | 45 ++++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/gx86/eclass/virtualx.eclass b/gx86/eclass/virtualx.eclass
index 0621b18..47116fd 100644
--- a/gx86/eclass/virtualx.eclass
+++ b/gx86/eclass/virtualx.eclass
@@ -1,4 +1,4 @@
-# Copyright 1999-2012 Gentoo Foundation
+# Copyright 1999-2013 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 # $Header: /var/cvsroot/gentoo-x86/eclass/virtualx.eclass,v 1.43 2012/10/03 22:47:12 chithanh Exp $
 
@@ -69,11 +69,12 @@ case ${VIRTUALX_REQUIRED} in
 		;;
 esac
 
-# @FUNCTION: virtualmake
+# @FUNCTION: virtualx
+# @USAGE: <argv>...
 # @DESCRIPTION:
-# Function which attach to running X session or start new Xvfb session
-# where the VIRTUALX_COMMAND variable content gets executed.
-virtualmake() {
+# Attach to a running X session or start a new Xvfb session, then run
+# the command passed as arguments.
+virtualx() {
 	debug-print-function ${FUNCNAME} "$@"
 
 	local i=0
@@ -83,14 +84,6 @@ virtualmake() {
 	local XHOST=$(type -p xhost)
 	local xvfbargs="-screen 0 1280x1024x24"
 
-	# backcompat for maketype
-	if [[ -n ${maketype} ]]; then
-		ewarn "QA: ebuild is exporting \$maketype=${maketype}"
-		ewarn "QA: Ebuild should be migrated to use VIRTUALX_COMMAND=${maketype} instead."
-		ewarn "QA: Setting VIRTUALX_COMMAND to \$maketype conveniently for now."
-		VIRTUALX_COMMAND=${maketype}
-	fi
-
 	# If $DISPLAY is not set, or xhost cannot connect to an X
 	# display, then do the Xvfb hack.
 	if [[ -n ${XVFB} && -n ${XHOST} ]] && \
@@ -145,10 +138,10 @@ virtualmake() {
 		# to kill Xvfb
 		debug-print "${FUNCNAME}: ${VIRTUALX_COMMAND} \"$@\""
 		if has "${EAPI}" 2 3; then
-			${VIRTUALX_COMMAND} "$@"
+			"$@"
 			retval=$?
 		else
-			nonfatal ${VIRTUALX_COMMAND} "$@"
+			nonfatal "$@"
 			retval=$?
 		fi
 
@@ -158,16 +151,34 @@ virtualmake() {
 		debug-print "${FUNCNAME}: attaching to running X display"
 		# Normal make if we can connect to an X display
 		debug-print "${FUNCNAME}: ${VIRTUALX_COMMAND} \"$@\""
-		${VIRTUALX_COMMAND} "$@"
+		"$@"
 		retval=$?
 	fi
 
 	# die if our command failed
-	[[ ${retval} -ne 0 ]] && die "${FUNCNAME}: the ${VIRTUALX_COMMAND} failed."
+	[[ ${retval} -ne 0 ]] && die "${FUNCNAME}: ${1} failed."
 
 	return 0 # always return 0, it can be altered by failed kill for Xvfb
 }
 
+# @FUNCTION: virtualmake
+# @DESCRIPTION:
+# Function which attach to running X session or start new Xvfb session
+# where the VIRTUALX_COMMAND variable content gets executed.
+virtualmake() {
+	debug-print-function ${FUNCNAME} "$@"
+
+	# backcompat for maketype
+	if [[ -n ${maketype} ]]; then
+		ewarn "QA: ebuild is exporting \$maketype=${maketype}"
+		ewarn "QA: Ebuild should be migrated to use VIRTUALX_COMMAND=${maketype} instead."
+		ewarn "QA: Setting VIRTUALX_COMMAND to \$maketype conveniently for now."
+		VIRTUALX_COMMAND=${maketype}
+	fi
+
+	virtualx ${VIRTUALX_COMMAND} "$@"
+}
+
 # @FUNCTION: Xmake
 # @DESCRIPTION:
 # Same as "make", but set up the Xvfb hack if needed.
-- 
1.8.1.2

[gentoo-dev] [PATCH virtualx.eclass 2/5] Use eqawarn from eutils.eclass.

[Michał Górny]

Instead of ewarn "QA: ...
---
 gx86/eclass/virtualx.eclass | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/gx86/eclass/virtualx.eclass b/gx86/eclass/virtualx.eclass
index 47116fd..0da3066 100644
--- a/gx86/eclass/virtualx.eclass
+++ b/gx86/eclass/virtualx.eclass
@@ -9,6 +9,8 @@
 # Original author: Martin Schlemmer <azarah@gentoo.org>
 # @BLURB: This eclass can be used for packages that needs a working X environment to build.
 
+inherit eutils
+
 # @ECLASS-VARIABLE: VIRTUALX_REQUIRED
 # @DESCRIPTION:
 # Variable specifying the dependency on xorg-server and xhost.
@@ -46,15 +48,15 @@ case ${VIRTUALX_REQUIRED} in
 		;;
 	optional|tests)
 		# deprecated section YAY.
-		ewarn "QA: VIRTUALX_REQUIRED=optional and VIRTUALX_REQUIRED=tests are deprecated."
-		ewarn "QA: You can drop the variable definition completely from ebuild,"
-		ewarn "QA: because it is default behaviour."
+		eqawarn "VIRTUALX_REQUIRED=optional and VIRTUALX_REQUIRED=tests are deprecated."
+		eqawarn "You can drop the variable definition completely from ebuild,"
+		eqawarn "because it is default behaviour."
 
 		if [[ -n ${VIRTUALX_USE} ]]; then
 			# so they like to specify the useflag
-			ewarn "QA: VIRTUALX_USE variable is deprecated."
-			ewarn "QA: Please read eclass manpage to find out how to use VIRTUALX_REQUIRED"
-			ewarn "QA: to achieve the same behaviour."
+			eqawarn "VIRTUALX_USE variable is deprecated."
+			eqawarn "Please read eclass manpage to find out how to use VIRTUALX_REQUIRED"
+			eqawarn "to achieve the same behaviour."
 		fi
 
 		[[ -z ${VIRTUALX_USE} ]] && VIRTUALX_USE="test"
@@ -170,9 +172,9 @@ virtualmake() {
 
 	# backcompat for maketype
 	if [[ -n ${maketype} ]]; then
-		ewarn "QA: ebuild is exporting \$maketype=${maketype}"
-		ewarn "QA: Ebuild should be migrated to use VIRTUALX_COMMAND=${maketype} instead."
-		ewarn "QA: Setting VIRTUALX_COMMAND to \$maketype conveniently for now."
+		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
 
@@ -186,8 +188,8 @@ virtualmake() {
 Xmake() {
 	debug-print-function ${FUNCNAME} "$@"
 
-	ewarn "QA: you should not execute make directly"
-	ewarn "QA: rather execute Xemake -j1 if you have issues with parallel make"
+	eqawarn "you should not execute make directly"
+	eqawarn "rather execute Xemake -j1 if you have issues with parallel make"
 	VIRTUALX_COMMAND="emake -j1" virtualmake "$@"
 }
 
-- 
1.8.1.2

[gentoo-dev] [PATCH virtualx.eclass 3/5] Convert X* functions to the new API.

[Michał Górny]

---
 gx86/eclass/virtualx.eclass | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gx86/eclass/virtualx.eclass b/gx86/eclass/virtualx.eclass
index 0da3066..096c37a 100644
--- a/gx86/eclass/virtualx.eclass
+++ b/gx86/eclass/virtualx.eclass
@@ -190,7 +190,7 @@ Xmake() {
 
 	eqawarn "you should not execute make directly"
 	eqawarn "rather execute Xemake -j1 if you have issues with parallel make"
-	VIRTUALX_COMMAND="emake -j1" virtualmake "$@"
+	virtualx emake -j1 "$@"
 }
 
 # @FUNCTION: Xemake
@@ -199,7 +199,7 @@ Xmake() {
 Xemake() {
 	debug-print-function ${FUNCNAME} "$@"
 
-	VIRTUALX_COMMAND="emake" virtualmake "$@"
+	virtualx emake "$@"
 }
 
 # @FUNCTION: Xeconf
@@ -208,5 +208,5 @@ Xemake() {
 Xeconf() {
 	debug-print-function ${FUNCNAME} "$@"
 
-	VIRTUALX_COMMAND="econf" virtualmake "$@"
+	virtualx econf "$@"
 }
-- 
1.8.1.2

[gentoo-dev] [PATCH virtualx.eclass 4/5] Deprecate virtualmake in favor of the new syntax.

[Michał Górny]

---
 gx86/eclass/virtualx.eclass | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/gx86/eclass/virtualx.eclass b/gx86/eclass/virtualx.eclass
index 096c37a..f576335 100644
--- a/gx86/eclass/virtualx.eclass
+++ b/gx86/eclass/virtualx.eclass
@@ -172,12 +172,13 @@ virtualmake() {
 
 	# backcompat for maketype
 	if [[ -n ${maketype} ]]; then
-		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
 
+	eqawarn "Using virtualmake is considered deprecated."
+	eqawarn "Please use the new, cleaner API instead:"
+	eqawarn "	virtualx ${VIRTUALX_COMMAND}"
+
 	virtualx ${VIRTUALX_COMMAND} "$@"
 }
 
-- 
1.8.1.2

[gentoo-dev] [PATCH virtualx.eclass 5/5] (Optionally) deprecate all X* wrappers.

[Michał Górny]

The new syntax seems simple enough that we can think of deprecating all
those short-hand forms.
---
 gx86/eclass/virtualx.eclass | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/gx86/eclass/virtualx.eclass b/gx86/eclass/virtualx.eclass
index f576335..9d5045d 100644
--- a/gx86/eclass/virtualx.eclass
+++ b/gx86/eclass/virtualx.eclass
@@ -189,8 +189,10 @@ virtualmake() {
 Xmake() {
 	debug-print-function ${FUNCNAME} "$@"
 
-	eqawarn "you should not execute make directly"
-	eqawarn "rather execute Xemake -j1 if you have issues with parallel make"
+	eqawarn "Using Xmake is considered deprecated."
+	eqawarn "Please use the new, cleaner API instead:"
+	eqawarn "	virtualx emake -j1 ..."
+
 	virtualx emake -j1 "$@"
 }
 
@@ -200,6 +202,10 @@ Xmake() {
 Xemake() {
 	debug-print-function ${FUNCNAME} "$@"
 
+	eqawarn "Using Xemake is considered deprecated."
+	eqawarn "Please use the new, cleaner API instead:"
+	eqawarn "	virtualx emake ..."
+
 	virtualx emake "$@"
 }
 
@@ -209,5 +215,9 @@ Xemake() {
 Xeconf() {
 	debug-print-function ${FUNCNAME} "$@"
 
+	eqawarn "Using Xeconf is considered deprecated."
+	eqawarn "Please use the new, cleaner API instead:"
+	eqawarn "	virtualx econf ..."
+
 	virtualx econf "$@"
 }
-- 
1.8.1.2

Re: [gentoo-dev] [RFC/PATCH] A cleaner API for virtualx.eclass

[Diego Elio Pettenò]

I'd say, "Go for it!"

But on the other hand I wonder if it might make sense to have
something more generic, so that one only has to call something in a
way such as

virtualx_setup
run_tests --foo
virtualx_cleanup

The reason why I'm wondering this is that we need some more "virtual
environments" for testing purposes, really: so many packages need a
D-Bus session (and I'd rather have them using a test session than a
system one!), and at least in Ruby world we often need a database
(sometimes more than one)...
Diego Elio Pettenò — Flameeyes
flameeyes@flameeyes.eu — http://blog.flameeyes.eu/


On Mon, Feb 11, 2013 at 11:14 PM, Michał Górny <mgorny@gentoo.org> wrote:
> Hello, fellow developers,
>
> The current virtualx.eclass API is a bit insane. It seems a bit like
> stacking of a few next APIs, mostly designed to quickly run 'make
> check', then extended to general functions.
>
> For example running a function 'run_tests' with parameter '--foo' would
> look like:
>
>         VIRTUALX_COMMAND=run_tests virtualmake --foo
>
> which is really awful, considering that '--foo' is a parameter to
> 'run_tests' and not virtualmake.
>
> My patches introduce a single wrapper with argv-as-parameter syntax.
> That is, the fore-mentioned example would look like:
>
>         virtualx run_tests --foo
>
> Depending on the maintainer decisions and your feedback, I believe that
> even all the X* short-hand functions could be deprecated. They are a bit
> shorter:
>
>         Xemake check
>
> vs:
>
>         virtualx emake check
>
> but I don't think that's much of a difference.
>
> What are your thoughts?
>
>

Re: [gentoo-dev] [RFC/PATCH] A cleaner API for virtualx.eclass

[Gilles Dartiguelongue]

Le lundi 11 février 2013 à 23:48 +0100, Diego Elio Pettenò a écrit :
> I'd say, "Go for it!"
> 
> But on the other hand I wonder if it might make sense to have
> something more generic, so that one only has to call something in a
> way such as
> 
> virtualx_setup
> run_tests --foo
> virtualx_cleanup

This sounds nice but is imho but can be elaborated later on.


-- 
Gilles Dartiguelongue <eva@gentoo.org>
Gentoo

Re: [gentoo-dev] [RFC/PATCH] A cleaner API for virtualx.eclass

[Andreas K. Huettel]

[-- Attachment #1: Type: Text/Plain, Size: 852 bytes --]

Am Montag, 11. Februar 2013, 23:14:38 schrieb Michał Górny:
> 
> What are your thoughts?

Same as Diego I like the general idea, but an even more generic framework 
might make sense. Say test dbus session, say setting up some test file 
structure, ...

Oh, and one more thing... please before you commit anything talk to the kde 
guys... we inherit virtualx.eclass everywhere via kde4-*.eclass so we can 
easily use it (and many many kde packages need it for the tests). Changes in 
virtualx.eclass already broke many kde packages once...

[IMHO I'd say it's perfectly fine if you determine the best way to go without 
specific kde regression testing, but before it goes in we'd like to have a 
chance to adapt our eclasses...]


-- 

Andreas K. Huettel
Gentoo Linux developer 
dilfridge@gentoo.org
http://www.akhuettel.de/


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [gentoo-dev] [RFC/PATCH] A cleaner API for virtualx.eclass

["Paweł Hajdan, Jr."]

[-- Attachment #1: Type: text/plain, Size: 342 bytes --]

On 2/11/13 11:14 PM, Michał Górny wrote:
> My patches introduce a single wrapper with argv-as-parameter syntax.
> That is, the fore-mentioned example would look like:
> 
> 	virtualx run_tests --foo

Maybe we can just adapt Ubuntu's (I think) xvfb-run? More
standardization == profit.

Thank you for working on this!

Paweł



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 203 bytes --]

Re: [gentoo-dev] [RFC/PATCH] A cleaner API for virtualx.eclass

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 625 bytes --]

On Tue, 12 Feb 2013 18:42:28 +0100
""Paweł Hajdan, Jr."" <phajdan.jr@gentoo.org> wrote:

> On 2/11/13 11:14 PM, Michał Górny wrote:
> > My patches introduce a single wrapper with argv-as-parameter syntax.
> > That is, the fore-mentioned example would look like:
> > 
> > 	virtualx run_tests --foo
> 
> Maybe we can just adapt Ubuntu's (I think) xvfb-run? More
> standardization == profit.

That's probably a good idea. However, I just stepped up here to
replace the ugly API with something better. I think the actual eclass
maintainers should have the final word here.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]