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

chore: prepare tokio-macros v2.0.0 #5580

Merged
merged 4 commits into from Mar 27, 2023
Merged

Conversation

Darksonn
Copy link
Contributor

2.0.0 (March 24th, 2023)

This major release updates the dependency on the syn crate to 2.0.0, and increases the MSRV to 1.56.

As part of this release, we are adopting a policy of incrementing the tokio-macros major version more often. This reduces the chance of issues where people are confused about the same version of Tokio not behaving in the same way, if you happen to be using two different versions of tokio-macros.

  • macros: update syn (#5572)
  • macros: accept path as crate rename (#5557)

@Darksonn Darksonn added the A-tokio-macros Area: The tokio-macros crate label Mar 24, 2023
@taiki-e
Copy link
Member

taiki-e commented Mar 24, 2023

As part of this release, we are adopting a policy of incrementing the tokio-macros major version more often. This reduces the chance of issues where people are confused about the same version of Tokio not behaving in the same way, if you happen to be using two different versions of tokio-macros.

I don't think increasing the major version is the right solution here. tokio-macros is also dependent on tokio's private APIs, so it is not always easy to determine in exactly which cases you need to increase the major version or not.

Instead, I would recommend using the same version number as tokio and using the = version requirement in tokio's Cargo.toml. This will force the use of a fully compatible version. This is the approach used by serde, async-stream, futures, pin-project, etc.

@carllerche
Copy link
Member

carllerche commented Mar 24, 2023

@taiki-e I think using = is a reasonable solution moving forward, but we haven't been doing that to date, so we need some way to prevent earlier versions of tokio from pulling in the latest tokio-macros. A major bump will do that.

Do you agree @taiki-e?

@taiki-e
Copy link
Member

taiki-e commented Mar 24, 2023

If we were intended to support the use of the latest tokio-macros and older tokio together, I think it may need a major release.

However, if we did not intend to support it, the current behavior of being able to depend on a newer version that may not work by mistake is a bug. (Older versions could break at any time anyway, and may also already be broken.)
If so, we may need to backport similar changes to the LTS version, but I do not think a major release is needed.

That said, I'm not opposed to making a major release here. It indeed helps for previous versions that are not currently broken.
(Although I think it is not needed based on the previous cases such as serde, futures, async-stream, etc.)

@carllerche
Copy link
Member

If we were intended to support the use of the latest tokio-macros and older tokio together, I think it may need a major release.

This isn't a requirement, but what we need to avoid is breaking LTS releases. Unfortunately, all the current LTS releases do not use = with their tokio-macros dependency.

So, I think the proposal is:

  • Release tokio-macros 2.0 to avoid breaking LTS releases
  • Future releases of tokio will depend on tokio-macros with an = dependency.

We should be good moving forward. Do you agree?

@carllerche
Copy link
Member

I don't think we actually want = though, we want to pull in newest patch releases but not minor releases. Correct?

Copy link
Member

@carllerche carllerche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a comment based on the discussino.

tokio/Cargo.toml Outdated
@@ -97,7 +97,7 @@ stats = []
autocfg = "1.1"

[dependencies]
tokio-macros = { version = "1.7.0", path = "../tokio-macros", optional = true }
tokio-macros = { version = "2.0.0", path = "../tokio-macros", optional = true }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should change this to pin the dependence to 2.0.x per the discussion.

@taiki-e
Copy link
Member

taiki-e commented Mar 24, 2023

  • Release tokio-macros 2.0 to avoid breaking LTS releases
  • Future releases of tokio will depend on tokio-macros with an = dependency.

That's okay too. I was thinking of the following (a way that doesn't bump to 2.0):

  • New tokio LTS releases with = requirement to tokio-macros current latest or the version included in its branch.
  • New tokio release with = requirement to new version of tokio-macros.

I don't think we actually want = though, we want to pull in newest patch releases but not minor releases. Correct?

Yeah, ~x.y requirement is okay too. However, it may need more work to determine which changes require a minor release, because we need to consider not only API changes, but also whether to use tokio's new private API, etc.

@carllerche
Copy link
Member

@taiki-e, our policy is patch releases for "important" bug fixes only. Minor releases for everything else.

@Darksonn
Copy link
Contributor Author

How about this solution?

  1. We bump to 2.0.0 now.
  2. Tokio uses ~ dependencies on tokio-macros.
  3. We don't try to sync the tokio and tokio-macros minor version number. This avoids having to release a tokio-macros every time we release a new tokio version.
  4. Patch releases are used sparingly. We would mainly use them to fix a bug discovered right after a release. For normal changes, prefer a new minor release.

@taiki-e
Copy link
Member

taiki-e commented Mar 25, 2023

@Darksonn Sounds good to me!


@carllerche

our policy is patch releases for "important" bug fixes only. Minor releases for everything else.

I was mainly thinking about the case of tokio's private API being changed to fix a bug in tokio-macros (and the opposite).
For example, it is a change like #4211 (although that PR itself was released in a minor version).

That said, I think my concern is addressed by the solution proposed by Darksonn:

4. Patch releases are used sparingly. We would mainly use them to fix a bug discovered right after a release. For normal changes, prefer a new minor release.

Copy link
Member

@carllerche carllerche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks

@Darksonn Darksonn merged commit 614fe35 into master Mar 27, 2023
52 checks passed
@Darksonn Darksonn deleted the alice/prepare-tokio-macros-2.0.0 branch March 27, 2023 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-macros Area: The tokio-macros crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants