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

[Rusty Russell]

On Thu, 21 Jun 2012 17:49:26 +0200, Jelmer Vernooij <jelmer at samba.org> wrote:
> On Mon, Jun 18, 2012 at 09:46:32AM +0930, Rusty Russell wrote:
> > On Fri, 15 Jun 2012 01:10:01 +0200, Jelmer Vernooij <jelmer at samba.org> wrote:
> > > On Thu, Jun 14, 2012 at 06:57:30PM -0400, simo wrote:
> > > > I am really oppoe to make samba-util a dependency for tdb, samba util is
> > > > in NO shape to become a real public library. Let's not do the worst
> > > > thing we could please.
> 
> > > I wasn't saying anything about making it public. ccan already *is* a
> > > dependency of tdb and it's not public either. Neither is a great
> > > situation.
> 
> > BTW, you mean ntdb/tdb2, not tdb?
> 
> Sorry, yes.
> 
> > But yes, lots of confusion here.  Technically, CCAN is built as a shared
> > lib (not for any good reason), but it's also linked directly into
> > libtdb2/libntdb as it's a private dependency.
> 
> > It's not *public*, that would be wrong.
> 
> There is cost and value associated with putting things into a
> shared library rather than just copying the code (be it source or
> binary) into lots of different places.
> 
> > > It would be nice to make samba-util public, but that would (as you
> > > said earlier) require some effort to clean it up and define a
> > > stable API.
> 
> > Nice?  It's a terrible idea!
> 
> > Why is public better than private?  More maintenance overhead for us,
> > more bloat and cruft as we can't remove old APIs, and it's not like
> > anyone else is going to use libsamba-util.so.
> Sure, it has its disadvantages. You have to be careful what you
> declare stable and public. The various bits in samba-util haven't
> changed in a long time though, and they are all pretty independent.
> I'm not proposing making them public in their current form, we can
> clean them up first and cherrypick.

OK, I think that's worth looking at, esp. given what you say below.

> The cost of changing a public ABI isn't just in maintaining backwards
> compatibility; like any API change (private or public), all existing
> call sites will also need to be updated.

But for non-public API you can find all the callers, trivially.  In
Samba we often take easy path and avoid it, but I think that's a bad
thing.  I'd like to do more cleanups, where it make sense.

> openchange has been using some of these libraries for a
> while, even though they have only been semi-public. We originally
> started installing these headers files so OpenChange could link
> against Samba 4 rather than be a fork of it. For example, our
> pidl-generated DCE/RPC code (which OpenChange uses) uses some of these
> functions.

Erk.  That does make it more important that we clean things up and
decide what's actually public :(

> The alternative is including these lumps of code in each of the
> generated files, or have openchange ship its own copy.
> That has got its disadvantages too. Changes are only made in one
> copy (e.g. bug fixes); you have to make sure the symbols don't clash.
> If there are data structures that are part of the ABI you need to make
> sure that those created by the Samba version of your utility code
> never end up in the OpenChange version, ...

Yes, and we should make them proper libraries, because then waf will at
least do rudimentry checks that we don't break the ABI (it doesn't check
structure definitions).

> > If you insist on making everything shared a public shared library, you
> > end up with coders cut & pasting to avoid the hassle.  That's what tdb
> > did: cut & paste the hash code :(
> Hasn't that already happened? My impression of ccan is as a repository
> with little snippets of code that coders can copy-paste into their
> source to reuse. Is that an incorrect view of it?

Partially, but we're also trying to ease sharing and on-sharing, by
encouraging people to use whole modules.  It's much easier to
understand, update and reuse some code that's been dropped in a ccan/foo
subdirectory than to dig into lib/tdb/common/hash.c for the 290 lines
(about half of the hash module) which have been cut & paste into there.

> I'm not advocating making every single function in Samba public, nor
> am I advocating branding samba-util's existing semi-public API as stable.
> I'm not insisting on making everything shared a public shared library.
> But samba-util is already public to some degree, and changes to the
> ccan ABI have an impact too (see my other email).

Yes, I hadn't realized the state with samba-util.  What do we need to do
to clean that up?

Thanks!
Rusty.