From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) by finch.gentoo.org (Postfix) with ESMTP id 946AA138010 for ; Sun, 24 Mar 2013 20:01:20 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 482AAE088A; Sun, 24 Mar 2013 20:01:16 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) (using TLSv1 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 4ED1BE07FB for ; Sun, 24 Mar 2013 20:01:15 +0000 (UTC) Received: from portable (AMontpellier-651-1-406-46.w81-251.abo.wanadoo.fr [81.251.237.46]) (using SSLv3 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) (Authenticated sender: aballier) by smtp.gentoo.org (Postfix) with ESMTPSA id 89FF4335DF3; Sun, 24 Mar 2013 20:01:13 +0000 (UTC) Date: Sun, 24 Mar 2013 21:01:08 +0100 From: Alexis Ballier To: gentoo-dev@lists.gentoo.org Cc: mgorny@gentoo.org Subject: Re: [gentoo-dev] [PATCH 2/2] Support wrapping headers for multilib ABIs. Message-ID: <20130324210108.4a408c31@portable> In-Reply-To: <20130324164115.1af3b499@pomiocik.lan> References: <20130323172532.1b1100e2@pomiocik.lan> <1364055998-23674-2-git-send-email-mgorny@gentoo.org> <20130324161452.63a55140@portable> <20130324164115.1af3b499@pomiocik.lan> Organization: Gentoo X-Mailer: Claws Mail 3.9.0 (GTK+ 2.24.17; x86_64-pc-linux-gnu) Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-dev@lists.gentoo.org Reply-to: gentoo-dev@lists.gentoo.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Archives-Salt: cc26ef97-8bb6-4cd8-95c4-2820736922f1 X-Archives-Hash: 2188138d88901fa3063cc0dc724f1ba8 On Sun, 24 Mar 2013 16:41:15 +0100 Micha=C5=82 G=C3=B3rny wrote: > On Sun, 24 Mar 2013 16:14:52 +0100 > Alexis Ballier wrote: >=20 > > On Sat, 23 Mar 2013 17:26:38 +0100 > > Micha=C5=82 G=C3=B3rny wrote: > >=20 > > > --- > > > gx86/eclass/autotools-multilib.eclass | 82 > > > +++++++++++++++++++++++++++++++++++ 1 file changed, 82 > > > insertions(+) > > >=20 > > > diff --git a/gx86/eclass/autotools-multilib.eclass > > > b/gx86/eclass/autotools-multilib.eclass index d7372b0..c65c777 > > > 100644 --- a/gx86/eclass/autotools-multilib.eclass > > > +++ b/gx86/eclass/autotools-multilib.eclass > >=20 > >=20 > > why not multilib-build ? >=20 > Because it has two parts which have to be called at the right time. > It doesn't have a public API yet, so I'm putting it where I can test > it sanely. Well, it has nothing to do with autotools :/ I'm not sure how to do it properly though... the function should be "shared" but multilib-build doesn't export any phase function by design. Maybe put the function and the variable documentation in multilib-build, call it from autotools-multilib and add documentation to autotools-multilib like: "this eclass supports header wrapping, see multilib-build documentation about MULTILIB_WRAPPED_HEADERS variable" [...] > > I'd just go for paths relative to $EPREFIX/usr/include (or > > $ED/usr/include) for MULTILIB_WRAPPED_HEADERS. That would simplify > > the code. >=20 > That's true. But on the other hand, that would introduce yet another > root for paths which would probably end up being confusing. > Using /usr/include/... feels more natural IMO. if you have to strip /usr/include to use the input, then just don't require /usr/include as input ;) > And of course, it leaves the possibility of supporting other locations > in the future. But you're right: There's no need to unnecessarily ban corner cases. > > > + # and then usr/include > > > + f=3D${f#usr/include/} > > > + > > > + local dir=3D${f%/*} > > > + > > > + # $CHOST shall be set by multilib_toolchain_setup > > > + dodir "/tmp/multilib-include/${CHOST}/${dir}" > > > + mv "${ED}/usr/include/${f}" > > > "${ED}/tmp/multilib-include/${CHOST}/${dir}/" || die + > >=20 > > why not use $T rather than $ED/tmp ? >=20 > I prefer using $D rather than $T for files which are intended to be > installed. Purely theoretically, $T could be on another filesystem > than $D. Then, moving the file back and forth could cause it to lose > some of the metadata -- and will be slower, of course. and if some file is left behind it will install stuff in /tmp :/ I usually tend to consider $D as a write-only file system and consider it cleaner that way, but that's true, moving files around is likely cleaner if done within $D >=20 > > > + if [[ ! -f ${ED}/tmp/multilib-include/${f} ]]; > > > then > > > + dodir "/tmp/multilib-include/${dir}" > > > + cat > "${ED}/tmp/multilib-include/${f}" > > > <<_EOF_ || die +/* This file is auto-generated by > > > autotools-multilib.eclass > > > + * as a multilib-friendly wrapper to /${f}. For the original > > > content, > > > + * please see the files that are #included below. > > > + */ > > > +_EOF_ > > > + fi > > > + > > > + local defs > > > + case "${ABI}" in > >=20 > > didn't you say $ABI may have name collisions? > > considering the code below, it seems much safer to match on $CHOST >=20 > Yes, that's why I've put the whole matching inline instead of > introducing a function to obtain the values. The current design > of the eclass -- which was quite stupid of me -- doesn't provide > a way to access that ABI_* thing. I am going to change that > in the future but don't want to step into two different issues > at a time. >=20 > At a first glance, $CHOST sounds like a neat idea. But I'm afraid it's > not a really safe choice. From a quick glance, CHOST values for x86 > were: >=20 > i*86-pc-linux-gnu > x86_64-pc-linux-gnu > x86_64-pc-linux-gnux32 >=20 > I feel like there's a lot of variables here, ends up in a bit ugly > pattern matching IMO. Well, almost every matching I've seen uses CHOST, because it's the safest choice. For your example:=20 i?86*) ;; x86_64*gnux32) ;; x86_64*) ;; I haven't checked the details, but that'd be even better: freebsd defines a different ABI (which is correct because it's incompatible) but the #ifery for this wrapper is the same, so with this CHOST, matching freebsd (or other OSes) comes for free. In the end it'll be shorter :) > > [...] > >=20 > > It'd be nice to have an attempt to support all the ABIs gentoo > > supports in that file: I'd prefer to spot possible problems with > > this solution vs. sedding a template for example to be seen before > > having to rewrite it. > > For example, IIRC, ppc64 defines both __powerpc__ and __powerpc64__, > > the natural way would be: > > #if defined(__powerpc64__) > > ppc64 stuff > > #elif defined(__powerpc__) > > ppc stuff > > #endif > >=20 > > with your approach that'd be: > > #if defined(__powerpc__) && !defined(__powerpc64__) > > ppc stuff > > #elif defined(__powerpc64__) > > ppc64 stuff > > #endif >=20 > I had the same problem with x32. I've chosen the solution which worked > and was easy to implement. it's easy too :p > > doing with a template has its disadvantages but allows more > > flexibility in how the #ifery is written; I don't want to realize > > your approach cannot deal with some weird arches after it has been > > deployed. >=20 > To be honest, I can't really imagine how we could work with a template > here. It would at least require the function to be aware of other ABIs > which I really wanted to avoid. Of course, then there's the whole code > to handle it, and honestly I don't have a vision how it would work. Template (short version): #if defined(__powerpc64__) ABI=3Dppc64 #elif defined(__powerpc__) ABI=3Dppc #else #error "fail" #endif When installing a header for $ABI: sed -e "s/ABI=3D${ABI}\$/#include /" After installing everything: sed -e "s/ABI=3D.*/#error \"sorry, no support for \0\"/" See, there's no need to be aware of any ABI and you even get more meaningful #errors since you now know the #if path it used (because of the \0) :) The clear disadvantage of this approach is that every wrapped header will have a long list of #if #elif #endif for every gentoo ABI, most of the paths through these conditions will be invalid, and it will be harder to read. I really prefer generating the wrapper on the fly like you do but I want to be sure it's good enough for all our use cases. Alexis.