[PATCH] some fixes for waf issues

Christian Ambach ambi at samba.org
Wed Jan 15 14:21:29 MST 2014


ENOATTACHMENT.. sorry


Am 15.01.14 22:20, schrieb Christian Ambach:
> Hi Andrew,
>
>>>>> 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.
>>>
>>> If we go with option 1 and change the behavior of configure checks to
>>> only make use of a function if the prototype is present and the
>>> resulting binary could be linked successfully, it might open up a can of
>>> worms if the change is done to the configure checks in general,
>>> affecting all checks.
>>> There are known examples that will fail then, like fdatasync on MacOS.
>>>
>>> So this check should better be local to the gettext configure checks.
>>> How about changing them to only enable gettext support if the prototypes
>>> could be found and linking succeeds?
>>
>> Yes.  That is the standard approach for this kind of situation.
>
> Okay, the attached patchset implements the change of --with-gettext
> to an automatic option: if nothing is given and detection fails,
> use of gettext will be skipped. If user has given --with-gettext and
> detection fails, configure will abort.
> As a safety net, --without-gettext is still around that will enforce
> that gettext is not used.
>
> Please review.
>
>>> Is gettext really that important? Not having it will "just" leave you
>>> without localized error messages for the minority of Samba binaries and
>>> libraries. When the new patches have landed, this can be removed again.
>>
>> Indeed.  I was surprised to see it made mandatory, when important things
>> like ACLs, xattr and LDAP were not at that point.  Just make sure we
>> undo the build farm changes if we change the rules here.
>
> Making that mandatory was a short-sighted mistake. The new approach
> should be better.
>
> Cheers,
> Christian

-------------- next part --------------
From 07ad4ea27c1b224ac034390cd4bd5ff1d90c2b50 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/6] 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

Signed-off-by: Christian Ambach <ambi at samba.org>
---
 source3/build/charset.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/source3/build/charset.py b/source3/build/charset.py
index 44852a6..8cef7da 100644
--- a/source3/build/charset.py
+++ b/source3/build/charset.py
@@ -33,6 +33,12 @@ def CHECK_SAMBA3_CHARSET(conf, crossbuild=False):
 	    default_unix_charset="UTF-8"
             # TODO: this used to warn about the set charset on cross builds
 
+        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');
+            default_dos_charset = "ASCII"
+            default_unix_charset = "UTF8"
+
         conf.DEFINE('DEFAULT_DOS_CHARSET', default_dos_charset, quote=True)
         conf.DEFINE('DEFAULT_UNIX_CHARSET', default_unix_charset, quote=True)
 
-- 
1.8.1.2


From 7856b2a080ead038ff778ce8ecbf0c402ee0b1e9 Mon Sep 17 00:00:00 2001
From: Christian Ambach <ambi at samba.org>
Date: Thu, 2 Jan 2014 23:28:20 +0100
Subject: [PATCH 2/6] waf:lib/replace correct detection of libiconv

add -liconv as a complete command line argument,
not all characters on their own

Bug: https://bugzilla.samba.org/show_bug.cgi?id=9911

Signed-off-by: Christian Ambach <ambi at samba.org>
---
 lib/replace/wscript | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/replace/wscript b/lib/replace/wscript
index a316032..09efdc8 100644
--- a/lib/replace/wscript
+++ b/lib/replace/wscript
@@ -390,7 +390,7 @@ removeea setea
             # Some hosts need lib iconv for linking with lib intl
             # So we try with flags just in case it helps.
             oldflags = conf.env['EXTRA_LDFLAGS'];
-            conf.env['EXTRA_LDFLAGS'].extend("-liconv")
+            conf.env['EXTRA_LDFLAGS'].extend(["-liconv"])
             conf.CHECK_FUNCS_IN('dgettext gettext bindtextdomain textdomain bind_textdomain_codeset',
                                 'intl', checklibc=False, headers='libintl.h')
             conf.env['EXTRA_LDFLAGS'] = oldflags
-- 
1.8.1.2


From 79c10c76ca6de29b1a03abb49af96ce1a08891ef Mon Sep 17 00:00:00 2001
From: Christian Ambach <ambi at samba.org>
Date: Thu, 2 Jan 2014 22:23:16 +0100
Subject: [PATCH 3/6] waf:lib/replace fix up libintl related checks

on a default installation of AIX, libintl.a exists but
libintl.h does not
So check for the declarations of those functions as well
to make sure that the build works.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=9911

Signed-off-by: Christian Ambach <ambi at samba.org>
---
 lib/replace/wscript | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/replace/wscript b/lib/replace/wscript
index 09efdc8..6784dd3 100644
--- a/lib/replace/wscript
+++ b/lib/replace/wscript
@@ -371,6 +371,7 @@ removeea setea
     if not Options.options.disable_gettext:
         conf.CHECK_HEADERS('libintl.h')
         conf.CHECK_LIB('intl')
+        conf.CHECK_DECLS('dgettext gettext bindtextdomain textdomain bind_textdomain_codeset', headers="libintl.h")
         # *textdomain functions are not strictly necessary
         conf.CHECK_FUNCS_IN('bindtextdomain textdomain bind_textdomain_codeset',
                             '', checklibc=True, headers='libintl.h')
