Skip to content
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

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

Closed
neachdainn opened this issue Apr 4, 2019 · 19 comments
Closed

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

neachdainn opened this issue Apr 4, 2019 · 19 comments
Assignees

Comments

@neachdainn
Copy link
Collaborator

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 neachdainn self-assigned this Apr 5, 2019
@neachdainn
Copy link
Collaborator Author

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

@jeikabu
Copy link
Owner

jeikabu commented Apr 5, 2019

Should be fine.

@jeikabu
Copy link
Owner

jeikabu commented Apr 8, 2019

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
Copy link

gdamore commented Apr 8, 2019 via email

@neachdainn
Copy link
Collaborator Author

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
Copy link
Owner

jeikabu commented Apr 14, 2019

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

@neachdainn
Copy link
Collaborator Author

neachdainn commented Apr 14, 2019

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
Copy link
Owner

jeikabu commented Apr 15, 2019

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:

  • Crate and nng version numbers diverge
  • You never update a crate (once you push 1.2 anything else is "lower priority" wrt crates.io)
  • You accept crates.io not displaying the "correct" thing (this is what I was doing before- I assume anyone that actually care's that much about the nng version as opposed to it just being a downstream dependency will look at the docs and/or github)

@neachdainn
Copy link
Collaborator Author

neachdainn commented Apr 15, 2019

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
Copy link
Owner

jeikabu commented Apr 15, 2019

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
Copy link
Owner

jeikabu commented Apr 16, 2019

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

@neachdainn
Copy link
Collaborator Author

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
Copy link
Owner

jeikabu commented Apr 22, 2019

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
Copy link
Collaborator Author

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
Copy link
Owner

jeikabu commented Jan 7, 2020

@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
Copy link
Collaborator Author

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
Copy link

gdamore commented Jan 7, 2020

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
Copy link
Owner

jeikabu commented Jan 7, 2020

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
Copy link
Collaborator Author

neachdainn commented Jan 7, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants