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

Reply via email to