You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2016/09/21 18:55:31 UTC

Re: svn commit: r1761714 - in /httpd/httpd/trunk: CHANGES CMakeLists.txt docs/log-message-tags/next-number modules/filters/config.m4 modules/filters/mod_brotli.c os/win32/BaseAddr.ref


On 09/21/2016 12:38 PM, kotkov@apache.org wrote:
> Author: kotkov
> Date: Wed Sep 21 10:38:48 2016
> New Revision: 1761714
> 
> URL: http://svn.apache.org/viewvc?rev=1761714&view=rev
> Log:
> mod_brotli: Add initial implementation.
> 
> This new module supports dynamic Brotli (RFC 7932) compression.  Existing
> mod_deflate installations can benefit from better compression ratio by
> sending Brotli-compressed data to the clients that support it:
> 
>     SetOutputFilter BROTLI_COMPRESS;DEFLATE
> 
> The module features zero-copy processing, which is only possible with the
> new API from the upcoming 1.0.x series of brotli [1].  The Linux makefile
> works against libbrotli [2], as currently the core brotli repository doesn't
> offer a way to build a library [3].  Apart from that, only the CMake build
> is now supported.
> 
> [1] https://github.com/google/brotli
> [2] https://github.com/bagder/libbrotli
> [3] https://github.com/google/brotli/pull/332
> 
> Added:
>     httpd/httpd/trunk/modules/filters/mod_brotli.c   (with props)
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/CMakeLists.txt
>     httpd/httpd/trunk/docs/log-message-tags/next-number
>     httpd/httpd/trunk/modules/filters/config.m4
>     httpd/httpd/trunk/os/win32/BaseAddr.ref
> 
>

