erickt / rust-zmq

Rust zeromq bindings.
Apache License 2.0
886 stars 189 forks source link

Move to zeromq-src 0.2 #339

Closed jean-airoldie closed 1 year ago

jean-airoldie commented 2 years ago

Note that im unsure whether libsodium should be linked by default since I'm assuming not all users use CURVE.

Jasper-Bekkers commented 2 years ago

@jean-airoldie it looks like this is still on zeromq-src 0.1.6

Jasper-Bekkers commented 2 years ago
diff --git a/zmq-sys/Cargo.toml b/zmq-sys/Cargo.toml
index 7c40f3c..50ceb64 100644
--- a/zmq-sys/Cargo.toml
+++ b/zmq-sys/Cargo.toml
@@ -22,7 +22,7 @@ libc = "0.2.15"

 [build-dependencies]
 metadeps = "1"
-zeromq-src = { version = "0.1.6", optional = true }
+zeromq-src = { version = "0.2.0", optional = true }

 [package.metadata.pkg-config]
 libzmq = "4.1"
diff --git a/zmq-sys/build/vendored.rs b/zmq-sys/build/vendored.rs
index 653dcd4..00ef5c5 100644
--- a/zmq-sys/build/vendored.rs
+++ b/zmq-sys/build/vendored.rs
@@ -6,6 +6,5 @@ pub fn configure() {

     let wants_debug = env::var("PROFILE").unwrap() == "debug";

-    let artifacts = zeromq_src::Build::new().link_static(true).build_debug(wants_debug).build();
-    artifacts.print_cargo_metadata();
+    zeromq_src::Build::new().build_debug(wants_debug).build()
 }
Jasper-Bekkers commented 2 years ago

@erickt It would be really nice to see a new release with this PR. This would fully remove the cmake dependency from zmq, which in turn would remove the cmake dependency from Google's evcxr and my dependency tree.

Additionally it brings an update to the latest upstream zmq release, which includes websocket support among other things.

jean-airoldie commented 2 years ago

it looks like this is still on zeromq-src 0.1.6

Lol you're right. Fixed that.

erickt commented 2 years ago

Hi all! I handed maintenance over of this crate some time ago to @jedisct1 and @rotty, since it’s been years since I last used zeromq.

If they’ve gotten too busy to help maintain it, perhaps we could get some new maintainers to help out? Would any of you be interested in helping out?

jean-airoldie commented 2 years ago

it’s been years since I last used zeromq.

Same here. Won't be able to help unfortunately.

birkenfeld commented 2 years ago

I'm still using the crate actively, so I'd bet interested in helping out.

Jasper-Bekkers commented 2 years ago

Hi all! I handed maintenance over of this crate some time ago to @jedisct1 and @rotty, since it’s been years since I last used zeromq.

If they’ve gotten too busy to help maintain it, perhaps we could get some new maintainers to help out? Would any of you be interested in helping out?

We could do it under the Traverse-Research org, and have contributed most of this conversion and update already. ☺️

jedisct1 commented 2 years ago

Unfortunately, the last application using ZeroMQ I had was rewritten in Zig, so I'm not using that crate any longer either.

I you could take over maintenance, @Jasper-Bekkers that would be great!

Jasper-Bekkers commented 2 years ago

@jedisct1 please email (firstname@traverseresearch.nl ), then we can set everything up that way ☺️

dr-orlovsky commented 2 years ago

I can also help with maintaining this crate, since I am using it very extensively in multiple projects

Jasper-Bekkers commented 2 years ago

Unfortunately, the last application using ZeroMQ I had was rewritten in Zig, so I'm not using that crate any longer either.

I you could take over maintenance, @Jasper-Bekkers that would be great!

Let me know if you need anything from me, this crate is quite outdated and could use some dependency updates as well as code cleanup, that I'd be happy to do as well as removing cmake and moving to a later zmq release.

Jasper-Bekkers commented 2 years ago

Any update on this? Or at least on a release with this PR?

Jasper-Bekkers commented 2 years ago

Due to the lack of updates here I've created https://crates.io/crates/zmq2 that integrates these changes.

birkenfeld commented 2 years ago

The zmq2 repo link on crates.io still points to this repo - can you update to your fork? Thanks!

clintfred commented 2 years ago

Thanks to those who are trying to keep this crate alive.

@erickt Would it be possible to hand over the crate ownership to the fork maintainers? Having another crate isn't ideal.

Also, is the fork going to continue to use https://github.com/erickt/rust-zmq for issues, but https://github.com/Traverse-Research/zmq2 for PRs? That's will confuse people, I think. It might be better to make a clean break and not use a fork at all.

MarijnS95 commented 2 years ago

@erickt Would it be possible to hand over the crate ownership to the fork maintainers? Having another crate isn't ideal.

@jedisct1 seems to be the person to ask.

Also, is the fork going to continue to use https://github.com/erickt/rust-zmq for issues, but https://github.com/Traverse-Research/zmq2 for PRs?

If users find issues while using zmq2 it makes sense to report that over there; users with issues on rust-zmq should report them here and simply be redirected to first test if zmq2 has already addressed their problem.

That's will confuse people, I think. It might be better to make a clean break and not use a fork at all.

What difference does it make? Besides the fact that GitHub shows "Forked from erickt/rust-zmq" and "Branch is XXX commits ahead of erickt/rust-zmq:master" issues and PRs can still be created as normal on the forked repo. Besides, issues reported here mean more traffic in the maintainers(') inbox, in hopes of pushing them to follow up on https://github.com/erickt/rust-zmq/pull/339#issuecomment-1043886109.

clintfred commented 2 years ago

@erickt Would it be possible to hand over the crate ownership to the fork maintainers? Having another crate isn't ideal.

@jedisct1 seems to be the person to ask.

Also, is the fork going to continue to use https://github.com/erickt/rust-zmq for issues, but https://github.com/Traverse-Research/zmq2 for PRs?

If users find issues while using zmq2 it makes sense to report that over there; users with issues on rust-zmq should report them here and simply be redirected to first test if zmq2 has already addressed their problem.

I don't currently see an "Issues" section on zmq2. I was originally under the impression that maybe forks couldn't have issues, but I think they just need to be enabled.

That's will confuse people, I think. It might be better to make a clean break and not use a fork at all.

What difference does it make? Besides the fact that GitHub shows "Forked from erickt/rust-zmq" and "Branch is XXX commits ahead of erickt/rust-zmq:master" issues and PRs can still be created as normal on the forked repo. Besides, issues reported here mean more traffic in the maintainers(') inbox, in hopes of pushing them to follow up on #339 (comment).

MarijnS95 commented 2 years ago

I don't currently see an "Issues" section on zmq2. I was originally under the impression that maybe forks couldn't have issues, but I think they just need to be enabled.

You are definitely right, I hadn't seen that :sweat_smile:. They can be enabled on forks as well, but are - as far as I'm aware - disabled by default when creating a fork. Not sure why PRs are enabled by default.

erickt commented 2 years ago

I'm incredibly sorry for dropping the ball on this. @Jasper-Bekkers since you did the fork, I sent you an invite to collaborate on this repo, and invited you to own https://crates.io/crates/zmq. Please feel free to merge your changes in here.

daniel-brenot-apcapital commented 2 years ago

@Jasper-Bekkers Are you going to merge your changes in here? I'm looking to add async support but want to know that it's going to be accepted. Also i want to make sure i'm working off of the correct codebase

clintfred commented 2 years ago

@Jasper-Bekkers Are you going to merge your changes in here? I'm looking to add async support but want to know that it's going to be accepted. Also i want to make sure i'm working off of the correct codebase

@daniel-brenot-apcapital You may want to look at https://github.com/cetra3/tmq. I've used that in the past and had a fairly good experience.

daniel-brenot-apcapital commented 2 years ago

@Jasper-Bekkers Are you going to merge your changes in here? I'm looking to add async support but want to know that it's going to be accepted. Also i want to make sure i'm working off of the correct codebase

@daniel-brenot-apcapital You may want to look at https://github.com/cetra3/tmq. I've used that in the past and had a fairly good experience.

Like the other crate i've looked at, this seems to be linux only at a first glance. Am i correct? I don't see how it would work on windows when it uses unix apis.

clintfred commented 2 years ago

@Jasper-Bekkers Are you going to merge your changes in here? I'm looking to add async support but want to know that it's going to be accepted. Also i want to make sure i'm working off of the correct codebase

@daniel-brenot-apcapital You may want to look at https://github.com/cetra3/tmq. I've used that in the past and had a fairly good experience.

Like the other crate i've looked at, this seems to be linux only at a first glance. Am i correct? I don't see how it would work on windows when it uses unix apis.

I think you're right about windows support. See https://github.com/cetra3/tmq/issues/17 It looks like it wouldn't be too hard to add to that project, if you're so inclined.

daniel-brenot-apcapital commented 2 years ago

@Jasper-Bekkers Are you going to merge your changes in here? I'm looking to add async support but want to know that it's going to be accepted. Also i want to make sure i'm working off of the correct codebase

@daniel-brenot-apcapital You may want to look at https://github.com/cetra3/tmq. I've used that in the past and had a fairly good experience.

Like the other crate i've looked at, this seems to be linux only at a first glance. Am i correct? I don't see how it would work on windows when it uses unix apis.

I think you're right about windows support. See cetra3/tmq#17 It looks like it wouldn't be too hard to add to that project, if you're so inclined.

Right, but why not make async support built into the main library then? Having multiple libraries that add async support when it could be added as something supported by the main library means that there is a high chance the async library might stop being supported when this one is still going, or vice versa. I'm using this in a professional context, so it would be nice to have fewer libraries to have to trust as well.

Also if the worry is about bulk, then just ask that the implementation of async be thin and behind a feature. I'm confused as to why there is so much hesitance to adding a base feature that would make the library much more practical.

Jasper-Bekkers commented 2 years ago

@Jasper-Bekkers Are you going to merge your changes in here? I'm looking to add async support but want to know that it's going to be accepted. Also i want to make sure i'm working off of the correct codebase

I've PR'd the changes here #345, it would be good to get some eyes on and some wider testing before merging this in.

As for async support in the "main library", I think we should start up another issue to discuss this. I'm personally very much not a fan of Rust's async/await design in general, the heavy runtime dependencies it brings and I don't really think the async/await design fit with how zmq is supposed to be driven. I also am quite hesitant of taking in major changes, one of the key attraction points for me in the rust-zeromq crate is that it's a light abstraction around the C library. This is why my personal opinion is that these things should live in a separate crate (potentially built on top of this one or on the raw C bindings this crate has) as I also have no desire to maintain an async/await implementation myself.

However, if we should go in-depth I don't think this issue is the right place for this discussion and we should open up another one.

Like the other crate i've looked at, this seems to be linux only at a first glance. Am i correct? I don't see how it would work on windows when it uses unix apis.

While this crate does support windows, it's not ideal. The PR I've linked previously should address those issues. We've been using it successfully with those changes on Windows. Please help review those changes if you can :-)

dr-orlovsky commented 2 years ago

I'm personally very much not a fan of Rust's async/await design in general

+1.

Proper microservice architecture done with ZMQ allows to make non-blocking projects without using async at all - so adding tokio will lead to forking this project and maintaining a no-async version of the lib for those who do not need async.

Basically ZMQ is a kind of "channel" alternative to async, and it is strange to mix this two together. If somebody needs async, he probably splits tasks between microservices in a wrong way - or uses ZMQ just for RPC API to monolithic server, which is an edge case for ZMQ.

I am running 20+ microservices communicating over 13+ ZMQ sockets, working also with TCP, database, high-load data processing etc, and never had required async.

daniel-brenot-apcapital commented 2 years ago

I'm personally very much not a fan of Rust's async/await design in general

+1.

Proper microservice architecture done with ZMQ allows to make non-blocking projects without using async at all - so adding tokio will lead to forking this project and maintaining a no-async version of the lib for those who do not need async.

Basically ZMQ is a kind of "channel" alternative to async, and it is strange to mix this two together. If somebody needs async, he probably splits tasks between microservices in a wrong way - or uses ZMQ just for RPC API to monolithic server, which is an edge case for ZMQ.

I am running 20+ microservices communicating over 13+ ZMQ sockets, working also with TCP, database, high-load data processing etc, and never had required async.

"Proper microservice architecture" Doesn't prescribe for or against async. That's a bit of a ridiculous notion. Also yes you can make non blocking projects with this without async, but it will inevitably end up just being some other way to do async.

Using ZMQ to perform RPC isn't exactly monolithic.

Receiving asynchronously on a socket is a great example of where a proper async implementation would outperform any possible implementation you have. With using something like libuv, you can have the operating system resume an async task once data is received on a socket. This would dramatically improve throughput of an application, as well as reduce the number of threads needed to serve users.

Also, async is not tokio. Adding async to the library allows it to be used with any async runner. This shows the complete lack of understanding in the topic being discussed.

Lastly, just because you are running 20 microservices over 13 sockets and haven't required async, doesn't mean that you did it well. This is kind of a strange way to make your point as it doesn't really prove anything.

Anyways, i'm content with letting this issue die or moving this discussion to a different thread, but the sheer lack of understanding about how async works and what implementing it properly means is very surprising to me. Rusts async seems to be a strangely controversial topic, despite it being one of the most efficient implementations out there(See how activex performs in relation to other web servers).

dr-orlovsky commented 1 year ago

@daniel-brenot-apcapital here by async I meant not the consept of async programming, but the specifics of the rust async implementation - and any async implementations that do not use shared nothing principle and utilizes shared memory instead.

I also never said that async==tokio and I was referring to the proposal of adding tokio support to zmq directly. Nevertheless, no desire or time in arguing on what I meant, when my statements are just taken out of context.