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

Requiring version number to match upstream Rust version is unnecessary #6683

Open
Mark-Simulacrum opened this issue Feb 5, 2021 · 6 comments
Labels
A-infra Area: CI issues and issues that require full access for GitHub/CI

Comments

@Mark-Simulacrum
Copy link
Member

#6499 proposed locking the version number of clippy-the-cargo project to the version number of Rust. In practice, this causes some work for the release team during release week, and I'm not sure why it matters - to my knowledge, clippy isn't depended on by any projects outside the monorepo at rust-lang/rust and this repository, so the version number shouldn't meaningfully affect anyone. Can we remove the tests, at least? Cargo for example does not bump the version number in lockstep with the monorepo, for example, so it doesn't seem like this is all that necessary.

If there's some purpose to the version number then we can work on documenting the process to update in https://forge.rust-lang.org/release/process.html#bump-the-stable-version-number-t-6-days-friday-the-week-before, but it would be great to avoid that - we're hoping to eventually automate the release process pretty much start to finish, and the fewer files need to be edited (particularly relatively complex ones like Cargo.toml) the easier that will be.

@matthiaskrgr
Copy link
Member

Could we just disable the test when running clippy tests from the rustc repo?
This way, there should be no failure when the rustc version is bumped in the rustc repo, but as soon as we do the next rustc->clippy sync (and bump the compiler version inside the clippy repo) the test would fail inside clippy CI and we'd be alerted to bump the version in our repo.

matthiaskrgr added a commit to matthiaskrgr/rust-clippy that referenced this issue Feb 5, 2021
… inside the rustc repo.

Do not check if clippy version matches rustc version when runnning tests inside the rustc repo.
This makes sure that upstream rustc maintainers do not have to deal with our test failing/mismatching versions
when the rustc version bump is happening.
cc rust-lang#6683
@Manishearth
Copy link
Member

Yeah I think @matthiaskrgr's proposed plan is good here.

@Mark-Simulacrum
Copy link
Member Author

That seems fine to me, anything that means we don't need to worry about it upstream basically solves my concerns. That said I'm not convinced you want this - but I'm not going to argue that, you know constraints better than I.

bors added a commit that referenced this issue Feb 6, 2021
…=flip1995

tests: ignore check_that_clippy_has_the_same_major_version_as_rustc()inside the rustc repo

Do not check if clippy version matches rustc version when runnning tests inside the rustc repo.
This makes sure that upstream rustc maintainers do not have to deal with our test failing/mismatching versions
when the rustc version bump is happening.
cc #6683

We already do the "don't run inside the rustc-repo" workaround for the dogfood test:
https://github.com/rust-lang/rust-clippy/blob/a507c27660d05f37307369d30bee9e82ce3a11e1/tests/dogfood.rs#L16

changelog: None
@camsteffen camsteffen added the A-infra Area: CI issues and issues that require full access for GitHub/CI label Feb 6, 2021
@Manishearth
Copy link
Member

Yeah I don't think we really need this, but it's easy and convenient to have the version number somewhere in tree.

Clippy reports the rustc version on the CLI, and it would be kinda weird if that weren't somewhere in tree as well.

@Mark-Simulacrum
Copy link
Member Author

Hm. I guess I don't see that as at all odd. But up to y'all.

@blyxyas
Copy link
Member

blyxyas commented May 6, 2024

Is this fixed by #6684?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-infra Area: CI issues and issues that require full access for GitHub/CI
Projects
None yet
Development

No branches or pull requests

5 participants