[gentoo-dev] x-modular.eclass: A modified approach to EAPI support

[gentoo-dev] x-modular.eclass: A modified approach to EAPI support

[Donnie Berkholz]

[-- Attachment #1.1: Type: text/plain, Size: 1022 bytes --]

I decided to try something a little different because I had some ideas 
for improving the existing EAPI patches I've seen going into other 
eclasses. So here is my patch for x-modular.eclass. I tested it with 
ebuilds using EAPIs 0, 1, and 2, and it appeared to work fine. It 
already happened to have a function called src_configure, so that 
doesn't appear in the patch.

One thing I see as an improvement is a lack of EAPI value checks 
throughout the ebuild to avoid repetition between the function export 
and the function call. Things just check whether a function was 
exported, which is the only place where EAPI value checks happen.

Additionally, the fallback in case statements is "I don't know what to 
do" and supported EAPIs are explicitly defined. This will make it 
obvious when the eclass doesn't support a new EAPI instead of it 
randomly failing after you try it.

Any thoughts?

-- 
Thanks,
Donnie

Donnie Berkholz
Developer, Gentoo Linux
Blog: http://dberkholz.wordpress.com

[-- Attachment #1.2: x-modular.eclass.diff --]
[-- Type: text/plain, Size: 1638 bytes --]

--- /home/donnie/src/gentoo-x86/eclass/x-modular.eclass	2009-03-06 12:11:38.000000000 -0800
+++ x-modular.eclass	2009-03-06 12:16:25.000000000 -0800
@@ -26,6 +26,21 @@
 # there. You may also want to change the SLOT.
 XDIR="/usr"
 
+EXPORTED_FUNCTIONS="src_unpack src_compile src_install pkg_preinst pkg_postinst pkg_postrm"
+
+case "${EAPI:-0}" in
+	0|1)
+		;;
+	2)
+		EXPORTED_FUNCTIONS="${EXPORTED_FUNCTIONS} src_prepare src_configure"
+		;;
+	*)
+		die "Unknown EAPI ${EAPI}"
+		;;
+esac
+
+EXPORT_FUNCTIONS ${EXPORTED_FUNCTIONS}
+
 IUSE=""
 HOMEPAGE="http://xorg.freedesktop.org/"
 
@@ -297,6 +312,15 @@
 	elibtoolize
 }
 
+# @FUNCTION: x-modular_src_prepare
+# @USAGE:
+# @DESCRIPTION:
+# Prepare a package after unpacking, performing all X-related tasks.
+x-modular_src_prepare() {
+	x-modular_patch_source
+	x-modular_reconf_source
+}
+
 # @FUNCTION: x-modular_src_unpack
 # @USAGE:
 # @DESCRIPTION:
@@ -306,8 +330,7 @@
 	x-modular_server_supports_drivers_check
 	x-modular_dri_check
 	x-modular_unpack_source
-	x-modular_patch_source
-	x-modular_reconf_source
+	has src_prepare ${EXPORTED_FUNCTIONS} || x-modular_src_prepare
 }
 
 # @FUNCTION: x-modular_font_configure
@@ -390,7 +413,7 @@
 # @DESCRIPTION:
 # Compile a package, performing all X-related tasks.
 x-modular_src_compile() {
-	x-modular_src_configure
+	has src_configure ${EXPORTED_FUNCTIONS} || x-modular_src_configure
 	x-modular_src_make
 }
 
@@ -645,5 +668,3 @@
 create_font_cache() {
 	font_pkg_postinst
 }
-
-EXPORT_FUNCTIONS src_unpack src_compile src_install pkg_preinst pkg_postinst pkg_postrm

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[gentoo-dev] Re: x-modular.eclass: A modified approach to EAPI support

[Christian Faulhammer]

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

Hi,

Donnie Berkholz <dberkholz@gentoo.org>:
> Any thoughts?

 Nice is the centralisation of the EAPI check although I also like to
see function execution by EAPI on the spot.  What I don't like is dying
on an unknown EAPI.  Is it in the range of possibilities that the
provided phases will change drastically with EAPI > 2?

V-Li


-- 
Christian Faulhammer, Gentoo Lisp project
<URL:http://www.gentoo.org/proj/en/lisp/>, #gentoo-lisp on FreeNode

<URL:http://www.faulhammer.org/>

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

Re: [gentoo-dev] x-modular.eclass: A modified approach to EAPI support

[Petteri Räty]

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

Donnie Berkholz wrote:
> I decided to try something a little different because I had some ideas 
> for improving the existing EAPI patches I've seen going into other 
> eclasses. So here is my patch for x-modular.eclass. I tested it with 
> ebuilds using EAPIs 0, 1, and 2, and it appeared to work fine. It 
> already happened to have a function called src_configure, so that 
> doesn't appear in the patch.
> 
> One thing I see as an improvement is a lack of EAPI value checks 
> throughout the ebuild to avoid repetition between the function export 
> and the function call. Things just check whether a function was 
> exported, which is the only place where EAPI value checks happen.
> 
> Additionally, the fallback in case statements is "I don't know what to 
> do" and supported EAPIs are explicitly defined. This will make it 
> obvious when the eclass doesn't support a new EAPI instead of it 
> randomly failing after you try it.
> 
> Any thoughts?
> 
> 

The other option instead of die would be DEPEND="THIS-IS-NOT-VALID".

Regards,
Petteri


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

Re: [gentoo-dev] x-modular.eclass: A modified approach to EAPI support

[Rémi Cardona]

Le 06/03/2009 21:57, Donnie Berkholz a écrit :
> Any thoughts?

Looks pretty good to me. I don't have much else to say :)

Cheers,

Rémi

Re: [gentoo-dev] x-modular.eclass: A modified approach to EAPI support

