Add a CMakeLists.txt file

[bsergean]

Hi there, thanks for this super useful and fast library.

I added a simple CMake file which should make it so that more users can use your library. One good recent feature of cmake is called FetchContent, which make it real easy to include another library by downloading it from github and building it. I added some doc about it in the README file.

[ebiggers]

Maintaining multiple build systems would be error-prone and extra work. (And I only have time to work in libdeflate in my free time, so anything that avoids introducing additional maintenance burden would be very helpful.) So the only way I might accept this is if CMake fully replaced the current Makefile. That means the CMake replacement would need to support everything the current Makefile supports, and the README would need to be fully updated, and all scripts in the scripts/ directory that are currently running make would need to be updated to use the CMake replacement instead.

Generally, my preference is to keep the plain Makefile. FWIW, libdeflate actually used CMake originally, but pretty quickly I replaced it with a plain Makefile since it seemed just as simple, and it was easier to use (e.g. make -j8 <options> instead of rm -r build; mkdir build; cd build; cmake .. <options>; make -j8 instead of just make -j8). And I don't really buy claims like "CMake is becoming a standard now", as lots of projects instead use GNU autotools, a plain Makefile, Meson, Bazel, etc...

It's still possible that I could be convinced if there were large benefits to switching to CMake in particular, though, and if it actually replaced the current Makefile as mentioned above, and if you did a really good job with the changes.

[bsergean]

Thanks for taking a look, you're making some good points.

I have a project where I still have a very tiny makefile in the top folder which takes care of invoking cmake for the common case which can be a chore. I do all my development on mac or linux.

I don't know at which pace I can improve this PR, but the current version is relatively complete, I'll look in more details at what is inside the scripts folder.

One advantage of cmake is that it is truly cross platform and works well on Windows, while autotools is not, and I haven't followed the latest trends, but bazel seems to be for niche projects or google only projects, same with buck (the facebook one). Not sure what big project uses Meson, I can't think of any, maybe jsoncpp. Also FetchContent is quite cool I think.

I know C++ more, but serious projects that now have cmake support are libcurl, libuv, spdlog, rapidjson, openssl. If your project has cmake it can be included more easily in vcpkg which is Microsoft attempt at adding a real package manager for C/C++. (there's something called cdeps too for just C).

[bsergean]

I found something interesting where a build fail through cmake, but not with the regular make build. (the xenial warning config). The other outstanding thing I'm looking at is making shared libraries, where I get missing symbols.

In file included from /home/travis/build/bsergean/libdeflate/lib/adler32.c:63:0:

/home/travis/build/bsergean/libdeflate/lib/arm/adler32_impl.h: In function ‘adler32_neon_chunk’:

/home/travis/build/bsergean/libdeflate/lib/arm/adler32_impl.h:102:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]

  *s1 += v_s1[0] + v_s1[1] + v_s1[2] + v_s1[3];

  ^

/home/travis/build/bsergean/libdeflate/lib/arm/adler32_impl.h:103:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]

  *s2 += v_s2[0] + v_s2[1] + v_s2[2] + v_s2[3];

  ^

cc1: all warnings being treated as errors

[ebiggers]

In file included from /home/travis/build/bsergean/libdeflate/lib/adler32.c:63:0:

/home/travis/build/bsergean/libdeflate/lib/arm/adler32_impl.h: In function ‘adler32_neon_chunk’:

/home/travis/build/bsergean/libdeflate/lib/arm/adler32_impl.h:102:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]

  *s1 += v_s1[0] + v_s1[1] + v_s1[2] + v_s1[3];

  ^

/home/travis/build/bsergean/libdeflate/lib/arm/adler32_impl.h:103:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]

  *s2 += v_s2[0] + v_s2[1] + v_s2[2] + v_s2[3];

  ^

cc1: all warnings being treated as errors

That's a known false positive compiler warning with some gcc versions. In .travis.yml (https://github.com/ebiggers/libdeflate/blob/master/.travis.yml#L43), this warning is suppressed on xenial + gcc + arm64, which is the only tested case where this warning is seen.

[bsergean]

I updated the PR @Spacelm

On Jan 11, 2021, at 1:19 AM, SpaceIm [email protected] wrote:

@SpaceIm commented on this pull request.

