-
Notifications
You must be signed in to change notification settings - Fork 755
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
Add a minimal-versions check #2015
Conversation
So a thing which this check doesn't accomplish is that each crate is not tested independently. In order to accomplish that they have to be removed from the workspace. Easiest way to accomplish this (that I am aware of) is to remove the Once that is done, we can see the following build failure in
|
Hmm, it seems like --- hypothetically --- we could just have the CI job run |
@udoprog just to clarify, is this ready to merge now? |
If you're OK with the following limitations:
The branch also appears to be outdated now, I'll bump it when I get a minute. |
@udoprog do you know if it's possible to use |
I don't believe it is possible with cargo-hack (also note I do use it for removing dev-dependencies above)! I started poking around with a cargo extension that would create a crate using the built-in packaging mechanism and unpack it into a clean directory where it can run the specified command over it. But I've yet to make much progress with it. As mentioned in the related Tokio issue hunting down and fixing min-version violations is fairly time consuming. If there was solid tooling to do it* it would be easier to motivate spending the effort on a personal level. *: A tool would be something where you can just run a script to do all the right tests, and another script which will do other work necessary to just tell you the lowest versions of packages that have been automatically bisected to work within some MSRV constraint. |
I agree with it is impossible for the current cargo-hack by itself to handle this correctly. cargo-minimal-versions (PR 4 to be exact) should do most of the right things about this, but as far as running builds within the current workspace, it seems to be impossible to handle the issue related to optional dependencies. (There is also the problem that the current implementation of PR 4 does not work well when tools like rust-analyzer and cargo-watch are running in the background.) (For library) The way to really seem to be right about this is to remove the path dependencies within the current workspace, then specify the library you want to build as a dependency of a temporary crate created outside the current workspace, and build that temporary crate. (Note that the temporary crate needs to be per feature, not per crate. Also, to catch linkage problems, subcommand needs to be I plan to implement that approach in the future in cargo-minimal-versions, but I don't know ETA. |
I'm glad to see you're putting some mileage into the problem @taiki-e! The second issue I briefly skimmed over here and I find the most time consuming is the lack of a tool that helps you discover the versions of specific dependencies which are actually build-compatible with a specific msrv. In my mind that would require something like bisecting each dependency from the current specification up until the latest available version and find the latest (or earliest) version of it that still allows the project to build. The way I currently do it is to:
|
Given that this PR does fix some existing minimal versions issues, I'd like to go ahead and merge this now, even if it isn't exhaustive. Would it make sense to merge this branch as-is, and continue working on minimal-version checking as the tooling improves? |
Go for it! |
This adds a minimal-versions check to the tracing project. Adapted from `tokio-rs/tokio`. Adding this avoids breaking downstream dependencies from accidentally under-constraining minimal versions of dependencies when they depend on tracing. I've currently just introduced the check. I will try to and do encourage others to add patches to fix this where possible since it can be a fair bit of work to chase down a version of all dependencies that passes minimal-versions and is msrv. I've also seen some really odd windows-specific issues (which are not being tested for here). This is currently only testing `tracing`, `tracing-core`, and `tracing-subscriber`. Packages such as `tracing-futures` are proving to be a bit harder to deal with due to having features which enable very old dependencies. Steps to test the build minimal versions locally: ```sh cargo install cargo-hack rustup default nightly cargo hack --remove-dev-deps --workspace cargo update -Z minimal-versions cargo hack check --all-features --ignore-private ``` CC: tokio-rs/tokio#4513
This adds a minimal-versions check to the tracing project. Adapted from `tokio-rs/tokio`. Adding this avoids breaking downstream dependencies from accidentally under-constraining minimal versions of dependencies when they depend on tracing. I've currently just introduced the check. I will try to and do encourage others to add patches to fix this where possible since it can be a fair bit of work to chase down a version of all dependencies that passes minimal-versions and is msrv. I've also seen some really odd windows-specific issues (which are not being tested for here). This is currently only testing `tracing`, `tracing-core`, and `tracing-subscriber`. Packages such as `tracing-futures` are proving to be a bit harder to deal with due to having features which enable very old dependencies. Steps to test the build minimal versions locally: ```sh cargo install cargo-hack rustup default nightly cargo hack --remove-dev-deps --workspace cargo update -Z minimal-versions cargo hack check --all-features --ignore-private ``` CC: tokio-rs/tokio#4513
This adds a minimal-versions check to the tracing project. Adapted from
tokio-rs/tokio
. Adding this avoids breaking downstream dependencies from accidentally under-constraining minimal versions of dependencies when they depend on tracing.I've currently just introduced the check. I will try to and do encourage others to add patches to fix this where possible since it can be a fair bit of work to chase down a version of all dependencies that passes minimal-versions and is msrv. I've also seen some really odd windows-specific issues (which are not being tested for here).
This is currently only testing
tracing
,tracing-core
, andtracing-subscriber
. Packages such astracing-futures
are proving to be a bit harder to deal with due to having features which enable very old dependencies.Steps to test the build minimal versions locally:
CC: tokio-rs/tokio#4513