Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 239123 - Support for EAPI-2 in gnome2.eclass
Summary: Support for EAPI-2 in gnome2.eclass
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: High normal (vote)
Assignee: Gentoo Linux Gnome Desktop Team
URL:
Whiteboard:
Keywords: Inclusion
Depends on:
Blocks: 253888
  Show dependency tree
 
Reported: 2008-09-30 07:28 UTC by Alexis Ballier
Modified: 2009-02-17 16:08 UTC (History)
6 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
proposed patch (gnome2.eclass.patch,893 bytes, patch)
2008-09-30 07:58 UTC, Alexis Ballier
Details | Diff
Add support for EAPI=2 (gnome2-eapi2-fixes.patch,714 bytes, patch)
2008-09-30 08:53 UTC, Nirbheek Chauhan (RETIRED)
Details | Diff
EAPI-2 patch (gnome2-eclass.patch,941 bytes, patch)
2008-10-05 02:16 UTC, Jorge Manuel B. S. Vicetto (RETIRED)
Details | Diff
gnome2.eclass-EAPI2.patch (gnome2.eclass-EAPI2.patch,838 bytes, patch)
2009-01-22 12:35 UTC, Peter Volkov (RETIRED)
Details | Diff
Naive patch for EAPI-2 (gnome2-eapi2.patch,1.36 KB, patch)
2009-02-08 21:37 UTC, Peter Alfredsen (RETIRED)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexis Ballier gentoo-dev 2008-09-30 07:28:56 UTC
If I have an ebuild using EAPI-2 inheriting gnome2 eclass and doing nothing else, it'll call econf twice, once with the default src_configure and once at gnome2_src_compile. Maybe it'd be possible to export gnome2_src_configure and call it from gnome2_src_compile only if eapi<2; that doesn't seem that reliable for future eapis though.
Comment 1 Mart Raudsepp gentoo-dev 2008-09-30 07:38:36 UTC
I personally planned to first take care of GNOME-2.24 and then start looking into this, and not use EAPI-2 in gnome2.eclass inheriting packages till then. Maybe someone else from the team wants to look at it before GNOME-2.24 is finished, but where do you need this? We are still more than 30 days away from portage-2.2 stabilization anyhow, so I don't see the hurry - we'd certainly take care of it soon (I'm quite sure later this week), so that revbumps with EAPI-2 using gnome2.eclass utilizing packages can be in ~arch for about exactly 30 days when portage-2.2 goes stable..
Comment 2 Alexis Ballier gentoo-dev 2008-09-30 07:42:26 UTC
I'd need this for ogmrip, bug #238965
for now I'll probably add the ugly hack:
# lousy way of avoiding double econf until gnome2 eclass gets fixed
src_configure() {
    :
}

use deps are very interesting for ogmrip :/
Comment 3 Alexis Ballier gentoo-dev 2008-09-30 07:44:01 UTC
Thinking about it, a way to avoid messing with eapi would be to set a variable at the end of gnome2_src_configure and not call it from src_compile if it's true.
Comment 4 Alexis Ballier gentoo-dev 2008-09-30 07:58:44 UTC
Created attachment 166803 [details, diff]
proposed patch

This appears to do the trick; needs testing with stable portage, eapi-0/1 ebuilds as I'm not sure how it would behave with src_configure being exported.
Comment 5 Mart Raudsepp gentoo-dev 2008-09-30 08:16:03 UTC
Problem with hack in comment #2 is that it'll break as soon as gnome2.eclass starts supporting EAPI=2, I think
Comment 6 Nirbheek Chauhan (RETIRED) gentoo-dev 2008-09-30 08:53:57 UTC
Created attachment 166812 [details, diff]
Add support for EAPI=2

(In reply to comment #0)
> Maybe it'd be possible to export gnome2_src_configure and
> call it from gnome2_src_compile only if eapi<2; that doesn't seem that reliable
> for future eapis though.
> 

Seeing the current trend of official EAPIs, they all seem to be incremental in nature, so test eapi -lt 2 seems to be the best way to go about it (IMO)

The attached patch implements/exports src_prepare and src_configure and does special-case code when ${EAPI} -lt 2
Comment 7 Nirbheek Chauhan (RETIRED) gentoo-dev 2008-09-30 08:55:50 UTC
(In reply to comment #4)
> Created an attachment (id=166803) [edit]
> proposed patch
> 
> This appears to do the trick; needs testing with stable portage, eapi-0/1
> ebuilds as I'm not sure how it would behave with src_configure being exported.
> 

IMO, this patch is too special-case. It handles only src_configure problems, and will have to be changed once EAPI=2 support is properly integrated into the eclass.

Also, why use a file when you can check a variable? :)
Comment 8 Rémi Cardona (RETIRED) gentoo-dev 2008-09-30 08:56:40 UTC
EAPIs are not numbers, they are strings. As such, I believe doing integer
comparison is just the best way to shoot ourselves in the foot :)
Comment 9 Nirbheek Chauhan (RETIRED) gentoo-dev 2008-09-30 09:06:39 UTC
(In reply to comment #8)
> EAPIs are not numbers, they are strings. As such, I believe doing integer
> comparison is just the best way to shoot ourselves in the foot :)
> 

Yeah, *red-faced* I realised that, and fixed it in the patch, but forgot to fix it in the comment ;p (as I realise now)
Comment 10 Alexis Ballier gentoo-dev 2008-09-30 15:51:09 UTC
(In reply to comment #7)
> (In reply to comment #4)
> > Created an attachment (id=166803) [edit]
> > proposed patch
> > 
> > This appears to do the trick; needs testing with stable portage, eapi-0/1
> > ebuilds as I'm not sure how it would behave with src_configure being exported.
> > 
> 
> IMO, this patch is too special-case. It handles only src_configure problems,
> and will have to be changed once EAPI=2 support is properly integrated into the
> eclass.

Unless we have a way to ask "has my eapi src_configure support" i don't see anything better; and yes it only deals with src_configure problems as that's what is annoying me at the moment :) Adding src_prepare support will be more difficult because we want it to be called in src_unpack if we know it won't be called later.

Adding checks for specific eapis will require it to be updated for each new eapi which in the end will result in maintaining functions like "has my eapi foo support" there and will kill the usage of experimental eapis; this might be the way to go, but i don't think that's gnome2.eclass's job.

> Also, why use a file when you can check a variable? :)

Using a file sounds more reliable to me, ala what elibtoolize does.
Comment 11 Alexis Ballier gentoo-dev 2008-09-30 16:12:00 UTC
(In reply to comment #5)
> Problem with hack in comment #2 is that it'll break as soon as gnome2.eclass
> starts supporting EAPI=2, I think

maybe :/ it depends on how support is added; i'll probably change it to call gnome2_src_configure and add a src_compile with emake || die, so that it shouldn't break but kills the advantages of having functions exported from the eclass. (Of course that's unless you take a quick (as in today) decision on how to handle this, otherwise there is no hurry and I can remove the code duplication later)
Comment 12 Jorge Manuel B. S. Vicetto (RETIRED) Gentoo Infrastructure gentoo-dev 2008-10-05 02:16:16 UTC
Created attachment 167240 [details, diff]
EAPI-2 patch

This is a different alternative to add support for EAPI-2 to the eclass.
An important note regarding nirbheek's patch is that you can't export src_prepare and src_configure for EAPI!=2.
Comment 13 Olivier Crete (RETIRED) gentoo-dev 2009-01-20 01:16:10 UTC
As an additional note, you probably don't need to export src_unpack() and src_compile() in EAPI=2
Comment 14 Peter Volkov (RETIRED) gentoo-dev 2009-01-22 12:35:42 UTC
Created attachment 179325 [details, diff]
gnome2.eclass-EAPI2.patch

Updated Jorge's patch with  Olivier suggestion. Gnome team, can I commit that?
Comment 15 Daniel Gryniewicz (RETIRED) gentoo-dev 2009-01-24 20:46:24 UTC
That actually looks good.  Peter: If you've tried it with a EAPI=2 and a EAPI<2, go ahead and commit.
Comment 16 Jonathan Callen (RETIRED) gentoo-dev 2009-01-25 06:09:10 UTC
I would suggest that instead of doing this:

case "${EAPI}" in
  2) EXPORT_FUNCTIONS ... ;;
  *) EXPORT_FUNCTIONS ... ;;
esac

you use this:

case "${EAPI:-0}" in
  0|1) EXPORT_FUNCTIONS ... ;;
  *) EXPORT_FUNCTIONS ... ;;
