jeikabu / nng-rust

MIT License
18 stars 8 forks source link

Move to Nanomsg domain and publish on Crates.io #17

Closed neachdainn closed 4 years ago

neachdainn commented 5 years ago

I think we are ready to begin this process and that this crate is in a state where we can start moving our respective wrappers to it. I do want to keep it at an *-rc version until after we've both moved to it and have given it a little time to settle.

neachdainn commented 5 years ago

I am willing to contact Garrett / handle the logistics / push to Crates.io as soon as @jeikabu gives me the go-ahead.

jeikabu commented 5 years ago

Should be fine.

jeikabu commented 5 years ago

Going to mention that all that needs to be done is transfer ownership of this repo to @gdamore. I might need to do it as the current owner.

gdamore commented 5 years ago

I dont think that is necessary. I need to add you to the nanomsg project then we can move it. Sent from my Verizon, Samsung Galaxy smartphone -------- Original message --------From: jeikabu notifications@github.com Date: 4/8/19 12:54 AM (GMT-08:00) To: jeikabu/nng-rust nng-rust@noreply.github.com Cc: gdamore garrett@damore.org, Mention mention@noreply.github.com Subject: Re: [jeikabu/nng-rust] Move to Nanomsg domain and publish on Crates.io (#17) Going to mention that all that needs to be done is transfer ownership of this repo to @gdamore. I might need to do it as the current owner.

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread. {"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/jeikabu/nng-rust","title":"jeikabu/nng-rust","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/jeikabu/nng-rust"}},"updates":{"snippets":[{"icon":"PERSON","message":"@jeikabu in #17: Going to mention that all that needs to be done is transfer ownership of this repo to @gdamore.\r\nI might need to do it as the current owner."}],"action":{"name":"View Issue","url":"https://github.com/jeikabu/nng-rust/issues/17#issuecomment-480724459"}}} [ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/jeikabu/nng-rust/issues/17#issuecomment-480724459", "url": "https://github.com/jeikabu/nng-rust/issues/17#issuecomment-480724459", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

neachdainn commented 5 years ago

While NNG 1.2 and the Nanomsg Org items are pending, should I upload this to Crates.io as v1.1.1-rc.1? I have a branch that is working off of this repo that I'm eager to merge into my master branch.

jeikabu commented 5 years ago

That should be fine. It's been stable for a while now.

neachdainn commented 5 years ago

I should have it uploaded some time this weekend. I'm assuming you won't merge #18 until after NNG v1.2 comes out?


EDIT: It has been uploaded. It looks like Crates.io will default to showing the latest non-rc release (see here) which might make the planned versioning system of <nng-version>.rc<crate-version> problematic. I'm OK with this for now but I'll probably want to figure something out before NNG 1.2 is released.

EDIT: By "figure something out", I really mean "confirm that's how we want to do things". Crates.io displaying the wrong version won't impede the development of Runng or Nng-rs since we're in the know, but it may be an issue for people who want to use the raw bindings or are writing their own wrapper.

jeikabu commented 5 years ago

Yeah, that 1.2 branch is for whatever the "next" release of nng is called. Probably shouldn't merge it because it contains several changes that are definitely not compatible with 1.1.1.

The way crates.io/cargo deals with everything after the - is surprising. If you yank the earlier crates it might display like you expect. Last time I looked there was no satisfactory solution. You need to settle with:

neachdainn commented 5 years ago

Originally, I decided that diverging was fine but tried to minimize it by saying that the major and minor version numbers will always match with NNG. In theory this is fine since the API for NNG shouldn't change between patch versions and it would allow the crate to make updates via the patch version. As long as the crate was very careful about the changes between patch versions, things generally work as expected.

I do like the idea of <nng version>-<crate version> except for two things: Crates.io will always display the wrong version unless versions are yanked, which I am not a fan of, and putting nng-sys = "*" in the Cargo.toml will always download an old version. That being said, I think we should change it from e.g., 1.2.0-rc.1 to 1.2.0-1.0.0 since Semver should still approximately work in that 1.2.0-1.0.0 < 1.2.0-1.0.1 < 1.2.0-1.1.0, etc.

Another thing I considered was controlling the NNG version via feature flags. I think it strikes a good compromise between allowing the crate to be updated as needed but making it clear to the user which NNG version they are using (with the default feature being the last release). In my opinion, it doesn't matter what the crate versions are when the NNG version is right next to it: nng = { version = "5.4.2", features = ["nng-1.2.0"] }. The major downside of this is that it makes the maintenance of the crate much more difficult.

Looking at the FFI binding crates on Crates.io, it looks like most of them have chosen to have diverging version numbers. Unfortunately, I didn't see any that specified how their versioning work until libsqlite3-sys which uses feature flags to specify the version (among other things). sdl2-sys links to the system installed version but provides a bundled version behind a feature flag in case there are issues. Overall, it doesn't seem like there isn't any community consensus on what to do.


I think we can agree that never updating the crate is not a good choice. I originally chose to diverge on the patch version since I was maintaining the bindings by hand but, now that we're using Bindgen, I don't think it is a better option than the others. On my own, I would probably have this crate use feature flags since Cargo will tend to do what the user is expecting.

I will be fine with the <nng version>-<crate version> system if you have a strong preference for it, I would just like to move away from -rc.x to -x.y.z. I'll be good with pretty much anything as long as we've officially decided by the 1.2 release.

jeikabu commented 5 years ago

I believe there was a problem with -x.y.z when I tried it (or maybe it was only with -x).

Semver already approximately works: 1.1.1-rc.1 < 1.1.1-rc.2 < 1.2.0-rc.1. Both -rc.X and -X.Y.Z make it a pre-release. Given that there's very little going on in the package (it's mostly generated), probably don't need a lot of versioning info there; 1.1.0-1.1.0 is a bit verbose. I don't think semver lets you have 1.1.1-1.0.0 and then push 1.1.1-1.1.0 to introduce breaking changes.

