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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make bumping the wire format an API breaking change #749

Open
therealfrauholle opened this issue Apr 5, 2023 · 4 comments
Open

Make bumping the wire format an API breaking change #749

therealfrauholle opened this issue Apr 5, 2023 · 4 comments

Comments

@therealfrauholle
Copy link
Contributor

Hey @Urhengulas after the latest release of the changes in #747 I am still having problems aligning versions in my setup 馃槗

It appears to me that we missed bumping the dependency defmt-macros of defmt in 3789241 and also earlier in 1cbbb33, it should now depend on defmt-macros-0.3.4.

Following this I tried to wrap my head around how the wire format plays together with the SemVer guarantees, and I think that bumping the wire format is an API breaking change.

This is related to the changes introduced as of #540, which solved #287.

@Dirbaio
Copy link
Contributor

Dirbaio commented Apr 8, 2023

The original rationale for decoupling the wire format version from the defmt semver version was that doing a defmt semver-major bump is a giant pain. You can't mix defmt versions in a binary (#426), and this seems fundamentally unfixable with the defmt design. So upgrading defmt requires ALL libraries using it to update. And in turn, that update requires a major bump of the library, because it'll break binaries that are still using the older version.

This happened in the defmt 0.2 -> 0.3 upgrade. It was a giant pain, it took MONTHS.

Decoupling the versions allows defmt to evolve while avoiding regularly inflicting this pain into the ecosystem. I really don't think we should walk back on this decision.

However, I remember discussing making defmt-decoder support multiple wire format versions, which it seems it never happened. Currently old defmt-decoder/probe-run versions support only wire 3, newer ones support only 4, so you have to keep switching between them if some projects have been updated and some other haven't. I agree this is not great.

If the new versions added support for 4 instead of dropping support for 3 the experience would be much better: you upgrade defmt, you get an error from probe-run saying "please update probe-run", and then everything would be working again.

@jannic
Copy link
Contributor

jannic commented Apr 12, 2023

Would it be another option to do a semver bump for defmt-decoder only when the wire format changes?
That way, binaries could support multiple wire format versions. Not as nice as having multiple versions supported by defmt-decoder, but perhaps a viable option if direct support is too difficult to add?

@Dirbaio
Copy link
Contributor

Dirbaio commented Apr 12, 2023

It'd be nicer to have a single defmt-decoder version support multiple wire format versions.

If it's one-to-one, bins can hack it by depending on multiple defmt-decoder versions at once, yes, but:

  • that work needs to be repeated for each project that uses defmt-decoder
  • needs manual work to add new versions, vs just cargo updateing to a newer defmt-decoder.

@datdenkikniet
Copy link

datdenkikniet commented Apr 12, 2023

Requiring an update to projects using defmt-decoder sounds far less problematic, IMO.

Crates actually using that are far fewer, and being able to support multiple wire versions so that actual users can safely cargo install <decoder thing> without having to worry that decoding logs from projects locked to older versions of defmt completely breaks (without cargo install-ing very specific versions of <decoder thing> locked to a specific version of defmt-decoder).

The fact that probe-run has defmt-decoder = "=0.3.5" (because if it didn't, how would I ever run a defmt 3 binary without patching a version of probe-run?) so it didn't update anyways isn't particularly supportive for the use case... Great that all my projects updated but the tools didn't :/

For defmt the rationale makes sense, but for defmt-decoder I don't think there's a good enough reason if backwards compatibility is not maintained, or backwards compatibility can't be maintained by 3rd party tools.

bors bot added a commit that referenced this issue May 3, 2023
750: Add support for decoding wire format version 3 r=Urhengulas a=jannic

As suggested in #749 (comment), it would be nice if defmt-decoder could handle older wire format versions transparently.

This patch implements support for version 3. A version of `probe-run` compiled against this was verified to work with defmt versions 0.3.2, 0.3.3 and 0.3.4.

Co-authored-by: Jan Niehusmann <jan@gondor.com>
Co-authored-by: Johann Hemmann <johann.hemmann@code.berlin>
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

4 participants