In CMakeLists.txt https://github.com/ebiggers/libdeflate/pull/101#discussion_r554909272:

+set(lint-vla $<$BOOL:${DEFLATE_LINT_VLA}:-Wvla>) +set(lint-implicit-fallthrough $<$BOOL:${DEFLATE_LINT_IMPLICIT_FALLTHROUGH}:-Wimplicit-fallthrough>)

+target_compile_options(deflate PRIVATE ${lint-all}) +target_compile_options(deflate PRIVATE ${lint-undef}) +target_compile_options(deflate PRIVATE ${lint-pedantic}) +target_compile_options(deflate PRIVATE ${lint-declaration-after-statement}) +target_compile_options(deflate PRIVATE ${lint-missing-prototypes}) +target_compile_options(deflate PRIVATE ${lint-strict-prototypes}) +target_compile_options(deflate PRIVATE ${lint-vla}) +target_compile_options(deflate PRIVATE ${lint-implicit-fallthrough}) + +# The CFLAGS environment variable is read and handled automatically by cmake + +if (BUILD_SHARED_LIBS)

• target_compile_options(deflate PRIVATE -fvisibility=hidden -D_ANSI_SOURCE) There is an abstraction of visibility preset in CMake: https://cmake.org/cmake/help/latest/prop_tgt/LANG_VISIBILITY_PRESET.html https://cmake.org/cmake/help/latest/prop_tgt/LANG_VISIBILITY_PRESET.html — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ebiggers/libdeflate/pull/101#pullrequestreview-565173766, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2O6UPSVS3QPYD3B2JCIPTSZK7ANANCNFSM4TB4PY7Q.

[bsergean]

$ mkdir build
$ cd build
$ cmake -DDEFLATE_BUILD_TEST_PROGRAMS=ON ..
$ make ...
$ make test
Running tests...
Test project /Users/benjaminsergeant/src/foss/github/libdeflate/build
    Start 1: test_checksums
1/6 Test #1: test_checksums ...................   Passed    0.25 sec
    Start 2: test_custom_malloc
2/6 Test #2: test_custom_malloc ...............   Passed    0.24 sec
    Start 3: test_incomplete_codes
3/6 Test #3: test_incomplete_codes ............   Passed    0.22 sec
    Start 4: test_litrunlen_overflow
4/6 Test #4: test_litrunlen_overflow ..........   Passed    0.24 sec
    Start 5: test_slow_decompression
5/6 Test #5: test_slow_decompression ..........   Passed    0.21 sec
    Start 6: test_trailing_bytes
6/6 Test #6: test_trailing_bytes ..............   Passed    0.21 sec

100% tests passed, 0 tests failed out of 6

Total Test time (real) =   1.37 sec

[bsergean]

@ebiggers I've changed the PR to be purely additive, and not conflict with any existing makefiles or scripts.

I know you made your point about wanting to have a single build-system, but would you consider accepting the cmake files as community maintained, which can be highlighted even more in the readme? Project like libuv or curl have multiple build systems.

A bunch of folks use IDE projects generators, and having cmake support will let them build libdeflate when used in their project that way. Also the cmake ninja generator will work fine here.

[rleigh-codelibre]

I was just looking at CMake support for libdeflate (for use with libTIFF), and noticed all the hard work had already been done! This is really nice.

For building with Windows with MinGW or MSVC, CMake is a real improvement on the two Makefiles. It would make it much easier to package and distribute with vcpkg for example. The Makefile.msc has no install targets and no support for debug vs release runtimes. CMake can provide all that directly.

Some comments on the PR:

• If libdeflate depends upon ZLIB, it would be nice to have a transitive dependency on ZLIB::ZLIB and have the exported configuration use find_dependency to automatically find and create the imported target. You can see an example of this here L226-249 and this template. Note: this was originally done with a fairly old CMake version; there may now be a more transparent way of importing the transitive dependencies.
• Shared library versioning doesn't look quite right; if the existing Makefiles don't do this either it would be nice to set up some proper versions rather than 0.0.0 for all build systems
• You might want to set the debug postfix to "d" for Windows, so we can have debug and release libraries coexisting (would also be nice for getting libdeflate packaged with vcpkg). Example:

# Debug postfix
if(MSVC)
    set(CMAKE_DEBUG_POSTFIX "d")