Keep in mind how the package will most likely be used. I would expect the most important thing being nng compatibility. Subsequent releases in the "1.1.1" series would involve things like trivial packaging changes, documentation, etc. Most people just want the latest compatible version and to do that they can specify 1.1.1-rc, that will pull 1.1.1-rc.1 or whatever the latest is.

You might want to make a test crate and try pulling/building different versions.

Using feature flags seems like it would get messy pretty fast. Especially with how clumsy it is to deal with mutually exclusive flags.

jeikabu commented 5 years ago

If you don't like the "rc" change it to something else. I only used that because it seemed to be pretty common.

neachdainn commented 5 years ago

Using feature flags seems like it would get messy pretty fast. Especially with how clumsy it is to deal with mutually exclusive flags.

This is true.

Most people just want the latest compatible version and to do that they can specify 1.1.1-rc, that will pull 1.1.1-rc.1 or whatever the latest is.

Wouldn't this mean that the changes we can make are controlled by the changes in NNG? We lose the ability to inform Cargo and the user if the update breaks something or adds features, as it is essentially a patch version. I don't anticipate there to be much of a need to make breaking changes, but adding that version_check crate definitely opens the possibility of utilizing new Rust features.

You're latest commit on the 1.2 branch does constitute a breaking change since TryFrom is not in the prelude. Are we fine with requiring the user to understand that things may break between 1.1.1-rc.1 and 1.2.0.rc.1, even though SemVer disallows that?

I personally would tentatively answer "yes" because (1) Cargo won't do anything stupid since the user does have to fully specify the NNG version to get the latest and (2) it is very likely that the only direct users of this crate will be Nng-rs and Runng.

If you don't like the "rc" change it to something else. I only used that because it seemed to be pretty common.

The thing I don't like about "rc" is just that it almost universally means "release candidate", which would imply that we never have actual releases. But, I did have an idea concerning that: if we follow the existing Rust precedent of only supporting the latest version, we could do non-RC releases right before we release a new version.

