1 |
Hello, |
2 |
I am seeking a positive code review on the following change to |
3 |
flag-o-matic.eclass, diff is below (reasons are below that): |
4 |
|
5 |
%% cvs diff |
6 |
Index: flag-o-matic.eclass |
7 |
=================================================================== |
8 |
RCS file: /var/cvsroot/gentoo-x86/eclass/flag-o-matic.eclass,v |
9 |
retrieving revision 1.126 |
10 |
diff -u -r1.126 flag-o-matic.eclass |
11 |
--- flag-o-matic.eclass 3 Nov 2008 05:52:39 -0000 1.126 |
12 |
+++ flag-o-matic.eclass 25 Nov 2008 18:36:04 -0000 |
13 |
@@ -417,7 +417,8 @@ |
14 |
|
15 |
x="" |
16 |
for x in "$@" ; do |
17 |
- test-flag-${comp} "${x}" && flags="${flags}${flags:+ }${x}" |
18 |
+ test-flag-${comp} "${x}" && flags="${flags}${flags:+ }${x}" || \ |
19 |
+ ewarn "removing ${x} because ${comp} rejected it" |
20 |
done |
21 |
|
22 |
echo "${flags}" |
23 |
@@ -656,7 +657,7 @@ |
24 |
ewarn "Appending a library link instruction |
25 |
(${flag}); libraries to link to should not be passed through LDFLAGS" |
26 |
done |
27 |
|
28 |
- export LDFLAGS="${LDFLAGS} $*" |
29 |
+ export LDFLAGS="${LDFLAGS} $(test-flags "$@")" |
30 |
return 0 |
31 |
} |
32 |
|
33 |
Reason: |
34 |
We hit this little gem in Gentoo Prefix when some ebuilds started |
35 |
using flags that non-GNU linkers didn't accept and actually aborted |
36 |
on. For example, the darwin ld does not accept --no-as-needed. So, the |
37 |
initial work-around was to check to see if we had a GNU ld, and only |
38 |
apply if so. (aside, further investigation revealed that GNU and |
39 |
darwin ld are actually pretty non-clever in this regard. For example, |
40 |
the hp-ux linker will just ignore non-valid flags) But this is not |
41 |
very friendly to the lack of Gentoo Prefix developer availability. ;) |
42 |
|
43 |
A convienient side effect of the above patch is that it protects |
44 |
Gentoo users from typos in ebuilds. For example, "append-ldflags |
45 |
--foo" will cause compilation to abort, but with the above patch, a |
46 |
ewarn will be issued instead and compilation will continue. Besides |
47 |
typos, if the GNU ld ever changes in some non-compatible way, there |
48 |
will be less breakage. |
49 |
|
50 |
There will be claims that additional checking to the flags should not |
51 |
be added to Gentoo Linux because it is redundant (Gentoo only uses GNU |
52 |
ld). However, time test-flags "$@" only takes 0m0.017s on my host. I |
53 |
guess this number could be larger on less modern hosts..I don't know. |
54 |
|
55 |
Comments? Good, bad, or otherwise? The Gentoo Prefix team always aims |
56 |
to have as minimal diffs as possible from the gentoo-x86 tree, hence |
57 |
the motivation for this request to review and inclusion into the |
58 |
gentoo-x86 tree. |
59 |
|
60 |
Thanks, |
61 |
Jeremy |