endif()

Edit: One other oddity noticed while working on libdeflate support in libtiff. Why does libdeflate have a "lib" prefix on Windows? It's easy enough to work around, but CMake's find library doesn't find "deflate" by default because it only adds a "lib" prefix on Unix platforms. Why are the platform library naming conventions not being followed on Windows (in Makefile.msc)? Looks like this is one thing that the CMake build will do correctly.

As an aside, FindDeflate might be of interest. The library naming doesn't quite match the static Makefile names; having the debug postfixes would be necessary to make this work properly. If you can make the exported configuration stuff work properly, this can be removed, but just mentioning it in case it's of use to others who want to use libdeflate with CMake (as opposed to building with CMake).

Kind regards, Roger

[bsergean]

Thanks Roger, I’ll try to incorporate your feedback in the PR.

1. I don’t think that the library depends on zlib, just the tests and not even sure
2. Getting the shared library versioning shouldn’t be too hard I’ll try to get that working
3. The debug postfix looks really easy to do.

Last thing is that we could put that in a repo maybe called libdeflate-cmake, similar to openssl ? I know that supporting multiple build system can be annoying. I think have a ‘community level’ support for CMake would be ideal.

Ps: we use (as opposed to build) libdeflate with CMake through ExternalProject version and it works alright.

On Mar 7, 2021, at 9:56 AM, Roger Leigh [email protected] wrote:

I was just looking at CMake support for libdeflate (for use with libTIFF), and noticed all the hard work had already been done! This is really nice.

For building with Windows with MinGW or MSVC, CMake is a real improvement on the two Makefiles. It would make it much easier to package and distribute with vcpkg for example. The Makefile.msc has no install targets and no support for debug vs release runtimes. CMake can provide all that directly.

Some comments on the PR:

If libdeflate depends upon ZLIB, it would be nice to have a transitive dependency on ZLIB::ZLIB and have the exported configuration use find_dependency to automatically find and create the imported target. You can see an example of this here https://gitlab.com/codelibre/ome/ome-files-cpp/-/blob/master/lib/ome/files/CMakeLists.txt#L226 L226-249 and this template https://gitlab.com/codelibre/ome/ome-files-cpp/-/blob/master/lib/ome/files/OMEFilesConfig.cmake.in. Note: this was originally done with a fairly old CMake version; there may now be a more transparent way of importing the transitive dependencies. Shared library versioning doesn't look quite right; if the existing Makefiles don't do this either it would be nice to set up some proper versions rather than 0.0.0 for all build systems You might want to set the debug postfix to "d" for Windows, so we can have debug and release libraries coexisting (would also be nice for getting libdeflate packaged with vcpkg). Example:

Debug postfix

if(MSVC) set(CMAKE_DEBUG_POSTFIX "d") endif() As an aside, FindDeflate https://gitlab.com/libtiff/libtiff/-/blob/master/cmake/FindDeflate.cmake might be of interest. The library naming doesn't quite match the static Makefile names; having the debug postfixes would be necessary to make this work properly. If you can make the exported configuration stuff work properly, this can be removed, but just mentioning it in case it's of use to others who want to use libdeflate with CMake (as opposed to building with CMake).

Kind regards, Roger

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ebiggers/libdeflate/pull/101#issuecomment-792323879, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2O6UIDAVKFVYWV6H46O23TCO46NANCNFSM4TB4PY7Q.

[SpaceIm]

I have created libdeflate recipe in conan public repository, and I'm also interested in CMake support in libdeflate. Indeed libdeflate is now an optional dependency of libtiff (since 4.2.0) and gdal (since 3.2.0), enabled by default, so it's indirectly included in the dependency graph of a lot of libraries. Therefore a robust build would be welcome.

[ivan-volnov]

Any news here?

[serge2016]

I am also very interested!

[diizzyy]

I think most would appreciate using CMake as framework :-)

[ckerr]

At the risk of piling on, I'd also like to see CMake support for libdeflate. It would make embedding it in my own CMake-based application much simpler. :)

[ebiggers]

Given the interest, I'm willing to switch to CMake, but it has to replace the current Makefile instead of being added alongside it.

[bsergean]

I think there should be a mini transition period where both makefile and cmakefiles exists, so that we can know that the new system actually works.

