noloader / cryptopp-cmake

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

Missing compiler id #4

Closed fxha closed 6 years ago

fxha commented 6 years ago

When building cryptopp on apple (compiler id AppleClang) the build will fail, due to missing architecture flags. This is because in L432

if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
                          ^^^^^^^^

we expect Clang. The right way is to check for both of them: e.g. CMAKE_CXX_COMPILER_ID MATCHES "Clang".

Furthermore, test headers missing ${CMAKE_CURRENT_SOURCE_DIR}/factory.h and test sources validat4.cpp.


I can't place code under Public Domain, due to german copyright restrictions. So I cannot create a pull request or is CC0 1.0 valid?

noloader commented 6 years ago

@mouse07410, @MarcelRaad,

Can you handle this change? I don't have CMake installed to test it.

mouse07410 commented 6 years ago

Yeah, I can. I use CMake in a few projects (r.g., EncFS and CryFS).

BTW, I think we should consider keeping CMake-related stuff as a branch of the main repo.

mouse07410 commented 6 years ago

@noloader what's your take on removing factory.h from the installed .h? It is used only by benchmarks and test.cpp.

noloader commented 6 years ago

@mouse07410,

what's your take on removing factory.h from the installed .h? It is used only by benchmarks and test.cpp.

factory.h is kind of interesting. It is only used in the test suite at the moment. However, it does not exist in the Test:: namespace because it can be used in regular code. For example, I have a clone that experiments with an ENGINE interface. The Engines provide an abstraction to things like Apple's CommonCrypto. I use the factory classes to get a handle to one of the engine classes.

Right now it can be removed. But I don't think it is the best choice.

mouse07410 commented 6 years ago

@fxha, thanks for your fixes. factory.h stays, other changes have been applied. I tested that it works.

Thanks again!