search

noloader / cryptopp-cmake

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

OpenMP #68

Closed smessmer closed 3 years ago

smessmer commented 3 years ago

By default, when using the cryptopp-cmake script, or when depending on cryptopp via Conan, it seems to build without OpenMP.

For the cryptopp-cmake script, I worked around this so far by adding my own hacks around it in a separate CMakeLists.txt, and to be honest, those hacks are somewhat complicated, see here: https://github.com/cryfs/cryfs/blob/38575f258a16c2dd847013749c93eaefe1c58af0/vendor/cryptopp/CMakeLists.txt

I would like to switch that project to Conan, but that means I won't be able to use this hack anymore. OpenMP would have to be supported natively by the cryptopp conan package, which I think means that cryptopp-cmake would have to natively support it.

Are there plans to add OpenMP build functionality? Possibly some code from the hack I linked above could be copied, it seems to be relatively cross-platform.

noloader commented 3 years ago

Hi @smessmer,

OpenMP would probably be a good option to support for folks who want to use it.

I'm not a CMake expert, so I can't really comment on your patch.

I sent you a Collaborator invite. I run my GitHubs a little differently. I like allowing others access to the code. It removes barriers for other folks.

If you add OpenMP support, please ensure it works with CMake 2.8.6. CMake 2.8.6 is still found in the field. Otherwise, things will break when I test on some platforms, like Ubuntu 14 ERS and Solaris 11.

By the way, in general, Crypto++ slows down with OpenMP. Some algos do enjoy a speedup (like Scrypt), but in general there's no gain. Also see https://www.cryptopp.com/wiki/OpenMP.

smessmer commented 3 years ago

Thanks. CryFS depends on scrypt, that's why OpenMP is important. Are you maintaining the conan package? Is my assumption correct that it is built based on this cmake script? Is there a conanfile.py somewhere?

How is the CI situation of cryptopp-cmake, if I make a commit pass CI could I trust that it wouldn't break anything?

noloader commented 3 years ago

Are you maintaining the conan package? Is there a conanfile.py somewhere?

I am not aware of a Conan package. Maybe you can start one?

If you do, I'll write a wiki page on it and point people to it. The centralization will help users by ensuring they know where to get the latest semi-official package.

If you start a GitHub repo, I recommend the name cryptopp-conan. The name is consistent with cryptopp-autotools, cryptopp-cmake, cryptopp-android, etc. I think consistency is important for users.

How is the CI situation of cryptopp-cmake

We do some basic testing through Travis. I have to manually test some platforms, like Solaris.

If something breaks I usually ping the person who performed the commit and ask them to look at it.

smessmer commented 3 years ago

Oh there is one already, but wasn't sure who's maintaining it. But seems conan packages are in a public GitHub repo, found it here https://github.com/conan-io/conan-center-index/blob/master/recipes/cryptopp/all/conanfile.py

noloader commented 3 years ago

Oh there is one already, but wasn't sure who's maintaining it.

Well, if you want, you can contribute to that one. Or you can start your own.

I'll still write the wiki page so users can find it, and users will know that's the semi-official one.

smessmer commented 3 years ago

Sg. The right page to link users to would probably be https://conan.io/center/cryptopp not the GitHub repository above

smessmer commented 3 years ago

I think I was too slow in accepting your collaboration invitation, it expired. I opened a PR though and CI is currently running over it. https://github.com/noloader/cryptopp-cmake/pull/70

smessmer commented 3 years ago

@noloader Can you resend your collaboration invitation or alternatively take a look at and merge that PR?

smessmer commented 3 years ago

@noloader ping

smessmer commented 3 years ago

Thank you, I merged the OpenMP code :)