CMake Proposal

[Dmitri Pal]

Hello,

Sorry for the top post.
This is a very good summary and gets right to the point of many "whys".
The only thing that I want to add is that Red Hat was ready to offer
cycles and effort in converting to CMake because we already had the
expertise and could have aligned our priorities to effectively use the
expertise for the benefits of the Samba project. The proposed approach
does not make us well suited for the job. We will be glad to participate
in the collaborative effort in improving build system but we
unfortunately can't take on the responsibility of making the change in
the given circumstances.  So we will focus on other things that we have
expertise and can work on.  

Thank you,
Dmitri



tridge at samba.org wrote:
> Hi Bill,
>
> A number of people (including yourself) have asked me to explain
> exactly what it would take for me to be happy with cmake as a build
> system for Samba.
>
> The first and most important criterion is that it be better than the
> current system. To work out if it meets that criterion we first need
> to look at what is wrong with the current system.
>
> To illustrate one of the problems with the current system I'd like to
> point you at this recent commit:
>
>   http://git.samba.org/?p=samba.git;a=commitdiff;h=1203de99b178a2d7f6c7c6534c42d05242322fe3
>
> all looks quite reasonable, yes? Simo is removing a subsystem called
> 'SCHANNELDB' from the gensec component of Samba4 as it has been
> replaced with a different approach (one based on a tdb).
>
> There is a problem though. He missed a place where we list a
> dependency on SCHANNELDB. Have a look at this bit of build config:
>
>   http://git.samba.org/?p=samba.git;a=blob;f=source4/rpc_server/config.mk;h=5ff1bd572263ce8f19b4c550c68a963a8796249e;hb=1203de99b178a2d7f6c7c6534c42d05242322fe3
>
> that is the build rules for the rpc_server component of Samba4. Look
> at line 131, in the rules for the dcerpc_server subsystem. You'll see
> that it still lists a dependency on the SCHANNELDB subsystem. It is an
> easy mistake to make.
>
> Our current build system allows this type of error. That is because it
> spits out a Makefile, and in the make language using an undefined
> variable is not an error.
>
> Let's do the same thing in cmake. This patch makes the
> talloc_testsuite in the cmake rules Andreas wrote depend on the
> non-existent SCHANNELDB subsystem:
>
>   --- a/lib/talloc/CMakeLists.txt
>   +++ b/lib/talloc/CMakeLists.txt
>   @@ -72,7 +72,7 @@ set(talloc_SRCS
>
>    # build libraries and binaries
>    add_library(${LIBTALLOC_LIBRARY} SHARED ${talloc_SRCS})
>   -target_link_libraries(${LIBTALLOC_LIBRARY} ${LIBREPLACE_LIBRARY})
>   +target_link_libraries(${LIBTALLOC_LIBRARY} ${LIBREPLACE_LIBRARY} ${SCHANNELDB})
>
>    set_target_properties(
>        ${LIBTALLOC_LIBRARY}
>
> Now try and build talloc with cmake. You'll get no errors and no
> warnings. So just like our current buildsystem, cmake has swallowed a
> non-existent dependency.
>
> If we do the same thing with waf:
>
>   --- a/lib/talloc/wscript
>   +++ b/lib/talloc/wscript
>   @@ -22,4 +22,4 @@ def build(bld):
>
>        bld.SAMBA_BINARY('talloc_testsuite',
>                         'testsuite.c testsuite_main.c',
>   -                     deps='talloc')
>   +                     deps='talloc SCHANNELDB')
>
> then this is what we get:
>
>   object 'SCHANNELDB' was not found in uselib_local (required by add_objects 'talloc_testsuite')
>
> and it refuses to build until you fix the missing dependency.
>
> Ok, so that is the first criterion for a successful use of cmake or
> any other build system as a replacement for our current system. It
> much catch the types of errors that tend to accumulate in large
> projects and that have accumulated in Samba (waf has found many
> examples of this type of thing in our current build rules).
>
> What other problems do we have in our current build system? Well, the
> build rules that contributors to Samba have to routinely edit are too
> esoteric. Some of them are quite clear, for example, if we look at the
> build rule for the dcerpc_server:
>
>   [MODULE::dcerpc_netlogon]
>   INIT_FUNCTION = dcerpc_server_netlogon_init
>   SUBSYSTEM = dcerpc_server
>   PRIVATE_DEPENDENCIES = \
> 		DCERPC_COMMON \
> 		SCHANNELDB \
> 		NDR_STANDARD \
> 		auth_sam \
> 		LIBSAMBA-HOSTCONFIG
>
> then it is pretty clear what is meant. It doesn't have much error
> checking (as evidenced by the SCHANNELDB problem), but it is at least
> fairly clear what is intended, even if you haven't worked with the
> build system before.
>
> A few lines further up in the same file is something that is less
> clear:
>
>   ################################################
>   # Start MODULE dcerpc_srvsvc
>   [MODULE::dcerpc_srvsvc]
>   INIT_FUNCTION = dcerpc_server_srvsvc_init
>   SUBSYSTEM = dcerpc_server
>   PRIVATE_DEPENDENCIES = \
> 		DCERPC_COMMON NDR_SRVSVC share
>   # End MODULE dcerpc_srvsvc
>   ################################################
>
>   dcerpc_srvsvc_OBJ_FILES = $(addprefix $(rpc_serversrcdir)/srvsvc/, dcesrv_srvsvc.o srvsvc_ntvfs.o)
>
>   $(eval $(call proto_header_template,$(rpc_serversrcdir)/srvsvc/proto.h,$(dcerpc_srvsvc_OBJ_FILES:.o=.c)))
>
> The main module declaration of dcesrv_srvsvc is fairly clear, but the
> 'eval' bit isn't. If you stare at it long enough you may realise it is
> saying that we need to build the srvsvc/proto.h prototype file using
> an external rule called proto_header_template. The use of the eval is
> pretty hard to understand though.
>
> I also think the dcerpc_srvsvc_OBJ_FILES part could be clearer. It
> works by magically appending "_OBJ_FILES" to the end of the module
> name to find the variable which lists the object files the module
> needs. It is not awful, but it could be better.
>
> For the wscript build my auto-conversion script read the above snippet
> and created this instead:
>
>   bld.SAMBA_MODULE('dcerpc_srvsvc',
>                    source='srvsvc/dcesrv_srvsvc.c srvsvc/srvsvc_ntvfs.c',
>                    autoproto='srvsvc/proto.h',
>                    subsystem='dcerpc_server',
>                    init_function='dcerpc_server_srvsvc_init',
>                    deps='DCERPC_COMMON NDR_SRVSVC share'
>                    )
>
> which (at least to me!) is much clearer, especially for the automatic
> prototype generation. Even for someone who had never worked with this
> build system before I think they could probably work out how to make
> changes to it. If they do get it wrong, then at least the build system
> is likely to tell them what they did wrong so they can fix it. That
> comes from using a language that has error checking. I've also 
> adding extra ASSERT() statements in the waf samba rules to add
> additional checks to ensure that common mistakes give very clear
> errors. For example:
>
>     bld.ASSERT(subsystem is not None, "You must specify a subsystem for init_function '%s'" % init_function)
>
> Ok, so that is the 2nd criterion for a successful conversion to a new
> build system. Common maintenance tasks of Samba must be possible by
> people who are not really familiar with the internals of the build
> system.
>
> Next, we need to be able to encode the project rules. One of the
> biggest problems with the Samba4 build at the moment is that we
> generate massive binaries are libraries, about 10x larger than we
> should. This happens because subsystems get included many times in
> different libraries and get included in lots of binaries. As you saw
> in my previous email to Simo, I suggested that we need a set of
> 'project rules' that make this sort of thing an error. I know how to
> encode those sorts of rules in waf, as I've already done it for the
> related dependency checking rules. For example, I've got a rule that
> checks that we don't have any circular dependencies, and it produces
> this:
>
>  ERROR: Circular dependency for auth_system_session: auth_system_session->auth_session->SAMDB->auth_system_session
>  Removing dependency auth_session from target auth_system_session
>
> the same type of waf rules can be used to enforce any type of project
> rule you like. Can you do that in cmake? Quite possibly, but you'd
> need to demonstrate it to convince me. 
>
> I hope that is enough for you to understand what I'm personally
> looking for in a build system. Other team members may have other
> priorities. Once we have a replacement available that works we will
> have to look at it as a team and decide if it really is better than
> what we have now. If it isn't then it won't be used.
>
> If you do decide to pursue this, then I would strongly encourage you
> to try to build a tool that auto-converts the existing config.mk files
> in Samba. Otherwise you will spend a very long time re-creating the
> equivalent by hand. You may wish to look at my (extremely hackish!)
> mktowscript.pl conversion program in my waf-wip branch to get
> started. It shouldn't be hard to modify it to output whatever cmake
> needs.
>
> I should also mention that it could happen that I hit some major
> stumbling block with waf, and I can't make a Samba build system with
> waf that is better than our current system. I don't think that will
> happen as I'm far enough along to already be looking at the hardest
> parts of the conversion, but if it does happen then I might look at
> cmake instead, or maybe we'll just try to fix the existing build
> system if cmake can't handle the basic criterion I have given above.
>
> What if we end up with both a set of cmake rules and a set of waf
> rules that meet these basic criterion? In that case I suspect it will
> come down to the ancillary stuff, like usability for our existing
> users (eg. --prefix --enable-developer etc), and dependence on
> external tools. It may be that cmake has some advantages that could be
> weighed against those. In which case someone would have to argue what
> those are, and we'd need to make a choice.
>
> Either way, this whole conversion is going to take a long time and a
> lot of effort. So let's please make sure we end up with something that
> really is better than what we have now. To get started on that, it is
> essential that the person doing the conversion really understands the
> current system.
>
> Cheers, Tridge
>   


-- 
Thank you,
Dmitri Pal

Engineering Manager IPA project,
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/