You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by William A Rowe Jr <wr...@rowe-clan.net> on 2017/04/11 03:55:28 UTC

Re: svn commit: r1790806 - /httpd/httpd/branches/2.4.x/STATUS

On Mon, Apr 10, 2017 at 6:50 AM,  <ji...@apache.org> wrote:
> Author: jim
> Date: Mon Apr 10 11:50:26 2017
> New Revision: 1790806
>
> URL: http://svn.apache.org/viewvc?rev=1790806&view=rev
> Log:
> With v0.60 of https://github.com/google/brotli released,
> this is now viable again.

Agreed this is no longer 'being worked' but is again proposed...

> --- httpd/httpd/branches/2.4.x/STATUS (original)
> +++ httpd/httpd/branches/2.4.x/STATUS Mon Apr 10 11:50:26 2017
> @@ -177,17 +177,6 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>                    protocol input, also seems unwise)
>        jailletc36: needs r1790457 but can be merged afterwards. No functionnal change.
>
> -PATCHES/ISSUES THAT ARE BEING WORKED
> -  [ New entried should be added at the START of the list ]
> -
> -  *) core: Disallow multiple Listen on the same IP:port when listener buckets
> -     are configured (ListenCoresBucketsRatio > 0), consistently with the single
> -     bucket case (default), thus avoiding the leak of the corresponding socket
> -     descriptors on graceful restart.
> -     trunk patch: http://svn.apache.org/r1789220
> -     2.4.x patch: trunk works (modulo CHANGES)
> -     +1: ylavic
> -
>    *) mod_brotli: Backport of mod_brotli filter
>       trunk patch: http://svn.apache.org/r1761714
>                    http://svn.apache.org/r1762512
> @@ -198,8 +187,17 @@ PATCHES/ISSUES THAT ARE BEING WORKED
>                    http://svn.apache.org/r1779699
>       2.4.x patch: http://home.apache.org/~jim/patches/brotli-2.4.patch
>       +1: jim, jorton,
> -     -1: wrowe (Premature, waiting on github.com/google/brotli 0.6 release)
> -     NOTE: Awaiting next release post 0.5.2

I was happy with the state of master as of Friday. I have not reviewed
the final release package, however.

I was very very happy with the huge documentation improvements
today. One key aspect I identified, that --deflate ++brotli is a net loss
for our examples, is unacceptable. Increasing the total traffic by ripping
out one universal compression method for a solution supported by only
half of the clients is, I'm sure you agree, a joke. Brotli can only be added
as a supplemental compression scheme in addition to gzip for older
clients at this time, particularly since the introduction of this compression
adds a large incremental cpu tax on the server.

I was disappointed to learn from the docs that input compression is not
even supported by the proposed enhancement. The browser clients
may not support it, but obviously other significant clients such as svn
should be adding it promply, the cpu cost to the clients pushing data
is not our worry at all, and the cpu cost of httpd decoding the brotli
compression is hardly measurable if not an actual win.

I'm amused by your rejection of my completing code review of this
submission, Jim. Classy.

Re: svn commit: r1790806 - /httpd/httpd/branches/2.4.x/STATUS

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Tue, Apr 11, 2017 at 6:39 AM, Jim Jagielski <ji...@jagunet.com> wrote:
>
>> On Apr 10, 2017, at 11:55 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>>
>>> -     -1: wrowe (Premature, waiting on github.com/google/brotli 0.6 release)
>>> -     NOTE: Awaiting next release post 0.5.2
>>
>> I'm amused by your rejection of my completing code review of this
>> submission, Jim. Classy.
>
> Bill, your veto is quoted above. As such, with the release of 0.6
> it was no long applicable.

And is my responsibility to remove, generally from one vote state to
another vote state. E.g. a -1 was blocking the release. Other reasons
for the veto had been stated on list, and

> As far as your problems w/ building brotli, maybe you simply aren't
> doing it right.
>
>  ../configure-cmake

You missed the step where you checked out github (via repo source
link, perhaps) so you were using v0.6.0.tar.gz vs Brotli-0.6.0.tar.gz
I had attempted to use... this wasn't distinguished in the package
names. (The later might better have been named Brotli_py-0.6.0);

Brotli-0.6.0.tar.gz
Brotli-0.6.0.zip
Source code (zip)
Source code (tar.gz)

Seems a complete source package is in the works, but v0.6.0.tar.gz
seems to be useable. Will be adjusting my vote shortly based on the
feedback in https://github.com/google/brotli/issues/539

Re: svn commit: r1790806 - /httpd/httpd/branches/2.4.x/STATUS

Posted by Jim Jagielski <ji...@jaguNET.com>.
> On Apr 10, 2017, at 11:55 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> 
>> 
>> -     -1: wrowe (Premature, waiting on github.com/google/brotli 0.6 release)
>> -     NOTE: Awaiting next release post 0.5.2
> 
> 
> I'm amused by your rejection of my completing code review of this
> submission, Jim. Classy.

Bill, your veto is quoted above. As such, with the release of 0.6
it was no long applicable.

As far as your problems w/ building brotli, maybe you simply aren't
doing it right.

 ../configure-cmake