I'm still very interested in having this PR land, but I have also a lot of other things on my plate so I don't know when I'll get to this. @SpaceIm / how you could you add your proposed changes to this PR ? By making a PR in my repo ?

[SpaceIm]

@SpaceIm / how you could you add your proposed changes to this PR ? By making a PR in my repo ?

Sure.

[SpaceIm]

@bsergean Please consider https://github.com/bsergean/libdeflate/pull/1

• fix export of symbols for non-Windows if shared
• change target to libdeflate, and imported target to libdeflate::libdeflate
• honor lib name from original Makefile if windows & static (but still drop the lib prefix if msvc, and .lib for mingw in favor of .a or .dll.a)
• add VERSION
• improve CMake config file
• install a pkgconfig file
• always build the cli & honor name from Makefile. Also install it in bin folder
• better separation between cli & tests
• link zlib with tests only & don't forget to find it with find_package(). Also properly express the scope of dependencies of each target.
• edit README regarding CMake:
  • more generic commands
  • don't speak about FetchContent. Proper way to consume a library is to install it, then call find_package() in your own CMakeLists

So I think it also addresses @eli-schwartz review.

Tested on macOS with AppleClang 13, static & shared.

[eli-schwartz]

I commented on that PR. The generated pkg-config file has an edge case that leads to extremely broken behavior if the user configures with -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_INSTALL_LIBDIR=/usr/lib (using an absolute path for values other than prefix).

This is very difficult to fix properly due to the design of cmake (although meson handles it perfectly), although there are ways to make it less terrible.

[SpaceIm]

another one: https://github.com/bsergean/libdeflate/pull/2

MinGW is broken by the way (for utility & tests). -municode must be injected during compilation of the the utility & tests, but I can't make it work with CMake.

[ebiggers]

At the risk of piling on, but for the record: I for one would likely lose any interest in contributing to libdeflate if it switched from a Makefile to CMake.

Can you elaborate on why that is?

[ivan-volnov]

At the risk of piling on, but for the record: I for one would likely lose any interest in contributing to libdeflate if it switched from a Makefile to CMake.

But no great loss: I haven't yet contributed to libdeflate in any case. 😃

Nice try ))

Well, I personally believe that CMake is very important for many developers. CMake support allows easy integration with current big projects that use the library.

Even Boost, having their own build system, is gradually switching to CMake.

So, in my opinion, the library can gain a lot of contemporary developers by only switching the build system.

[eli-schwartz]

(Boost's own build system also has the curious status of being worse than handwritten Makefiles.)

[SunBlack]

For me: decades of experience with Make, versus an opinion that CMake scripts are inflexible and obscure.

From my point of view CMake has a lot of benefits over Makefile, especially on bigger projects which are multiplatform:

• Generation for various IDEs (there are still some projects which provides solutions for each MSVC version in their repo)
• You don't need to search the whole Makefiles for available options (not all are adding a documentation at top of the Makefile)
• Generation of installer or deb files (last thing would be e.g. nice to have here)
• Checking of available features (in this MR I missing e.g. the AVX detection, this could resolve the issue, that the MSVC build result is slower than this of mingw)
• Easy integration within other projects via imported targets. I don't know e.g. how do check the minimum required version with a Makefile.
• ...

Of course CMake is not perfect (as it is a scripting language), therefore tools like CMake format are not even perfect. But compared to using plain Makefiles it is a dream (if you are using CMake 3.x) ;-)

[eli-schwartz]

• not everyone cares about or likes IDEs, but you seem to be talking more about MSVC than about IDEs?

• Makefiles aren't a real build system, real build systems like autotools and meson have --help or configure commands that do this for you. Sadly, this is actually a point against cmake, since cmake is like raw Makefiles and requires reading the entire build files to find undocumented variables that can be and usually are anywhere. Cmake doesn't have a help command. You can run a command after you unsuccessfully configured the project with the wrong options, that prints every internal state variable whether they are options or not, without context or guidance, though.

• Generation of deb files is an antipattern, please use the distro tooling or something like checkinstall. :)

• again Makefiles aren't a real build system. Real build systems like autotools and meson and cmake too, lets you check for features before building. It's hardly unique to cmake, but you can certainly do it with cmake.

