code duplication in ccan (was: Re: snprintf on SunOS)

[Jelmer Vernooij]

On Sat, Jun 09, 2012 at 08:40:51AM +1000, Andrew Bartlett wrote:
> On Fri, 2012-06-08 at 18:23 +0200, Jelmer Vernooij wrote:
> > On Thu, Jun 07, 2012 at 02:34:52PM +1000, Andrew Bartlett wrote:
> > > On Wed, 2012-06-06 at 03:13 -0700, Mihai-Radu, Orza wrote:
> > > > Thank you for the patch. I managed to go past the asn1 linkage problem, so it helped.
> > > > 
> > > > But now I get the following build error:
> > > > [ 522/3534] Compiling lib/ccan/failtest/failtest.c
> > > > ../lib/ccan/failtest/failtest.c:8:17: err.h: No such file or directory
> > > > Waf: Leaving directory `/tmp/samba-4.0.0beta1/bin'
> > > > Build failed:  -> task failed (err #1):
> > > >         {task: cc failtest.c -> failtest_1.o}
> > > > 
> > > > The err.h C header does not come with SunOS so I guess the compiler should use the replacement functions defined in source4\heimdal_build\replace.c. Is there some config param I'm missing?
> > > > 
> > > > Thanks again and regards,
> > > > Mihai
> > > 
> > > In the short term, it seems you should be able to
> > > 
> > > rm -rf lib/ccan/failtest
> > > 
> > > as this code is unused.  You may have noticed I've mailed the ccan
> > > maintainer to understand what we are going to use the code for, and how
> > > to best handle this in the future. 
> > This is related to something I've been wondering about for a while.
> > Ccan reinvents the wheel and is separate from lib/util (of which it
> > duplicates a lot) and has its own test infrastructure. 
> One problem is that tdb2/ntdb uses ccan (but it uses a lot of it for
> only for failtest), and is an 'independent' (or at least able to be
> built as independent) package.  
> 
> We could deprecate parts of libsamba-util for ccan of course.
> Presumably the licences also set much the same course (ccan being LGPL
> or less to build an LGPL tdb2/ntdb).
We used to have a single library for portability (libreplace), and one
for common convenience functions (libsamba-util).  ccan adds yet another
library which overlaps with both of these.

We can deprecate functionality from samba-util, but we'll still end up
with the same issue - two libraries that do essentially the same
thing.

In addition, ccan has a modular nature that makes sense for embedded
projects but not really for Samba, since we're just building it as a
library anyway. The library can't be public since it doesn't have a
public ABI (modules can be excluded). Exporting each of the ccan
modules as a shared library doesn't really make sense, as they're so
small.

> > In addition, it
> > looks like most of ccan isn't actually used anywhere in Samba.
> I was surprised how much of this was the case when I worked with rusty
> to re-enable failtest (the tdb2 tests didn't match a glob, and so were
> never built).  
> 
> The ccan modules not just used in failtest are:
> 
> ccan-hash ccan-ilog ccan-likely ccan-tally
> 
> > Can we remove it in favor of libsamba-util?
> I don't think we can, but for all the fuss and bother this thing has
> caused, what is left to be installed on disk is quite an almost trivial
> amount of code. 
It's not really about the duplicated bytes in the executable, which I
agree is minimal.

The issue is that ccan isn't a real library (see above) and
duplicates our other efforts. If these functions are just included for
the sake of ntdb, can we perhaps just embed the ones we need
(privately) in ntdb?

ccan also has its own test infrastructure, which means custom code to
run the ccan and tdb tests, and no integration with the test reporting
in "make test" or the build farm.

Cheers,

Jelmer