third_party (ex-lib_3p) now ready for review.
Amitay Isaacs
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
> patches.
>
> 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:
>
> Samba:
> http://git.samba.org/?p=ira/wip.git;a=shortlog;h=refs/heads/third_party
>
> Third party git tree:
> http://git.samba.org/?p=ira/third_party.git;a=summary
>
> Thanks,
>
> -Ira
>
Hi Ira,
Looking at the patches, I am assuming third_party would be dropped at the
top-level.
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
exception.
You missed one substitution of LOCATION_OF_THIRD_PARTY in lib/ldb/wscript.
- bld.RECURSE('lib/popt')
+
+ if os.path.exists(srcdir+'/lib_3p'):
+ bld.RECURSE('lib_3p/popt')
Also, instead of doing the checks in wscript, all the checking can be done
in samba_third_party.py.
- conf.RECURSE('lib/popt')
+
+ if conf.CHECK_FOR_THIRD_PARTY():
+ conf.RECURSE(conf.LOCATION_OF_THIRD_PARTY() + '/popt')
+ else:
+ 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
place.')
+ else:
+ 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?
Amitay.
More information about the samba-technical
mailing list