Re: [gentoo-dev] new old eclass - wxwidgets.eclass

[Ryan Hill]

Mart Raudsepp wrote:
> Hello,
>
> On E, 2007-09-24 at 21:14 -0600, Ryan Hill wrote:
>> since everyone is getting into the reviewing mood, i thought it
>> would be a good time to get some opinions on the wxwidgets
>> eclass rewrite i've done.
>
>
> I figured I'd reply to the old review request on the list instead of
> doing this privately, so here we go.
>
> The notes are based on the latest eclass located at
> http://overlays.gentoo.org/dev/dirtyepic/browser/eclass/wxwidgets.eclass
> at current revision 6:
>
> * I just assume the documentation markup for the new way of doing eclass
> documenting is good. The documentation text itself seems mostly good, if
> maybe not so professional. Some nitpicking:

Bah, professionalism is for professionals. ;)
 
> ** Ebuilds are also required set the global -> s/required/required to/?

Yep.
 
> ** The description gives the impression we support installation of
> release/debug side-by-side - don't think this is true. Maybe we should
> think about a way for users to be able to set their own wx-config to a
> debug build, but have the system USE flag dependent. Hmm... Maybe I
> should revise my stance on debug + release co-existance in some form

This is basically what I had previously (though much uglier).  I figured
we might want to rethink it later so I made it simple to re-add again if
we want.  For now though I'd just like to get this out to the public.
 
> ** Once 2.8 is in, we should update the simple usage example to that

For sure.
 
> ** With EAPI=1 SLOT depends can be used now, but as wx is always with a
> version and SLOT matching a way that * wildcard can be used, it's fine
> to not require things to use EAPI=1 indeed I'd say. Just noting.

With EAPI=1 we can also do something like

DEPEND=">=x11-libs/wxGTK-2.6.2:2.6"

which neatly solves our issue with needing a minimum version in a SLOT.

I'll make a note of this in the documentation.
 
> ** In simple usage example I would rather refer to RDEPEND, not DEPEND,
> or both

Hmm?  RDEPEND defaults to DEPEND if not defined but not the other way
around.  I will refer to both though since wxGTK does definitely need
to be in RDEPEND for proper binpkgs.

> * Perhaps we should take care of setting the wxGTK DEPEND and RDEPEND
> ourselves based on a WX_GTK_VER in ebuilds global scope? Might cause
> problems for things using wx conditionally for some optional GUI.
> Talking of which, it seems it already relies on global scope - does that
> work fine with conditional wx usage? need-wxwidgets for that, I guess?
> What's the view on eclasses touching DEPEND/RDEPEND these days, anyway?

I believe this would force everything with an optional wxwidgets USE flag
to require wxGTK.  I never really thought about how the global code affects
the conditional case until now, but considering that all it does is set a
variable if wxGTK is installed it would seem that it could take advantage
of it as well and only call need-wxwidgets if it needs something other than
the default.  Course calling need-wxwidgets is still a good idea.

> * Taking care of DEPEND and RDEPEND in the eclass itself could allow us
> to migrate things over to wxBase package in the future without any
> ebuild migration work, once we fix upstream configure.in to work with a
> system provided wxBase libraries and not have file collisions. However
> this probably requires a global variable for base-ansi/base-unicode
> passing instead of a need-wxwidgets function for that to be possible to
> work out

Yes, that would be a future goal.
 
> * I hate any unconditional compiler flags workarounds, such as
> "append-flags -fno-strict-aliasing". I have some long pending upstream
> work to do to simply fix it in that one header where it makes noise and
> once that finally gets resolved (or maybe it already is in 2.8), we have
> no reason to keep passing this, as to my knowledge it also means less
> optimization chances for gcc. Do we have a plan on how to get rid of
> that CFLAGS append once the user is dealing with a wx library that has
> this fixed?

I believe I added that due to an aliasing bug in the gcc-4.2.0 prerelease.
I think I've since gotten the patch applied to our GCC though so it
shouldn't be a problem to remove.  If we could silence that warning all the
better.

> At least this isn't the configure argument mess we used to have in
> wxlib.eclass where you'd get some important class configured out just
> because it was broken in an old version :)
>
> * Are you sure we want to ewarn unconditionally on all need-wxwidgets
> function calls?

Right, that should be an einfo.  It was originally for debugging but I
found it to be pretty useful in practice.

> * What's with the TODO on _wxerror() and the function?

I thought a broken out error reporting function would save a bunch of
duplicated code, but I ended up with a lot fewer cases than I thought
I would.  I think I might just remove it.
 
> The previous points encompass mostly only things noted while going over
> the code - I have yet to look at it with a logic and interaction sense.
> I figured I'll get this current list off my hands before I get sunk into
> work.
>
>
> Many many thanks for pushing wxGTK forwards while I'm occupied with
> work, gnome stuff... and procrastinating

Thanks for looking at it.  I know you have a lot on your plate.

-- 
                  fonts / wxWindows / gcc-porting / treecleaners
  EFFD 380E 047A 4B51 D2BD  C64F 8AA8 8346 F9A4 0662 (0xF9A40662)
-- 
gentoo-dev@g.o mailing list