I.e., right before we release 1.2.0-rc.1 we upload 1.1.1 to Crates.io. This would mean that the rc actually does stand for "release candidate" and Crates.io / Cargo will start doing sensible things when the user doesn't fully specify the version. If the user want's the latest version but doesn't do their research on the quirks of this crate, doing nng-sys = "*" will at least get them something more recent than 0.1.3.

jeikabu commented 5 years ago

Are we fine with requiring the user to understand that things may break between 1.1.1-rc.1 and 1.2.0.rc.1, even though SemVer disallows that?

I also think it's "yes". It's the same whether we use 1.1.1-rc.X or 1.1.1-X.Y.Z. If we extend the nng version numbers we may end up with a "breaking" change as far as SemVer is concerned when we map things to Rust.

"rc" seemed fine because we're just an extension of nng proper. There really is no "final" 1.1.1 because we may always need to adjust documentation, etc. I would have preferred to use 1.1.1-x or 1.1.1-x.y but that didn't play nice with cargo; you couldn't specify nng-sys = "1.1.1-" (maybe you can now).

Not sure how much we need to worry about nng-sys = "*" since that is discouraged. But you're right, cargo/crates.io "preferring" the last x.y.z release (with no pre-release or meta-data) is unfortunate.

Anyway, I don't think there's a "perfect" solution. Pretty sure we're only going to get two of:

  1. Match nng version numbers
  2. Semver compliance
  3. Supported by cargo/crates.io/docs.rs

I'm ok with whatever you want to change it to. After messing with it for a while I gave up and accept that nng-sys just isn't the intended use-case. =)

neachdainn commented 5 years ago

I would have preferred to use 1.1.1-x or 1.1.1-x.y but that didn't play nice with cargo; you couldn't specify nng-sys = "1.1.1-" (maybe you can now).

Ah. I wasn't quite sure what you meant before. This makes sense.

I'm ok with whatever you want to change it to

I guess we should just leave it for now. We already have an *-rc release so we may as well stay consistent. That being said, I will probably push another release (maybe 1.0.0) for the purpose of making the default landing page on Crates.io very explicit about the way the crate is being versioned, because right now it is very incorrect.

I like the idea of releasing a 1.0.0 because the Rust community does have a problem with crates never reaching a release version and I would like to inform people that this crate is a release crate. Unfortunately, that would prevent us from ever releasing any 1.0.0-rc versions... But are we ever going to do that?

jeikabu commented 4 years ago

@neachdainn Just close this? I thought a "neutral" third-party would be best, but, honestly, it's easier if one of us just owns it. If you're not happy with me having it, I'll transfer it to you. No biggie.

neachdainn commented 4 years ago

I liked the idea of having an "official" Rust binding, but you owning it is fine. It doesn't seem likely that yet another Rust NNG wrapper will appear.

gdamore commented 4 years ago

Were you guys looking for something from me? I’m happy to have this have a home on nanomsg.org.

Note there is also a scaporust pure rust implementation of the protocols.

jeikabu commented 4 years ago

If possible yeah, I was just looking to get rid of the dangling issues. Scaporust hasn't been touched in almost 2 years, but I'm fine with having a "pure" implementation as well.

From your April 8 comment it sounded like we needed to be added to the nanomsg org? Or, I can transfer the repo to you, whatever is required.

neachdainn commented 4 years ago

Scaporust hasn't been touched in almost 2 years, but I'm fine with having a "pure" implementation as well.

Also, for whatever reason, Scaproust is largely unused (2,167 total downloads) compared to the NNG wrappers (9,711) and the Nanomsg wrapper (22,006). Even if there was a real push for a pure-Rust version of the protocols, Rust's asynchronous I/O situation has changed pretty dramatically over the last two years and the author has stated that it would probably require a rewrite.

I’m happy to have this have a home on nanomsg.org.

I would love to have it there (ideally with nanomsg-sys, but that's a different issue). Just because thesys crates really aren't anything more than auto-generated "headers" and trying to use more than a single sys crate for a library causes compilation issues for the end user. Having an "official" one give the community a solid common sys crate.