The Wrapper Project

Fri Nov 29 03:04:22 MST 2013

On Friday 29 November 2013 14:09:58 Andrew Bartlett wrote:
> On Thu, 2013-11-28 at 17:22 +0100, Andreas Schneider wrote:
> > On Thursday 28 November 2013 06:17:19 Andrew Bartlett wrote:
> > > One of the oddities of Samba is that we don't do that very much at all.
> > > We are very hesitant to rely on external free software projects, for
> > > better or worse.
> > 
> > So on day we will add glibc to our tree? :)
> > 
> > > In this case, I'm particularly hesitant about the idea of requiring it
> > > to be installed to permit testing, because our build farm, old, unloved
> > > and broken as it is, is a particularly poor environment for requiring
> > > the installation of external software.
> > 
> > You managed to install python to be able to build Samba, I don't see how
> > other stuff could be much harder ... (We will come to the reason later.)
> The folks who did the effort to install python told me was an absolute
> pain to do.  I'm keen that we work out a plan here before we proceed
> changing the basis for our test system.  I thought we had agreed to have
> it in-tree using our build system.
> 
> > > As I've said before, in my previous work on this code, I've found and
> > > fixed some very subtle bugs that show up only in bizarre results in our
> > > make test.
> > 
> > Yeah, cause the stuff was simply untested. There were/are no unit tests to
> > verify that the wrapper functions work as expected. Only nss_wrapper has a
> > truckload of good tests from Günther. I've added more test and we have a
> > code coverage of 74.9 %
> > 
> > http://mock.cryptomilk.org/index.php?project=nsswrapper
> > 
> > >  I would want to ensure we had a very strict rule as to
> > > 
> > > exactly the version of these wrappers that we used against a specific
> > > version of Samba.  I think bundling these like we do popt et al is the
> > > best approach for now.
> > 
> > Why are you (an others) always come up with these lame excuses. Why don't
> > you just admit that you are lazy and don't want to install any of the
> > 3rdparty software.
> 
> This isn't about being lazy or otherwise, it is about ensuring that the
> fundamental elements of our test infrastructure remain consistent for
> all test runs.
> 
> The way I see it is this:  Either the wrapper project is complete, and
> so won't need updates in-tree, or we will need to manage updating the
> build farm, user and developer instances.  Those updates will need to
> strictly (presumably by pkg-config tests strictly checking version
> numbers) match new revisions at exactly the same point where the
> matching change is made in Samba.
> 
> I'm particularly nervous about this in a 'git bisect' scenario.

If you looking for a bug and do a 'git bisect' is an argument to have them 
outside of the tree, cause the bisect will not go to a revision where an 
important patch for a wrapper is not applied. The wrapper will always be the 
same version with the same patch level.

As I said I can understand integrating them but then the development will not 
take place in the Samba tree but outside. The more people who use it the more 
eyes we get on it, the better the code will be. And if there is a bug then we 
need a unit test for it so we will not regress in future.

socket_wrapper still lacks of a lot of tests. Those need to written first 
before we can move on.

> 
> > Last heimdal update: July 2011 (Heimdal last release was done 2012-01-11)
> > Quick copy: 68 files changed, 6750 insertions(+), 3142 deletions(-)
> > 
> > Last zlib update: before 2008 (moved to lib/ in 2008)
> > Last iniparser update: before 2010 (moved to lib/ in 2010)
> > Last popt update: before 2010 (moved to lib/ in 2010)
> 
> I'm not sure what you point is here.
> 
> > > The other thing I would miss is the ability to invoke this via CPP
> > > macros.  We removed smb_wrapper because we determined the LD_PRELOAD
> > > approach was determined not to be viable in the long term.  I guess the
> > > difference here is that the socket API is more limited, as because CPP
> > > also can't override libc calls, we have altered all the callers anyway.
> > 
> > What is this about now? So I should stop to get 'make test' working with
> > the LD_PRELOAD wrappers?
> 
> It is my preference that we can use this either way, by CPP or
> LD_PRELOAD.

I prefer one way and get it working right so it can be used against external 
applications too.

> 
> > > It would, even as he has left Samba development, be insightful to ask
> > > for Tridge's view on this, because he has more experience than anyone
> > > else I know on LD_PRELOAD over many years.
> > 
> > What do you want, that I stop working on this? Why do we dance around the
> > issue here?
> 
> I wrote positively about this effort when you first spoke of it, and
> raised some issues.  You have returned, with what feels like a 'take it
> or leave it' offer, and without any significant effort to address the
> concerns.  What do you expect me to say?
> 
> It is entirely your right to fork a bit of Samba's code, and develop it
> yourself.  However, we should rightly hold it and the changes you
> propose to close scrutiny when you propose to change how Samba uses it.

This code is still WIP. There are still issues on different operating system 
which need to be fixed and if I find the time to work on it it the code 
changes often several time a day and like for uid_wrapper I did more or less a 
complete rewrite cause the code was wrong and not thread safe.

> Getting back to fundamental issues, I'm particularly nervous about your
> dlopen change in ldb, and that it demonstrates some broader challenges
> with the LD_PRELOAD approach:
> 
> https://git.samba.org/?p=asn/samba.git;a=commitdiff;h=c4d1bd522c6850065cd941
> 3ee860856cac52c4df

That's why I asked about a review. The wrapper_project branch is only tested 
on Linux, it works but there are still issues. Especially if you have a 
correct uid_wrapper like we have now.

The ldb change needs to be fixed in a different way. Maybe a ldb_wrapper 
overwriting the ldb_modules_load_path() function or by having a

#ifdef DEVELOPER_BUILD
#endif

to disable it just for 'make test'.

Metze already told me that I need to rewrite the root_mode code probably also 
using a wrapper or a #ifdef

> At the very least, like we have agreed to do for other areas
> (libwbclient), I think this needs to be set only for build-time
> binaries, so we don't cause issues in the BIND9 server.

Yes, either create a wrapper or use a #ifdef for the developer build.

To go back. The idea of this work was to have one day a AD DC you can ship in 
a Linux enterprise distribution. Currently it is not possible. So we need or 
want to start a MIT KDC like we do with smbd. So to simply start the installed 
mit kdc and run 'make test' you need to preload the wrappers.

The other way is to completely integrate MIT KDC into the Samba tree. If I 
compile Samba, I don't want to compile Kerberos.

	-- andreas

-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org