streamvbyte icon indicating copy to clipboard operation
streamvbyte copied to clipboard

Create cmake target

Open victor1234 opened this issue 2 years ago • 4 comments

  • Create a cmake target for proper installation
  • Update README.md with CMake integration instructions
  • Add cmake find_package(streamvbyte) check to Ubuntu 22.04 CI
  • Set version in cmake according to last tag

victor1234 avatar Apr 24 '23 22:04 victor1234

@victor1234 Do you know why it fails under Visual Studio?

lemire avatar Apr 25 '23 01:04 lemire

PR is not complete now, I need 1-2 days

victor1234 avatar Apr 25 '23 07:04 victor1234

@lemire PR is in draft mode now. I'm still working. It's not ready for review.

victor1234 avatar Apr 26 '23 06:04 victor1234

This is a comprehensive PR and I would like to discuss whether these changes are suitable for your project. The main goal is to create modern cmake targets that install correctly and are available through various integration methods, making it easier to use the library and build packages for it. In addition, global variables are not used in modern cmake (they are propagated to the main project when the library is integrated via add_subdirectory()), and all properties should be assigned to each target specifically. As I understand it, all properties ( sanitizer, ASAN_OPTIONS, __restrict ) should only apply to the streamvbyte target and not propagate to the main project when linking. Only _ARM_NEON__ should be propagated to the example.

Please answer these questions:

  1. Do we need pure Makefile support?
  2. I would not mention DCMAKE_INSTALL_PREFIX in the readme at all, as installation defaults to the standard location where the target can easily found by find_package(streamvbyte).
  3. Which of the compilation, linking and definition flags should only be left with streamvbyte, and which should propagate to the entire project that streamvbyte is linked to?
  4. There is no arm support in github CI, how can I check if the PR works correctly and arm optimization enabled on my raspberry pi4?
  5. I would suggest moving all includes into one folder, it would be #include <streamvbyte/streamvbyte.h>. But this will not be compatible with older versions.

victor1234 avatar Apr 29 '23 18:04 victor1234