esac

This would be forward-compatible with any future EAPIs in so far as they would
be incremental additions to EAPI=2.  In other words, I think you should treat
EAPI=3 as EAPI=2 instead of EAPI=0.  Just a thought.
Comment 17 Jorge Manuel B. S. Vicetto (RETIRED) Gentoo Infrastructure gentoo-dev 2009-01-26 01:59:51 UTC
(In reply to comment #16)
,..
> you use this:
> 
> case "${EAPI:-0}" in
>   0|1) EXPORT_FUNCTIONS ... ;;
>   *) EXPORT_FUNCTIONS ... ;;
> esac
> 
> This would be forward-compatible with any future EAPIs in so far as they would
> be incremental additions to EAPI=2.  In other words, I think you should treat
> EAPI=3 as EAPI=2 instead of EAPI=0.  Just a thought.
> 

Unfortunately, there's a problem with that - EAPI-0 ebuilds may not include the EAPI var, so the default must always be assumed to be EAPI-o.
Comment 18 Jonathan Callen (RETIRED) gentoo-dev 2009-01-26 02:44:42 UTC
> Unfortunately, there's a problem with that - EAPI-0 ebuilds may not include
> the EAPI var, so the default must always be assumed to be EAPI-o.

That is why the code used ${EAPI:-0} - if the value of $EAPI is null or unset, then it assumes that EAPI is 0, because EAPI-0 is the only EAPI that allows a null or unset $EAPI
Comment 19 Jorge Manuel B. S. Vicetto (RETIRED) Gentoo Infrastructure gentoo-dev 2009-01-26 03:21:07 UTC
(In reply to comment #18)
> > Unfortunately, there's a problem with that - EAPI-0 ebuilds may not include
> > the EAPI var, so the default must always be assumed to be EAPI-o.
> 
> That is why the code used ${EAPI:-0} - if the value of $EAPI is null or unset,
> then it assumes that EAPI is 0, because EAPI-0 is the only EAPI that allows a
> null or unset $EAPI
> 

ok, I missed that.
Anyway, you're making assumptions for future EAPIs to "ease" the upgrade to newer EAPI versions, but that goes against the basic rule that you must never assume anything about a future EAPI. If EAPI-3 were to rename src_ functions, your code would kill the eclass until someone fixed it. An important point is that it is possible to define new EAPIs that are not backward compatible with prior versions and although I'm talking about prior and later EAPI versions, there is no "ordering" and it would be legitimate to have an "Iamnotanumber" EAPI.
Although having to manually touch the eclass for any future EAPI is "boring", it is an "unavoidable" consequence of the defintion of EAPI.
Comment 20 Gilles Dartiguelongue (RETIRED) gentoo-dev 2009-01-26 23:04:22 UTC
@pva, doesn't you patch calls gnome_src_prepare 2 times if EAPI=2 ? or is there some "don't execute this phase 2 times" protection I'm unaware of ?
Comment 21 Jorge Manuel B. S. Vicetto (RETIRED) Gentoo Infrastructure gentoo-dev 2009-01-27 03:32:47 UTC
(In reply to comment #20)
> @pva, doesn't you patch calls gnome_src_prepare 2 times if EAPI=2 ? or is there
> some "don't execute this phase 2 times" protection I'm unaware of ?
> 

Giles, you're right. I missed that.
Peter, you missed the point in my patch. The call for src_prepare and src_configure in respectively src_unpack and src_compile should only be done for EAPI!=2.
Comment 22 Nirbheek Chauhan (RETIRED) gentoo-dev 2009-01-27 03:42:04 UTC
(In reply to comment #20)
> @pva, doesn't you patch calls gnome_src_prepare 2 times if EAPI=2 ? or is there
> some "don't execute this phase 2 times" protection I'm unaware of ?
> 

Yeah, the "don't execute this phase 2 times" protection is "don't export the phase twice" :p

If EAPI=2, src_unpack isn't exported, and the ebuild uses default_src_unpack. Exactly how src_compile isn't exported when EAPI=2.
Comment 23 Peter Volkov (RETIRED) gentoo-dev 2009-01-27 07:12:31 UTC
(In reply to comment #20)
> @pva, doesn't you patch calls gnome_src_prepare 2 times if EAPI=2 ? or is
> there some "don't execute this phase 2 times" protection I'm unaware of ?

The magic is inside this code:

case "${EAPI}" in
    2)
        EXPORT_FUNCTIONS src_unpack src_prepare src_configure src_install pkg_preinst pkg_postinst pkg_postrm
        ;;
    *)
        EXPORT_FUNCTIONS src_unpack src_compile src_install pkg_preinst pkg_postinst pkg_postrm
        ;;
esac

Let's consider what will be called in EAPI=2. In this case only src_unpack src_prepare src_configure src_install pkg_preinst pkg_postinst pkg_postrm are exported. So...

EAPI=2
phase              function to be called

src_unpack         default_src_unpack
src_prepare        gnome_src_prepare
src_configure      gnome_src_configure
src_compile        default_src_compile
and so on...

Giles, at which point to you think gnome_src_configure will be called twice?

(In reply to comment #21)
> Peter, you missed the point in my patch. The call for src_prepare and
> src_configure in respectively src_unpack and src_compile should only be done
> for EAPI!=2.
 
And as I see this is the case. What do I miss?
Comment 24 Mart Raudsepp gentoo-dev 2009-02-03 01:43:42 UTC
(In reply to comment #23)
> (In reply to comment #20)
> > @pva, doesn't you patch calls gnome_src_prepare 2 times if EAPI=2 ? or is
> > there some "don't execute this phase 2 times" protection I'm unaware of ?
> 
> The magic is inside this code:

I can fully understand the logic here, and see that it will work as intended, but I'm going to have to reject this patch because it is indeed magic.
We don't want to have such undocumented magic in the eclass, that can easily break with future changes, for example when:

a) It is deemed necessary to have more code for src_compile than just the standard thing - easy to accidentally forget to export gnome2_src_compile for EAPI-2
b) if src_compile gets exported for EAPI-2 for a), easy to not notice src_configure will get called twice.

Please rework the patch to export in EAPI-2 all the same functions than EAPI-1 does, while working properly.
I might be also happy with big fat warning comments in all relevant places to say to edit things where necessary when changing stuff as per above, but that still feels icky.
Comment 25 Mart Raudsepp gentoo-dev 2009-02-03 01:44:38 UTC
I think that means Jorge's patch is either suitable for inclusion or a good base for the final one?
Comment 26 Nirbheek Chauhan (RETIRED) gentoo-dev 2009-02-03 08:10:26 UTC
(In reply to comment #24)
> We don't want to have such undocumented magic in the eclass, that can easily
> break with future changes, for example when:
> 

Okay, then let's document it with comments.

> a) It is deemed necessary to have more code for src_compile than just the
> standard thing - easy to accidentally forget to export gnome2_src_compile for
> EAPI-2
> b) if src_compile gets exported for EAPI-2 for a), easy to not notice
> src_configure will get called twice.
> 

