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

version-bump check doesn't know a subcrate hasn't bumped on nightly if we've bumped it on beta #12347

Closed
weihanglo opened this issue Jul 11, 2023 · 1 comment · Fixed by #12395
Labels
A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@weihanglo
Copy link
Member

weihanglo commented Jul 11, 2023

Problem

We've added cargo unpublished xtask and ci/validate-version-bump.sh to help the project to detect if a pull request has touched any subcrate and needs a version bump for it. This works well when there is no bump on beta branch. Bad thing happens when we already have a bump on the current beta but not yet published, cargo published would assume we've already bumped that crate.

In reality, we should consider crate version in beta branch as “published” and check the version bump against that instead of published one on crates.io. Every crate gets into beta branch of the 6 weeks release window should be considered as “published”.

Steps

In #12310, the check thought crates-io crate has been bumped. Actually it is required to update to 0.37.0, since 0.36.1 will be published with 1.72.0, but #12310 is targeted at 1.73.0. or 1.74.0 if merged.

Possible Solution(s)

It is quite tricky to solve this. Subcrates are not necessary to follow the train release model with Rust and Cargo. However, there are some requirements we need to pay attention to

  • Subcrates are somehow loosely bound to Cargo. Sometimes a specific change is required by a certain version of Cargo, so it needs to be released when cargo gets released.
  • We may not want to have a separate release cadence for each subcrate. It would be a maintenance burden.
  • We may not want to release a subcrate when the cargo version it is bound to has yet hit stable, since everything could change before that.

I don't have a good answer for the release model of subcrates, but here is a proposal that may make it more explicit:

  • A subcrate version can contain build metadata to show with which cargo version it is supposed to be released. That is, for crates-io 0.36.1 it would be 0.36.1+1.71.0, so that is pretty clear it is targeted to be released with Rust 1.71.0.
  • If we need to backport to a subcrate on beta, for example crates-io@0.36.1 to 0.37.0, then we just bump it to 0.37.0+1.71.0. We then need a xtask automation to detect that on beta it has got a version bump so on nightly we need to adjust accordingly.

Version

This is about project infra but let me record the point of time it happened.

cargo 1.70.0 (ec8a8a0ca 2023-04-25)
release: 1.70.0
commit-hash: ec8a8a0cabb0e0cadef58902470f6c7ee7868bdc
commit-date: 2023-04-25
@weihanglo weihanglo added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. labels Jul 11, 2023
@weihanglo
Copy link
Member Author

I believe cargo-semver-checks could help this part.

Does the crate I'm checking have to be published on crates.io?

No, it does not have to be published anywhere. You'll just need to use a flag to help cargo-semver-checks locate the version to use as a baseline for semver-checking.

cc @obi1kenobi

@weihanglo weihanglo added S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. and removed S-triage Status: This issue is waiting on initial triage. labels Jul 18, 2023
bors added a commit that referenced this issue Jul 26, 2023
ci: rewrite bump check and respect semver

### What does this PR try to resolve?

This ports `ci/validate-version-bump.sh` and #12200 to the new xtask `bump-check`. It also adds the ability to set the correct baseline revision when checking version bump. That is, it fixes #12347.

In addition, this integrates the community tool `cargo-semver-checks`. SemVer violation can now be detected earlier.

### How should we test and review this PR?

This PR extracts the registry query part from `xtask-unpublished` and removes other pars. I don't feel like we need it in the short term.

For how it works, please the check doc comment in each function.

One concern is that this check is still a required job for bors. I believe `@obi1kenobi` is quite responsive and willing to help if there is something wrong. So, waiting for an upstream fix won't be a problem for cargo. Also as a good citizen in the community, we can always contribute back.
(Take it easy `@obi1kenobi,` don't be stressed out 🙂)
<!-- homu-ignore:end -->
bors added a commit that referenced this issue Jul 26, 2023
ci: rewrite bump check and respect semver

### What does this PR try to resolve?

This ports `ci/validate-version-bump.sh` and #12200 to the new xtask `bump-check`. It also adds the ability to set the correct baseline revision when checking version bump. That is, it fixes #12347.

In addition, this integrates the community tool `cargo-semver-checks`. SemVer violation can now be detected earlier.

### How should we test and review this PR?

This PR extracts the registry query part from `xtask-unpublished` and removes other pars. I don't feel like we need it in the short term.

For how it works, please the check doc comment in each function.

One concern is that this check is still a required job for bors. I believe `@obi1kenobi` is quite responsive and willing to help if there is something wrong. So, waiting for an upstream fix won't be a problem for cargo. Also as a good citizen in the community, we can always contribute back.
(Take it easy `@obi1kenobi,` don't be stressed out 🙂)
<!-- homu-ignore:end -->
@bors bors closed this as completed in 6145d0c Aug 1, 2023
bors added a commit that referenced this issue Aug 15, 2023
[beta-1.72.0] bump cargo-util to 0.2.5

#11442 changed `cargo-util` at the last minute but we failed to bump the version.

And due to #12347, version bump check didn't work well at that time.

In order to make CI pass, the following PRs are also cherry-picked:

* #12450
* #12491
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant