Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Rust change policy #6718

CasperN started this conversation in General
Oct 21, 2020 · 23 comments · 1 reply
Discussion options

We have a bunch of work lined up, developing Flatbuffers in Rust. (Issues). Some of these, such as fixing #5589 would require breaking changes. We need a stability and change policy.

Some precedent:

Proposed policy

  1. Critical Rust code-breaking changes may happen with 1 weeks notice.

  2. Minor Rust code-breaking changes may happen with 12 weeks notice

    • This includes adopting new compiler features (thus forcing a compiler upgrade), renaming constants to be more "Rust-y", etc
    • Change due to new features like the verifier Implement Flatbuffer Verifier for Rust #6161 fall into this category.

"notice" basically means mentioning it on an issue tagged rust in this repo.

Thoughts?

@rw @aardappel @krojew @jean-airoldie @andygrove @jcrevier @plwalsh @dvtomas @maxburke

You must be logged in to vote

Replies: 23 comments 1 reply

Comment options

The time frames suggested here seem reasonable to me. One thought though: It might be wise to publish or document somewhere which commit of flatc can generate working code for the corresponding rust flatbuffer crate. The crate documentation instructs users to use HEAD. But if the changes in generated code are significant and there's a bit of churn, it might take library users a bit longer to upgrade / transition. Being able to generate working code for any given version of the flatbuffers crate seems important.

Ideally we pin a version of the flatbuffers crate to a version of flatc but given how infrequently versions are released, I think we could just tie it to a specific commit.

You must be logged in to vote
0 replies
Comment options

I'm all for this. Breaking changes for the sake of active development is always okay in my book. Thanks for keeping us in the loop. And I agree with @jcrevier's suggestions above.

You must be logged in to vote
0 replies
Comment options

I think we should also consider #6149 which makes things more consistent.

You must be logged in to vote
0 replies
Comment options

CasperN
Oct 22, 2020
Maintainer Author

I think we should also consider #6149 which makes things more consistent.

I see the following tradeoffs to making the crate's version match flatc's:
(+) We work (and test) from HEAD anyway so it makes sense to snapshot them at the same point.
(+) We'd maintain a simpler compatibility mapping.
(-) Rust's version could be bumped due to irrelevant changes in flatc
(-) I'm not sure if Flatbuffers' version follows https://semver.org/ as rigorously as some Rust folk would like

Overall, it sounds good to me.

You must be logged in to vote
0 replies
Comment options

(-) Rust's version could be bumped due to irrelevant changes in flatc

That's really not an issue, since FB rarely uses patch version and we can leverage that (or any metadata). Only the major and minor versions should be relevant to the API.

(-) I'm not sure if Flatbuffers' version follows https://semver.org/ as rigorously as some Rust folk would like

I could rant for an hour about how crate authors fail to follow semver, but that's not the place for that. Nevertheless, it would be great if FB actually followed it, right @aardappel ? :)

You must be logged in to vote
0 replies
Comment options

(-) I'm not sure if Flatbuffers' version follows https://semver.org/ as rigorously as some Rust folk would like

I agree the simpler compatibility mapping would be a big win. However, I think the semver concerns are a problem. If we release a 1.12 version of the crate and then want to make backwards incompatible changes to just the crate, we will need a 2.0 release of the crate for things to play nicely with semver and cargo -- but is that a good enough reason for @aardappel to bump the major version of the entire project? I suspect we're better off overall without trying to couple these together.

You must be logged in to vote
0 replies
Comment options

CasperN
Oct 22, 2020
Maintainer Author

and cargo

Oh yea if automation enforces rigorous semver within the Rust ecosystem then I agree we should avoid coupling.

You must be logged in to vote
0 replies
Comment options

and cargo

Oh yea if automation enforces rigorous semver within the Rust ecosystem then I agree we should avoid coupling.

Yep, cargo will happily grab 1.13 if you have flatbuffers = "1.12" specified in your cargo.toml

You must be logged in to vote
0 replies
Comment options

Given the changes coming to FB, I would be happy with a semver-compatible 2.0 release.

You must be logged in to vote
0 replies
Comment options

CasperN
Oct 22, 2020
Maintainer Author

I would be happy with a semver-compatible 2.0 release.

That'd be an @aardappel decision, but having a single rigorous semver for all flatbuffers languages and flatc seems like a bad idea to me: There'll be far too many irrelevant "breaking changes" that are not observable in most projects since they don't use all the features/languages.

You must be logged in to vote
0 replies
Comment options

A breaking change is never irrelevant. If something fundamental is changing very often, there's something really wrong with the design.

You must be logged in to vote
0 replies
Comment options

