Add "resolver = 2" option to workspace manifest examples #10625
Conversation
It seems like `resolver = 2` is just a basic requirement to build any package with dependencies. To make the examples work it's needed. Apparently there is work to make it default (rust-lang/rust#90148) but right now it's required.
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
epage
changed the title
|
FYI I updated the PR description to more closely match what this does. When I read it, I assumed it was going to apply to all manifest examples, even if the edition is set. |
ehuss
added
S-waiting-on-author
In looking over rust-lang#10625, I remembered we've been having growing pains with the workspace documentation. It was originally written when there were a lot fewer workspace features. As more workspace features have been added, they've been tacked on to the documentation. This re-thinks the documentation by focusing on the schema, much like `manifest.md` does.
In looking over rust-lang#10625, I remembered we've been having growing pains with the workspace documentation. It was originally written when there were a lot fewer workspace features. As more workspace features have been added, they've been tacked on to the documentation. This re-thinks the documentation by focusing on the schema, much like `manifest.md` does.
In looking over rust-lang#10625, I remembered we've been having growing pains with the workspace documentation. It was originally written when there were a lot fewer workspace features. As more workspace features have been added, they've been tacked on to the documentation. This re-thinks the documentation by focusing on the schema, much like `manifest.md` does.
In looking over rust-lang#10625, I remembered we've been having growing pains with the workspace documentation. It was originally written when there were a lot fewer workspace features. As more workspace features have been added, they've been tacked on to the documentation. This re-thinks the documentation by focusing on the schema, much like `manifest.md` does.
In looking over rust-lang#10625, I remembered we've been having growing pains with the workspace documentation. It was originally written when there were a lot fewer workspace features. As more workspace features have been added, they've been tacked on to the documentation. This re-thinks the documentation by focusing on the schema, much like `manifest.md` does.
In looking over rust-lang#10625, I remembered we've been having growing pains with the workspace documentation. It was originally written when there were a lot fewer workspace features. As more workspace features have been added, they've been tacked on to the documentation. This re-thinks the documentation by focusing on the schema, much like `manifest.md` does.
In looking over rust-lang#10625, I remembered we've been having growing pains with the workspace documentation. It was originally written when there were a lot fewer workspace features. As more workspace features have been added, they've been tacked on to the documentation. This re-thinks the documentation by focusing on the schema, much like `manifest.md` does.
In looking over rust-lang#10625, I remembered we've been having growing pains with the workspace documentation. It was originally written when there were a lot fewer workspace features. As more workspace features have been added, they've been tacked on to the documentation. This re-thinks the documentation by focusing on the schema, much like `manifest.md` does.
docs(ref): Clarify workspace settings ### What does this PR try to resolve? In reviewing the status of #10625, I was reminded - that we are having growing pains with the workspace documentation - that `workspace.resolver` isn't documented So I re-organized the workspace docs to have a high level intro / behavior description and then to focus on being a field reference, much like `manifest.md`. I could see splitting it specifically into tutorial/reference like the overriding dependencies document does it. When adding `workspace.resolver`, I remembered in the nested workspace discussion there were other workspace related sections that are not present. We now link out to `profile`, `patch`, and `replace`. In doing this, I realized that `patch` and `replace` do not specify their workspace behavior, so I do that. ### How should we test and review this PR? Look at it commit by commit to get more digestible chunks. Unfortunately, the first commit didn't split up so easily. ### Additional information Other information you want to mention in this PR, such as prior arts, future extensions, an unresolved problem, or a TODO list. --> <!-- homu-ignore:end -->
|
☔ The latest upstream changes (presumably #11082) made this pull request unmergeable. Please resolve the merge conflicts. |
In looking over rust-lang#10625, I remembered we've been having growing pains with the workspace documentation. It was originally written when there were a lot fewer workspace features. As more workspace features have been added, they've been tacked on to the documentation. This re-thinks the documentation by focusing on the schema, much like `manifest.md` does.
|
Ping @mousetail. Just checking in to see if you are still interested in working on this, or if you had any questions. By the way, workspace.md has got a content re-org recently. You may need to resolve some conflicts before proceeding. |
In looking over rust-lang#10625, I remembered we've been having growing pains with the workspace documentation. It was originally written when there were a lot fewer workspace features. As more workspace features have been added, they've been tacked on to the documentation. This re-thinks the documentation by focusing on the schema, much like `manifest.md` does.
In looking over rust-lang#10625, I remembered we've been having growing pains with the workspace documentation. It was originally written when there were a lot fewer workspace features. As more workspace features have been added, they've been tacked on to the documentation. This re-thinks the documentation by focusing on the schema, much like `manifest.md` does.
In looking over rust-lang#10625, I remembered we've been having growing pains with the workspace documentation. It was originally written when there were a lot fewer workspace features. As more workspace features have been added, they've been tacked on to the documentation. This re-thinks the documentation by focusing on the schema, much like `manifest.md` does.
src/doc/src/reference/workspaces.md
Outdated
| The [`resolver`] key effects how a package that is depended on in multiple | ||
| ways with different features are resolved. Some libraries depend on it being | ||
| set to 2. | ||
|
|
||
| An empty `[workspace]` table can be used with a `[package]` to conveniently | ||
| create a workspace with the package and all of its path dependencies. |
There was a problem hiding this comment.
This doesn't quite fit within the current section
Maybe we should move this into the "virtual workspace" section and only update that example?
There was a problem hiding this comment.
I removed this paragraph, seems like this concept is explained well enough in the virtual workspace section
There was a problem hiding this comment.
The focus of my concern wasn't so much the duplication of information about virtual workspaces but that we are in the section about members and exclude keys and introducing the resolver key as if it belongs in that section. We then include the resolver key in each further section. I get the point of including the resolver key in each subsequent section, to try to remind people to use it, but it doesn't quite fit seeing an unexplained key and one that is only relevant when its a virtual workspace. This is why I was recommending to make this entire PR only touch the virtual workspace section.
This PR does not have to do all of the lifting for helping users. We also have #10910.
| ### Workspace selection | ||
|
|
| @@ -128,6 +136,7 @@ used: | |||
| [workspace] | |||
| members = ["path/to/member1", "path/to/member2", "path/to/member3/*"] | |||
| default-members = ["path/to/member2", "path/to/member3/foo"] | |||
| resolver = "2" | |||
| @@ -225,6 +234,7 @@ configuration in `Cargo.toml`. For example: | |||
| ```toml | |||
| [workspace] | |||
| members = ["member1", "member2"] | |||
| resolver = "2" | |||
| @@ -78,6 +83,7 @@ version = "0.1.0" # the current version, obeying semver | |||
| authors = ["Alice <[email protected]>", "Bob <[email protected]>"] | |||
| ``` | |||
|
|
|||
|
|
|||
| The [`resolver`] key here effects how a package that is depended on in multiple | ||
| ways with different features are resolved. Some libraries depend on it being | ||
| set to 2. |
There was a problem hiding this comment.
I'd recommend shifting the focus to fit within this section. We delegate explanation of the field to the resolver page.
Something like
Without a root package to fall back on, the [`resolver`] field should be set in the `[workspace]` table.(at first I was worried this was too strong of a recommendation but it is roughly what the resolver page says)
|
Ping @mousetail. Just checking in to see if you are still interested in working on this doc improvement. Feel free to ask any question here or on Zuilp. |
|
I'm going to close due to inactivity. I think it might be good to more clearly recommend setting the resolver field in a workspace, but I'm not sure this PR quite hit the right balance of focus. I'd also mention #10112 might be a stronger indicator than assuming people read the documentation. Thanks! |
It seems like
resolver = 2is just a basic requirement to build any package with dependencies. To make the examples work it's needed. Apparently there is work to make it default (rust-lang/rust#90148) but right now it's required.What does this PR try to resolve?
The examples in the documentation about workspaces don't work in the latest edition of rust. This would have saved me many hours of debugging.
Ideally there would also be a description of what the option actually does but I don't really understand that myself.
How should we test and review this PR?
It's a documentation change. Check if you think the information is useful.
Additional information
Relevant links: