Resolve overlap between cmake/make

[RobDickinson]

The build system currently has duplicate logic for downloading 3rd party libraries and building the shared library and test programs. This logic appears in both CMakeLists.txt and the Makefile (specifically the thirdparty and sharedlib targets).

The installation section of the README (https://github.com/pmem/pmemkv#installation) should be updated accordingly to fit the new approach.

[RobDickinson]

@tomaszkapela and @GBuella, just looking for your guidance here.

I really like the set of top-level commands that we have today -- for example, being able to make a code change and run make test and everything rebuilds and runs. We want to be careful not to go backwards on usability in our enthusiasm to standardize on cmake.

What cmake does better is in managing all of the little compiler switches that I tend to screw up anyway, so I would love to build the shared library (and executables) exclusively through cmake. But I think the rest of what the Makefile provides today works fairly well.

So what I was thinking is that we'd remove the thirdparty target from the Makefile, and rewire sharedlib to call cmake instead.

CURRENT VERSION

thirdparty:
	rm -rf 3rdparty && mkdir 3rdparty
	wget https://github.com/google/googletest/archive/release-1.7.0.tar.gz -O 3rdparty/gtest.tar.gz
	mkdir 3rdparty/gtest && tar -xzf 3rdparty/gtest.tar.gz -C 3rdparty/gtest --strip 1
	wget https://github.com/pmem/nvml/archive/1.2.tar.gz -O 3rdparty/nvml.tar.gz
	mkdir 3rdparty/nvml && tar -xzf 3rdparty/nvml.tar.gz -C 3rdparty/nvml --strip 1
	sh -c 'cd 3rdparty/nvml && make install prefix=`pwd`'

sharedlib:
	$(CXX) src/pmemkv.cc -o libpmemkv.so $(SO_FLAGS) $(USE_NVML) $(CXX_FLAGS)

PROPOSED CHANGE

(thirdparty target is removed)

sharedlib:
	cmake --build ./cmake-build-debug --target all -- -j 8

The example and test targets would use the executables produced by the cmake build:

CURRENT VERSION

example: reset sharedlib
	$(CXX) src/pmemkv_example.cc -o pmemkv_example $(USE_PMEMKV) $(CXX_FLAGS)
	PMEM_IS_PMEM_FORCE=1 ./pmemkv_example

PROPOSED CHANGE

example: reset sharedlib
	PMEM_IS_PMEM_FORCE=1 ./cmake-build-debug/pmemkv_example

Other top-level targets like make install and make clean would need adjustments for new file locations but would function the same.

Is this what you had in mind? The other obvious alternative would be to define all of our custom targets in cmake...but that seems more verbose and less descriptive than what we have now.

I'm not so much a purist in the end -- I don't like having duplicate logic in the build today, and I prefer cmake for building libraries/binaries, but I wouldn't at all be bothered to still have top-level make commands as described in the README today. (https://github.com/pmem/pmemkv#installation)

I'm curious if anybody has a different take on this.

Thanks!

[GBuella]

Well, the main advantage here of a manually maintained Makefile is allowing a build without cmake. So one doesn't have to install it.

If cmake would be required even for builds with the prepared Makefile, then I believe just keeping cmake would be preferred.
The cmake equivalent targets could be:

make check           # build everything from scratch and run tests
make                      # build everything - automatically added target
make clean            # remove built targets - automatically added target
make example       # build and run example program
make stress          # build and run stress test
make test             # build and run unit tests
make install          # install to DESTDIR
make uninstall      # remove from DESTIR
make sharedlib     # build .so, if it needs to be a separate target

We already use most of this in pmemfile and syscall_intercept.
Some advantages:

• Having a configure step before builds, where flags, install destination, etc... can be selected, libraries are detected, compiler is chosen
• Everything always works with out-of-source builds. For example I don't see how could I have a debug build and an optimized build of pmemkv_stress at the same time using only the manually created Makefile. Whereas with cmake I can already do that:

mkdir pmemkv_dbg
cd pmemkv_dbg
ccmake ~/code/pmemkv # and choose Debug build in ccmake
make pmemkv_stress
./pmemkv_stress

 cd ..

 mkdir pmemkv_release
 cd pmemkv_release
 ccmake ~/code/pmemkv # and choose Release build in ccmake
 make pmemkv_stress
 ./pmemkv_stress

• Handling RPATH automatically, thus LD_LIBRARY_PATH wouldn't be needed for anyone.
• If this ever needs to be ported to something other than GNU/Linux, cmake makes it almost trivial. Especially if that other is Windows, see how NVML fares with manually maintaining makefiles and VS solution files in parallel...
• Handling most dependencies out of the box. No need to track manually what needs to be rebuilt when some .cc file or some .h file changes. As I see, at the moment, with the Makefile one can rebuild everything, or nothing. There are one or two ways of tracking dependencies in a Makefile -- which usually cmake does right. For example the pmemkv_stress.cc file is never mentioned in the Makefile as a dependency, it is recompiled every time, whether it changed or not. Meanwhile, an equivalent cmake generated make target make stress can recognise if e.g.: pmemkv_stress.cc did not change, no need to compile it, but pmemkv.cc changed, so it needs to be compiled, and if static libraries are used, pmemkv_stress must be linked again, but not if dynamic libraries are used.

[GBuella]

By the way, if a single command build is really needed, one can always add a build.sh to the source, which does something like:
mkdir build && cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make && ctest

[tomaszkapela]

I'm with @GBuella on this one. Maintaining both will almost certainly become a bother, probably sooner than later. Calling cmake from the Makefiles provides no benefit besides backward compatibility, which at this point in time should not be taken into account. I don't see a reason why this shouldn't be ported to Windows, although I might be missing something. Speaking from experience, maintaining two build systems is tedious. I opt for dropping Makefiles in favor of cmake.

[RobDickinson]

OK, you have me convinced that all heavy lifting done by the build should be done exclusively by cmake. If you look at our latest build, you'll see that no build logic is duplicated now. I also documented out-of-source builds in the README.

That said, I still greatly prefer to use a top-level Makefile rather than copy/paste long cmake commands from the README, like is being done in pmemfile. I don't want to have to manually copy/paste at all. I'd like to have all scripts under source control, and the be able to easily handle when the directory structure changes (like we just did, getting rid of the 3rdparty directory) through a script rather than sending out an email somehow. I'd like to be able to take those build scripts and run them with a continuous integration server (like Jenkins) or use them from a chatbot/slackbot. It's a core idea within the devops community that you always automate things like this where you'd otherwise dig for documentation, and in this case, the cost of more complete automation is very small.

So while you've completely convinced me of the power and utility of cmake, let me try to convince you that the user experience shown by NVML is the right one. For a first-time user, we shouldn't ever be doing anything more complicated than git clone followed by a second simple command to build/test the library.

I think this latest version gives the power and benefits of cmake without giving up full automation for what we consider our "standard build" -- but as always, I'm open to better ideas and strong feelings.

I also reached out to @sarahjelinek on this issue, and it sounds like she would prefer a higher level of automation rather than what's being done there with cmake today. I'll let her comment for herself.

If it really bothers people that the top-level script is a Makefile script, rather than a bash script or a Ruby script or whatever...then we can have that debate. Let's acknowledge though that this latest version puts all significant build logic in cmake, so we're not maintaining two build systems any more. (the current version also enforces minimal dependencies when it comes to rebuilding, because all the real logic is handled by cmake)

[RobDickinson]

@GBuella, @tomaszkapela, ok to close this, or are there unaddressed concerns? I'm definitely open if there are still suggestions about how to improve this, but if not then let's move on to other things. 😀

[GBuella]

OK, I think.
Once we might clean the install/uninstall process a little bit, e.g. one can't set a prefix with the top level makefile, also, the top level makefile's install target copies the .so, into which cmake has built rpath:

$ readelf -a libpmemkv.so | grep RUNPATH
 0x000000000000001d (RUNPATH)            Library runpath: [/home/tej/local/lib]

But let's just make a ticket about install/uninstall and packaging for different distros.

[RobDickinson]

Thanks for all the help on this thread.

I created #62 to handle building native packages, which doesn't need to be done yet.

I also modeled the build system in the pmemkv-jni repo after our work here.