madsim-rs / madsim

Magical Deterministic Simulator for distributed systems in Rust.
Apache License 2.0
662 stars 48 forks source link

include original crate's version #145

Closed xxchan closed 11 months ago

xxchan commented 1 year ago

e.g., 0.2.17+aws-sdk-s3.0.21 or 0.2.17+0.21

Other examples:

This is called "Build metadata" in semver: https://semver.org/spec/v2.0.0.html#spec-item-10

wangrunji0408 commented 1 year ago

Good idea. Will change to this style from next version.

xxchan commented 1 year ago

I'm now wondering whether we should do a major version bump when we update the (major) version of the original crate, e.g., #147. πŸ˜‡

xxchan commented 1 year ago

.. Otherwise people may unexpectedly have breaking changes, unless they use precise version of madsim πŸ˜‡

wangrunji0408 commented 1 year ago

According to semver:

Build metadata MUST be ignored when determining version precedence.

Therefore we should bump a major version if the original crate bumps.

Since madsim-tonic depends on both madsim and tonic, we should assign an independent version to it. Maybe 0.3.0+madsim.0.2+tonic.0.9 πŸ˜‡

xxchan commented 1 year ago

Oops, didn't see edited message in email πŸ€ͺ

One problem is the madsim version is also included. Just like arrow-* all have the same version… πŸ€”οΈ

On Thu, 20 Jul 2023 at 07:03, Runji Wang @.***> wrote:

According to semver https://semver.org/spec/v2.0.0.html#spec-item-10:

Build metadata MUST be ignored when determining version precedence.

Therefore we should bump a major version if the original crate bumps.

β€” Reply to this email directly, view it on GitHub https://github.com/madsim-rs/madsim/issues/145#issuecomment-1643194364, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJBQZNPTNIRDKFBVQC3ZXSLXRC3YNANCNFSM6AAAAAAYQT2XEI . You are receiving this because you authored the thread.Message ID: @.***>

xxchan commented 1 year ago

The benefit of letting all sub-crates with the same version like arrow-*:

It seems such policy isn't too common in the ecosystem. e.g., tokio-* has different versions.

I think arrow is also special because the crates are highly related and are basic building blocks (maybe except things like flight...), so it's natural to update them all at once.

For tokio/madsim, it makes less sense to update all things together. e.g., update tokio-utils without updating tokio, and update madsim-tonic without updating madsim and madsim-s3.

.. But it might be OK to require users to update all tokio-* after tokio bumps major version.

.... But for madsim it might be more problematic, since each madsim-* is a relatively critical dependency. πŸ€”οΈ Ideally, each madsim-* can work with multiple major versions of madsim, like icelake can work with multiple major versions of arrow. But of course that brings maintenance burden.

xxchan commented 1 year ago

Regarding whether to include +madsim0.2 in the build metadata, I guess it's optional, since users can know the version from the dependency tree.

This also makes me re-consider whether madsim-tonic 0.3.0+0.9.2 is really needed for the same reason... For zstd 0.12.3+zstd.1.5.2, it really doesn't have this information as zstd is outside of Cargo's control. But it might be nice to have? I'm not sure now and don't know any project for reference. πŸ€ͺ

xxchan commented 1 year ago

It seems such policy isn't too common in the ecosystem. e.g., tokio-* has different versions.

Well, but I also read this πŸ€ͺ

All tonic-* crates owned by this repository will now be versioned together to make it easier to understand which crate matches the core tonic crate version.

wangrunji0408 commented 1 year ago

I prefer not to maintain the same version because each madsim-* is a mock of external crate. We don't have control of these crates. If we keep the same version, every version bump from external crate (such as tonic 0.8 -> 0.9) will force a bump of madsim ecosystem. This is weird. πŸ€ͺ

wangrunji0408 commented 1 year ago

When using madsim-*, users have to be aware of both madsim version and the original crate version. Because:

  1. All madsim dependencies must be compatible, otherwise there will be multiple async runtime. (similar to the situation of tokio 0.1, 0.3, 1)
  2. In normal build, users are actually using the original crate, e.g. tonic, even if they see madsim-tonic.

So it is ideal to have both versions in the metadata:

madsim-tonic 0.3.0+madsim.0.2+tonic.0.9

But it looks a little verbose. Maybe omitting the madsim version is also fine.

madsim-tonic 0.3.0+0.9
xxchan commented 1 year ago

Yes I think generally we are in the same page. The last thing is that maybe +0.9 can also be ignored, as it's known in the dependency tree. πŸ˜‡