lib_3p system

Wed Jul 16 23:04:49 MDT 2014

On Thu, 2014-07-17 at 05:13 +0200, Jelmer Vernooij wrote:
> On Wed, Jul 16, 2014 at 10:52:38PM -0400, Ira Cooper wrote:
> > On Wed, Jul 16, 2014 at 9:50 PM, Jelmer Vernooij <jelmer at samba.org> wrote:
> > 
> > > On Wed, Jul 16, 2014 at 09:42:59PM -0400, Ira Cooper wrote:
> > > > On Wed, Jul 16, 2014 at 8:36 PM, Jelmer Vernooij <jelmer at samba.org>
> > > wrote:
> > > >
> > > > > On Wed, Jul 16, 2014 at 07:03:55PM -0400, Ira Cooper wrote:
> > > > > > In all the work I've been asked for my rationale for the work I am
> > > done.
> > > > > >
> > > > > > It is about cleanliness and solid software engineering.  We can't be
> > > sure
> > > > > > we AREN'T using libraries that are totally integrated into our build
> > > > > > system.  While we can try, and we can claim, the true proof is
> > > removing
> > > > > > them.
> > > > > >
> > > > > > In the interests of doing this, I've actually gone ahead and removed
> > > > > them,
> > > > > > on a branch. and put in support for downloading a tar file, manually.
> > > > > >
> > > > > > The first patch on my new branch shows a file that was including our
> > > > > local
> > > > > > popt headers, instead of following and finding the system ones.  I
> > > > > suspect
> > > > > > it is the only one, but until there is more testing on more systems,
> > > I
> > > > > > won't feel truly sure of that.
> > > > > >
> > > > > > If we wish to support a "fat" tarball for our releases, that is fine.
> > > > >  But
> > > > > > for day to day to development, the intent of this change is to make
> > > it so
> > > > > > developers who don't wish to have these libraries or their sources on
> > > > > their
> > > > > > system, do not have them there.
> > > > > >
> > > > > > If you want more rationale than Simo's rationale, Jeremy's and
> > > Volker's,
> > > > > I
> > > > > > suggest you look at my first patch on this branch.  It shows what I
> > > truly
> > > > > > fear.  Insidious errors.  This error was innocent it looks like...
> > > > > > thankfully.
> > > > > >
> > > > > > This is why I do not support third party libraries in the tree.
> > >  These
> > > > > type
> > > > > > of mistakes are too easy to make, and too easy to tempt ourselves
> > > into.
> > > > > >
> > > > > > Git branch is at:
> > > > > >
> > > > > > http://git.samba.org/?p=ira/wip.git;a=shortlog;h=refs/heads/lib_3p
> > > > > >
> > > > > > The supporting "lib_3p" tar file:
> > > > > >
> > > > > > http://www.samba.org/~ira/lib_3p.tar  (This should move to a better
> > > > > > location and be versioned etc, if we do this.)
> > > > > >
> > > > > > I'll construct the actual git repo to go with the tarfile tomorrow.
> > > > > >
> > > > > > I suggest you look at the code, and the overall concept.  I think
> > > you'll
> > > > > > find it a vast improvement, and a solid middle ground.
> > > > > >
> > > > > > (Yes, this is a request for review, and comments.)
> > > > >
> > > > > Can you rename it to something with a more obvious name? "p3" doesn't
> > > > > mean anything to me. Perhaps "lib/external" ?
> > > > >
> > > > > Please update lib/update-external.sh, which is there to update
> > > > > some of these libraries. (I've got a pending patch to make it update
> > > zlib)
> > > > >
> > > > > Can you move each library in a separate commit? Git doesn't deal well
> > > > > with moves of lots of files unless you specify the magic options (see
> > > > > e.g. the output of "git log lib/zlib").
> > > > lib_3p stands for "Libraries, Third Party." "external" is a bit too vague
> > > > for this use.  I really want an exact meaning.
> > > In that case, what about just naming it lib/third_party or
> > > lib/3rd_party ?
> > lib/ is for the first party libraries.
> It's for both, at least not at the moment.
> 
> If you'd like to change that, and have the third party stuff somewhere
> else, can we please call that "third_party/" ? "Libraries" is also
> a misleading name since some of the third party stuff we bundle are
> things like Python modules.
> 
> >  > As far as what I really intend, it was clear I missed a bit on my real
> > > > intent.
> > > >
> > > > Look at http://git.samba.org/?p=ira/lib_3p.git;a=summary ; and you'll
> > > see
> > > > the missing piece.
> > > >
> > > > The intent is that there is a second git repo, and that all of this data
> > > is
> > > > properly kept under SCM.
> > >
> > > What's the advantage of a second git repository, versus just removing
> > > your local copy of lib_3p ?
> > The advantage is to "default to safe".  The first commit shows some of the
> > danger of leaving lib_3p in the tree.
> This will make building Samba from git more painful for anybody who
> has a system that currently lacks one of these bundled libraries.
> 
> I can see the value in being able to build without any bundled
> libraries, mostly for packagers and developers verifying we don't
> accidentally rely on bundled libraries. It's not worth inconveniencing
> regular users who are trying to build Samba.

The attached patch changes the dist to distcheck in the samba-libs
target.  This does yet another build of Samba, but this time from the
tarball and also an install/uninstall in /tmp.  If using /tmp would be
an issue, an additional waf patch will be required. 

The idea is to show we are not using any of the tarball-excluded source
paths. 

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-autobuild-Do-a-distcheck-in-the-samba-libs-target-to.patch
Type: text/x-patch
Size: 1271 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140717/87db631e/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140717/87db631e/attachment.pgp>