1 |
On Sun, 24 Mar 2013 16:41:15 +0100 |
2 |
Michał Górny <mgorny@g.o> wrote: |
3 |
|
4 |
> On Sun, 24 Mar 2013 16:14:52 +0100 |
5 |
> Alexis Ballier <aballier@g.o> wrote: |
6 |
> |
7 |
> > On Sat, 23 Mar 2013 17:26:38 +0100 |
8 |
> > Michał Górny <mgorny@g.o> wrote: |
9 |
> > |
10 |
> > > --- |
11 |
> > > gx86/eclass/autotools-multilib.eclass | 82 |
12 |
> > > +++++++++++++++++++++++++++++++++++ 1 file changed, 82 |
13 |
> > > insertions(+) |
14 |
> > > |
15 |
> > > diff --git a/gx86/eclass/autotools-multilib.eclass |
16 |
> > > b/gx86/eclass/autotools-multilib.eclass index d7372b0..c65c777 |
17 |
> > > 100644 --- a/gx86/eclass/autotools-multilib.eclass |
18 |
> > > +++ b/gx86/eclass/autotools-multilib.eclass |
19 |
> > |
20 |
> > |
21 |
> > why not multilib-build ? |
22 |
> |
23 |
> Because it has two parts which have to be called at the right time. |
24 |
> It doesn't have a public API yet, so I'm putting it where I can test |
25 |
> it sanely. |
26 |
|
27 |
|
28 |
Well, it has nothing to do with autotools :/ |
29 |
|
30 |
I'm not sure how to do it properly though... the function should be |
31 |
"shared" but multilib-build doesn't export any phase function by design. |
32 |
Maybe put the function and the variable documentation in |
33 |
multilib-build, call it from autotools-multilib and add documentation |
34 |
to autotools-multilib like: "this eclass supports header wrapping, see |
35 |
multilib-build documentation about MULTILIB_WRAPPED_HEADERS variable" |
36 |
|
37 |
[...] |
38 |
> > I'd just go for paths relative to $EPREFIX/usr/include (or |
39 |
> > $ED/usr/include) for MULTILIB_WRAPPED_HEADERS. That would simplify |
40 |
> > the code. |
41 |
> |
42 |
> That's true. But on the other hand, that would introduce yet another |
43 |
> root for paths which would probably end up being confusing. |
44 |
> Using /usr/include/... feels more natural IMO. |
45 |
|
46 |
if you have to strip /usr/include to use the input, then just don't |
47 |
require /usr/include as input ;) |
48 |
|
49 |
> And of course, it leaves the possibility of supporting other locations |
50 |
> in the future. |
51 |
|
52 |
But you're right: There's no need to unnecessarily ban corner cases. |
53 |
|
54 |
|
55 |
> > > + # and then usr/include |
56 |
> > > + f=${f#usr/include/} |
57 |
> > > + |
58 |
> > > + local dir=${f%/*} |
59 |
> > > + |
60 |
> > > + # $CHOST shall be set by multilib_toolchain_setup |
61 |
> > > + dodir "/tmp/multilib-include/${CHOST}/${dir}" |
62 |
> > > + mv "${ED}/usr/include/${f}" |
63 |
> > > "${ED}/tmp/multilib-include/${CHOST}/${dir}/" || die + |
64 |
> > |
65 |
> > why not use $T rather than $ED/tmp ? |
66 |
> |
67 |
> I prefer using $D rather than $T for files which are intended to be |
68 |
> installed. Purely theoretically, $T could be on another filesystem |
69 |
> than $D. Then, moving the file back and forth could cause it to lose |
70 |
> some of the metadata -- and will be slower, of course. |
71 |
|
72 |
and if some file is left behind it will install stuff in /tmp :/ |
73 |
|
74 |
I usually tend to consider $D as a write-only file system and consider |
75 |
it cleaner that way, but that's true, moving files around is likely |
76 |
cleaner if done within $D |
77 |
|
78 |
> |
79 |
> > > + if [[ ! -f ${ED}/tmp/multilib-include/${f} ]]; |
80 |
> > > then |
81 |
> > > + dodir "/tmp/multilib-include/${dir}" |
82 |
> > > + cat > "${ED}/tmp/multilib-include/${f}" |
83 |
> > > <<_EOF_ || die +/* This file is auto-generated by |
84 |
> > > autotools-multilib.eclass |
85 |
> > > + * as a multilib-friendly wrapper to /${f}. For the original |
86 |
> > > content, |
87 |
> > > + * please see the files that are #included below. |
88 |
> > > + */ |
89 |
> > > +_EOF_ |
90 |
> > > + fi |
91 |
> > > + |
92 |
> > > + local defs |
93 |
> > > + case "${ABI}" in |
94 |
> > |
95 |
> > didn't you say $ABI may have name collisions? |
96 |
> > considering the code below, it seems much safer to match on $CHOST |
97 |
> |
98 |
> Yes, that's why I've put the whole matching inline instead of |
99 |
> introducing a function to obtain the values. The current design |
100 |
> of the eclass -- which was quite stupid of me -- doesn't provide |
101 |
> a way to access that ABI_* thing. I am going to change that |
102 |
> in the future but don't want to step into two different issues |
103 |
> at a time. |
104 |
> |
105 |
> At a first glance, $CHOST sounds like a neat idea. But I'm afraid it's |
106 |
> not a really safe choice. From a quick glance, CHOST values for x86 |
107 |
> were: |
108 |
> |
109 |
> i*86-pc-linux-gnu |
110 |
> x86_64-pc-linux-gnu |
111 |
> x86_64-pc-linux-gnux32 |
112 |
> |
113 |
> I feel like there's a lot of variables here, ends up in a bit ugly |
114 |
> pattern matching IMO. |
115 |
|
116 |
Well, almost every matching I've seen uses CHOST, because it's the |
117 |
safest choice. For your example: |
118 |
i?86*) ;; |
119 |
x86_64*gnux32) ;; |
120 |
x86_64*) ;; |
121 |
|
122 |
I haven't checked the details, but that'd be even better: freebsd |
123 |
defines a different ABI (which is correct because it's incompatible) |
124 |
but the #ifery for this wrapper is the same, so with this CHOST, |
125 |
matching freebsd (or other OSes) comes for free. In the end it'll be |
126 |
shorter :) |
127 |
|
128 |
|
129 |
> > [...] |
130 |
> > |
131 |
> > It'd be nice to have an attempt to support all the ABIs gentoo |
132 |
> > supports in that file: I'd prefer to spot possible problems with |
133 |
> > this solution vs. sedding a template for example to be seen before |
134 |
> > having to rewrite it. |
135 |
> > For example, IIRC, ppc64 defines both __powerpc__ and __powerpc64__, |
136 |
> > the natural way would be: |
137 |
> > #if defined(__powerpc64__) |
138 |
> > ppc64 stuff |
139 |
> > #elif defined(__powerpc__) |
140 |
> > ppc stuff |
141 |
> > #endif |
142 |
> > |
143 |
> > with your approach that'd be: |
144 |
> > #if defined(__powerpc__) && !defined(__powerpc64__) |
145 |
> > ppc stuff |
146 |
> > #elif defined(__powerpc64__) |
147 |
> > ppc64 stuff |
148 |
> > #endif |
149 |
> |
150 |
> I had the same problem with x32. I've chosen the solution which worked |
151 |
> and was easy to implement. |
152 |
|
153 |
it's easy too :p |
154 |
|
155 |
|
156 |
> > doing with a template has its disadvantages but allows more |
157 |
> > flexibility in how the #ifery is written; I don't want to realize |
158 |
> > your approach cannot deal with some weird arches after it has been |
159 |
> > deployed. |
160 |
> |
161 |
> To be honest, I can't really imagine how we could work with a template |
162 |
> here. It would at least require the function to be aware of other ABIs |
163 |
> which I really wanted to avoid. Of course, then there's the whole code |
164 |
> to handle it, and honestly I don't have a vision how it would work. |
165 |
|
166 |
Template (short version): |
167 |
|
168 |
#if defined(__powerpc64__) |
169 |
ABI=ppc64 |
170 |
#elif defined(__powerpc__) |
171 |
ABI=ppc |
172 |
#else |
173 |
#error "fail" |
174 |
#endif |
175 |
|
176 |
When installing a header for $ABI: |
177 |
sed -e "s/ABI=${ABI}\$/#include <foo>/" |
178 |
|
179 |
After installing everything: |
180 |
sed -e "s/ABI=.*/#error \"sorry, no support for \0\"/" |
181 |
|
182 |
|
183 |
See, there's no need to be aware of any ABI and you even get more |
184 |
meaningful #errors since you now know the #if path it used (because of |
185 |
the \0) :) |
186 |
|
187 |
The clear disadvantage of this approach is that every wrapped header |
188 |
will have a long list of #if #elif #endif for every gentoo ABI, most of |
189 |
the paths through these conditions will be invalid, and it will be |
190 |
harder to read. |
191 |
I really prefer generating the wrapper on the fly like you do but I |
192 |
want to be sure it's good enough for all our use cases. |
193 |
|
194 |
Alexis. |