-- 
1.8.1.2


From 64b939550cfbd74af34fcdd61e2104c50d3e9397 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/6] waf:lib/replace change detection of gettext

convert this to an automatic check: if no option is given, try to find gettext
and if found, use it
if user has specified --with-gettext, then bail out if it could not be found
in case of --without-gettext, skip all gettext related configure checks

Bug: https://bugzilla.samba.org/show_bug.cgi?id=9911

Signed-off-by: Christian Ambach <ambi at samba.org>
---
 buildtools/wafsamba/wscript | 3 +--
 lib/replace/wscript         | 9 +++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/buildtools/wafsamba/wscript b/buildtools/wafsamba/wscript
index fe2e515..7984227 100755
--- a/buildtools/wafsamba/wscript
+++ b/buildtools/wafsamba/wscript
@@ -80,8 +80,7 @@ def set_options(opt):
                    match = ['Checking for library iconv', 'Checking for iconv_open', 'Checking for header iconv.h'])
     opt.add_option('--with-gettext',
                    help='additional directory to search for gettext',
-                   action='store', dest='gettext_location', default='/usr/local',
-                   match = ['Checking for library intl', 'Checking for header libintl.h'])
+                   action='store', dest='gettext_location', default='None')
     opt.add_option('--without-gettext',
                    help=("Disable use of gettext"),
                    action="store_true", dest='disable_gettext', default=False)
diff --git a/lib/replace/wscript b/lib/replace/wscript
index 6784dd3..5f29e9f 100644
--- a/lib/replace/wscript
+++ b/lib/replace/wscript
@@ -367,6 +367,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:
         conf.CHECK_HEADERS('libintl.h')
@@ -398,10 +399,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 != 'None' and (not conf.env['HAVE_GETTEXT'] or not conf.env['HAVE_DGETTEXT']):
+        conf.fatal('library gettext not found at specified location')
 
     conf.CHECK_FUNCS_IN('pthread_create', 'pthread', checklibc=True, headers='pthread.h')
 
-- 
1.8.1.2


From 8062fa0b2c7ce91b702ca59eb4b510b1b5277981 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 5/6] waf:lib/replace fix gettext detection

if the user has specified a path for gettext, add it to CFLAGS and LDFLAGS
so we can find it during configure and build

Bug: https://bugzilla.samba.org/show_bug.cgi?id=9911

Signed-off-by: Christian Ambach <ambi at samba.org>
---
 lib/replace/wscript | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/replace/wscript b/lib/replace/wscript
index 5f29e9f..650dc06 100644
--- a/lib/replace/wscript
+++ b/lib/replace/wscript
@@ -370,6 +370,13 @@ removeea setea
     # 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?
+        if not Options.options.gettext_location == 'None':
+           conf.env['CFLAGS'].extend(["-I%s" % Options.options.gettext_location]);
+           conf.env['LDFLAGS'].extend(["-L%s" % Options.options.gettext_location]);
+        else:
+           conf.env['CFLAGS'].extend(["-I/usr/local"]);
+           conf.env['LDFLAGS'].extend(["-L/usr/local"]);
         conf.CHECK_HEADERS('libintl.h')
         conf.CHECK_LIB('intl')
         conf.CHECK_DECLS('dgettext gettext bindtextdomain textdomain bind_textdomain_codeset', headers="libintl.h")
-- 
1.8.1.2


From c9fea6b74b9b05b67f6ea8e39a400da584f03659 Mon Sep 17 00:00:00 2001
From: Christian Ambach <ambi at samba.org>
Date: Thu, 25 Jul 2013 19:41:02 +0200
Subject: [PATCH 6/6] waf:lib/replace gettext configure checks

Make sure we only try to work with gettext if we found
the prototypes and were able to link

Bug: https://bugzilla.samba.org/show_bug.cgi?id=9911

Signed-off-by: Christian Ambach <ambi at samba.org>
---
 lib/replace/wscript | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/replace/wscript b/lib/replace/wscript
index 650dc06..ab95018 100644
--- a/lib/replace/wscript
+++ b/lib/replace/wscript
@@ -407,6 +407,15 @@ removeea setea
                 # save for dependency definitions
                 conf.env.intl_libs='iconv intl'
 
+    # did we find both prototypes and a library to link against?
+    # if not, unset the detected values (see Bug #9911)
+    if not (conf.env['HAVE_GETTEXT'] and conf.env['HAVE_DECL_GETTEXT']):
+       conf.undefine('HAVE_GETTEXT')
+       conf.undefine('HAVE_DECL_GETTEXT')
+    if not (conf.env['HAVE_DGETTEXT'] and conf.env['HAVE_DECL_DGETTEXT']):
+       conf.undefine('HAVE_DGETTEXT')
+       conf.undefine('HAVE_DECL_DGETTEXT')
+
     # did the user insist on gettext (--with-gettext)?
     if Options.options.gettext_location != 'None' and (not conf.env['HAVE_GETTEXT'] or not conf.env['HAVE_DGETTEXT']):
         conf.fatal('library gettext not found at specified location')
-- 
1.8.1.2


More information about the samba-technical mailing list