third_party (ex-lib_3p) now ready for review.
amitay at gmail.com
Wed Jul 23 06:55:21 MDT 2014
On Wed, Jul 23, 2014 at 6:22 PM, Ira Cooper <ira at samba.org> wrote:
> The functionality of the patch as far as a "user" is concerned is much the
> same. But it has been refactored underneath so it has less code
> duplication, and it is prepared for the day when we want to change the name
> of the third_party directory. (While I haven't plumbed it into configure,
> one could.)
> The patchset itself has been broken out into smaller more understandable
> If you have comments on how to make it better, I'm all ears.
> The main thing, you will find me strongly opposed to changing is the last
> commit, removing the libraries, hopefully reading the popt commit will help
> you understand why, along with all the discussions along the way.
> I encourage you to see this as a "final draft". (Or RC1.) I do think it
> is much more ready to go than when I started, thank you for the review
> comments! They truly did prove useful.
> Trees for review:
> Third party git tree:
Looking at the patches, I am assuming third_party would be dropped at the
In that case, it would not build any of the subdir builds (e.g. lib/ldb).
And you forgot CTDB. ;-)
LOCATION_OF_THIRD_PARTY has be relative to the srcdir, possibly something
like Utils.g_module.srcdir + '/third_party'. Or better still would be use
mechanism similar to DIST_DIRS to find the correct location of a component.
For DIST_DIRS, instead of trying to construct the path using
LOCATION_OF_THIRD_PARTY, may be we can just leave it as
third_party/popt:third_party/popt. For building tarballs, you would need
to drop third_party libraries in the correct place anyway.
- Patch: third_party/popt: Initial support for popt.
In this patch, lib/ldb/wscript needs to include Utils to raise WafError
You missed one substitution of LOCATION_OF_THIRD_PARTY in lib/ldb/wscript.
+ if os.path.exists(srcdir+'/lib_3p'):
Also, instead of doing the checks in wscript, all the checking can be done
+ if conf.CHECK_FOR_THIRD_PARTY():
+ conf.RECURSE(conf.LOCATION_OF_THIRD_PARTY() + '/popt')
+ if not conf.CHECK_POPT():
+ raise Utils.WafError('popt development packages have not
been found.\nIf third_party is installed, check that it is in the proper
+ conf.define('USING_SYSTEM_POPT', 1)
How about macros similar to conf.RECURSE, say conf.THIRD_PARTY('popt') and
bld.THIRD_PARTY('popt') ? Thinking about this a little more, it appears to
be harder because we need to include the check for system library if
third_party is missing. May be we can just add macros per third_party
library (e.g. conf.THIRD_PARTY_POPT and bld.THIRD_PARTY_POPT) and keep all
the details under the implementation?
More information about the samba-technical