On 17.08.2012 10:11, Joe Orton wrote: > On Thu, Aug 16, 2012 at 08:36:31PM +0200, Kaspar Brand wrote: >> I wonder if we should add support for module-specific CFLAGS etc., >> which would always appear before the EXTRA_XXX stuff in the compile >> and link commands, i.e. in rules.mk we would have: >> >> ALL_CFLAGS = $(MOD_CFLAGS) $(EXTRA_CFLAGS) $(NOTEST_CFLAGS) $(CFLAGS) >> ALL_CPPFLAGS = $(DEFS) $(INTERNAL_CPPFLAGS) $(MOD_CPPFLAGS) >> $(EXTRA_CPPFLAGS) $(NOTEST_CPPFLAGS) $(CPPFLAGS) >> ALL_INCLUDES = $(INCLUDES) $(MOD_INCLUDES) $(EXTRA_INCLUDES) >> >> ALL_LDFLAGS = $(MOD_LDFLAGS) $(EXTRA_LDFLAGS) $(NOTEST_LDFLAGS) $(LDFLAGS) >> >> A particular module could then set its specific MOD_CFLAGS etc. in >> modules.mk, and these would always have priority over those possibly >> inserted by other modules. > > Doing CFLAGS et al like that doesn't generalise brilliantly, because > they are per-directory (modules/xxx) not strictly per-module, but it > could be done anyway, and that wouldn't matter for mod_ssl. Yeah, > probably a good idea.
I gave it a try, and so far it seems to work as expected, see the attached patch (against r1358166, to reduce clutter). Right now the following modules are affected: - modules/cache: mod_socache_dc (--with-distcache) - modules/filters: mod_deflate (--with-z), mod_xml2enc (--with-libxml2), mod_proxy_html (--with-libxml2) - modules/lua: mod_lua (--with-lua, --enable-luajit) - modules/ssl: mod_ssl (--with-ssl) I.e. there's currently only a potential clash in modules/filters. > It should be possible to override LDFLAGS truly per-module by tweaking > the SH_LINK line generated in modules.mk. I didn't pursue this option for the time being, as it would currently only be of potential benefit for mod_deflate and mod_xml2enc/mod_proxy_html. If you (or other devs) think it's an important aspect, please let me know and I'll have a closer look. Feedback and comments about the proposed approach - especially from build system experts - is very much welcome. Kaspar
Index: acinclude.m4 =================================================================== --- acinclude.m4 (revision 1358166) +++ acinclude.m4 (working copy) @@ -149,12 +149,20 @@ fi ]) +dnl the list of build variabled which are available for customization on a +dnl per module directory basis (to be inserted into modules.mk with a "MOD_" +dnl prefix, i.e. MOD_CFLAGS etc.). Used in APACHE_MODPATH_{INIT,FINISH}. +define(mod_buildvars, [CFLAGS CXXFLAGS CPPFLAGS LDFLAGS LIBS INCLUDES]) +dnl dnl APACHE_MODPATH_INIT(modpath) AC_DEFUN(APACHE_MODPATH_INIT,[ current_dir=$1 modpath_current=modules/$1 modpath_static= modpath_shared= + for var in mod_buildvars; do + eval MOD_$var= + done test -d $1 || $srcdir/build/mkdir.sh $modpath_current > $modpath_current/modules.mk ])dnl @@ -163,6 +171,11 @@ echo "DISTCLEAN_TARGETS = modules.mk" >> $modpath_current/modules.mk echo "static = $modpath_static" >> $modpath_current/modules.mk echo "shared = $modpath_shared" >> $modpath_current/modules.mk + for var in mod_buildvars; do + if eval val=\"\$MOD_$var\"; test -n "$val"; then + echo "MOD_$var = $val" >> $modpath_current/modules.mk + fi + done if test ! -z "$modpath_static" -o ! -z "$modpath_shared"; then MODULE_DIRS="$MODULE_DIRS $current_dir" else @@ -480,7 +493,7 @@ dnl Determine the OpenSSL base directory, if any AC_MSG_CHECKING([for user-provided OpenSSL base directory]) - AC_ARG_WITH(ssl, APACHE_HELP_STRING(--with-ssl=DIR,OpenSSL base directory), [ + AC_ARG_WITH(ssl, APACHE_HELP_STRING(--with-ssl=PATH,OpenSSL installation directory), [ dnl If --with-ssl specifies a directory, we use that directory if test "x$withval" != "xyes" -a "x$withval" != "x"; then dnl This ensures $withval is actually a directory and that it is absolute @@ -497,7 +510,6 @@ saved_CPPFLAGS="$CPPFLAGS" saved_LIBS="$LIBS" saved_LDFLAGS="$LDFLAGS" - SSL_LIBS="" dnl Before doing anything else, load in pkg-config variables if test -n "$PKGCONFIG"; then @@ -514,10 +526,10 @@ ap_openssl_found="yes" pkglookup="`$PKGCONFIG --cflags-only-I openssl`" APR_ADDTO(CPPFLAGS, [$pkglookup]) - APR_ADDTO(INCLUDES, [$pkglookup]) + APR_ADDTO(MOD_INCLUDES, [$pkglookup]) pkglookup="`$PKGCONFIG --libs-only-L --libs-only-other openssl`" APR_ADDTO(LDFLAGS, [$pkglookup]) - APR_ADDTO(SSL_LIBS, [$pkglookup]) + APR_ADDTO(MOD_LDFLAGS, [$pkglookup]) fi PKG_CONFIG_PATH="$saved_PKG_CONFIG_PATH" fi @@ -525,12 +537,12 @@ dnl fall back to the user-supplied directory if not found via pkg-config if test "x$ap_openssl_base" != "x" -a "x$ap_openssl_found" = "x"; then APR_ADDTO(CPPFLAGS, [-I$ap_openssl_base/include]) - APR_ADDTO(INCLUDES, [-I$ap_openssl_base/include]) + APR_ADDTO(MOD_INCLUDES, [-I$ap_openssl_base/include]) APR_ADDTO(LDFLAGS, [-L$ap_openssl_base/lib]) - APR_ADDTO(SSL_LIBS, [-L$ap_openssl_base/lib]) + APR_ADDTO(MOD_LDFLAGS, [-L$ap_openssl_base/lib]) if test "x$ap_platform_runtime_link_flag" != "x"; then APR_ADDTO(LDFLAGS, [$ap_platform_runtime_link_flag$ap_openssl_base/lib]) - APR_ADDTO(SSL_LIBS, [$ap_platform_runtime_link_flag$ap_openssl_base/lib]) + APR_ADDTO(MOD_LDFLAGS, [$ap_platform_runtime_link_flag$ap_openssl_base/lib]) fi fi @@ -548,8 +560,10 @@ if test "x$ac_cv_openssl" = "xyes"; then ap_openssl_libs="-lssl -lcrypto `$apr_config --libs`" - APR_ADDTO(SSL_LIBS, [$ap_openssl_libs]) + APR_ADDTO(MOD_LDFLAGS, [$ap_openssl_libs]) APR_ADDTO(LIBS, [$ap_openssl_libs]) + dnl SSL_LIBS is used for building ab (in support/Makefile) + APR_SETVAR(SSL_LIBS, [$MOD_LDFLAGS]) APACHE_SUBST(SSL_LIBS) dnl Run library and function checks @@ -585,7 +599,7 @@ ac_cv_serf=no serf_prefix=/usr SERF_LIBS="" - AC_ARG_WITH(serf, APACHE_HELP_STRING([--with-serf=PREFIX], + AC_ARG_WITH(serf, APACHE_HELP_STRING([--with-serf=PATH], [Serf client library]), [ if test "$withval" = "yes" ; then @@ -611,7 +625,7 @@ if test "$ac_cv_serf" = "yes"; then AC_DEFINE(HAVE_SERF, 1, [Define if libserf is available]) APR_SETVAR(SERF_LIBS, [-L$serf_prefix/lib -lserf-0]) - APR_ADDTO(INCLUDES, [-I$serf_prefix/include/serf-0]) + APR_ADDTO(MOD_INCLUDES, [-I$serf_prefix/include/serf-0]) fi ]) Index: build/rules.mk.in =================================================================== --- build/rules.mk.in (revision 1369276) +++ build/rules.mk.in (working copy) @@ -22,13 +22,16 @@ # the user-defined flags can always override the configure ones, if needed. # Note that includes are listed after the flags because -I options have # left-to-right precedence and CPPFLAGS may include user-defined overrides. +# The "MOD_" prefixed variable are provided to allow modules to insert their +# (per-subdirectory) settings through definitions in modules.mk, with highest +# precedence. # -ALL_CFLAGS = $(EXTRA_CFLAGS) $(NOTEST_CFLAGS) $(CFLAGS) -ALL_CPPFLAGS = $(DEFS) $(INTERNAL_CPPFLAGS) $(EXTRA_CPPFLAGS) $(NOTEST_CPPFLAGS) $(CPPFLAGS) -ALL_CXXFLAGS = $(EXTRA_CXXFLAGS) $(NOTEST_CXXFLAGS) $(CXXFLAGS) -ALL_LDFLAGS = $(EXTRA_LDFLAGS) $(NOTEST_LDFLAGS) $(LDFLAGS) -ALL_LIBS = $(EXTRA_LIBS) $(NOTEST_LIBS) $(LIBS) -ALL_INCLUDES = $(INCLUDES) $(EXTRA_INCLUDES) +ALL_CFLAGS = $(MOD_CFLAGS) $(EXTRA_CFLAGS) $(NOTEST_CFLAGS) $(CFLAGS) +ALL_CPPFLAGS = $(DEFS) $(INTERNAL_CPPFLAGS) $(MOD_CPPFLAGS) $(EXTRA_CPPFLAGS) $(NOTEST_CPPFLAGS) $(CPPFLAGS) +ALL_CXXFLAGS = $(MOD_CXXFLAGS) $(EXTRA_CXXFLAGS) $(NOTEST_CXXFLAGS) $(CXXFLAGS) +ALL_LDFLAGS = $(MOD_LDFLAGS) $(EXTRA_LDFLAGS) $(NOTEST_LDFLAGS) $(LDFLAGS) +ALL_LIBS = $(MOD_LIBS) $(EXTRA_LIBS) $(NOTEST_LIBS) $(LIBS) +ALL_INCLUDES = $(MOD_INCLUDES) $(INCLUDES) $(EXTRA_INCLUDES) # Compile commands Index: modules/cache/config.m4 =================================================================== --- modules/cache/config.m4 (revision 1369276) +++ modules/cache/config.m4 (working copy) @@ -42,7 +42,7 @@ dnl Determine the distcache base directory, if any AC_MSG_CHECKING([for user-provided distcache base]) - AC_ARG_WITH(distcache, APACHE_HELP_STRING(--with-distcache=DIR, Distcache installation directory), [ + AC_ARG_WITH(distcache, APACHE_HELP_STRING(--with-distcache=PATH, Distcache installation directory), [ dnl If --with-distcache specifies a directory, we use that directory or fail if test "x$withval" != "xyes" -a "x$withval" != "x"; then dnl This ensures $withval is actually a directory and that it is absolute @@ -63,7 +63,7 @@ if test "x$ap_distcache_base" != "x"; then APR_ADDTO(CPPFLAGS, [-I$ap_distcache_base/include]) - APR_ADDTO(INCLUDES, [-I$ap_distcache_base/include]) + APR_ADDTO(MOD_INCLUDES, [-I$ap_distcache_base/include]) APR_ADDTO(LDFLAGS, [-L$ap_distcache_base/lib]) APR_ADDTO(ap_distcache_ldflags, [-L$ap_distcache_base/lib]) if test "x$ap_platform_runtime_link_flag" != "x"; then Index: modules/filters/config.m4 =================================================================== --- modules/filters/config.m4 (revision 1369276) +++ modules/filters/config.m4 (working copy) @@ -34,7 +34,7 @@ APACHE_MODULE(deflate, Deflate transfer encoding support, , , most, [ - AC_ARG_WITH(z, APACHE_HELP_STRING(--with-z=DIR,use a specific zlib library), + AC_ARG_WITH(z, APACHE_HELP_STRING(--with-z=PATH,use a specific zlib library), [ if test "x$withval" != "xyes" && test "x$withval" != "x"; then ap_zlib_base="$withval" @@ -66,6 +66,7 @@ ap_zlib_ldflags="" if test "$ap_zlib_base" != "/usr"; then APR_ADDTO(INCLUDES, [-I${ap_zlib_base}/include]) + APR_ADDTO(MOD_INCLUDES, [-I${ap_zlib_base}/include]) dnl put in CPPFLAGS temporarily so that AC_TRY_LINK below will work CPPFLAGS="$CPPFLAGS $INCLUDES" APR_ADDTO(LDFLAGS, [-L${ap_zlib_base}/lib]) @@ -82,13 +83,13 @@ APR_ADDTO(MOD_DEFLATE_LDADD, [$ap_zlib_ldflags -lz])], [AC_MSG_RESULT(not found) enable_deflate=no - INCLUDES=$ap_save_includes if test "x$ap_zlib_with" = "x"; then AC_MSG_WARN([... Error, zlib was missing or unusable]) else AC_MSG_ERROR([... Error, zlib was missing or unusable]) fi ]) + INCLUDES=$ap_save_includes LDFLAGS=$ap_save_ldflags CPPFLAGS=$ap_save_cppflags APR_REMOVEFROM(LIBS, [-lz]) @@ -98,7 +99,7 @@ AC_DEFUN(FIND_LIBXML2, [ AC_CACHE_CHECK([for libxml2], [ac_cv_libxml2], [ AC_ARG_WITH(libxml2, - [APACHE_HELP_STRING(--with-libxml2,location for libxml2)], + [APACHE_HELP_STRING(--with-libxml2=PATH,location for libxml2)], [test_paths="${with_libxml2}"], [test_paths="/usr/include/libxml2 /usr/local/include/libxml2 /usr/include /usr/local/include"] ) @@ -122,7 +123,7 @@ APACHE_MODULE(xml2enc, i18n support for markup filters, , , , [ FIND_LIBXML2 if test "$ac_cv_libxml2" = "yes" ; then - APR_ADDTO(CFLAGS, [-I${XML2_INCLUDES}]) + APR_ADDTO(MOD_CFLAGS, [-I${XML2_INCLUDES}]) APR_ADDTO(MOD_XML2ENC_LDADD, [-lxml2]) else enable_xml2enc=no @@ -131,7 +132,7 @@ APACHE_MODULE(proxy_html, Fix HTML Links in a Reverse Proxy, , , , [ FIND_LIBXML2 if test "$ac_cv_libxml2" = "yes" ; then - APR_ADDTO(CFLAGS, [-I${XML2_INCLUDES}]) + APR_ADDTO(MOD_CFLAGS, [-I${XML2_INCLUDES}]) APR_ADDTO(MOD_PROXY_HTML_LDADD, [-lxml2]) else enable_proxy_html=no Index: modules/lua/config.m4 =================================================================== --- modules/lua/config.m4 (revision 1369276) +++ modules/lua/config.m4 (working copy) @@ -128,7 +128,7 @@ AC_MSG_NOTICE([using '${LUA_LIBS}' for Lua Library]) AC_ARG_ENABLE(luajit, APACHE_HELP_STRING(--enable-luajit,Enable LuaJit Support), - APR_ADDTO(CPPFLAGS, ["-DAP_ENABLE_LUAJIT"])) + APR_ADDTO(MOD_CPPFLAGS, ["-DAP_ENABLE_LUAJIT"])) ifelse([$1], , , $1) fi ]) @@ -138,7 +138,7 @@ APACHE_MODULE(lua, Apache Lua Framework, $lua_objects, , , [ CHECK_LUA() if test "x$enable_lua" != "xno" ; then - APR_ADDTO(INCLUDES, [$LUA_CFLAGS]) + APR_ADDTO(MOD_INCLUDES, [$LUA_CFLAGS]) APR_ADDTO(MOD_LUA_LDADD, [$LUA_LIBS]) fi ]) Index: modules/ssl/config.m4 =================================================================== --- modules/ssl/config.m4 (revision 1369276) +++ modules/ssl/config.m4 (working copy) @@ -40,11 +40,10 @@ APACHE_MODULE(ssl, [SSL/TLS support (mod_ssl)], $ssl_objs, , most, [ APACHE_CHECK_OPENSSL if test "$ac_cv_openssl" = "yes" ; then - APR_ADDTO(MOD_SSL_LDADD, [\$(SSL_LIBS)]) if test "x$enable_ssl" = "xshared"; then # The only symbol which needs to be exported is the module # structure, so ask libtool to hide everything else: - APR_ADDTO(MOD_SSL_LDADD, [-export-symbols-regex ssl_module]) + APR_ADDTO(MOD_LDFLAGS, [-export-symbols-regex ssl_module]) fi else enable_ssl=no