[SCM] Samba Shared Repository - branch master updated

[simo]

On Mon, 2010-05-31 at 22:07 +0200, Jelmer Vernooij wrote:
> On Mon, 2010-05-31 at 13:52 -0400, simo wrote:
> > On Mon, 2010-05-31 at 12:23 -0500, Jelmer Vernooij wrote:
> > > The branch, master has been updated
> > >        via  7f75ee0... ldb: Install ldb_handlers.h header.
> > >        via  fe8302b... ldb: Remove Samba-specific symbols.
> > >        via  1bc53f0... ldb: Move utility functions to separate file.
> > >        via  82d56b9... ldb: Fix dependencies when building with system ldb.
> > >       from  471ed70... s3:smbd map_username() doesn't need sconn anymore
> > > 
> > > http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> > > 
> > > 
> > > - Log -----------------------------------------------------------------
> > > commit 7f75ee025ff9c02763fb9201d94af4b2739c8e03
> > > Author: Jelmer Vernooij <jelmer at samba.org>
> > > Date:   Mon May 31 18:29:11 2010 +0200
> > > 
> > >     ldb: Install ldb_handlers.h header.
> > 
> > Jelmer,
> > why are you installing this header ?
> > It is a private header afaik.
> It's used by ldb-samba, so we need the functions in there exposed somehow. Could we perhaps move them to ldb.h ?

Does ldb-samba now build against libldb ?
Otherwise you can directly reference the header, no ?

> > > diff --git a/source4/lib/ldb/tools/cmdline.c b/source4/lib/ldb/tools/cmdline.c
> > > index 180923f..c2b595f 100644
> > > --- a/source4/lib/ldb/tools/cmdline.c
> > > +++ b/source4/lib/ldb/tools/cmdline.c
> > > @@ -21,12 +21,9 @@
> > >     License along with this library; if not, see <http://www.gnu.org/licenses/>.
> > >  */
> > >  
> > > -#include "ldb_includes.h"
> > > -#include "ldb.h"
> > > -#include "tools/cmdline.h"
> > > -
> > >  #if (_SAMBA_BUILD_ >= 4)
> > >  #include "includes.h"
> > > +#include <ldb.h>
> > >  #include "lib/cmdline/popt_common.h"
> > >  #include "lib/ldb-samba/ldif_handlers.h"
> > >  #include "auth/gensec/gensec.h"
> > 
> > 
> > This also doesn't look right.
> > When you are building the tools you want to build them against the
> > headers that are going to be installed, and not against the headers that
> > are currently available on the machine.
> > We shouldn't reference <ldb.h> but explicitly the headers in the tree.
> We don't use any internal APIs so it should be possible to build against
> the system LDB library. We still need to build custom versions of
> ldb{add,del,search,edit} because there are "#ifdef _SAMBA_BUILD_" lines
> in those utilities' source code.

No, the problem is that when you compile a *new* version of ldb you
really want to have the tools built against the *new* headers, or they
may even fail to build if the new headers have some functions they
depend on. Unless you split building library and tools and require
people to install the library first and then build the tools.

> We supported a similar thing in the old build system but in that case we
> always had lib/ldb/include in the include path. We don't have that in
> the case of the waf build, and I'd like to avoid it (also to prevent
> accidentally including ldb_private.h somewhere while we link against the
> system ldb).

It's ok for the rest of samba, but is it problematic for the tools
themselves.

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>