Home | Docs | Forums | Lists | Bugs | Planet | Store | GMN | Get Gentoo!
Not eligible to see or edit group visibility for this bug.
View Bug Activity | Format For Printing | XML | Clone This Bug
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.
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..
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 :/
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.
Created an attachment (id=166803) [details] 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.
Problem with hack in comment #2 is that it'll break as soon as gnome2.eclass starts supporting EAPI=2, I think
Created an attachment (id=166812) [details] 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
(In reply to comment #4) > Created an attachment (id=166803) [edit] [details] > 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? :)
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 :)
(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)
(In reply to comment #7) > (In reply to comment #4) > > Created an attachment (id=166803) [edit] [details] > > 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.
(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)
Created an attachment (id=167240) [details] 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.
As an additional note, you probably don't need to export src_unpack() and src_compile() in EAPI=2
Created an attachment (id=179325) [details] gnome2.eclass-EAPI2.patch Updated Jorge's patch with Olivier suggestion. Gnome team, can I commit that?
That actually looks good. Peter: If you've tried it with a EAPI=2 and a EAPI<2, go ahead and commit.
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.
(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.
> 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
(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.
@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 ?
(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.
(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.
(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?
(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.
I think that means Jorge's patch is either suitable for inclusion or a good base for the final one?
(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.
(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.
(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.
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)
(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.
Created an attachment (id=181390) [details] Naive patch for EAPI-2 Is there any reason why a naive patch such as this one cannot be committed?
Not that I can see. Maybe someone else has a problem?
(In reply to comment #32) > Not that I can see. Maybe someone else has a problem? > Just commit it already. :)
ACK from me too.
Committed. Let the EAPI 2 floodgates open.
