Skip to content

Update CMake variables and paths #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
skmono opened this issue Oct 25, 2022 · 1 comment
Open

Update CMake variables and paths #46

skmono opened this issue Oct 25, 2022 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@skmono
Copy link

skmono commented Oct 25, 2022

For better cmake usability, I would like to suggest two changes in the ipp-crypto cmake configuration.

Replace all CMAKE_BINARY_DIR

For ipp-crypto standalone build, this may not cause any issue, but when building ipp-crypto with cmake FetchContent, within another project, it will pollute the build folder of the project with several package files, generated from:
https://github.com/intel/ipp-crypto/blob/46944bd18e6dbad491ef9b9a3404303ef7680c09/sources/cmake/ippcp-gen-pkg-config.cmake#L42-L45
https://github.com/intel/ipp-crypto/blob/46944bd18e6dbad491ef9b9a3404303ef7680c09/sources/cmake/ippcp-gen-pkg-config.cmake#L49
https://github.com/intel/ipp-crypto/blob/46944bd18e6dbad491ef9b9a3404303ef7680c09/sources/cmake/ippcp-gen-pkg-config.cmake#L65-L70

It would be cleaner and provide better usage if all CMAKE_BINARY_DIR could be replaced with PROJECT_BINARY_DIR or CMAKE_CURRENT_BINARY_DIR.

Use GNUInstallDirs variables for installation

The current installation destination for headers and libraries are using constant strings:
https://github.com/intel/ipp-crypto/blob/46944bd18e6dbad491ef9b9a3404303ef7680c09/sources/ippcp/CMakeLists.txt#L483-L486
https://github.com/intel/ipp-crypto/blob/46944bd18e6dbad491ef9b9a3404303ef7680c09/sources/ippcp/crypto_mb/src/CMakeLists.txt#L121-L131

For better usability, I suggest replacing all:
"lib" with ${CMAKE_INSTALL_LIBDIR} and
"include" with ${CMAKE_INSTALL_INCLUDEDIR}

Reason is similar to above suggestion - some customization when building ipp-crypto within another project could make it a lot easier to maintain project codes and build paths.

Please let me know what you think.

@aelizaro aelizaro added the enhancement New feature or request label Jun 29, 2023
@aelizaro
Copy link

Hi @skmono, sorry for the delayed responce! Thank you for the proposal we will consider update of CMake

@aelizaro aelizaro self-assigned this Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants