Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
nemequ opened this issue Jun 14, 2016 · 15 comments
Closed

Comments

@nemequ
Copy link
Contributor

nemequ commented Jun 14, 2016

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):

  • Do you want to install a library and the header files?
    • If so, do you want to generate and install a pkg-config file? pkg-config is the preferred way to convey information about how other people should link to your library for pretty much everyone except CMake.
  • If you have unit tests, would you like to install them? It's not very common, but https://wiki.gnome.org/Initiatives/GnomeGoals/InstalledTests lists some reasons you may want to.
  • Would you like to provide an autotools-style configure script as an optional wrapper for your build system (see configure-cmake for details)

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
Copy link
Collaborator

eustas commented Jun 15, 2016

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:

  • Premake is straightforward, but might be inconvenient
  • Bazel is also straightforward, but I'm not sure if it is possible to pull BUILD files out of subdirectories
  • CMake looks powerful, but complicated

@nemequ
Copy link
Contributor Author

nemequ commented Jun 20, 2016

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
Copy link

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:

  • Snow Leopard (OSX 10.6.8) on Mac
  • Centos 5 for Linux
  • FreeBSD 7

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:

  • it doesn't compile on FreeBSD 7 - I guess gcc is too old
  • earlier versions worked on OSX, but yesterday's version expands instead of compressing (separate issue)
  • the version I downloaded yesterday doesn't build on OSX 10.6.8 with make

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
Copy link
Contributor Author

nemequ commented Jun 22, 2016

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
Copy link
Collaborator

eustas commented Jun 22, 2016

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

  • use xcodebuild
  • remove -Wl,--start-group / -Wl,--end-group enclosure in build files
  • call directly make brotli to avoid building everything (brotli.make seem to be not corrupted)

@eustas
Copy link
Collaborator

eustas commented Jun 22, 2016

And, yes, cmake will be added soon.

@eustas
Copy link
Collaborator

eustas commented Jun 22, 2016

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

@eustas
Copy link
Collaborator

eustas commented Jun 27, 2016

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

@nemequ
Copy link
Contributor Author

nemequ commented Jun 27, 2016

@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
Copy link
Collaborator

eustas commented Jun 28, 2016

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
Copy link
Contributor Author

nemequ commented Jun 28, 2016

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
Copy link
Contributor Author

nemequ commented Jun 29, 2016

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
Copy link
Contributor Author

nemequ commented Jul 23, 2016

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:

  • Not sure if you're willing to use an unmodified configure-cmake, but if so I'm happy to add a patch to add it.
  • I'm willing to add a pkg-config file and install a shared library, but I'm not sure you want to do that until the API is stable. Unless you want to version everything (i.e., libbrotli0.5, brotli-0.5.pc, /usr/include/brotli/0.5/*, etc.), which I'd be happy to throw together.
  • If you want I could probably port most of Squash's tests over to Brotli (especially if you're okay with using µnit like Squash does).

@eustas
Copy link
Collaborator

eustas commented Jul 28, 2016

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

@nemequ
Copy link
Contributor Author

nemequ commented Aug 4, 2016

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.

@nemequ nemequ closed this as completed Aug 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants