-
Notifications
You must be signed in to change notification settings - Fork 193
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
Upgrade to zmq2
#345
Upgrade to zmq2
#345
Conversation
* Update to zeromq-src with bumps libzmq version to 4.3.4 and enables building without cmake. * Link against libsodium for encryption support for security and performance reasons.
`metadeps` hasn't seen an update and crate release for 4-5 years, with `system-deps` being a fork and actively maintained successor to it.
…ke dll export settings got mixed up somewhere since we're looking for __impl_* definitions
Any movement on getting this merged? I use a small corner of ZMQ with Rust, probably not enough to be majorly useful against the full feature set of ZMQ - but am happy to look at test, review, CI etc. if I can be of help. |
Please do, this PR is actively looking for reviews @martynp |
Spent some time on this PR again;
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these changes are good, but some are still fork-related. IMO this can be merged, once my requested changes are solved.
use zmq::{z85_decode, z85_encode, DecodeError, EncodeError}; | ||
//use quickcheck::{quickcheck, Arbitrary, Gen}; | ||
//use rand::Rng; | ||
//use std::iter; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this code is fully redundant I think it'd be better to fully remove it from the codebase. Same goes for the comments later in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is still a valid test, but one that just needs to be updated eventually so I'll opt to leave it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's good to merge!
Hi, I know this is already some time ago, but @Jasper-Bekkers could you shortly elaborate why the |
This mainly fixes a lot of build issues we were having building cross platform libraries for zmq. It's quite a few commits, but we've been running with this in production ever since the other crate got released more or less (which, for us, means we've tested Linux and Windows, not macOS).
This also updates the crate to point to the latest upstream release.