> Modified: httpd/httpd/trunk/modules/filters/config.m4
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/config.m4?rev=1761714&r1=1761713&r2=1761714&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/filters/config.m4 (original)
> +++ httpd/httpd/trunk/modules/filters/config.m4 Wed Sep 21 10:38:48 2016
> @@ -141,6 +141,127 @@ APACHE_MODULE(proxy_html, Fix HTML Links
>  ]
>  )
>  
> +dnl
> +dnl APACHE_CHECK_BROTLI
> +dnl
> +dnl Configure for Brotli, giving preference to
> +dnl "--with-brotli=<path>" if it was specified.
> +dnl
> +AC_DEFUN([APACHE_CHECK_BROTLI],[
> +  AC_CACHE_CHECK([for Brotli], [ac_cv_brotli], [
> +    dnl initialise the variables we use
> +    ac_cv_brotli=no
> +    ac_brotli_found=""
> +    ac_brotli_base=""
> +    ac_brotli_libs=""
> +    ac_brotli_mod_cflags=""
> +    ac_brotli_mod_ldflags=""
> +
> +    dnl Determine the Brotli base directory, if any
> +    AC_MSG_CHECKING([for user-provided Brotli base directory])
> +    AC_ARG_WITH(brotli, APACHE_HELP_STRING(--with-brotli=PATH,Brotli installation directory), [
> +      dnl If --with-brotli 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
> +        ac_brotli_base="`cd $withval ; pwd`"
> +      fi
> +    ])
> +    if test "x$ac_brotli_base" = "x"; then
> +      AC_MSG_RESULT(none)
> +    else
> +      AC_MSG_RESULT($ac_brotli_base)
> +    fi
> +
> +    dnl Run header and version checks
> +    saved_CPPFLAGS="$CPPFLAGS"
> +    saved_LIBS="$LIBS"
> +    saved_LDFLAGS="$LDFLAGS"
> +
> +    dnl Before doing anything else, load in pkg-config variables
> +    if test -n "$PKGCONFIG"; then
> +      saved_PKG_CONFIG_PATH="$PKG_CONFIG_PATH"
> +      if test "x$ac_brotli_base" != "x" -a \
> +              -f "${ac_brotli_base}/lib/pkgconfig/libbrotlienc.pc"; then
> +        dnl Ensure that the given path is used by pkg-config too, otherwise
> +        dnl the system libbrotlienc.pc might be picked up instead.
> +        PKG_CONFIG_PATH="${ac_brotli_base}/lib/pkgconfig${PKG_CONFIG_PATH+:}${PKG_CONFIG_PATH}"
> +        export PKG_CONFIG_PATH
> +      fi
> +      ac_brotli_libs="`$PKGCONFIG --libs-only-l --silence-errors libbrotlienc`"
> +      if test $? -eq 0; then
> +        ac_brotli_found="yes"
> +        pkglookup="`$PKGCONFIG --cflags-only-I libbrotlienc`"
> +        APR_ADDTO(CPPFLAGS, [$pkglookup])
> +        APR_ADDTO(MOD_CFLAGS, [$pkglookup])
> +        pkglookup="`$PKGCONFIG --libs-only-L libbrotlienc`"
> +        APR_ADDTO(LDFLAGS, [$pkglookup])
> +        APR_ADDTO(MOD_LDFLAGS, [$pkglookup])
> +        pkglookup="`$PKGCONFIG --libs-only-other libbrotlienc`"
> +        APR_ADDTO(LDFLAGS, [$pkglookup])
> +        APR_ADDTO(MOD_LDFLAGS, [$pkglookup])
> +      fi
> +      PKG_CONFIG_PATH="$saved_PKG_CONFIG_PATH"
> +    fi
> +
> +    dnl fall back to the user-supplied directory if not found via pkg-config
> +    if test "x$ac_brotli_base" != "x" -a "x$ac_brotli_found" = "x"; then
> +      APR_ADDTO(CPPFLAGS, [-I$ac_brotli_base/include])
> +      APR_ADDTO(MOD_CFLAGS, [-I$ac_brotli_base/include])
> +      APR_ADDTO(LDFLAGS, [-L$ac_brotli_base/lib])
> +      APR_ADDTO(MOD_LDFLAGS, [-L$ac_brotli_base/lib])
> +      if test "x$ap_platform_runtime_link_flag" != "x"; then
> +        APR_ADDTO(LDFLAGS, [$ap_platform_runtime_link_flag$ac_brotli_base/lib])
> +        APR_ADDTO(MOD_LDFLAGS, [$ap_platform_runtime_link_flag$ac_brotli_base/lib])
> +      fi
> +    fi
> +
> +    ac_brotli_libs="${ac_brotli_libs:--lbrotlienc `$apr_config --libs`} "
> +    APR_ADDTO(MOD_LDFLAGS, [$ac_brotli_libs])

This breaks compilation of trunk if libbrotlienc is not present as it is added unconditionally.
But even if would be added conditionally I sense that all filters would be linked against
libbrotlienc. So better only add to MOD_BROTLI_LDADD instead of MOD_LDFLAGS

> +    APR_ADDTO(LIBS, [$ac_brotli_libs])
> +
> +    dnl Run library and function checks
> +    liberrors=""
> +    AC_CHECK_HEADERS([brotli/encode.h])
> +    AC_MSG_CHECKING([for Brotli version >= 1.0.0])
> +    AC_TRY_COMPILE([#include <brotli/encode.h>],[
> +const uint8_t *o = BrotliEncoderTakeOutput((BrotliEncoderState*)0, (size_t*)0);],
> +      [AC_MSG_RESULT(OK)
> +       ac_cv_brotli="yes"],
> +      [AC_MSG_RESULT(FAILED)])
> +
> +    dnl restore
> +    CPPFLAGS="$saved_CPPFLAGS"
> +    LIBS="$saved_LIBS"
> +    LDFLAGS="$saved_LDFLAGS"
> +
> +    dnl cache MOD_LDFLAGS, MOD_CFLAGS
> +    ac_brotli_mod_cflags=$MOD_CFLAGS
> +    ac_brotli_mod_ldflags=$MOD_LDFLAGS
> +  ])
> +  if test "x$ac_cv_brotli" = "xyes"; then
> +    APR_ADDTO(MOD_LDFLAGS, [$ac_brotli_mod_ldflags])
> +
> +    dnl Ouch!  libbrotlienc.1.so doesn't link against libm.so (-lm),
> +    dnl although it should.  Workaround that in our LDFLAGS:
> +
> +    APR_ADDTO(MOD_LDFLAGS, ["-lm"])
> +    APR_ADDTO(MOD_CFLAGS, [$ac_brotli_mod_cflags])
> +  fi
> +])
> +
> +APACHE_MODULE(brotli, Brotli compression support, , , most, [
> +  APACHE_CHECK_BROTLI
> +  if test "$ac_cv_brotli" = "yes" ; then
> +      if test "x$enable_brotli" = "xshared"; then
> +         # The only symbol which needs to be exported is the module
> +         # structure, so ask libtool to hide everything else:
> +         APR_ADDTO(MOD_BROTLI_LDADD, [-export-symbols-regex brotli_module])
> +      fi
> +  else
> +      enable_brotli=no
> +  fi
> +])
> +
>  APR_ADDTO(INCLUDES, [-I\$(top_srcdir)/$modpath_current])
>  
>  APACHE_MODPATH_FINISH
> 