• Integration with other projects via imported targets is an antipattern, pkg-config exists and has the benefit of working with projects that aren't cmake. That being said, you can install cmake config files from any build system, it doesn't require you to build with cmake. Also yes, if you already use cmake you can add another cmake project using one of 3 different verbose and clunky methods which all break picking up system copies without even more scripting. At least you can do it! You cannot do it at all with raw Makefiles.

...

Basically you're making a lot of arguments in favor of having a build system at all. I agree. Real build systems are good. ~~Even cmake is better than no build system at all.~~

[1100101]

In my opinion you ignore the fact that CMake can be used on Windows, while Make cannot.

Also, CMake offers ccmake or cmake-gui for easy configuration and a full overview over all available options.

[jmarshall]

I wrote a brief comment as a counterpoint to similarly content-free brief comments from CMake advocates, in response to https://github.com/ebiggers/libdeflate/pull/101#issuecomment-718326339. I did not wish to derail this PR or add to @ebiggers's workload, so I have deleted my comments.

I suggest @SunBlack refrain from making baseless & incorrect comments on other people's experience and expertise.

[SunBlack]

• not everyone cares about or likes IDEs, but you seem to be talking more about MSVC than about IDEs?

Not only, but as this repo contains a Makefile.msc, so you usually use MSVC when using this. But of course you could also use Qt Creator, Eclipse or some other IDEs.

• Makefiles aren't a real build system, real build systems like autotools and meson have --help or configure commands that do this for you. Sadly, this is actually a point against cmake, since cmake is like raw Makefiles and requires reading the entire build files to find undocumented variables that can be and usually are anywhere. Cmake doesn't have a help command. You can run a command after you unsuccessfully configured the project with the wrong options, that prints every internal state variable whether they are options or not, without context or guidance, though.

True, going through the configure step of CMake is more like going through a Wizard than reading the man page before using the CLI. Nevertheless you forgot: configure is again just a typical script on Unix platforms. So you need again another script for non Unix platforms.

• Generation of deb files is an antipattern, please use the distro tooling or something like checkinstall. :)

Why this is an antipattern? When using checkinstall you need to add everything yourself, which is not standard (e.g. required data files). Therefore you have always remember which extra information you have to add (usually you store the last used call somewhere). The integration of CMake doesn't need it - this can be even dynamic depends on your build selection. Another good thing: You have to define most information just once and not multiple times. E.g. information like the version, company,... are stored in a header file (to show it in the program itself), DLLs (As far as I know Linux doesn't show this information on a .so file) and installer. So you have to adjust just one single point to adjust it everywhere.

• again Makefiles aren't a real build system. Real build systems like autotools and meson and cmake too, lets you check for features before building. It's hardly unique to cmake, but you can certainly do it with cmake.

Yep, that why play Makefiles are sth. you used 10 years ago. As software is getting more complex and you usually sometimes want to enable or disable specific hardware features, Makefiles aren't the best anymore for it (even it is still possible)

• Integration with other projects via imported targets is an antipattern, pkg-config exists and has the benefit of working with projects that aren't cmake. That being said, you can install cmake config files from any build system, it doesn't require you to build with cmake.

pkg-config is usually just Unix (never saw it someone using on Windows as there is vcpkg more common) and to have lines like pkg-config --libs --cflags glib-2.0 within your Makefile is not really the best in terms of readability (when you starting to make the version configurable).

Basically you're making a lot of arguments in favor of having a build system at all. I agree. Real build systems are good. ~Even cmake is better than no build system at all.~

Yep that is the point, as the current integration of libdeflate makes no fun.

[SpaceIm]

Integration with other projects via imported targets is an antipattern, pkg-config exists and has the benefit of working with projects that aren't cmake.

It's not an anti-pattern. C++ ecosystem is fragmented, there is no official tool. Some libraries provide CMake config files & pkgconfig files (and most of the time it's broken in libraries with complex or option-dependant dependency graph), some other just one flavor, some other a custom shell config file, some other don't even care. I always advice CMake imported targets over pkgconfig files in a CMake based project, it's usually more robust, and doesn't require to install pkgconfig in addition to CMake (it can be tedious on Windows).

[bsergean]

I just closed this, this is where the work is happening now -> https://github.com/ebiggers/libdeflate/pull/235