Skip to content

[c++] CMake improvements#28

Merged
carltimmer merged 9 commits intoJeffersonLab:evio-6.0from
DraTeots:fix-filesystem-header
Mar 20, 2025
Merged

[c++] CMake improvements#28
carltimmer merged 9 commits intoJeffersonLab:evio-6.0from
DraTeots:fix-filesystem-header

Conversation

@DraTeots
Copy link
Contributor

@DraTeots DraTeots commented Mar 19, 2025

Various fixes and improvements to CMakeList.txt

  • Crete evioConfig.cmake during install. This allows users simply use find_package(evio) in their CMake setups
  • Improve Disruptor.cpp handling:
    • better Disruptor search, now one can also provide -DDISRUPTOR_CPP_DIR or to be compatible with environement variable name -DDISRUPTOR_CPP_HOME cmake flags.
    • automatic fetch if external Disruptor-cpp is not found, it is automatically fetched and built by cmake. It can be controlled with DISRUPTOR_ALLOW_FETCH cmake option
  • improve lz4 handling (fix false positive error messages and reorder lookup)
  • make coherent USE_FILESYSTEMLIB switch between C++17 and boost

Comments:

  • I was using latest ubuntu image and the code didn't compiled because of #ifdef __cpp_lib_filesystem. The #ifdef __cpp_lib_filesystem use were mixed with #ifdef USE_FILESYSTEMLIB definition use across files. I changed everything to #ifdef USE_FILESYSTEMLIB, the rationale is that it allows more control over compilation and testing. One can turn it on or off and see the outcome, while __cpp_lib_filesystem work automatically and can produce unwanted untested artifacts like on my setup.

  • Added both DISRUPTOR_CPP_HOME and DISRUPTOR_CPP_DIR variables that can be set with -D flag. The rationale: DISRUPTOR_CPP_HOME corresponds to possible environment variable DISRUPTOR_CPP_HOME which I guess is commonly used in coda setup. DISRUPTOR_CPP_DIR is used as in modern CMake <PACKAGE-NAME>_DIR flags are anticipated for setting package directories

  • The handling logic for Disruptor-cpp became more complex and occupied a significant portion of the main CMake file. To improve maintainability, I moved it to a dedicated FindDisruptor.cmake script, as is commonly done in such cases.

  • New cmake options and flags:

    • option USE_FILESYSTEMLIB - default OFF. When ON - use C++17 instead of Boost
    • option DISRUPTOR_ALLOW_FETCH - default ON. When ON - if disruptor-cpp is not found by EVN or flags, cmake fetches it
    • DISRUPTOR_CPP_DIR and DISRUPTOR_CPP_HOME - possible flags where to look installed disruptor-cpp files
  • fixes CMake looks for Disruptor.h in the wrong directories #27

P.S. I carefully tried to keep full backward compatibility. With all changes to cmake file make sure, that with all that changes it behaves as before on systems that uses cmake / environment variables as before. But I don't have coda or whatever setup and can't test changes in production like environments.

@jonzarling
Copy link
Collaborator

Thanks! I tested it both ways, building Disruptor-CPP externally and with the -DDISRUPTOR_ALLOW_FETCH=1 option. Both ways of building worked properly for me. I image @carltimmer might want to have a look as well before merging.

@carltimmer carltimmer merged commit 88a79e9 into JeffersonLab:evio-6.0 Mar 20, 2025
@DraTeots DraTeots deleted the fix-filesystem-header branch March 20, 2025 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CMake looks for Disruptor.h in the wrong directories

3 participants