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

Separate C and C++ libraries and CMake fixes #6

Merged
merged 2 commits into from Aug 3, 2018

Conversation

perfinion
Copy link
Contributor

CMake: version sofiles and respect libdir

Version the libraries like libnsync.so.1.20.0 and set the symlinks
properly. The version is taken from the VERSION file in the root, it
should be updated with new releases. The versioning requires CMake 3.0
so the minimum version is updated.

64bit distros use /usr/lib64 not /usr/lib so this needs to be
configurable as well.

The source properties for C++ are set outside of enabling the tests
otherwise building the C++ library with tests disabled fails.

CMake: add separate libnsync_cpp library target so C/C++ do not conflict

The libraries should be named differently otherwise they cannot be
installed together on the system. libnsync.so is the C library.
libnsync_cpp.so is the C++ library. The header files for both are
identical.

Both are built from the same source files so the C++ source files must
be copied so that the source file property LANGUAGE can be set. The copy
is done during build so the configure step does not need to be re-done
after any code changes. Per-OS settings are now all done together and
then two functions set the C / C++ properties on the build targets.

The C library is the recommended one and the C++ library takes
significantly longer to build and test so it is possible to only build
and install only the C library.

$ mkdir build; cd build
$ cmake ..
$ make nsync
$ make test_c
$ make install

Will build and install only the C library. make all will still build
both libraries like normal. There are also test_c / test_cpp targets
to run only C/C++ tests.

Signed-off-by: Jason Zaman jason@perfinion.com

Version the libraries like libnsync.so.1.20.0 and set the symlinks
properly. The version is taken from the VERSION file in the root, it
should be updated with new releases. The minimum is kept CMake version
is kept at 2.8 but >=3.0 is strongly recommended. Since the minimum is
<3.0, policy CMP0048 must be set to NEW.

64bit distros use /usr/lib64 not /usr/lib so this needs to be
configurable as well.

The source properties for C++ are set outside of enabling the tests
otherwise building the C++ library with tests disabled fails.

Signed-off-by: Jason Zaman <jason@perfinion.com>
The libraries should be named differently otherwise they cannot be
installed together on the system. libnsync.so is the C library.
libnsync_cpp.so is the C++ library. The header files for both are
identical.

Both are built from the same source files so the C++ source files must
be copied so that the source file property LANGUAGE can be set. The copy
is done during build so the configure step does not need to be re-done
after any code changes. Per-OS settings are now all done together and
then two functions set the C / C++ properties on the build targets.

The C library is the recommended one and the C++ library takes
significantly longer to build and test so it is possible to only build
and install only the C library.

$ mkdir build; cd build
$ cmake ..
$ make nsync
$ make test_c
$ make install

Will build and install only the C library. make all will still build
both libraries like normal. There are also test_c / test_cpp targets
to run only C/C++ tests.

Signed-off-by: Jason Zaman <jason@perfinion.com>
@perfinion
Copy link
Contributor Author

perfinion commented Aug 3, 2018

I force-pushed an update to keep the cmake min required version at 2.8.
Also added to the documentation saying that >=3.0 is strongly recommended.

Copy link
Collaborator

@m3bm3b m3bm3b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.
Many thanks.

@m3bm3b m3bm3b merged commit 9120459 into google:master Aug 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants