CMake build system cleanups and restructuring #2453
Conversation
|
@amadio - the improvements look great! Much more idiomatic from the CMake side. Two thoughts, both for the "next round" I think:
|
|
Thanks for taking a look at the PR!
Yes, I'm coming for you, haha. I recently realized that scitokens-cpp does not export a version anywhere, so it's not possible to detect what version is installed. It would be nice to at least export something in the headers or a As for bundling dependencies, well, I'm not a big fan of that. When I was working in ROOT I was always fighting against it, but wasn't heard. I think bundled dependencies can easily lead to security vulnerabilities and suffer from code rot, since they are rarely updated. In XRootD, I've removed the bundled |
|
I'm going to split this into two parts, one which I will merge soon to master and which contains part of the changes here, plus a few extras. The other, which I will keep in this pull request, is to do the refactoring for moving |
9ea6394 to
5496199
Compare
This makes the placement of binaries and libraries independent of the directory structure of the CMake build system, which makes it easier to set paths to client and server binaries in tests, as well as ensuring that libraries and plugins from the build itself are always used when running the test suite. Moreover, this allows to migrate the build system to use more idiomatic CMake by using, for example, add_subdirectory(Xrd*) and src/Xrd*/CMakeLists.txt instead of the current include(Xrd*) and src/Xrd*.cmake, to keep plugins in the same place as other libraries. Note: These variables must be set in the CACHE to allow Python builds using setup.py to override them, so that libraries are placed in the correct directory for later installation by pip.
This is the first version that provides the multi interface, so it allows us to remove checks and ifdefs for this from the project.
This is to make the headers, source files, namespaces, etc all the same name as the plugin itself, for consistency.
…opriate The intention is to allow XRootD to be embedded with add_subdirectory(xrootd) within another CMake project.
In preparation to address issues #2415 and #2432, I started to perform a general restructuring of the build system for XRootD 5.8, to use more idiomatic and modern CMake constructs. This pull request is my current draft of it, which I would like you to review.
Before doing anything, I had to set the output directory for binaries and libraries to be
<BUILD_DIR>/binand<BUILD_DIR>/lib, otherwise paths to binaries such asxrdcp, etc, change depending on which CMake directory they were build. Overall, I think this makes it much simpler to managePATHin tests, etc, since you only need<BUILD_DIR>/binand notsrc/for server binaries andsrc/XrdCl/for client binaries, etc. Libraries and plugins are automatically loaded from the build directory when specified only by name, so in configuration files, if you have full paths, you can just use the basename without the directory part in the future.I am moving most
src/Xrd*.cmakescripts tosrc/Xrd*/CMakeLists.txtand moving build logic as close as possible to where it's needed, such as calls tofind_package, etc. I would also like to use this opportunity (since I will need to touch almost all build system files) to update the CMake style we use to be more consistent across the whole repository, maybe something similar to what's shown here and here. Please let me know your opinions, I'd prefer to adjust style before merging this.I still have to deal with
XrdDaemons.cmake,XrdHeaders.cmake,XrdServer.cmake, andXrdUtils.cmake. I'd like to hear your thoughts on how to move their bits down to subdirectories. My plan is to create the libraries and binaries insrc/CMakeLists.txtand usetarget_sourcesin subdirectories to add the files to the targets as needed.After this restructuring is over, I will work on improving the exported files (i.e.
XRootDConfig.cmake) and consider creating functions to manage libraries and plugins in a more central way in case global changes are needed. For now, I'm just using plain CMake commands everywhere.Thanks for reviewing!
Update: I've finished most of the remaining work. Now only
XrdHeaders.cmakeneeds to be dealt with. I've dealt with #2415 and #2432 as the last thing in the pull request, but it's still not over yet, I might add a few more changes before adding tomasterand into 5.8.0.