search

noloader / cryptopp-cmake

CMake files for Crypto++ project
BSD 3-Clause "New" or "Revised" License
92 stars 68 forks source link

Change ${SRC_DIR} into a user settable variable #85

Closed laitingsheng closed 2 years ago

laitingsheng commented 2 years ago

I think a better solution than #54 and #67 would be just turning the ${SRC_DIR} into a user-settable variable.

I would further suggest changing this variable name into something unique like "CRYPTOPP_SRC_DIR" or "CRYPTOPP_PROJECT_DIR" instead of the more common name "SRC_DIR" to avoid confusion in case others are using the same variable name.

noloader commented 2 years ago

Thanks @laitingsheng,

Was the change tested against Ubuntu 14? Ubuntu 14 is extended support. It can still be encountered in the field. Ubuntu 14 uses Cmake 2.8.6.

laitingsheng commented 2 years ago

Was the change tested against Ubuntu 14?

Honestly, not yet, since the oldest Ubuntu I have is 16.04 LTS.

But according to the documentation of CMake 2.8.6, the syntax of set(... ... CACHE PATH ...) is well supported at that version. Also, I've spotted you actually use it in

https://github.com/noloader/cryptopp-cmake/blob/e5f1db7dee2d7a41ae9e2a516c7e3df9b26bec0e/CMakeLists.txt#L116

I don't think it will break any current building systems.

noloader commented 2 years ago

Ok, thanks @laitingsheng . It does not look like you pushed the SRC_DIR → CRYPTOPP_PROJECT_DIR change.

laitingsheng commented 2 years ago

It does not look like you pushed the SRC_DIR → CRYPTOPP_PROJECT_DIR change.

Sorry, my fault.

I have changed this locally but haven't committed yet since I am waiting to see which name you would prefer. I will push this now with the variable name changed to ${CRYPTOPP_PROJECT_DIR}.

laitingsheng commented 2 years ago

@noloader I've checked that every ${SRC_DIR} has now been replaced by ${CRYPTOPP_PROJECT_DIR}.

noloader commented 2 years ago

Thank you very much.