Up to y'all.. so far, FlatBuffer has not been strict about breaking changes, in that, we try pretty hard to avoid them, but when they have to happen because of critical safety/performance or even major usability changes, we do them anyway. I think it is unavoidable, and better than the alternatives. My general rule of thumb has been that the more people we're likely to break the more critical the change must be for it to be ok. But that's hard to encode in a rule.

We're a small enough project (in terms of regular maintainers) that I personally wouldn't want to commit to strict timespans for giving notice.. I like to keep process as simple as possible. But if the people interested in Rust want to do this, I have no problem with it either.

As for versioning, I am famously apathetic to any versioning scheme that's not just dates or commit ids. I feel SemVer is pretty meaningless, and impossible to make work in an exact way in a project spanning public, internal and code-genned APIs across 10+ languages. Most of the time I can't even predict what can all break by looking at a PR.

Versions so far are just tied to global FlatBuffers releases, and we're not going to do these more often for the needs of 1 language, as least as long as I am doing them (anyone want to take over FlatBuffers releases, inquire within ;) That said, if y'all wanna push to Cargo more often than that and keep your own versioning, you're of course welcome to.

You must be logged in to vote
0 replies
Comment options

We're fine with breaking changes as long the changes are breaking only in API and not data as we rely on FB for inter-language serialization, but I assume that's what you mean anyways 🙂

You must be logged in to vote
0 replies
Comment options

I feel SemVer is pretty meaningless, and impossible to make work in an exact way in a project spanning public, internal and code-genned APIs across 10+ languages. Most of the time I can't even predict what can all break by looking at a PR.

I think trying to introduce semver can't hurt - after all, if it fails for us, we can still say we're not semver compatible, as it is now. On the other hand, if we can make it work, it's a good scheme to follow, which people will appreciate on upgrading.

You must be logged in to vote
0 replies
Comment options

@krojew to use SemVer as intented, the next few releases would have to be 2.0, 3.0, 4.0.. because there are breaking changes in ever release (or at least, there have been in all past 12+ releases).

We don't do releases other than that (for the whole repo).

Is this what you're proposing?

You must be logged in to vote
0 replies
Comment options

If there are backwards-incompatible changes, then yes - we should update the major version. This way we give everyone a compatible upgrade path for non-breaking changes and patches with minor and patch version bumps.

You must be logged in to vote
0 replies
Comment options

But that's the point: there have never been any minor version (non-breaking) releases.

SemVer would make sense if we released each language completely independently. But as long as we version FlatBuffers releases as a whole there is always some breaking change, even if some languages included did not have any breaking changes.

You must be logged in to vote
0 replies
Comment options

That actually doesn't really matter - even if we only have x.0.0 versions, the point is to explicitly state what is breaking or not (and in this edge case, each version is breaking). Right now our versioning is arbitrary, thus unknown to the outside world. Semver is a standard everyone recognizes and we loose nothing by following it. That gets more important when FB is used with languages where semver is assumed.

You must be logged in to vote
0 replies
Comment options

CasperN
Oct 23, 2020
Maintainer Author

But as long as we version FlatBuffers releases as a whole there is always some breaking change, even if some languages included did not have any breaking changes.

I agree, the problem with a single semver is that it doesn't scale with the size of the whole project. We'd need one for each independent API surface: The binary format and every language's code-generator/library.

You must be logged in to vote
0 replies
Comment options

I agree, the problem with a single semver is that it doesn't scale with the size of the whole project.

This is the problem Angular had with their myriad of libraries. They tried a matrix approach with different versions per lib with different compatibility. In the end, they switched to keeping every version consistent. If it worked for them, it can work for us.

You must be logged in to vote
0 replies
Comment options

I still think some sort of pinning of the crate version to a specific commit of flatc is a good idea. We could still have the latest version require a flatc from master, but especially for past versions it seems important to know how to generate working code for a given rust crate.

Also while the rust documentation is clear that the crate expects you to use the latest flatc from master, there's nothing on the crates.io page to indicate this. With more breaking changes in the code generator, I think this is already confusing folks: #6339

You must be logged in to vote
0 replies
Comment options

Perhaps we could add a Cargo package feature to the official flatbuffers crate, that uses build.rs to build the exact version of flatc that is needed?

You must be logged in to vote
0 replies
Comment options

This issue is stale because it has been open 6 months with no activity. Please comment or this will be closed in 14 days.

You must be logged in to vote
1 reply
Comment options

CasperN Jul 2, 2021
Maintainer Author

I moved this to a discussion since it seems more permanent than an issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Converted from issue

This discussion was converted from issue #6199 on July 02, 2021 17:56.

AltStyle によって変換されたページ (->オリジナル) /