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

[Jelmer Vernooij]

Hi Rusty,

On Sun, Jun 10, 2012 at 01:02:50PM +0930, Rusty Russell wrote:
> On Sat, 9 Jun 2012 01:18:29 +0200, Jelmer Vernooij <jelmer at samba.org> wrote:
> > 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.
> Why don't we just roll ccan into samba-util?  Either move ccan/ under
> util, or leave it where it is and just make samba-util pull in ccan.
That seems reasonable. We would have to make sure samba-util builds
without the rest of Samba so it can be a dependency of tdb, but that's
doable (can we name it libtutil?). There is still some overlap that
would be nice to avoid though (not just because of the amount of code
that's duplicated, but also because it leads to multiple ways to do
the same thing, which makes it harder to read code).

> If you think of CCAN as a separate project, the question becomes: should
> we help CCAN, or just do our own samba-specific code?
If we want to get our utility code from an external source we could
also look at using e.g. glib.  That's got a fair amount of
functionality, and is present on a lot of systems already. One of the
advantages would also be that we would no longer have to worry about
maintaining that code.  Instead, we could ditch parts of samba-util
that provide the same functionality.

I don't really understand why ccan was (silently) introduced as a
separate directory into Samba - it overlaps with something we already
have in Samba, and it wasn't discussed AFAIK. ccan as is is not really
appropriate to build as a shared library - it is more like a code collection
from which you can cherrypick what you like. In its current
form it has extra overhead with each module (a copy of the license and a
description) and a test system that's different from the rest of
Samba.

It took us a long time to go from two sets of utility functions in
Samba 3 and Samba 4 that had diverged back to a single set that is now
known as libsamba-util. This feels like going a step back.

> Obviously, I'd like to help CCAN, since it's my project, but not if it's
> too much hassle for SAMBA.
I wouldn't be opposed to using CCAN to seed some of the samba-util
code, and I don't see a problem with CCAN choosing to ship code that
is a taken from samba-util. The problem I have with ccan in its
current state is that it duplicates infrastructure and code and makes
life more complex without a good reason.

There is likely some code in CCAN that is better than that currently
in samba-util. Let's discuss replacing the existing code in samba-util
with the code from ccan, instead of adding yet more code.

> > 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?

> I thought about this, but I think you overestimate the duplication.
> The trivial stuff is duplicated, but it's also low-benefit.

> OTOH, I'd like SAMBA to unify on ccan/likely and ccan/time; the former
> because it has nice infrastructure for measuring branches which
> CCAN_LIKELY_DEBUG is set, and the latter because Samba doesn't have a
> unified time infrastructure; there are pieces all over the tree.
Note that samba already has a definition of likely() somewhere in
lib/util.

Samba has time functionality in lib/util/time.h, in what way is the
version in ccan better?

> I think ccan/build_assert and ccan/cast would also be minor wins.

cast seems to do the same thing as discard_const? I would really like
to avoid getting into the situation where some source files use
discard_const and others use cast_const, while the two do the same thing.

> It makes sense also to share some of samba-util with others, by turning
> them into CCAN modules, eg. dlinklist.
Why not just expose samba-util as a shared library? That seems like an
easier way of sharing it with others than CCAN.

> > 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.
> Well, ccan and tdb2/ntdb share the same test infrastructure.  I'd
> actually like to make that more common: having infrastructure for
> such low-level unit tests in Samba would be a significant benefit.
I don't see why we need to come up with a new infrastructure for
low-level unit tests for that, the existing code can be used for that
as well.

Cheers,

Jelmer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120615/9f86dd42/attachment.pgp>