[Ulrich Mueller]

>>>>> On Fri, 06 Mar 2009, Donnie Berkholz wrote:

> Any thoughts?

> +		*)
> +			die "Unknown EAPI ${EAPI}"
> +			    ;;

Is is safe to assume that an unknown EAPI will provide a "die"
function?

Ulrich

Re: [gentoo-dev] x-modular.eclass: A modified approach to EAPI support

[Nirbheek Chauhan]

On Sat, Mar 7, 2009 at 3:20 PM, Ulrich Mueller <ulm@gentoo.org> wrote:
>>>>>> On Fri, 06 Mar 2009, Donnie Berkholz wrote:
>
>> Any thoughts?
>
>> +             *)
>> +                     die "Unknown EAPI ${EAPI}"
>> +                         ;;
>
> Is is safe to assume that an unknown EAPI will provide a "die"
> function?
>

If we get all Ciaran-ey about that, then we can't even assume the
existence of a case statement in some future version of bash (which is
required by some EAPI)


-- 
~Nirbheek Chauhan

Re: [gentoo-dev] x-modular.eclass: A modified approach to EAPI support

[Alistair Bush]

Nirbheek Chauhan wrote:
> On Sat, Mar 7, 2009 at 3:20 PM, Ulrich Mueller <ulm@gentoo.org> wrote:
>>>>>>> On Fri, 06 Mar 2009, Donnie Berkholz wrote:
>>> Any thoughts?
>>> +             *)
>>> +                     die "Unknown EAPI ${EAPI}"
>>> +                         ;;
>> Is is safe to assume that an unknown EAPI will provide a "die"
>> function?
>>
> 
> If we get all Ciaran-ey about that, then we can't even assume the
> existence of a case statement in some future version of bash (which is
> required by some EAPI)
> 
> 

I think in these cases we just have to use common sense.  If a function 
is deprecated or "known to be 'on the way out'" then using them would 
obviously be a bad idea.  On the other hand even if they are used, 
surely someone would test an ebuild and discover this case pretty quickly.

Re: [gentoo-dev] x-modular.eclass: A modified approach to EAPI support

[Donnie Berkholz]

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

On 12:57 Fri 06 Mar     , Donnie Berkholz wrote:
> I decided to try something a little different because I had some ideas 
> for improving the existing EAPI patches I've seen going into other 
> eclasses. So here is my patch for x-modular.eclass. I tested it with 
> ebuilds using EAPIs 0, 1, and 2, and it appeared to work fine. It 
> already happened to have a function called src_configure, so that 
> doesn't appear in the patch.

FYI, using EXPORT_FUNCTIONS before inherit, as this patch caused 
x-modular.eclass to do, is broken in current portage releases. Zac said 
he would change this to be consistent with the lack of any ordering 
restriction in the PMS. Thanks to Tomáš Chvátal for tracking down this 
tricky bug!

I ran a quick check across everything in the tree and was happy to see 
nothing doing this besides my eclass patch. Here's the scriptlet I used:

for i in /usr/portage/eclass/*; do
  grep -q EXPORT_FUNCTIONS $i || continue
  grep -q inherit $i || continue
  LINE1=$(grep -n EXPORT_FUNCTIONS $i | grep -v ':#' | cut -d: -f1 | head -n1)
  LINE2=$(grep -n inherit $i | grep -v ':#' | cut -d: -f1 | tail -n1)
  if [[ $LINE1 -lt $LINE2 ]]; then
    echo $i $LINE1 $LINE2
  fi
done

Manual checking of the two results showed false positives.

-- 
Thanks,
Donnie

Donnie Berkholz
Developer, Gentoo Linux
Blog: http://dberkholz.wordpress.com

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [gentoo-dev] x-modular.eclass: A modified approach to EAPI support

[David Leverton]

On Sunday 08 March 2009 05:22:03 Donnie Berkholz wrote:
> FYI, using EXPORT_FUNCTIONS before inherit, as this patch caused
> x-modular.eclass to do, is broken in current portage releases. Zac said
> he would change this to be consistent with the lack of any ordering
> restriction in the PMS. Thanks to Tomáš Chvátal for tracking down this
> tricky bug!

Better to ask for PMS to be clarified.  All existing package managers do 
EXPORT_FUNCTIONS in more or less the same way, so changing it shouldn't 
happen without an EAPI bump.

Re: [gentoo-dev] x-modular.eclass: A modified approach to EAPI support

[Zac Medico]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

David Leverton wrote:
> On Sunday 08 March 2009 05:22:03 Donnie Berkholz wrote:
>> FYI, using EXPORT_FUNCTIONS before inherit, as this patch caused
>> x-modular.eclass to do, is broken in current portage releases. Zac said
>> he would change this to be consistent with the lack of any ordering
>> restriction in the PMS. Thanks to Tomáš Chvátal for tracking down this
>> tricky bug!
> 
> Better to ask for PMS to be clarified.  All existing package managers do 
> EXPORT_FUNCTIONS in more or less the same way, so changing it shouldn't 
> happen without an EAPI bump.

As discussed on irc, if we make it conditional on EAPI then you'll
practically never be able to call EXPORT_FUNCTIONS before inherit
since eclasses generally support multiple EAPIs. So, I've added a
warning message that is triggered when EXPORT_FUNCTIONS is called
before inherit. In a year or two we can consider having the warning
removed.
- --
Thanks,
Zac
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.10 (GNU/Linux)

iEYEARECAAYFAkm0SPsACgkQ/ejvha5XGaN/+ACgsS44TWTR2fODGzwSI0XH5xN7
xpkAoOJhSWeXQDnhO8OuoXuViB2MNe7F
=RJI6
-----END PGP SIGNATURE-----