More code for src_compile which will apply to _all_ packages importing gnome2.eclass? The last major change made to gnome2.eclass was more than *2 years* ago[1]. By the time we have to make another change, GNOME 3.0 will be out :p

Also, don't all changes made to eclasses have to go through proper testing and maintainer review? Also, since you now raised this point, the potential for EAPI-specific problems can be kept in mind via a comment.

> Please rework the patch to export in EAPI-2 all the same functions than EAPI-1
> does, while working properly.

The problem with that is that it gets bloody and ugly at the same time. Take a look at the kde eclasses -- it got so ugly that they decided to have EAPI=2 support only.

> I might be also happy with big fat warning comments in all relevant places to
> say to edit things where necessary when changing stuff as per above, but that
> still feels icky.
> 

I'm a follower of the principle of minimum source code change to get something working. The clearer and more concise the change, the better. Also, having case statements in every function is far more icky.

I am strongly in favour of getting Peter's patch into portage *asap*. Not being able to use EAPI=2 is very irritating; especially since it is now stable.
Comment 27 Peter Volkov (RETIRED) gentoo-dev 2009-02-03 08:56:42 UTC
(In reply to comment #24)
> I can fully understand the logic here, and see that it will work as intended,
> but I'm going to have to reject this patch because it is indeed magic.

Mart, although I disagree, you are maintainers, so it's your shot :) I don't want commit anything you guys deem bad.

> Please rework the patch to export in EAPI-2 all the same functions than EAPI-1
> does, while working properly.

Ok, you mean exactly patch Jorge Manuel B. S. Vicetto posted here in 2008 year? :)

