google / brotli

Brotli compression format
MIT License
13.54k stars 1.24k forks source link

Provide a build build system for both system-wide installation and bundling #374

Closed nemequ closed 8 years ago

nemequ commented 8 years ago

Since Squash bundles Brotli in our tree, we basically maintain a separate build system for Brotli. I'm trying to reduce the amount of duplication between Squash and the libraries it bundles, so I'd like to be able to reuse Brotli's build system, which would mean making it compatible with our own (CMake-based) build-system. Obviously such a system wouldn't be specific to Squash; any project which uses CMake and wants to bundle Brotli could benefit from it. With such a system, bundling Brotli would be as simple as something like:

set(BROTLI_BUNDLE_MODE ON)
add_subdirectory(brotli)

target_link_libraries(mycode brotli)

Brotli would then be built as part of the parent project, including unit tests (which could also automatically integrate with ctest, so running make test would run brotli's tests as well) and documentation (if applicable).

The rest of this is largely copied from quixdb/squash#202, edited slightly to be specific to brotli.

From Squash's perspective, as well as anyone else who wants to bundle the library, what we need is a way to build a static library (or two, if you want to separate the encoder and decoder), disable any installation, and control whether or not to have CTest run unit tests. Finally, it would be great if we could simply reference a target and have it automatically set any necessary flags (CMake ≥ 2.8.11 can do this, you just need to set the proper INTERFACE_* target properties).

If you would like to see an example of a system which works the way we want, take a look at BriefLZ's. I'm willing to create a similar system for Brotli, though obviously it would need to be a bit more extensive since Brotli is a larger project. I don't want to fork anything, so if you aren't interested we'll just stick with the current system for that plugin. Obviously not everyone is familiar with CMake so part of this is that I would help with maintenance as needed. However, before I proceed I need a few questions answered:

First, are you willing to provide a CMake-based build system in your tree? If so, are you okay with making it flexible enough to meet our needs as described above? If no to either of those, you can go ahead and close this issue and we'll keep interacting with Brotli just as we do today. Otherwise, there are a few ways to put this together:

  1. Place a single CMakeLists.txt file in the top level of your project, which contains all the logic for your entire project. If you're familiar with autotools, this is like a non-recursive build system.
  2. Place a CMakeLists.txt in the top level directory, as well as in each subdirectory. The top-level file will basically include each subdirectory's file. If you're familiar with autotools, it's a bit like a traditional recursive build system.
  3. Put the CMake build system in a subdirectory somewhere (such as contrib/cmake).

Note that it may be necessary to put supplementary modules in a single directory somewhere. I usually use a cmake subdirectory alongside the highest level CMakeLists.txt, wherever that is.

Options (1) and (2) basically treat CMake as a first-class citizen, whereas (3) makes it pretty clear that CMake is an option, but may not really be the recommended way to build your project. Option (1) vs. (2) is mostly just about style; (1) puts your entire build system in one place, whereas (2) keeps the logic closer to the code. I prefer (2), but it's up to you.

Would you want CMake to replace your current system, or would you prefer for the two to exist in parallel?

Also, there are some optional features which I need to know if you would like to support (assuming you don't already, obviously we can't really remove features):

Note that, as mentioned above, installation would need to be optional (projects integrating your project into their own tree probably don't want to install your CLI).

Finally, if custom CMake modules are necessary (see https://github.com/quixdb/squash/tree/master/cmake for some examples), would you prefer to copy the modules into your tree, or use a git submodule?

eustas commented 8 years ago

Hello, Evan. I like the option (1), but the help with writing CMakeLists.txt is much appreciated. I consider having multiple build systems on board, as long as they do not interfere and reside in root and build/ directories. Currently planned build systems are CMake, Premake, and Bazel:

nemequ commented 8 years ago

I consider having multiple build systems on board, as long as they do not interfere and reside in root and build/ directories. Currently planned build systems are CMake, Premake, and Bazel:

FWIW, I would avoid this if I were you. It's easy to let the build systems get out of sync. You're already forgetting about the build system for the python module.

Anyways, I put together a quick CMake system at https://github.com/nemequ/brotli. It would probably be a good idea to test it in Travis and/or AppVeyor, but unfortunately the python stuff is in the way. I'll try to put together a quick helper script when I have time. I think everything else should work, including running the tests (without depending on sh, so it should work on Windows) and installing the bro executable.

hashbackup commented 8 years ago

Building a static .a library would be helpful for integrating Brotli into projects. I've been developing a backup program, HashBackup, and everything is loaded statically into one self-contained executable.

Brotli is a great compression program, but is complex compared to zstd. I have to build on old platforms to have upward compatiblity, so that's:

These seem ancient, but I have enterprise-y customers still using them. Building zstd into Python/HashBackup was easy. Adding Brotli was not so easy:

Here's the make output with OSX 10.6.8:

[jim@mb brotli-master]$ make ==== Building brotli_common (release) ==== Creating obj/Release/brotli_common dictionary.c Linking brotli_common ld: warning: option -s is obsolete and being ignored ==== Building brotli_dec (release) ==== Creating obj/Release/brotli_dec bit_reader.c decode.c huffman.c state.c Linking brotli_dec ld: warning: option -s is obsolete and being ignored ld: unknown option: --start-group collect2: ld returned 1 exit status make[2]: * [../../bin/libbrotli_dec.so] Error 1 make[1]: * [brotli_dec] Error 2 make: *\ [all] Error 2

nemequ commented 8 years ago

That looks like an issue with premake. Maybe because the files were generated ahead of time instead of on the system you're (trying to) compile on?

Anyways, the cmake support in my repo builds 3 static libraries, just like the premake version. I've also tested it by embedding the repo into Squash's brotli plugin, so if you're using cmake for HashBackup you should be able to just add_subdirectory(brotli) and use the brotli libraries as-needed.

eustas commented 8 years ago

It is a problem that was recently introduced in premake5. I have plans to add postfilter to fix it. There are 3 workarounds:

eustas commented 8 years ago

And, yes, cmake will be added soon.

eustas commented 8 years ago

Running premake5 gmake on mac fixes the problem. Perhaps I'll remove generated buildfiles to avoid confusion.

eustas commented 8 years ago

Removed generated buildfiles to avoid further confusion. Going to pull CMake soon.

nemequ commented 8 years ago

@eustas I managed to get travis working on my repo: https://travis-ci.org/nemequ/brotli/builds/140581300

That brings up a few more questions… for Squash we test a bunch of different compilers on Travis. Do you want me to do something similar for Brotli? If so, which compiler(s) and versions? For ICC you would need a license, but everything else should be feasible…

Also, you never said whether you wanted a configure-cmake wrapper for this.

Eventually it would probably be a good idea to generate a pkg-config file and maybe install the tests, but that should probably wait until the API stabilizes and brotli is ready to install a shared library.

eustas commented 8 years ago

Wide list of compilers would be a nice addition (different versions of clang/gcc/mingw). Also I see, it is possible to build/test with ASAN / MSAN. That is a super-cool option. So if you add more variants, we will be very grateful. (Going to investigate ICC license question later).

configure-cmake seems to cover all possible use-cases. Perhaps, a more light-weight version of this wrapper would fit our tiny simple project more =)

About API stabilization - you are absolutely right. We plan that API will finally stabilize in next 6 months, and then going to publish packages for distros. Though, some background pre-work could be done earlier.

nemequ commented 8 years ago

Also I see, it is possible to build/test with ASAN / MSAN. That is a super-cool option.

Yes, and I'll do that, but it's worth noting that this is most useful with a more comprehensive test suite… as it is, you're not really throwing anything at brotli that it doesn't expect… if you want, I can look into porting at least some of the tests over from Squash (I'll open up a separate issue for that, though).

So if you add more variants, we will be very grateful.

Sure, I'll try to take care of it later today.

(Going to investigate ICC license question later).

https://software.intel.com/en-us/articles/open-source-contributor-faq

I don't think Google would meet the criteria for a free license, though being Google I guess it's likely you already own a license which could be used…

configure-cmake seems to cover all possible use-cases.

Heh, it really doesn't, but thank you for saying so ;). I like to think it comes close.

Perhaps, a more light-weight version of this wrapper would fit our tiny simple project more =)

Perhaps, but I don't see why you would want to reinvent the wheel here. configure-cmake already exists and is known to work, the license is permissive, it doesn't create any hard dependencies (it's just shell scripting, not even bash), it's a single file (well, two if you count the configuration), and you happen to know the jerk who wrote it so if there are issues you have someone to talk to / blame.

nemequ commented 8 years ago

Okay, I managed to get most of the stuff done for Travis. See https://travis-ci.org/nemequ/brotli/builds/140999421

I went with container builds instead of full VMs, which is faster and easier on Travis. Unfortunately, it means the clang 3.5-3.8 builds can't be enabled yet because Travis hasn't re-enabled the LLVM apt repositories (they were down for a while, so Travis disabled them, but they're back up now).

It also messes things up for the sanitizers, which are apparently broken with the compilers packages from the ubuntu-toolchain-r repository. It may be possible to fix this when the LLVM repositories are available by switching the sanitizer builds from gcc to clang.

I did not include any builds using premake, though I guess it wouldn't be hard to add. TBH, I just have no idea how to use premake.

nemequ commented 8 years ago

Virtually everything should be working now (see https://travis-ci.org/nemequ/brotli/builds/146808543). The only real hole is TSan, which I don't think is going to happen on 12.04… between an ancient version of CMake and updated compilers which expect newer binutils, there are just too many problems :(. I'm sure I could get it working on 14.04, but if you want to use Travis' container infrastructure 12.04 is required, and given the number of different builds I think the container infrastructure is definitely a good idea.

Remaining related issues:

eustas commented 8 years ago

PR #397 has landed. WooHoo! Kudos to Evan!

nemequ commented 8 years ago

The original issue is resolved, so I'm going to close this. I'm still willing to put together PRs for the other stuff mentioned in my previous comment; just let me know if you want me to.