Regards

R�diger

RE: svn commit: r1761714 - in /httpd/httpd/trunk: CHANGES CMakeLists.txt docs/log-message-tags/next-number modules/filters/config.m4 modules/filters/mod_brotli.c os/win32/BaseAddr.ref

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Original Message-----
> From: Evgeny Kotkov [mailto:evgeny.kotkov@visualsvn.com]
> Sent: Donnerstag, 22. September 2016 00:17
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r1761714 - in /httpd/httpd/trunk: CHANGES
> CMakeLists.txt docs/log-message-tags/next-number modules/filters/config.m4
> modules/filters/mod_brotli.c os/win32/BaseAddr.ref
> 
> Ruediger Pluem <rp...@apache.org> writes:
> 
> >> +    ac_brotli_libs="${ac_brotli_libs:--lbrotlienc `$apr_config --
> libs`} "
> >> +    APR_ADDTO(MOD_LDFLAGS, [$ac_brotli_libs])
> >
> > This breaks compilation of trunk if libbrotlienc is not present as it is
> > added unconditionally. But even if would be added conditionally I sense
> > that all filters would be linked against libbrotlienc. So better only
> add
> > to MOD_BROTLI_LDADD instead of MOD_LDFLAGS
> 
> Indeed, I totally messed up this part of the configuration script.  Sorry
> for that.
> 
> I committed a rework of this part in r1761824 [1], and it fixes this build
> issue for me.  Could you please check it in your environment as well?

Yes, this fixes it. Thanks.

Regards

Rüdiger


Re: svn commit: r1761714 - in /httpd/httpd/trunk: CHANGES CMakeLists.txt docs/log-message-tags/next-number modules/filters/config.m4 modules/filters/mod_brotli.c os/win32/BaseAddr.ref

Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Ruediger Pluem <rp...@apache.org> writes:

>> +    ac_brotli_libs="${ac_brotli_libs:--lbrotlienc `$apr_config --libs`} "
>> +    APR_ADDTO(MOD_LDFLAGS, [$ac_brotli_libs])
>
> This breaks compilation of trunk if libbrotlienc is not present as it is
> added unconditionally. But even if would be added conditionally I sense
> that all filters would be linked against libbrotlienc. So better only add
> to MOD_BROTLI_LDADD instead of MOD_LDFLAGS

Indeed, I totally messed up this part of the configuration script.  Sorry
for that.

I committed a rework of this part in r1761824 [1], and it fixes this build
issue for me.  Could you please check it in your environment as well?

[1] https://svn.apache.org/r1761824


Thanks,
Evgeny Kotkov