> I might be also happy with big fat warning comments 

Please, take a look at that patch: https://bugs.gentoo.org/attachment.cgi?id=167240

Do you still want any warnings? Actually I think we should not document EAPI's inside eclasses. Please, give me go ahead too or commit anything by yourself.
Comment 28 Gilles Dartiguelongue (RETIRED) gentoo-dev 2009-02-03 09:28:09 UTC
(In reply to comment #26)
> (In reply to comment #24)
> > We don't want to have such undocumented magic in the eclass, that can easily
> > break with future changes, for example when:
> > 
> 
> Okay, then let's document it with comments.

The thing here that is considered annoying here is that documentation in eclass is good but it is way to easy to fail at changing ebuilds when bumping, fixing and doing the usual laundry.

The eclass should be a helper here, not something that will potentially screw you because you forgot that I EAPI X it does this and EAPI Y it does it but in a _subtle_ different way.
Comment 29 Mart Raudsepp gentoo-dev 2009-02-05 09:53:04 UTC
Additionally we'll still want to be able to do stuff like

src_compile() {
   ... some ebuild specific stuff ...
   gnome2_src_compile
   ... perhaps some more ebuild specific stuff ...
}

in ebuilds, and not worry about it working with EAPI-0/1 but not working with EAPI-2 (lets imagine that ebuild specific stuff deals with the compile phase with EAPI-0, perhaps having to call emake in an additional extra directory that isn't recursed into otherwise)
Comment 30 Mart Raudsepp gentoo-dev 2009-02-05 09:57:41 UTC
(In reply to comment #17)
> (In reply to comment #16)
> ,..
> > you use this:
> > 
> > case "${EAPI:-0}" in
> >   0|1) EXPORT_FUNCTIONS ... ;;
> >   *) EXPORT_FUNCTIONS ... ;;
> > esac
> > 
> > This would be forward-compatible with any future EAPIs in so far as they would
> > be incremental additions to EAPI=2.  In other words, I think you should treat
> > EAPI=3 as EAPI=2 instead of EAPI=0.  Just a thought.
> > 
> 
> Unfortunately, there's a problem with that - EAPI-0 ebuilds may not include the
> EAPI var, so the default must always be assumed to be EAPI-o.


Err, doesn't ${EAPI:-0} mean it's going to be treated as "0" in the case checks when it is unset?
I'm not entirely sure right now if I'd like an unknown EAPI (e.g 3) to be treated as EAPI=2, or as an error.
Comment 31 Peter Alfredsen (RETIRED) gentoo-dev 2009-02-08 21:37:44 UTC
Created attachment 181390 [details, diff]
Naive patch for EAPI-2

Is there any reason why a naive patch such as this one cannot be committed?
Comment 32 Daniel Gryniewicz (RETIRED) gentoo-dev 2009-02-13 17:31:02 UTC
Not that I can see.  Maybe someone else has a problem?
Comment 33 Nirbheek Chauhan (RETIRED) gentoo-dev 2009-02-14 10:39:41 UTC
(In reply to comment #32)
> Not that I can see.  Maybe someone else has a problem?
> 

Just commit it already. :)
Comment 34 Rémi Cardona (RETIRED) gentoo-dev 2009-02-14 11:04:27 UTC
ACK from me too.
Comment 35 Daniel Gryniewicz (RETIRED) gentoo-dev 2009-02-17 16:08:05 UTC
Committed.  Let the EAPI 2 floodgates open.