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

[RFC] how to manage decoder versions? #596

Open
japaric opened this issue Sep 28, 2021 · 5 comments
Open

[RFC] how to manage decoder versions? #596

japaric opened this issue Sep 28, 2021 · 5 comments
Labels
status: needs decision The Knurling team needs to make a decision on this

Comments

@japaric
Copy link
Member

japaric commented Sep 28, 2021

(this is probably more of probe-run issue but a solution may involve changes in defmt-decoder so posting in this issue tracker)

we version defmt in defmt-decoder so now a tool like probe-run can tell the user it doesn't know about a newer defmt-version and that the tool needs to be updated.
that's all good but presumably we want newer patch releases of probe-run to also support older defmt-versions so question is how do we approach that

1. one defmt-decoder per defmt-version

one way is to do a new minor release of defmt-decoder every time defmt-version increases.
then probe-run can depend on multiple minor versions of defmt-decoder.
however, for this to work probe-run needs to extract the defmt-version from the ELF so it can pick the right minor version of defmt-decoder to parse the rest of the table.
presumably, it needs to extract the version from the ELF without using any version of defmt-decoder.
other tools, like defmt-print, would also need to version extraction logic to support multiple defmt-decoder versions

2. multiple defmt-versions per defmt-decoder

the other way I see is handling the versions transparently in defmt-decoder itself.
That would imply having different paths in the code to handle different versions.
with this approach, we can do newer patch releases of defmt-decoder that support multiple defmt-versions.
the advantage is that tools can depend on a single minor version of defmt-decoder and gain, without extra work, support for newer versions.

That would imply having different paths in the code to handle different versions.

naively, this sounds like more overall work that the first option but I guess it also depends on how many tools we are maintaining using option 1.

3. one probe-run per defmt-version

that's all good but presumably we want newer patch releases of probe-run to also support older defmt-versions so question is how do we approach that

the third option that requires the least amount of work is to NOT do this and instead do a new minor release of defmt-decoder AND a new minor release of probe-run when defmt-version is bumped.

thoughts? @jonas-schievink

@japaric japaric added the status: needs decision The Knurling team needs to make a decision on this label Sep 28, 2021
@japaric japaric added this to the v0.3.0 milestone Sep 28, 2021
@japaric
Copy link
Member Author

japaric commented Oct 4, 2021

as a data point, the wire format has broken twice since the defmt version split landed:

@japaric
Copy link
Member Author

japaric commented Oct 6, 2021

another data point, we maintain 3 consumers of the defmt-decoder crate:

  • probe-run
  • defmt-print
  • qemu-run this one is internal use only, but we may want to add multiple decoder version support to perform backward compatibility tests?

@jonas-schievink
Copy link
Contributor

as a data point, the wire format has broken twice since the defmt version split landed:

That sounds like we might want to only bump the version between crates.io releases, not when we're just working in git (just to make sure we don't end up with 13 decoders or something). Not sure how that interacts with back-compat tests though.

one way is to do a new minor release of defmt-decoder every time defmt-version increases.
then probe-run can depend on multiple minor versions of defmt-decoder.

Some cursory testing reveals that this is impossible – Cargo does not allow a package to depend on different non-breaking versions of the same dependency, only on multiple major versions.

I think I'd say we go with option 2 then, with the caveat that we'll only bump the protocol version between actual releases, not for git-only changes.

@japaric
Copy link
Member Author

japaric commented Oct 6, 2021

That sounds like we might want to only bump the version between crates.io releases, not when we're just working in git (just to make sure we don't end up with 13 decoders or something).

hmm, bumping the version number per PR would let git users of probe-run get the "pre Flash & Run" 'mismatch version' error message instead of a runtime decoder error.

I do agree that we should not release one decoder crate for each PR that breaks the wire format. we could do decoder release less often so that e.g. decoder 0.3.0 supports defmt-version-3 and decoder 0.4.0 supports defmt-version-7 with all the versions in between (4, 5, 6) being git only.

Some cursory testing reveals that this is impossible – Cargo does not allow a package to depend on different non-breaking versions of the same dependency, only on multiple major versions.

To clarify, I meant "minor version" in the context of a pre-1.0 crate so there can be semver breaking changes between those.
Depending on decoder 0.3 & 0.4 should be possible. This works, for instance:

[dependencies]
futures-01 = { package = "futures", version = "0.1.0" }
futures-03 = { package = "futures", version = "0.3.0" }

@japaric japaric changed the title how to manage decoder versions? [RFC] how to manage decoder versions? Oct 15, 2021
@japaric
Copy link
Member Author

japaric commented Oct 15, 2021

I don't think we need to block the 0.3.0 on a decision on this RFC. At the latest, we'll need a decision when it's time to make a release that needs to support more than one decoder and that will be needed when we land a backward incompatible wire change after the 0.3.0 release.

So, I'm going to de-milestone this issue and instead put the docs PR #615 on the 0.3.0 milestone.

@japaric japaric removed this from the v0.3.0 milestone Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs decision The Knurling team needs to make a decision on this
Projects
None yet
Development

No branches or pull requests

2 participants