[PATCH] some fixes for waf issues

[Jelmer Vernooij]

On Thu, Dec 12, 2013 at 10:26:02PM +0100, Christian Ambach wrote:
> From 8d14e1314f67849e9813420f0993ead0903cbf30 Mon Sep 17 00:00:00 2001
> From: Christian Ambach <ambi at samba.org>
> Date: Wed, 4 Dec 2013 22:50:11 +0100
> Subject: [PATCH 1/4] waf: improve iconv checks
> 
> there are broken iconv implementations around (e.g. on AIX) that you
> can compile against but that refuse any mapping requests
> 
> make sure we do the same as the autoconf-based build did and
> fall back to our own code
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=10308

Reviewed-By: Jelmer Vernooij <jelmer at samba.org>

> diff --git a/source3/build/charset.py b/source3/build/charset.py
> index 44852a6..cbbb320 100644
> --- a/source3/build/charset.py
> +++ b/source3/build/charset.py
> @@ -33,8 +33,14 @@ def CHECK_SAMBA3_CHARSET(conf, crossbuild=False):
>  	    default_unix_charset="UTF-8"
>              # TODO: this used to warn about the set charset on cross builds
>  
> -        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.

> +        else:
> +            conf.DEFINE('DEFAULT_DOS_CHARSET', default_dos_charset, quote=True)
> +            conf.DEFINE('DEFAULT_UNIX_CHARSET', default_unix_charset, quote=True)

> From a310d6fbdca7861c07d0bab8d4840ef0ff3e6095 Mon Sep 17 00:00:00 2001
> From: Christian Ambach <ambi at samba.org>
> Date: Thu, 21 Nov 2013 23:02:38 +0100
> 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?

> From 8208249ce57ffb5ed07bb133953ab124bf51fd00 Mon Sep 17 00:00:00 2001
> From: Christian Ambach <ambi at samba.org>
> Date: Thu, 12 Dec 2013 22:10:36 +0100
> Subject: [PATCH 3/4] waf:lib/replace fix gettext detection
> 
> if the user has specified a path for gettext, add it to CFLAGS and LDFLAGS

> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=9911
> Signed-off-by: Christian Ambach <ambi at samba.org>
> ---
>  lib/replace/wscript | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/replace/wscript b/lib/replace/wscript
> index 3e43366..e0728f9 100644
> --- a/lib/replace/wscript
> +++ b/lib/replace/wscript
> @@ -365,6 +365,10 @@ removeea setea
>  
>      conf.env.intl_libs=''
>      if not Options.options.disable_gettext:
> +        # any extra path given to look at?
> +        if Options.options.gettext_location:
> +           conf.env['CFLAGS'].extend("-I%s" % Options.options.gettext_location);
> +           conf.env['LDFLAGS'].extend("-L%s" % Options.options.gettext_location);
^^^ This seems wrong. Shouldn't it be .extend(["-I..."]) or
.append("-I...") ?

> From ba28292b0185e008db886a3125d91cacb43fb758 Mon Sep 17 00:00:00 2001
> From: Christian Ambach <ambi at samba.org>
> Date: Thu, 12 Dec 2013 22:12:07 +0100
> Subject: [PATCH 4/4] waf:lib/replace change detection of gettext
> 
> convert this to an automatic check: if no option is given, try to find gettext
> and use it if found
> if user has specified --with-gettext, then bail out it it could not be found
> in case of --without-gettext, skip all gettext related configure checks
> 
> Bug:ihttps://bugzilla.samba.org/show_bug.cgi?id=9911
> Signed-off-by: Christian Ambach <ambi at samba.org>

Reviewed-By: Jelmer Vernooij <jelmer at samba.org>

> ---
>  lib/replace/wscript | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/replace/wscript b/lib/replace/wscript
> index e0728f9..3ca1eb0 100644
> --- a/lib/replace/wscript
> +++ b/lib/replace/wscript
> @@ -363,6 +363,7 @@ removeea setea
>                          headers='netinet/in.h arpa/nameser.h resolv.h')
>  
>  
> +    # try to find libintl (if --without-gettext is not given)
>      conf.env.intl_libs=''
>      if not Options.options.disable_gettext:
>          # any extra path given to look at?
> @@ -399,10 +400,10 @@ removeea setea
>              if conf.env['HAVE_GETTEXT'] and conf.env['HAVE_DGETTEXT']:
>                  # save for dependency definitions
>                  conf.env.intl_libs='iconv intl'
> -            else:
> -                conf.fatal('library gettext not found, try specifying the path to ' +
> -                           'it with --with-gettext=</path/to/gettext> or ' +
> -                           '--without-gettext to build without''')
> +
> +    # did the user insist on gettext (--with-gettext)?
> +    if Options.options.gettext_location and not conf.env['HAVE_GETTEXT'] and conf.env['HAVE_DGETTEXT']:
> +            conf.fatal('library gettext not found at specified location')
^^^ Please use four-space indents.
>  
>      conf.CHECK_FUNCS_IN('pthread_create', 'pthread', checklibc=True, headers='pthread.h')

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20131212/239e4bfc/attachment.pgp>