cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Infer resolver version from workspace version

Open ehuss opened this issue 2 years ago • 13 comments

It would be very helpful to set the resolver version in a virtual workspace based on the edition field being added in RFC 2906. This has been requested several times and has caused some confusion for some users.

That is, if you have:

[workspace]
edition = "2021"
members = ["foo"]

Then this will default workspace.resolver = "2"

The logic for determining the version is here. It should also check the workspace edition. This should also be gated on workspace-inheritance on nightly.

ehuss avatar Apr 21 '22 00:04 ehuss

@rustbot claim

hi-rustin avatar Apr 21 '22 01:04 hi-rustin

@hi-rustin If you don't mind I would like to claim this. I want to add it to the RFC as something to do before we try to stabilize.

Muscraft avatar Apr 21 '22 03:04 Muscraft

I want to add it to the RFC as something to do before we try to stabilize.

I don't quite understand what you mean?

hi-rustin avatar Apr 21 '22 05:04 hi-rustin

I'm currently working on the RFC for workspace inheritance and would like to add this issue to the tracking issue as something to do before trying to stabilize it.

Muscraft avatar Apr 21 '22 05:04 Muscraft

You mean we don't need to implement it now? (Or is there something blocking it?)

The logic for determining the version is here. It should also check the workspace edition. This should also be gated on workspace-inheritance on nightly.

This confuses me 😖 I thought we were going to make it happen now.

Unassign myself.

hi-rustin avatar Apr 21 '22 05:04 hi-rustin

If it is now acceptable and achievable. Then I'm happy to implement it :(

hi-rustin avatar Apr 21 '22 05:04 hi-rustin

That's my bad for being confusing. I was just trying to say that I was just going to add it to my todo list before trying for stabilization of workspace inheritance.

If it's something you want to work on you are more than welcome to. There shouldn't be any blockers on it.

Muscraft avatar Apr 21 '22 06:04 Muscraft

@ehuss my comment in #5784 used the workspace.edition = "2018" syntax because that is what is in the inheritance RFC, the current inheritance implementation has the syntax as workspace.package.edition = "2018". Should this issue reflect whatever syntax is stablized for inheritance or should it use workspace.edition, regardless of what inheritance does?

Background: there is unresolved thread from the RFC that I think needs revisiting and so #10564 changed the inheritable syntax to workspace.package.edition = "2018" as a proposed change for stablization as recorded in #8415.

epage avatar Apr 21 '22 13:04 epage

Another thing to consider is having workspace.edition = true caused #10586 to some degree. By having things to inherit under workspace.package.{key} or [workspace.package], it makes it harder to mix up [workspace] fields and [package] fields.

Muscraft avatar Apr 21 '22 14:04 Muscraft

Ah, sorry for the confusion. I probably should not have marked this with an E tag as this is indeed part of the RFC 2906 work. @hi-rustin, it looks like you have several open issues assigned to you. I recommend trying to finish some of those off before claiming additional issues. I appreciate the interest in helping, but claiming too many issues at the same time can make coordination more difficult.

@epage Sorry, I did not see that had changed. That indeed puts a wrinkle in this. I wouldn't want those inheritance fields to have no intrinsic meaning, except for edition. But I wouldn't want to add workspace.edition by itself with no meaning other than changing the resolver (or adding implicit inheritance).

Unless anyone has particular ideas on how to deal with that, I'm leaning towards just closing this.

ehuss avatar Apr 21 '22 17:04 ehuss

Unless anyone has particular ideas on how to deal with that, I'm leaning towards just closing this.

Let's at least leave this open until 2906 is stablized in case things change during the stablization.

epage avatar Apr 21 '22 17:04 epage

@hi-rustin, it looks like you have several open issues assigned to you. I recommend trying to finish some of those off before claiming additional issues. I appreciate the interest in helping, but claiming too many issues at the same time can make coordination more difficult.

I've claimed 4 issues, two of which have PRs waiting for review. another one is just revising the documentation, and the remaining one is also related to workspace, so I was wondering if I could look at this issue together yesterday. sorry to bother you all. I'll avoid this kind of problem next time.

hi-rustin avatar Apr 22 '22 02:04 hi-rustin

With workspace inheritance being stabilized, and #10112 being brought up again (#10910 would close it). I was wondering if this was something to consider again before it would be a breaking change.

Currently setting edition in a member while the workspace does not specify resolver = "2", results in using the v1 resolver being used. There is no warning for this (currently) and seems like a misstep. Having edition set in [workspace.package] imply the resolver version (if it's not set) would mitigate this issue for users of workspace inheritance. If this is something we should move forward with It would be necessary to document it.

Muscraft avatar Jul 28 '22 17:07 Muscraft

Triage: I still lean towards implementing #10112 over this for now. I think making workspace.package.edition have extra meaning could be a problem, and adding workspace.edition could be confusing. It would also be a backwards-incompatible change to change workspace.package.edition now that it is stable (at least for 2021, we could make future editions have different meaning).

It is unfortunate to need the extra boilerplate of the resolver field, but I don't really know of an alternate solution that would be better. There is likely going to be more edition-specific behavior changes in the future, so we'll likely need to figure out some solution, I just don't know what that is. Either of the above are possibilities (workspace.edition or workspace.package.edition), but I'm not particularly happy with either of them.

ehuss avatar May 09 '23 19:05 ehuss