Summary
I'd like to propose two improvements to make isa-l_crypto more friendly for modern CMake consumption patterns (FetchContent, add_subdirectory, package managers like Conan/vcpkg).
Issue 1: Include Path Inconsistency
Current Behavior:
| Usage |
Include Path |
Usage |
After make install |
include/isa-l_crypto/*.h |
#include <isa-l_crypto/md5_mb.h> |
| Via FetchContent |
include/*.h |
#include <md5_mb.h> |
This inconsistency makes it difficult to write portable code that works with both consumption methods.
Proposed Solution:
Rename include/ directory to isa-l_crypto/ and adjust CMake paths:
target_include_directories(isal_crypto PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>
$<INSTALL_INTERFACE:include>)
This is a minimal change (directory rename only, no file content changes) that enables consistent usage:
#include <isa-l_crypto/md5_mb.h> // Works for all consumption methods
Issue 2: Generic Option Names
Current:
option(BUILD_TESTS "Build test applications" ON)
option(BUILD_PERF "Build performance applications" ON)
Problem: These generic names can conflict with parent project options when used via FetchContent/add_subdirectory.
Proposed:
option(ISAL_CRYPTO_BUILD_TESTS "Build test applications" ON)
option(ISAL_CRYPTO_BUILD_PERF "Build performance applications" ON)
Offer
If these proposals are acceptable, I'm happy to submit PRs for both changes.
Related PR: #170 (CMAKE_SOURCE_DIR fix for FetchContent compatibility)
Summary
I'd like to propose two improvements to make isa-l_crypto more friendly for modern CMake consumption patterns (FetchContent, add_subdirectory, package managers like Conan/vcpkg).
Issue 1: Include Path Inconsistency
Current Behavior:
make installinclude/isa-l_crypto/*.h#include <isa-l_crypto/md5_mb.h>include/*.h#include <md5_mb.h>This inconsistency makes it difficult to write portable code that works with both consumption methods.
Proposed Solution:
Rename
include/directory toisa-l_crypto/and adjust CMake paths:This is a minimal change (directory rename only, no file content changes) that enables consistent usage:
Issue 2: Generic Option Names
Current:
Problem: These generic names can conflict with parent project options when used via FetchContent/add_subdirectory.
Proposed:
Offer
If these proposals are acceptable, I'm happy to submit PRs for both changes.
Related PR: #170 (CMAKE_SOURCE_DIR fix for FetchContent compatibility)