-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Determine story around JSON message compatibility #12377
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Comments
I think versioning can work if the versioning rules are clearly defined, and the format is documented. If I understand the problem correctly, right now there is only one format version, changing any metadata field breaks the version guarantees, because the version is always the same. Tools and libraries that parse the metadata format can rely on the I personally like how Stripe versions their API to provide backwards compatibility guarantees without having to expose version numbers directly in their resources: https://stripe.com/blog/api-versioning. The way that this would work is probably different for Cargo, and it might require an RFC. These would be the steps I'd take to create a compelling versioning story:
|
The problem with using requested versions is trying to figure out how to map new concepts into the old version. Things like [profile.dev]
debug = "line-tables-only" Format version 1 specified that |
I understand that. From my point of view, the lack of versioning is what's allowing changing types in the first place. You cannot have a conversation about data design when nothing is really enforced. Making the versioning guidelines explicit will help to set expectations about keeping backwards compatibility. I worked at Docker for a while, and we were very cautious about breaking api compatibility, and sometimes failed to keep the data format compatible. However, the explicit guidelines forced those conversations during feature design, instead of later. In your example, if debug changes from integer to string, you'd be forced to bump the format version. The best time to have the conversation about backwards compatibility was when the change was propose, but if there are no guidelines or anything that reminds people of those guarantees, the expectation is low, or non-existent. |
This is the part I'm not quite following. Can you clarify what that means? My interpretation is:
We do have guidelines that say cargo is intended to be backwards compatible except where specified, but there were no automated tests or tooling to enforce it in this case and nobody recognized that this type snuck into the serialized format or recognized that it was a breaking change until it was too late. We also have versioning built-in to However, even if we did recognize it, I don't know what we would have done differently. None of the options seem particularly good to me. That is, they all seem to break tooling in one way or another. The answer might be that the only option we have is to break backwards compatibility in some way. Then the question is, which approach causes the least breakage? Of the options I see, it seems like we stumbled on the course with the least amount of breakage (although we should have been more explicit about it in the docs, and proactive in updating the |
oh, yeah, let me clarify. I'm not saying that a previous version should break if a new version makes changes, I'm proposing the opposite, to try to keep backwards compatibility as much as possible. I agree with you that this requires an extra level of awareness from maintainers/reviewers. Going back to your example. The initial type for Running this command,
Running this command,
New values in that option might not make sense in the old format. You could either decide to give them meaning, or have a fallback value that means "unknown/undefined".
This is a very valid concern. In my experience, being explicit by using version numbers leads to least surprises and better backwards guarantees. |
### What does this PR try to resolve? #11666 This adds an unstable `--message-format` flag to `cargo package` to help `--list` output in newline-delimited JSON format. See <https://github.com/weihanglo/cargo/blob/package-list-fmt/src/doc/man/cargo-package.md#package-options> for more on what is provided. Open questions - `--list json` or `--message-format json`, see #15311 (comment) - a single json blob vs N? What is N? See #15311 (comment) - Is the current format `plain` or `human`, see #15311 (comment) - snake_case or kebab-case, see #15311 (comment) ### How should we test and review this PR? * This currently outputs absolute paths. If we don't want to show absolute paths in the JSON output, could switch to relative path to either package root or cwd (I prefer the latter though). * The actual schema, format option names, and field names is open to discuss and change. See also <#12377> for reference.
So far, we have treated the JSON messages emitted by
--message-format=json
(docs) to be somewhat stable (with the caveats of adding new fields). However, that stability has been broken at times (see oli-obk/cargo_metadata#240 as an example). We don't have a strategy for how such changes could be made. We should probably come up with some story and documentation for how we manage the compatibility.This issue is to collect feedback and ideas on how this should be handled and documented.
Note that this somewhat transitively applies to rustc's diagnostic structure. There should probably be a consistent story with both that and cargo's own messages, which may involve consulting with the compiler team.
My thoughts/ideas:
Versioning doesn't work particularly well since introducing a new version that tooling doesn't recognize would cause them to break (and in the debuginfo case, would cause them to unnecessarily to break in most cases since that change only affected the rare projects that would use the string-based types).
We could potentially indicate that types of fields can change between versions, but otherwise meanings of existing fields wouldn't change. But that could be a challenge for deserializers like
cargo_metadata
since the only choice to stay compatible would be to deserialize to something likeserde_json::Value
, losing all structure. Or those tools could just accept that type changes would break their tooling.We could potentially just document that this format is not stable at all, and shed responsibility. An option for tools like
cargo_metadata
is that they could just do semver-breaking changes periodically.Another option is to document that the format is stable except in the presence of new opt-in features (like the debuginfo string example). This could make it challenging or impossible to make broader structural changes.
The text was updated successfully, but these errors were encountered: