Building as an ExternalProject in CMake.

[m-kurz]

I tried to incorporate the SmartRedis Client for Fortran into an existing project (FLEXI to be precise) using the ExternalProject package for CMake. This allows to download an external project (in this case SmartRedis) to be downloaded and compiled on the fly with the high-level project so that the users do not have to install the dependency on their own. However, I encountered two distinct problems when doing so and have attached a CMake file that allows to reproduce both issues: CMakeLists.txt

1. If the overall project is built in parallel (i.e. with make -j) this will be passed to the make call to build SmartRedis. This will break its build process, since building SmartRedis with make -j will fail. This can be circumvented by stating explicitly make -j1 lib-with-fortran
2. The resulting Fortran library install/lib/libsmartredis-fortran.so seems to miss the link to libsmartredis.so if inspected with the ldd tool. Said dependency is not found (libsmartredis.so => not found). Consequently, every code using the Fortran library will break because of this. The originally built libraries in the build/release folder, however, work just fine. There seems to be some problem within the install step.

To reproduce both issues one just uses the attached CMakeLists.txt and executes:

mkdir -p build && cd build
cmake .. && make -j

To enable the workaround of forcing serial compilation, just switch to the commented out CONFIGURE_COMMAND in the file.

Maybe these issues are only an error in my approach to include the SmartRedis library via CMake.. On a side note: Installing the library standalone and then finding and linking it into the project works fine.

I would suggest the following:

• Either adapting the build process or (way easier and my recommendation) add a commandline flag or environment variable with which serial building can be enforced by overwriting the NPROC variable explicitly to 1 in the Makefile.
• Maybe add an example with CMake's ExternalProject to the documentation (if there is not already one I have overlooked)

Tested with:

• Ubuntu 20.04 LTS
• OpenSUSE Leap 15.5

For Versions:

• v0.5.2
• v0.4.1

[ashao]

You are the first person to try adding SmartRedis as an ExternalProject, so thank you very much for pointing this out.

Serial build of SmartRedis

I wonder if you might be able to make more headway by trying to use native CMake commands instead of trying to through our Makefile (which is largely there out of convenience). As you can see in: https://github.com/CrayLabs/SmartRedis/blob/develop/Makefile#L124-L136

The CMakeLists.txt in the root directory should be sufficient: https://github.com/CrayLabs/SmartRedis/blob/develop/CMakeLists.txt

The other option which I think is flexible is to change the assignment of NPROC to NPROC?= so that users can choose their own (or by default, will try to use all the processors available).

Linking problems

This has been on my list of annoyances for a while and have not reworked our CMake build to ensure that RPATH is set correctly. This was relatively low on my list of things to fix though because in general when we dynamically link in the SmartRedis libraries to simulation code, you already have to update the LD_LIBRARY_PATH to link against libsmartredis.so (e.g. export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/path/to/SmartRedis/install/lib). This should also resolve the libsmartredis.so not found when doing an ldd libsmartredis-fortran.so.

Please let us know which of these solutions you would prefer and we'll start implementing them.

[m-kurz]

Thanks for the answer.

Serial Build

I can indeed compile SmartRedis using the following lines in the ExternalProject command:

PATCH_COMMAND make -j1 deps
CMAKE_ARGS -DSR_FORTRAN=ON

So, SmartRedis itself can be compiled using native CMake by passing the required arguments. However, I needed to misuse the PATCH_COMMAND to run the Makefile to install the dependencies beforehand, since this is currently not possible via SmartRedis' CMake file. I agree that changing the argument to NPROC?= would be the easiest option probably accompanied by a note/example in the documentation. Another option might be to also include the SmartRedis dependencies in its CMake file again using the ExternalProject module, which then would allow to compile SmartRedis with all its dependencies only using CMake. Of course I'm not sure if this is something you find sensible or whether there are some concerns going this route. If there is interest, I could also give it a try to implement a version of this approach in CMake?

Linking Problems

Tbh, I cannot really reproduce the problems I initially had, since currently my project is compiling perfectly using the libsmartredis-fortran.so out of the install folder. So at least for me the problem somehow fixed itself.

[ashao]

I agree that making the dependencies ExternalProjects for SmartRedis is the cleanest route. I have no concerns about that. We revamped the internals of the build process last year and I think we could take a look again. Why don't I take point on trying it out. We've got a bunch of testing that we need to do that I think it might be more efficient for me to do.

[m-kurz]

Perfect, also sounds like the best option for me. Thanks!

[ashao]

@m-kurz: Please see the PR here. I think it does what you want it to do. As you can see from the tests, there are a few small things I need to fix up to get everything working, but it should be pretty quick from my end.

Could you double check and let me know if there's anything that you would like changed?

[m-kurz]

I just tested it within FLEXI and it works like a charm. I can now incorporate SmartRedis as an external project in our codebase and all dependencies are built correctly, also when using a parallel build with make -jX. The only thing I would suggest is to add GIT_SHALLOW ON to the ExternalProject_Add commands for both redis++ and hiredis, since this reduces the amount of downloaded data quite a bit and the build uses distinct Git tags anyways.

Besides that I noticed that after SmartRedis' dependencies have been installed correctly in the build step, their build is triggered again (unnecessarily) during SmartRedis install step. However, since the libraries are correctly found, nothing is eventually re-built and the introduced overhead of running this twice is probably negligible. Just noticed and wanted to mention it.

Thanks for implementing and reaching out!

[ashao]

Excellent! Great suggestion on the GIT_SHALLOW. Interesting observation on the dual build call, let me see if I can figure out where it's coming from (got a couple of candidates).

Lastly, I'm going to take the opportunity to finally make a Config.cmake for SmartRedis. I think then we're closer to a fully functional CMake build