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

Documentation on when/why/how to write _impl.hpp #12

Open
springmeyer opened this issue Oct 17, 2016 · 3 comments
Open

Documentation on when/why/how to write _impl.hpp #12

springmeyer opened this issue Oct 17, 2016 · 3 comments

Comments

@springmeyer
Copy link
Contributor

springmeyer commented Oct 17, 2016

@tmpsantos thanks for working on mapbox/geojson-cpp#24. Now I'd like your help in documenting your work so others can do this again for further projects. Can you write up a quick guide on how you accomplished this, key terms, and why you chose _impl.hpp as an extention (let's standardize on that). Drop your notes into this ticket as a comment please and then we'll integrate into the main readme.

/cc @mapsam @GretaCB

refs mapbox/hpp-skel#3

@tmpsantos
Copy link

tmpsantos commented Oct 18, 2016

Motivations

Header-only libraries are often easier to include in a project than external dependencies because they get compiled together with the project using them and they can just get bundled together with the project source files in a src/third-party directory.

This will make sure they get compiled with the same compiler flags as you use for your project and you don't need to build a new dependency package for every new architecture/operating system combination you support.

C++ libraries heavily using templates make a good candidate for a header-only library because templates are often implemented completely in a header. This is true as long as the header-only library does not have a dependency by itself.

Making header-only optional, geojson-cpp case

A library like geojson-cpp is small enough to be a good candidate for a header-only library. The problem is it depends on rapidjson, which is another header-only library.

Building it as a static library means we can hide the dependency on rapidjson. Deploying it as a header-only library assumes the project using geojson-cpp also depends on rapidjson, which is the case for Mapbox GL Native but not exactly obvious for developers only interested on a GeoJSON parser.

The _impl.hpp pattern

Fortunately there is a clean way to make a dependency be both a pre-compiled library and a header-only library, giving developers choice.

In the case of geojson-cpp, the part of the project depending on rapidjson got isolated in another header called geojson_impl.hpp. The project has a geojson.cpp that includes the implementation header:

#include <mapbox/geojson_impl.hpp>

Now the developer using geojson-cpp as a project dependency has two choices:

    1. Include geojson.hpp and link with libgeojson.a.

or:

    1. Include geojson.hpp, geojson_impl.hpp and make sure the project also has the headers for rapidjson available in the include path.

Including geojson_impl.hpp AND linking with libgeojson.a will cause a link error because the symbols are going to get duplicated. Remember that libgeojson.a was made from geojson.cpp that includes geojson_impl.hpp. It is like having geojson_impl.cpp at the end of the day.

Mason package

mason handles header-only packages differently from static/shared libraries. The header-only package is explicitly marked as MASON_HEADER_ONLY=true on the package description script.

When packing a library that offers both options, the library has to be packaged twice for each release, as we did for geojson-cpp:

https://github.com/mapbox/mason/blob/master/scripts/geojson/0.3.2/script.sh
https://github.com/mapbox/mason/blob/master/scripts/geojson/0.3.2-hpp/script.sh

When publishing the package to AWS, they won't conflict because they get posted to different directories.

Using in a project with mason and cmake

A project using geojson-cpp static library would simple need to add the following line to the build system:

mason_use(geojson VERSION 0.3.2)

Another project using the header only version would have to do:

mason_use(rapidjson VERSION 1.1.0 HEADER_ONLY)
mason_use(geojson VERSION 0.3.2 HEADER_ONLY)

Note that rapidjson is also needed in this case. Your project will also need to build a .cpp file including geojson_impl.hpp to satisfy the missing symbols like we have in Mapbox GL Native.

@tmpsantos
Copy link

@springmeyer feel free to edit/add. Let me know if there is any other aspect of the _impl.hpp you need better explained.

@springmeyer
Copy link
Contributor Author

@tmpsantos this looks fantastic, thank you 🙏

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

2 participants