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