Hello,
I am seeking a positive code review on the following change to
flag-o-matic.eclass, diff is below (reasons are below that):

%% cvs diff
Index: flag-o-matic.eclass
===================================================================
RCS file: /var/cvsroot/gentoo-x86/eclass/flag-o-matic.eclass,v
retrieving revision 1.126
diff -u -r1.126 flag-o-matic.eclass
--- flag-o-matic.eclass 3 Nov 2008 05:52:39 -0000       1.126
+++ flag-o-matic.eclass 25 Nov 2008 18:36:04 -0000
@@ -417,7 +417,8 @@

       x=""
       for x in "$@" ; do
-               test-flag-${comp} "${x}" && flags="${flags}${flags:+ }${x}"
+               test-flag-${comp} "${x}" && flags="${flags}${flags:+ }${x}" || \
+                       ewarn "removing ${x} because ${comp} rejected it"
       done

       echo "${flags}"
@@ -656,7 +657,7 @@
                       ewarn "Appending a library link instruction
(${flag}); libraries to link to should not be passed through LDFLAGS"
       done

-       export LDFLAGS="${LDFLAGS} $*"
+       export LDFLAGS="${LDFLAGS} $(test-flags "$@")"
       return 0
 }

Reason:
We hit this little gem in Gentoo Prefix when some ebuilds started
using flags that non-GNU linkers didn't accept and actually aborted
on. For example, the darwin ld does not accept --no-as-needed. So, the
initial work-around was to check to see if we had a GNU ld, and only
apply if so. (aside, further investigation revealed that GNU and
darwin ld are actually pretty non-clever in this regard. For example,
the hp-ux linker will just ignore non-valid flags) But this is not
very friendly to the lack of Gentoo Prefix developer availability. ;)

A convienient side effect of the above patch is that it protects
Gentoo users from typos in ebuilds. For example, "append-ldflags
--foo" will cause compilation to abort, but with the above patch, a
ewarn will be issued instead and compilation will continue. Besides
typos, if the GNU ld ever changes in some non-compatible way, there
will be less breakage.

There will be claims that additional checking to the flags should not
be added to Gentoo Linux because it is redundant (Gentoo only uses GNU
ld). However, time test-flags "$@" only takes 0m0.017s on my host. I
guess this number could be larger on less modern hosts..I don't know.

Comments? Good, bad, or otherwise? The Gentoo Prefix team always aims
to have as minimal diffs as possible from the gentoo-x86 tree, hence
the motivation for this request to review and inclusion into the
gentoo-x86 tree.

Thanks,
Jeremy

Reply via email to