-
Notifications
You must be signed in to change notification settings - Fork 48
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
Release for Crypto++ 8.7.0 #1
Comments
I'm quite happy with the current state for Linux/MacOS/WIN platforms. I'm not adding any more enhancements before 8_7_0 release. Gonna give it a couple of days for people to check if the |
It appears noloader/cryptopp-cmake installed header files into a subfolder |
I'll fix this. Not intended. |
Before we do this work twice, I just wanted to make this an option which is presetable to something different. At least when amule started using cryptopp some distros did use crypto++ as dir for includes. I need this var anyways, as (at least in amule) this dir is passed as define to the code and the compiler is told to search in the parent dir. |
Would you like to suggest a PR for this @Vollstrecker ? |
In fact I planned this for later this day. You would like to have this separate, or combined with the overworked config export. I need to check, but I'm pretty sure 3 quarters of our Config.in would be generated by the export anyways, so this can be shrinked to much more readable ~10 lines. |
Could you please do two PRs? That way we can keep the concerns separate. Thank you. |
There is one more thing we need to decide. Currently, the crypto++ lib puts all its files directly at the repo root. The install now puts include files under I understood that some users would prefer to do a Currently the target include directories for cmake sub-project usage follow the direct include style, not the prefixed one. Should we adjust the crypto++ cloned repo to have a cryptopp prefix and then publish includes with a prefix? |
As this is exactly what I need that var for, I also would prefer to have this style on the target. We could do it with setting an BUILD_INTERFACE on the target and an INSTALL_INTERFACE to the installed one. And then add an define to the interface like CRYPTOPP_INCLUDE_PREFIX which holds the right subdir. I guess if we set both include-paths, everything should work no matter if the users do their #include with the whole path, or with one level up. it should even work when built as subproject with include-path PROJECT_BUILD_DIR/external and PROJECT_BUILD_DIR/external/ext_cryptopp-src and CRYPTOPP_INCLUDE_PREFIX set to ext_cryptopp-src. But for testing this I need at least a day more. |
So to clarify the target has then |
One comment in the context of Unix & Linux and Windows... during installation...
Debian and friends are Debian, Ubuntu, Mint, etc. Red Hat and friends are Red Hat, CentOS, Fedora, Scientific Linux, etc. There's some handwaiving going on since the include and lib directories are controlled by Ping @alonbl . I believe he helps with Gentoo, and he may use the updated CMake files if they work as expected. He can probably provide valuable feedback. Ping @geoffbeier . He is another person I rely on a lot for perspective. I think he does a lot of work with Conan. |
Hi, Not sure I can help, this build system is way too complex for me to digest, I am afraid it went too far with customization, but I have no time to review it. I can see static library is created
which is incorrect as it missing the $prefix in the vars. without shared libraries no distribution will apply the new buildsystem whatever it will be. very important to support both 32bit and 64bit side-by-side properly, so that include files are well behaved, and if generated are handled correctly. and not break so version backward compatibility. |
Sure that's why I suggested 2 include-dirs. amule for example uses ${prefix}/include and a define that tell the if it should look for cryptopp/hdr.h or crypto++/hdr.h. With your suggestion they would need to read the properties of cryptopp::cryptopp and construct their own target with the right stuff. clementine uses hardcoded cryptopp/hdr.h so I guess the maintainers will have to patch this, or the'll leave out the spotify-support. SecureFS seems to ship their own copy of an older version of cryptopp-cmake codecrypt hardcodes /usr/local/opt/cryptopp/ and tell their users it's better to use homebrew. I could list like tegrarcm, but all examples rely either on fixes places (which are not given) or on a define to get the right dir. A quick check in debian brought 5 out of 8 using the define, so if it's this OR that, I would clearly say the define variant should be preffered.
Sure, but this is covered by the config file, in the past and the future.
Noone really does, but their is a location where cmake searches for the configs by default, and if that one is found, noone needs to know where the libs, headers etc. are located because the config tells the build-tools and everything is fine.
As this is a decision made in the lib itself, the buildsystem doesn't matter, you won't get a shared version anyways (iiuc)
I don't really get this. If you build this for both (or just one) arches, you always have control over the location where they are installed. The headers are the same for both. I didn't read the headers, but I'm pretty sure if there are differences between them, this is handled with defines, which will be noted in the config if needed. |
We will fix the pkgconfig file once we agree on the install strategy. Also, we can easily support the lib install side by side for 32 and 64 bits. Could you guide us on what are the best practices here @alonbl ? The shared lib build is a crypto++ issue not a cmake issue. Maybe @noloader can comment on it. |
Tha pkg-config file should follow the build system, it is a template... even if you install files in the wrong locations it should match the wrong locations. You should test this to be working. The problem with 32bit and 64bit is the installation of headers in case there is platform settings within one or more. the libdir will be set by the distribution using standard cmake arguments. I do not understand how come the shared lib is not an issue of a build system. This is what a build systems do... Distributions always use shared library as otherwise an upgrade of the package will not effect the consumers. |
I just gave it a quick shot. The .so isn't part of the build when just using the default target (it s listed in all: so I don't know what happens there), but with make dynamic I am able to build it. So we maybe shouldn't forbid the build completely. Btw. respecting CMAKE_BUILD_SHARED_LIBS is ok, but I'm a big fan of being able to build static and shared in one run, but that's another topic. |
Crypto++ does not export symbol visibility correctly for shared lib/ dll builds and several templates are in .cpp files with no symbol export. The current shared lib build only exports a subset of the symbols for a soon to be end-of-life FIPS library. That's what I meant by a crypto++ issue. The build system does have everything it takes to build shared library but it's not what 's really expected. |
I've seen in the docs only hints that the dll is not recommended and not FIPS validated. Also without the use of libtool it's always let's say problematic to only export needed symbols. But if there is a wrapper already as example, why not just including that one to ease the usage for maintainers and endusers? |
I don't know where to begin with the problems with the FIPS DLL... The FIPS DLL is a subset of the Crypto++ library. The FIPS DLL only exports classes like DH, AES, SHA, RSA and DSA. Those classes are called "approved cryptography" (some handwaiving) by NIST. If you want a HexEncoder or a RIPEMD hash, then they are missing. It causes a lot of problems for users. Then there's the problem of symbol visibility. The only symbols exported are those for approved cryptography. I've tried several times over the years to create Windows DLL that functions like users expect. That is, the whole library is available as a DLL instead of a static LIB. Here are the attempts:
So far my attempts have failed. (Be sure to read the head notes/source code comments in dump2def.cpp seems like it should be a path forward. I seem to recall the problem with I think we need to get rid of the FIPS DLL. Then, we can apply __declspec dllexport to [nearly] all library symbols, and not just the approved cryptography. Oh, and the shared object on Unix & Linux is fine. We can export all symbols without a lot of trouble (even some that don't need to be exported). That's why we have a shared object for Unix & Linux, but not a DLL for Windows. |
So, basically we can enable a shared lib build on anything except MSVC, although that shared lib breaks the good rules of exported symbols and visibility. For MSVC, unfortunately I tried that already and it still does not work and MSVC is way more picky on dllexport. In the long run though, symbols need to be properly exported... |
If it's just a problem on windows we can 1. enable it everywhere else and 2. use WINDOWS_EXPORT_ALL_SYMBOLS which just assumes that all symbols have dllexport given. the user still need to add dllimport everywhere they need to use a function that doesn't have it, but this should be the smallest problem then. |
I did run a build with -fPIC and -fvisibility=default with g++ on Linux, and tried to link Please note that in the current builds, @Vollstrecker please do give it a try in case I was doing something wrong... |
I will. first I try to split this topic into some more issues that can be handled separate and start to work them one by one out, plus at least one other thing i've seen. I guess the list is getting longer. |
@noloader I see that even in the crypto++ GMakefile, you always link cryptest to the archive library and never to the shared lib... Is there a reason for that? I'm getting so many undefined symbols if I try to link it against the shared lib. |
Yes. It is easier to link against the static archive. Runtime linking against a shared object during testing can be very messy. If we linked against the shared object, then we have to worry about runtime linker paths, and use However, we do test runtime linking against the shared object in the CXX="${CXX}" CXXFLAGS="$DEBUG_CXXFLAGS" LINK_LIBRARY=libcryptopp.so \
"$MAKE" "${MAKEARGS[@]}" static dynamic cryptest.exe 2>&1 | tee -a "$TEST_RESULTS" There is also a test for a release build, and a pair of tests for OS X. We also perform a shared object symbol test in |
If you jump over to Linux and use the Makefile, you can see what the library does. The command output will show you the flags we use to build things for Linux. It is not much more than:
Here's the result on my Kubuntu machine: We don't mess around with |
Another difference to noloader/cryptopp-cmake that I am seeing is that the previous version installed CMake packages under |
When looking at the last build in noloader, the package is in share also, I didn't check, but if this changed somewhen in the past, it was before the fork. |
It's not a breaking change type of difference. I looked at tte most common practices with cmake packages and both practices are common. CMake is able to find packages in both places. |
CRYPTOPP_8_7_0 released. |
This is a tracker for general discussions regarding release 8.7.0, being prepared in the master branch.
Please provide feedback, and discuss potential changes here,
The issue will be closed when the release tag is published.
The text was updated successfully, but these errors were encountered: