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.

Sign up for GitHub

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

Jump to bottom

Config support for controlling MSRV-aware resolver #13540

Closed
Tracked by #9930
Mar 5, 2024 · 6 comments · Fixed by #14296
Closed
Tracked by #9930

Config support for controlling MSRV-aware resolver #13540

Mar 5, 2024 · 6 comments · Fixed by #14296
Labels
A-configuration Area: cargo config files and env vars A-dependency-resolution Area: dependency resolution and the resolver C-enhancement Category: enhancement

Comments

Copy link
Contributor

Split out of #9930 for RFC #3537 for a more focused conversation

We need a config field but this was left undefined in the RFC
A-configuration Area: cargo config files and env vars C-enhancement Category: enhancement A-dependency-resolution Area: dependency resolution and the resolver labels Mar 5, 2024
Copy link
Contributor Author

Mar 5, 2024 •
edited
Loading

The RFC proposed

[build]
resolver.precedence = "rust-version"  # Default with `v3`

with the following values (on first release)

  • maximum
  • rust-version (falls back to rustc version if not present)

Future possibilities

Copy link
Contributor Author

For rustc value, I think we should see how to align with the package.rust-version = "tbd" field so people can connect the dots more easily.

Like with term (#13337), I wonder if we should be doing more individual fields rather than be-all fields.

Something like:

[resolver]
maximum = <true|false>
rust-version = <true|false|manifest|toolchain|<X>.<Y>.<Z>>
# yanked = <allow|ignore>
# prerelease = <allow|ignore>

This would also give control over #13290, #4225

We can have a separate [install.resolver]

Thoughts

  • I'd prefer more consistency in field names and values
  • I'd like to cover cases of "deny <incompatible|yanked|prerelease>", "discourage <...>", "allow <...>"
  • To satisfy the above two, i suspect we should break out the "max rust version" from the "rust version policy" fields

This leads to something like

[resolver]
maximum = <true|false>
incompatible-rust-version = <deny|discourage|allow>
yanked = <deny|discourage|allow>
prerelease = <deny|discourage|allow>

rust-version = <workspace|toolchain|<X>.<Y>.<Z>>
  • Not thrilled with the names
  • Not liking the location of rust-version as it should also apply to cargo add (but not cargo check?)
    • Maybe [build] could have its own rust-version field
    • Maybe we could call this table [dependencies] and have it apply to cargo add?

Open
27 tasks
Copy link
Contributor Author

I wonder if, instead of maximum = <true|false> with minimum being what you get with maximum = false, if it'd be better to have a <something> = <maximum|minimum>.

Copy link
Contributor Author

Talking this through with @Eh2406

  • Fewer fields offers a higher level experience with a combinatoric problem
  • More fields offers a lower level experience, allowing people to build what they want, but some combinations might not make sense

There isn't really much pushing us one direction or the other, so one way to look at this is which way is easier to evolve. Having fewer fields makes it so we can change it so that field sets defaults on the others. Having more fields makes that a little messier.

Current proposal

[resolver]
precedence = "<maximum|rust-version>"
# precedence = "<maximum|rust-version|minimum>"
# future: rust-version = "<workspace|toolchain|<X>.<Y>.<Z>"

Apr 10, 2024
refactor: Track when MSRV is explicitly set, either way
2e75d66 
This will hopefully help when merging between CLI and config with rust-lang#13540.
Merged
bors added a commit that referenced this issue Apr 10, 2024
@bors
Auto merge of #13732 - epage:honor, r=Muscraft
f99d24f 
refactor: Track when MSRV is explicitly set, either way

### What does this PR try to resolve?

This will hopefully help when merging between CLI and config with #13540.

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

### Additional information
Merged
bors added a commit that referenced this issue Apr 13, 2024
@bors
Auto merge of #13748 - epage:kebab, r=weihanglo
8dd6db4 
refactor(config): Consistently use kebab-case

This shouldn't change the behavior but makes it safer if
- We add new fields where it will matter
- Copy/paste these for new structs

I did not change things related to the Index because we are already stuck with that case (whether we want it or not)

Came across this when working on #13540 and almost made the mistake of copying what was already there
Apr 17, 2024
fix(msrv): Put MSRV-aware resolver behind a config
4d09271 
This is a part of rust-lang#13540 which is a party of rust-lang#9930.

The config is `resolver.something-like-precedence` with values:
- `something-like-maximum` (default)
- `something-like-rust-version`

This is punting on the actual config schema so we can implement
`package.resolver` and `edition = "2024"` support as we want the
MSRV-aware resolver available without `cargo_features`.
Merged
Apr 18, 2024
fix(msrv): Put MSRV-aware resolver behind a config
259574b 
This is a part of rust-lang#13540 which is a party of rust-lang#9930.

The config is `resolver.something-like-precedence` with values:
- `something-like-maximum` (default)
- `something-like-rust-version`

This is punting on the actual config schema so we can implement
`package.resolver` and `edition = "2024"` support as we want the
MSRV-aware resolver available without `cargo_features`.
Apr 18, 2024
fix(msrv): Put MSRV-aware resolver behind a config
18a5940 
This is a part of rust-lang#13540 which is a party of rust-lang#9930.

The config is `resolver.something-like-precedence` with values:
- `something-like-maximum` (default)
- `something-like-rust-version`

This is punting on the actual config schema so we can implement
`package.resolver` and `edition = "2024"` support as we want the
MSRV-aware resolver available without `cargo_features`.
bors added a commit that referenced this issue Apr 18, 2024
@bors
Auto merge of #13769 - epage:msrv-config, r=weihanglo
2956296 
fix(msrv): Put MSRV-aware resolver behind a config

### What does this PR try to resolve?
This is a part of #13540 which is a party of #9930.

The config is `resolver.something-like-precedence` with values:
- `something-like-maximum` (default)
- `something-like-rust-version`

This is punting on the actual config schema so we can implement
`package.resolver` and `edition = "2024"` support as we want the
MSRV-aware resolver available without `cargo_features`.

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

One of the included test cases shows a bug with `cargo install`.  Resolving that will be tracked in #9930

### Additional information
Closed
5 tasks
This was referenced May 6, 2024
Merged
Closed
Copy link
Contributor Author

Jun 5, 2024 •
edited
Loading

We talked about this in the Cargo team meeting today.

We lean away from resolver.precedence / resolver.policy because

  • It will quickly run into scaling issues
  • It reads a little funny to have maximum and rust-version as mutually exclusive

When it came to resolver.rust-version, the biggest concerns were

  • Naming that aligns with potential future expansion
  • How to name the new resolver behavior of using incompatible-rust-version as a "last resort"

We leaned towards resolver.incompatible-rust-version

  • Right polarity for potential other fields
  • Keeps it distinct from setting a "resolve to rust version" field
  • Expands to support rejecting of incompatible rust version package versions in the future

For the resolver.incompatible-rust-version field, we have three potential states

  • "treat like everything else" (current behavior)
  • "use if no other option" (new behavior)
  • "use only if already in lockfile" (not planned atm)

Potential names for "use if no other option" include:

  • discourage
  • eschew
  • avoid
  • fallback

Fallback seems like just the right weight. The challenge is that its a
noun while other words we are considering are verbs (allow, deny).

Copy link
Contributor Author

Jul 10, 2024 •
edited
Loading

Goal: provide field values for

[resolver]
incompatible-rust-version = "<field>"  # can be one of two meanings for now, expandable to three

Stepping back to more fundamentals and playing a game of mad libs...

We need to pick one specific version of a software package from among many

  • candidates
  • potential deps
  • options
  • prospects
  • nominees

Some of these "candidate" versions of the package will be

  • filtered out
  • ignored
  • rejected
  • denied
  • disallowed

After that, we then choose the highest priority package from among "candidates". We aren't wanting to have users manually specify the priority order but just to say whether we

  • ignore
  • are agnostic of
  • allow
  • treat as "normal", "standard"

certain traits or if certain traits are

  • discouraged
  • eschewed
  • avoided

when no other option is available. Or in other words, these are "fallback" versions

We should also put these in a sentence with the trait

  • packages with "incompatible rust versions" are ...
  • ... packages with "incompatible rust versions"

Examples

  1. allow, deny, fallback
  • "allow packages with incompatible-rust-versions"
  • "deny packages with incompatible-rust-versions"
  • "fallback to packages with incompatible-rust-versions"

Jul 24, 2024
fix(config): Adjust MSRV resolve config field name / values
b965c56 
Fixes rust-lang#13540
Merged
Jul 24, 2024
fix(config): Adjust MSRV resolve config field name / values
655ae80 
Fixes rust-lang#13540
Jul 24, 2024
fix(config): Adjust MSRV resolve config field name / values
feecbd7 
Fixes rust-lang#13540
Jul 30, 2024
fix(config): Adjust MSRV resolve config field name / values
1bf2423 
Fixes rust-lang#13540
Jul 31, 2024
fix(config): Adjust MSRV resolve config field name / values
5befacf 
Fixes rust-lang#13540
bors added a commit that referenced this issue Jul 31, 2024
@bors
Auto merge of #14296 - epage:msrv, r=weihanglo
11dccd1 
fix(config): Adjust MSRV resolve config field name / values

### What does this PR try to resolve?

Fixes #13540

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

### Additional information
@bors bors closed this as completed in 8622918 Jul 31, 2024
stupendoussuperpowers pushed a commit to stupendoussuperpowers/cargo that referenced this issue Aug 7, 2024
@stupendoussuperpowers
fix(config): Adjust MSRV resolve config field name / values
e49807e 
Fixes rust-lang#13540
@Pat-Lafon Pat-Lafon mentioned this issue Dec 2, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-configuration Area: cargo config files and env vars A-dependency-resolution Area: dependency resolution and the resolver C-enhancement Category: enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(config): Adjust MSRV resolve config field name / values epage/cargo
1 participant