1 |
On Sun, 24 Mar 2013 16:14:52 +0100 |
2 |
Alexis Ballier <aballier@g.o> wrote: |
3 |
|
4 |
> On Sat, 23 Mar 2013 17:26:38 +0100 |
5 |
> Michał Górny <mgorny@g.o> wrote: |
6 |
> |
7 |
> > --- |
8 |
> > gx86/eclass/autotools-multilib.eclass | 82 |
9 |
> > +++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) |
10 |
> > |
11 |
> > diff --git a/gx86/eclass/autotools-multilib.eclass |
12 |
> > b/gx86/eclass/autotools-multilib.eclass index d7372b0..c65c777 100644 |
13 |
> > --- a/gx86/eclass/autotools-multilib.eclass |
14 |
> > +++ b/gx86/eclass/autotools-multilib.eclass |
15 |
> |
16 |
> |
17 |
> why not multilib-build ? |
18 |
|
19 |
Because it has two parts which have to be called at the right time. |
20 |
It doesn't have a public API yet, so I'm putting it where I can test it |
21 |
sanely. |
22 |
|
23 |
> > @@ -33,6 +33,28 @@ inherit autotools-utils multilib-build |
24 |
> > |
25 |
> > EXPORT_FUNCTIONS src_prepare src_configure src_compile src_test |
26 |
> > src_install |
27 |
> > +# @ECLASS-VARIABLE: MULTILIB_WRAPPED_HEADERS |
28 |
> > +# @DESCRIPTION: |
29 |
> > +# A list of headers to wrap for multilib support. The listed headers |
30 |
> > +# will be moved to a non-standard location and replace with a file |
31 |
> > +# including them conditionally to current ABI. |
32 |
> > +# |
33 |
> > +# This variable has to be a bash array. Paths shall be relative to |
34 |
> > +# installation root (${D}), and name regular files. Recursive |
35 |
> > wrapping +# is not supported. |
36 |
> > +# |
37 |
> > +# Please note that header wrapping is *discouraged*. It is preferred |
38 |
> > to +# install all headers in a subdirectory of libdir and use |
39 |
> > pkg-config to +# locate the headers. Some C preprocessors will not |
40 |
> > work with wrapped +# headers. |
41 |
> > +# |
42 |
> > +# Example: |
43 |
> > +# @CODE |
44 |
> > +# MULTILIB_WRAPPED_HEADERS=( |
45 |
> > +# /usr/include/foobar/config.h |
46 |
> > +# ) |
47 |
> > +# @CODE |
48 |
> > + |
49 |
> > autotools-multilib_src_prepare() { |
50 |
> > autotools-utils_src_prepare "${@}" |
51 |
> > } |
52 |
> > @@ -49,13 +71,73 @@ autotools-multilib_src_test() { |
53 |
> > multilib_foreach_abi autotools-utils_src_test "${@}" |
54 |
> > } |
55 |
> > |
56 |
> > +_autotools-multilib_wrap_headers() { |
57 |
> > + debug-print-function ${FUNCNAME} "$@" |
58 |
> > + local f |
59 |
> > + |
60 |
> > + for f in "${MULTILIB_WRAPPED_HEADERS[@]}"; do |
61 |
> > + # drop leading slash if it's there |
62 |
> > + f=${f#/} |
63 |
> > + |
64 |
> > + if [[ ${f} != usr/include/* ]]; then |
65 |
> > + die "Wrapping headers outside |
66 |
> > of /usr/include is not supported at the moment." |
67 |
> > + fi |
68 |
> |
69 |
> Do you really want to support this at some point? Why? |
70 |
|
71 |
Honestly? No. But if people need it, I don't see much of a problem to |
72 |
add the support in the future. |
73 |
|
74 |
> I'd just go for paths relative to $EPREFIX/usr/include (or |
75 |
> $ED/usr/include) for MULTILIB_WRAPPED_HEADERS. That would simplify the |
76 |
> code. |
77 |
|
78 |
That's true. But on the other hand, that would introduce yet another |
79 |
root for paths which would probably end up being confusing. |
80 |
Using /usr/include/... feels more natural IMO. |
81 |
|
82 |
And of course, it leaves the possibility of supporting other locations |
83 |
in the future. |
84 |
|
85 |
> > + # and then usr/include |
86 |
> > + f=${f#usr/include/} |
87 |
> > + |
88 |
> > + local dir=${f%/*} |
89 |
> > + |
90 |
> > + # $CHOST shall be set by multilib_toolchain_setup |
91 |
> > + dodir "/tmp/multilib-include/${CHOST}/${dir}" |
92 |
> > + mv "${ED}/usr/include/${f}" |
93 |
> > "${ED}/tmp/multilib-include/${CHOST}/${dir}/" || die + |
94 |
> |
95 |
> why not use $T rather than $ED/tmp ? |
96 |
|
97 |
I prefer using $D rather than $T for files which are intended to be |
98 |
installed. Purely theoretically, $T could be on another filesystem than |
99 |
$D. Then, moving the file back and forth could cause it to lose some |
100 |
of the metadata -- and will be slower, of course. |
101 |
|
102 |
> > + if [[ ! -f ${ED}/tmp/multilib-include/${f} ]]; then |
103 |
> > + dodir "/tmp/multilib-include/${dir}" |
104 |
> > + cat > "${ED}/tmp/multilib-include/${f}" |
105 |
> > <<_EOF_ || die +/* This file is auto-generated by |
106 |
> > autotools-multilib.eclass |
107 |
> > + * as a multilib-friendly wrapper to /${f}. For the original content, |
108 |
> > + * please see the files that are #included below. |
109 |
> > + */ |
110 |
> > +_EOF_ |
111 |
> > + fi |
112 |
> > + |
113 |
> > + local defs |
114 |
> > + case "${ABI}" in |
115 |
> |
116 |
> didn't you say $ABI may have name collisions? |
117 |
> considering the code below, it seems much safer to match on $CHOST |
118 |
|
119 |
Yes, that's why I've put the whole matching inline instead of |
120 |
introducing a function to obtain the values. The current design |
121 |
of the eclass -- which was quite stupid of me -- doesn't provide |
122 |
a way to access that ABI_* thing. I am going to change that |
123 |
in the future but don't want to step into two different issues |
124 |
at a time. |
125 |
|
126 |
At a first glance, $CHOST sounds like a neat idea. But I'm afraid it's |
127 |
not a really safe choice. From a quick glance, CHOST values for x86 |
128 |
were: |
129 |
|
130 |
i*86-pc-linux-gnu |
131 |
x86_64-pc-linux-gnu |
132 |
x86_64-pc-linux-gnux32 |
133 |
|
134 |
I feel like there's a lot of variables here, ends up in a bit ugly |
135 |
pattern matching IMO. |
136 |
|
137 |
> [...] |
138 |
> |
139 |
> It'd be nice to have an attempt to support all the ABIs gentoo supports |
140 |
> in that file: I'd prefer to spot possible problems with this solution |
141 |
> vs. sedding a template for example to be seen before having to rewrite |
142 |
> it. |
143 |
> For example, IIRC, ppc64 defines both __powerpc__ and __powerpc64__, |
144 |
> the natural way would be: |
145 |
> #if defined(__powerpc64__) |
146 |
> ppc64 stuff |
147 |
> #elif defined(__powerpc__) |
148 |
> ppc stuff |
149 |
> #endif |
150 |
> |
151 |
> with your approach that'd be: |
152 |
> #if defined(__powerpc__) && !defined(__powerpc64__) |
153 |
> ppc stuff |
154 |
> #elif defined(__powerpc64__) |
155 |
> ppc64 stuff |
156 |
> #endif |
157 |
|
158 |
I had the same problem with x32. I've chosen the solution which worked |
159 |
and was easy to implement. |
160 |
|
161 |
> doing with a template has its disadvantages but allows more flexibility |
162 |
> in how the #ifery is written; I don't want to realize your approach |
163 |
> cannot deal with some weird arches after it has been deployed. |
164 |
|
165 |
To be honest, I can't really imagine how we could work with a template |
166 |
here. It would at least require the function to be aware of other ABIs |
167 |
which I really wanted to avoid. Of course, then there's the whole code |
168 |
to handle it, and honestly I don't have a vision how it would work. |
169 |
|
170 |
-- |
171 |
Best regards, |
172 |
Michał Górny |