third_party (ex-lib_3p) now ready for review.

[Ira Cooper]

On Sat, Jul 26, 2014 at 10:52 AM, Jelmer Vernooij <jelmer at samba.org> wrote:

> On Sat, Jul 26, 2014 at 10:13:47AM -0700, Ira Cooper wrote:
> > On Sat, Jul 26, 2014 at 9:49 AM, Jelmer Vernooij <jelmer at samba.org>
> wrote:
> >
> > > On Sat, Jul 26, 2014 at 09:38:04AM -0700, Ira Cooper wrote:
> > > > On Sat, Jul 26, 2014 at 8:26 AM, Jelmer Vernooij <jelmer at samba.org>
> > > wrote:
> > > >
> > > > > On Wed, Jul 23, 2014 at 10:02:23AM -0700, Ira Cooper wrote:
> > > > > > On Wed, Jul 23, 2014 at 9:48 AM, Jelmer Vernooij <
> jelmer at samba.org>
> > > > > wrote:
> > > > > > > On 23 July 2014 17:20:31 BST, Ira Cooper <ira at samba.org>
> wrote:
> > > > >
> > > > > > > >Checks for specific libraries don't belong under
> > > buildtools/wafsamba/,
> > > > > > > >> which is our set of support libraries on top of waf. Please
> add
> > > > > > > >> these checks in wscript or wscript_third_party or something
> > > > > > > >> like that.
> > > > > > >
> > > > > > > >I wasn't a fan of it either... but adding the changes to
> wscript
> > > is
> > > > > > > >just
> > > > > > > >ugly and stops us from reusing them properly.
> > > > > > > I doubt it would be that much code. Zlib is only used by samba
> > > itself.
> > > > > For
> > > > > > > the others, we can just use pkg gonfig. Also, we'll have to do
> this
> > > > > > > eventually anyway when we split these libraries off from the
> main
> > > samba
> > > > > > > repo.
> > > > > > Any code duplication when it comes to maintaining that type of
> > > > > information
> > > > > > is asking for trouble, IMHO.  I'd like other people's feedback on
> > > this
> > > > > > issue before I make that type of change that I really tried to
> avoid.
> > > > > >
> > > > > > Also pkg-config is not the only way to detect libraries.  I want
> to
> > > > > > maintain the most compatibility we can with the current system,
> while
> > > > > > making this change.
> > > > >
> > > > > The only library for which this is relevant (because it's not just
> > > > > used by Samba, but by ldb and tdb too) is popt. The detection code
> for
> > > > > popt is a single line:
> > > > >
> > > > > conf.CHECK_BUNDLED_SYSTEM('popt', checkfunctions='poptGetContext',
> > > > > headers='popt.h')
> > > > >
>
> > > > What happens when we want to change the detection logic?
> > > What sort of change do you expect to this line of code?
> > >
> > > Also, why does the logic need to be the same? tdb might need a newer
> > > version of popt
> > > than ldb or samba. It might want to check for the presence of a
> function
> > > that was
> > > recently added. We might want to blacklist a broken popt version in the
> > > future because
> > > it triggers a bug in the way Samba (and Samba alone) uses popt.
> > >
> >
> > #include <popt.h>
> >
> > Will only pick up one version, unless you are using different CFLAGS for
> > the libraries to cause this problem.  The goal of this whole thing is to
> > simplify what we are doing.
> >
> > No, I'm not making the libraries all individuals.  This is a deliberate
> > design choice to simplify maintenance.
>
> I never suggested that we should support linking each library against a
> different
> version of popt simultaneously, or anything like that.
>
> Each library has different requirements. The check in
> tdb/wscript is there to make sure there is a popt present on the
> system that is sufficient for tdb's purposes. The check in ldb/wscript
> is there to check that the popt on the system is sufficient for ldb's
> purposes.
>

We disagree here.  This is a great way to fragment the logic all over the
place.

tdb has a number of external users (pulseaudio, libcanberra, rhythmbox)
> that build the standalone tdb.  Why should they be forced to install a
> newer
> popt if ldb started using a newer popt feature?
>

Because it is a non-issue?  Everyone in this situation will upgrade their
popt due to the N other packages that use popt.

 At the moment, tdb/ldb/samba support all versions of popt, but that may
> change
> in the future. Popt is also quite a simple library; it's more of an issue
> with libraries like Kerberos, tdb or LDAP.
>

You are correct, is is a much bigger issue with those libraries.  Getting
the detection right is hard.  We've all seen packages mess it up, and go
WTF?

Given the state of disrepair of even our current libraries... I'm voting to
combine this logic so it is kept up properly.

Sorry, we just disagree here.  I suggest we have some others weigh in, as I
suggested previously.

-Ira