[PATCH] some fixes for waf issues

[Jelmer Vernooij]

On Thu, Jan 09, 2014 at 09:14:24AM +1300, Andrew Bartlett wrote:
> On Wed, 2014-01-08 at 20:40 +0100, Christian Ambach wrote:
> > Am 12.12.13 23:32, schrieb Jelmer Vernooij:
> > 
> > thank you very much for your review comments.
> > 
> > >> -        conf.DEFINE('DEFAULT_DOS_CHARSET', default_dos_charset, quote=True)
> > >> -        conf.DEFINE('DEFAULT_UNIX_CHARSET', default_unix_charset, quote=True)
> > >> +        if default_dos_charset is False or default_unix_charset is False:
> > >> +        # we found iconv, but it failed to convert anything (e.g. on AIX)
> > >> +            conf.undefine('HAVE_NATIVE_ICONV');
> > >> +            conf.DEFINE('DEFAULT_DOS_CHARSET', "ASCII", quote=True)
> > >> +            conf.DEFINE('DEFAULT_UNIX_CHARSET', "UTF8", quote=True)
> > > ^^^  You might want to keep the code as is, but just set
> > > default_dos_charset = "ASCII" and default_unix_charset = "UTF-8" here.
> > 
> > Changed patch is in the attached set.
> > 
> > >> Subject: [PATCH 2/4] waf:lib/replace fix up libintl related checks
> > >>
> > >> checklibc=True and link=True gives back false-positives
> > >> on AIX for gettext/dgettext and so the build fails later when
> > >> libintl.h is not available
> > >>
> > >> So let's check this very pedantically
> > >
> > > My understanding is that this makes us skip the link step, which would
> > > actually make the check less pedantic rather than more (since we
> > > wouldn't see any potential linking issues).
> > >
> > > Isn't this just a bug in CHECK_FUNCS_IN when link=True is set?
> > 
> > I am not sure how we should correctly deal with this.
> > Examining the configure logs on AIX I we might want to change our
> > detection strategy:
> > The special case on AIX is that libintl.a exists, but libintl.h not.
> > Without the link check, waf fails to detect gettext and friends because
> > there is no header around that would declare them when it tries to
> > compile code like this:
> > int main(void){void *__x = (void *)dgettext;return (int)__x;return 0;}
> > 
> > When enabling the link check (that is enabled by default), it compiles
> > and links the following code:
> > #define gettext __fake__gettext
> > [...]
> > #undef gettext
> > #if defined __stub_gettext || defined __stub___gettext
> > #error "bad glibc stub"
> > #endif
> > extern char gettext();
> > int main() { return gettext(); }
> > 
> > This compiles (as gettext is declared) and links (as the symbol can be
> > found in libintl.a).
> > The bad thing for the "real" code is that we still do not have
> > prototypes around and so compilation fails (because those functions do
> > not return int, as assumed when the prototype is lacking).
> > This might also happen with other libraries we depend on, but for some
> > cases we need this functionality (e.g. when accessing functions in the
> > Kerberos libs that are not declared in the public headers).
> > 
> > I see two options here:
> > 1. refine the configuration checks so they fail if no prototype could be
> > found, a positive link check should not be good enough
> > 2. declare the prototypes in lib/replace when they are missing (as for
> > fdatasync)
> 
> We really should avoid that, except where we are very, very sure of the
> prototype. 
> 
> > I would lean towards option 2 as it is less intrusive than changing
> > the behavior of configure checks for everything that we check for.
> > This is what you can find in the attached patchset and what is reported
> > to fix the build on AIX by the bug reporter.
> 
> I think 1 is the better option, unless there is a very pressing need for
> intl on AIX and really no way to install the correct headers. 
+1 on option 1.

Cheers,

Jelmer