In gnome we need to always run elibtoolize to prevent overlinking. We are then running it for ages from gnome2_src_prepare (in gnome2.eclass). This started to be a bit problematic when this snipped was added to libtool.eclass: local outfunc="einfo" [[ -f ${d}/.elibtoolized ]] && outfunc="ewarn" ${outfunc} "Running elibtoolize in: ${d#${WORKDIR}/}/" if [[ ${outfunc} == "ewarn" ]] ; then ewarn " We've already been run in this tree; you should" ewarn " avoid this if possible (perhaps by filing a bug)" fi Then, we started to avoid it changing the order we call eautoreconf, and calling it before gnome2_src_prepare: eautoreconf && gnome2_src_prepare But now, with eapi6, we need to call it again in reverse order to allow us to apply user patches for example. Hence, I was wondering if maybe we could add a GNOME2_EAUTORECONF=yes variable to run eautoreconf instead of elibtoolize avoiding this problem of running elibtoolize two times
This would also help us to use more "PATCHES=()" (from eapi6) without needing to define a src_prepare phase later for running eautoreconf from there
I'm not too hot on adding another global variable to handle this. We can apply user patches (and we do) but simply not bother about eautoreconf as it was left out of the scope of the eapply calls and it is imho fine that it is not magic and it does not try to support all cases. Not being able to use PATCHES more is a bit annoying but we lived without it for a long time and imho the problem is due to the gaps in the implementation of src_default. In any case, I added a section explaining the current requirements for that order in the wiki [1] so there is at least a trace of why we do it this way. [1] https://wiki.gentoo.org/wiki/Project:GNOME/Gnome_Team_Ebuild_Policies#src_prepare
Well, many other eclasses have that kind of variables for the same purpose, that will also allow us to use PATCHES more easily, allow people to get advantage of our eautoreconf runs when we need them and prevent things like this: https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=c37001d20236fcded1ddffbecc9decb4815e3c50 And also is cleaner than try to hidden that we are calling elibtoolize to times running gnome2_src_prepare after eautoreconf Then, I guess prons are more than cons of having one variable (specially now that we don't have GCONF_DEBUG and GNOME2_LA_PUNT is not needed in most cases to be explicitly set as its default value covers all but removal of .la modules). Also having one line with this variable looks to me "hotter" than needing to write a custom src_prepare for trying to handle all this (Please note I am refering to simply run "eautoreconf" instead of "elibtoolize" when the variable is set... I am not suggesting that we add the "magic" from other eclasses like autotools-utils.eclass that were trying to automatically run eautoreconf, as I agree that should be handled in a more general place)
This is not strictly related but something to bear in mind. Technically all autotools-based ebuilds should at least call elibtoolize but relatively few do. Putting the call in src_configure was discussed but this would mean that PMS would need to include all the details about how elibtoolize works. It was suggested that it could be split out into a separate package to avoid this. I tried this as a proof of concept and it worked very well. I hope to wrap it up in a more formal proposal in the near future.
(In reply to Pacho Ramos from comment #1) > This would also help us to use more "PATCHES=()" (from eapi6) without > needing to define a src_prepare phase later for running eautoreconf from > there I just hitted this again: needing to convert and ebuild from using PATCHES to manually running the following due to needing to append a patch requesting eautoreconf: eapply ... eautoreconf gnome2_src_prepare As, otherwise, patches are applied after eautoreconf by gnome2_src_prepare (that we need to call at the end to prevent elibtoolize from running two times)
Created attachment 455142 [details, diff] 1.patch This is my suggestion (thanks to XFCE guys as it's inspired on their eclasses :))
[master 078a4fc] gnome2.eclass: Allow to decide more easily if we can run eautoreconf OR only elibtoolize, this will prevent elibtoolize from being run two times and also allow the honoring of eapply_user patches (#591584), apart of also allowing us to use PATCHES array in more situations. 1 file changed, 19 insertions(+), 3 deletions(-)
I thought we agreed on IRC that this is not the way to go but to make eapply stop being dumb like eapply_user ?
Yes, we did agree sort of to that affect, though Pacho wasn't on IRC, I believe, and we didn't comment here.
If we're going to keep it, I'm not too happy about inheriting autotools from gnome2.eclass; IF we keep it, maybe require the ebuild using GNOME2_EAUTORECONF to inherit it?
What is the problem with using autotools.eclass?
And no, I wasn't in that IRC talk...
And of course I wouldn't agree on that and keeping our current ugly behavior... but well, I am used to all this votings ending with 2 vs 1 ... but following this pattern I guess it will be 2 vs 0 soon
Our problem with it is that it's workarounding issues that are much deeper and in other stuff, so we just patch gnome2.eclass here and leave the bug exist everywhere else. It should be fixed properly, for everything, not have all the eclasses have to grow some sort of custom stuff for avoiding some semi-innocent ewarn. That is, the ugly is in elibtoolize and co, not pre-patch gnome2.eclass. My problem with autotools inherit is that of processing time for everything inheriting gnome2.eclass. I think if we demand this new var to be declared before inherit2 gnome, we can do the inherit autotools conditional in eclass?
(In reply to Mart Raudsepp from comment #14) > My problem with autotools inherit is that of processing time for everything > inheriting gnome2.eclass. I think if we demand this new var to be declared > before inherit2 gnome, we can do the inherit autotools conditional in eclass? Actually a real issue, than this pathological inherit speed, is implicitly doing it for ebuilds even without asking an eautoreconf, so ebuilds start relying on it being, and then when we can remove this stuff, things will break because ebuilds didn't bother to inherit autotools when using something from it themselves (because things just worked for them and didn't fail from inherit autotools missing in the ebuild itself). Relatedly, this new GNOME2_EAUTORECONF is sort of new API now in gnome2.eclass, so if kept longer, once we can remove it, we get to break all sort of overlays relying on it already for avoiding a dubious elibtoolize warning that should be done properly in the lower level eclasses instead. E.g comment #2 as already expressed here, not IRC. I'm sorry we missed to comment here after the patch was attached, with existing disagreements having already been expressed here.
The issues with the interactions or eautoreconf and the patching need to be addressed at EAPI level, then, you should open a bug for that if it doesn't exist (it probably exists as this issue is really old... and never gets fixed because nobody finally agreed on how to implement it at that level...). But, of course, that will be handled, if ever, on a future EAPI that will likely come in some months (or years) Also... talking about workarounding issues when we are workarounding the issue of the double elibtoolize call moving on purpose gnome2_src_prepare to the end for hiding the issue... but also causing the eapply stuff to be broken because of that.... And the funny thing is that we also need to call elibtoolize always as a workaround (as probably that should be run *always* without needing an eclass... but that is another endless topic). (In reply to Mart Raudsepp from comment #14) > My problem with autotools inherit is that of processing time for everything > inheriting gnome2.eclass. I think if we demand this new var to be declared > before inherit2 gnome, we can do the inherit autotools conditional in eclass? Even if I don't know the real measure time comparison between the two approaches, I agree that maybe we could reuse the logic and instead of setting AUTOTOOLS_AUTO_DEPEND we could append autotools.eclass to the inherit. I am not sure why I chose the other way (probably because of mimic other eclasses that are doing the same... I hope I am not forgetting about any issue I tried to avoid :/). It should be as simply as changing that chunk to: --- gnome2.eclass~ 2017-01-15 17:44:32.000000000 +0100 +++ gnome2.eclass 2017-01-16 23:10:22.000000000 +0100 @@ -16,13 +16,8 @@ # Run eautoreconf instead of only elibtoolize GNOME2_EAUTORECONF=${GNOME2_EAUTORECONF:-""} -if [[ ${GNOME2_EAUTORECONF} == 'yes' ]] ; then - AUTOTOOLS_AUTO_DEPEND=yes -else - : ${AUTOTOOLS_AUTO_DEPEND:=no} -fi - -inherit autotools eutils libtool gnome.org gnome2-utils xdg +[[ ${GNOME2_EAUTORECONF} == 'yes' ]] && inherit autotools +inherit eutils libtool gnome.org gnome2-utils xdg case "${EAPI:-0}" in 4|5) That looks, indeed, also shorter and better to me :)
(In reply to Mart Raudsepp from comment #15) > Relatedly, this new GNOME2_EAUTORECONF is sort of new API now in > gnome2.eclass, so if kept longer, once we can remove it, we get to break all > sort of overlays relying on it already for avoiding a dubious elibtoolize > warning that should be done properly in the lower level eclasses instead. > E.g comment #2 as already expressed here, not IRC. > I'm sorry we missed to comment here after the patch was attached, with > existing disagreements having already been expressed here. The way to deprecate the variable is exactly the same as we are doing for other for a long time, when this becomes unneeded (probably on a new eapi fixing this stuff in a more general way), we die as we did with the old GCONF_DEBUG or G2CONF, if not so difficult
yeah, I think we dropped the ball on the global stuff a bit; something more important probably came up :( I think Soap__ had more intimate knowledge of all this elibtoolize nonsense at the global level, so CCing him for potential input and encouragement to file a lower level bug *grin*
(In reply to Mart Raudsepp from comment #18) > yeah, I think we dropped the ball on the global stuff a bit; something more > important probably came up :( > I think Soap__ had more intimate knowledge of all this elibtoolize nonsense > at the global level, so CCing him for potential input and encouragement to > file a lower level bug *grin* If he knows about the more "base" level (I mean, handling all this issues for all the packages), I have always worried why Gentoo is not having a special kind of eclasses for handling also autotools build system (as it's done for cmake and other build systems), that would allow to handle this on a centralized place and our eclasses would simply need to re-use them providing our desired defaults or additions... but that is another topic too :/ Regarding the change in comment #16... I can commit it just now if you want... that should cover the issue of inheritting it unconditionally and I see no issue with that
Guys, if you want elibtoolize done globally at a lower level then what I mentioned in comment #4 is probably the way to go. As I said, it worked very well when I tried it and that was only as a hack, I didn't even put much time into it yet. It was discussed here: https://archives.gentoo.org/gentoo-dev/message/9f967c78d6b8f210ada1e89dd1abe171
(In reply to Pacho Ramos from comment #19) > Regarding the change in comment #16... I can commit it just now if you > want... that should cover the issue of inheritting it unconditionally and I > see no issue with that Yeah, if it's fine to do it this way conditionally, just commit it I guess. MAYBE warn in the documentation that it is likely to go away at some point when the workaround isn't needed anymore, so to be careful when using outside main tree or gnome overlay or something. Then I guess we can just keep it around for now, if can't use the manual ordering via eapply instead of PATCHES and such. And then I just hope a global solution is found rather sooner than later, to get rid of this rather soon :)
[master 4479f85] gnome2.eclass: simplify inheritting on autotools.eclass and not inherit it unconditionally (#591584, thanks to leio for the help) 1 file changed, 2 insertions(+), 7 deletions(-) And: https://wiki.gentoo.org/wiki/Project:GNOME/Gnome_Team_Ebuild_Policies#GNOME2_EAUTORECONF (feel free to improve the wording of the wiki page... I am a bit tired now since it's time to sleep here ;)
(In reply to James Le Cuirot from comment #20) > Guys, if you want elibtoolize done globally at a lower level then what I > mentioned in comment #4 is probably the way to go. As I said, it worked very > well when I tried it and that was only as a hack, I didn't even put much > time into it yet. It was discussed here: > > https://archives.gentoo.org/gentoo-dev/message/ > 9f967c78d6b8f210ada1e89dd1abe171 Regarding this, I am really all for that... but I am not sure about how can I help as I am not in the involved teams... feel free to tell me via mail if you think I can contribute on something :)