-- The C compiler identification is AppleClang 8.1.0.8020038
-- The CXX compiler identification is AppleClang 8.1.0.8020038
-- Check for working C compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc
-- Check for working C compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
-- Check for working CXX compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Looking for log2
-- Looking for log2 - found
-- Configuring done
CMake Warning (dev):
  Policy CMP0042 is not set: MACOSX_RPATH is enabled by default.  Run "cmake
  --help-policy CMP0042" for policy details.  Use the cmake_policy command to
  set the policy and suppress this warning.

  MACOSX_RPATH is not specified for the following targets:

   brotlicommon
   brotlidec
   brotlienc

This warning is for project developers.  Use -Wno-dev to suppress it.

-- Generating done
-- Build files have been written to: /usr/local2/src/brotli-0.6.0/out
<jimsys:src/brotli-0.6.0/out> % make
Scanning dependencies of target brotlicommon
[  3%] Building C object CMakeFiles/brotlicommon.dir/common/dictionary.c.o
[  7%] Linking C shared library libbrotlicommon.dylib
[  7%] Built target brotlicommon
Scanning dependencies of target brotlienc
[ 11%] Building C object CMakeFiles/brotlienc.dir/enc/backward_references.c.o
[ 14%] Building C object CMakeFiles/brotlienc.dir/enc/backward_references_hq.c.o
[ 18%] Building C object CMakeFiles/brotlienc.dir/enc/bit_cost.c.o
[ 22%] Building C object CMakeFiles/brotlienc.dir/enc/block_splitter.c.o
[ 25%] Building C object CMakeFiles/brotlienc.dir/enc/brotli_bit_stream.c.o
[ 29%] Building C object CMakeFiles/brotlienc.dir/enc/cluster.c.o
[ 33%] Building C object CMakeFiles/brotlienc.dir/enc/compress_fragment.c.o
[ 37%] Building C object CMakeFiles/brotlienc.dir/enc/compress_fragment_two_pass.c.o
[ 40%] Building C object CMakeFiles/brotlienc.dir/enc/dictionary_hash.c.o
[ 44%] Building C object CMakeFiles/brotlienc.dir/enc/encode.c.o
[ 48%] Building C object CMakeFiles/brotlienc.dir/enc/entropy_encode.c.o
[ 51%] Building C object CMakeFiles/brotlienc.dir/enc/histogram.c.o
[ 55%] Building C object CMakeFiles/brotlienc.dir/enc/literal_cost.c.o
[ 59%] Building C object CMakeFiles/brotlienc.dir/enc/memory.c.o
[ 62%] Building C object CMakeFiles/brotlienc.dir/enc/metablock.c.o
[ 66%] Building C object CMakeFiles/brotlienc.dir/enc/static_dict.c.o
[ 70%] Building C object CMakeFiles/brotlienc.dir/enc/utf8_util.c.o
[ 74%] Linking C shared library libbrotlienc.dylib
[ 74%] Built target brotlienc
Scanning dependencies of target brotlidec
[ 77%] Building C object CMakeFiles/brotlidec.dir/dec/bit_reader.c.o
[ 81%] Building C object CMakeFiles/brotlidec.dir/dec/decode.c.o
[ 85%] Building C object CMakeFiles/brotlidec.dir/dec/huffman.c.o
[ 88%] Building C object CMakeFiles/brotlidec.dir/dec/state.c.o
[ 92%] Linking C shared library libbrotlidec.dylib
[ 92%] Built target brotlidec
Scanning dependencies of target bro
[ 96%] Building C object CMakeFiles/bro.dir/tools/bro.c.o
[100%] Linking C executable bro
[100%] Built target bro
<jimsys:src/brotli-0.6.0/out> % sudo make install
[  7%] Built target brotlicommon
[ 74%] Built target brotlienc
[ 92%] Built target brotlidec
[100%] Built target bro
Install the project...
-- Install configuration: "Debug"
-- Installing: /usr/local/bin/bro
-- Installing: /usr/local/lib/libbrotlienc.0.6.0.dylib
-- Installing: /usr/local/lib/libbrotlienc.dylib
-- Installing: /usr/local/lib/libbrotlidec.0.6.0.dylib
-- Installing: /usr/local/lib/libbrotlidec.dylib
-- Installing: /usr/local/lib/libbrotlicommon.0.6.0.dylib
-- Installing: /usr/local/lib/libbrotlicommon.dylib
-- Up-to-date: /usr/local/include/brotli
-- Installing: /usr/local/include/brotli/decode.h
-- Installing: /usr/local/include/brotli/encode.h
-- Installing: /usr/local/include/brotli/port.h
-- Installing: /usr/local/include/brotli/types.h
-- Installing: /usr/local/lib/pkgconfig/libbrotlicommon.pc
-- Installing: /usr/local/lib/pkgconfig/libbrotlidec.pc
-- Installing: /usr/local/lib/pkgconfig/libbrotlienc.pc

Re: svn commit: r1790806 - /httpd/httpd/branches/2.4.x/STATUS

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Apr 10, 2017 at 10:55 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> I was happy with the state of master as of Friday. I have not reviewed
> the final release package, however.

EFAIL

cd Brotli-0.6.0 && \
  cmake -G "Unix Makefiles" \
-D CMAKE_INSTALL_LIBDIR=lib \
-D CMAKE_INSTALL_PREFIX=../dst/httpd-2.4.x \
../src/Brotli-0.6.0 && \
cd ..
CMake Error: The source directory "../src/Brotli-0.6.0" does not
appear to contain CMakeLists.txt.

Was working Friday. Awkward :)