Part 2 of RFC2906 -- allow inheriting from a different `Cargo.toml` #10517

Muscraft · 2022-03-29T22:06:25Z

Tracking issue: #8415
RFC: rust-lang/rfcs#2906

Part 1

This PR focuses on inheriting from a root workspace:

Allow inheriting from a different Cargo.toml
Add in searching for a workspace root in to_real_manifest as needed
Fixed problem where a package would try to pull a dependency from a workspace and specify { workspace = true, optional = true } and it would not respect the optional
Added tests to verify everything is in working order

Remaining implementation work for the RFC

Correctly inherit fields that are relative paths
- Including adding support for inheriting license_file, readme, and path-dependencies
Path dependencies infer version directive
Lock workspace dependencies and warn when unused
Optimizations, as needed
Evaluate any new fields for being inheritable (e.g. rust-version)

Problems:

There is duplication of code that can't be removed without significant refactoring
Potential to parse the same manifest many times when searching for a root
- This should not happen when a [package] specifies its workspace
- This should only happen if the workspace root is greater than one folder above

rust-highfive · 2022-03-29T22:06:28Z

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

Muscraft · 2022-03-29T22:54:06Z

r? @epage

epage

Thanks for moving this forward and sorry for the delay!

src/cargo/core/workspace.rs

src/cargo/util/toml/mod.rs

src/cargo/util/config/mod.rs

src/cargo/core/workspace.rs

src/cargo/util/toml/mod.rs

…onal` or `features` correctly

…t to its own function

src/cargo/core/workspace.rs

src/cargo/util/toml/mod.rs

epage

Thanks!

epage · 2022-04-05T17:04:48Z

@bors r+

bors · 2022-04-05T17:04:50Z

📌 Commit 8b3ce98 has been approved by epage

bors · 2022-04-05T17:04:56Z

⌛ Testing commit 8b3ce98 with merge e2e2ddd...

bors · 2022-04-05T17:47:40Z

☀️ Test successful - checks-actions
Approved by: epage
Pushing e2e2ddd to master...

Update cargo 5 commits in 1ef1e0a12723ce9548d7da2b63119de9002bead8..e2e2dddebe66dfc1403a312653557e332445308b 2022-03-31 00:17:18 +0000 to 2022-04-05 17:04:53 +0000 - Part 2 of RFC2906 -- allow inheriting from a different `Cargo.toml` (rust-lang/cargo#10517) - File Cache is valid if checkout or contents hasn't changed (rust-lang/cargo#10507) - Fix how scrape-examples handles proc macros (rust-lang/cargo#10533) - tools: update checkout action on CI (rust-lang/cargo#10521) - Don't error if no binaries were installed (rust-lang/cargo#10508)

Part 3 of RFC2906 - Add support for inheriting `license-path`, and `depednency.path` Tracking issue: #8415 RFC: rust-lang/rfcs#2906 [Part 1](#10497) [Part 2](#10517) This PR focuses on adding support for inheriting `license-path`, and `depednency.path`: - To adjust the relative paths from being workspace-relative to package-relative, we use `pathdiff` which `cargo-add` is also going to be using for a similar purpose - `ws_path` was added to `InheritableFields` so we can resolve relative paths from workspace-relative to package-relative - Moved `resolve` for toml dependencies from `TomlDependency::<P>` to `TomlDependency` - This was done since resolving a relative path should be a string - You should never inherit from a `.cargo/config.toml` which is the reason `P` was added Remaining implementation work for the RFC - Relative paths for `readme` - Path dependencies infer version directive - Lock workspace dependencies and warn when unused - Optimizations, as needed - Evaluate any new fields for being inheritable (e.g. `rust-version`)

Part 4 of RFC2906 - Add support for inheriting `readme` Tracking issue: #8415 RFC: rust-lang/rfcs#2906 [Part 1](#10497) [Part 2](#10517) [Part 3](#10538) This PR focuses on adding support for inheriting `readme`: - Added adjusting of a `readme` path when it is outside of the `package_root` - Copied from `license-file`'s implementation in `toml::prepare_for_publish()` - Added copying of a `readme` file when it is outside of the `package_root` for publishing - Copied from `license-file`'s implementation in `cargo_package::build_ar_list()` - Merged copying of a file outside of a `package_root` to reduce duplicated code and allow for other files in the future to be copied in a similar way Remaining implementation work for the RFC - Path dependencies infer version directive - Lock workspace dependencies and warn when unused - Optimizations, as needed - Evaluate any new fields for being inheritable (e.g. `rust-version`)

Part 5 of RFC2906 - Add support for inheriting `rust-version` Tracking issue: #8415 RFC: rust-lang/rfcs#2906 Part 1: #10497 Part 2: #10517 Part 3: #10538 Part 4: #10548 This PR focuses on adding support for inheriting `rust-version`. While this was not in the original RFC it was decided that it should be added per [epage's comment](#8415 (comment)). - Changed `rust-version` to `Option<MaybeWorkspace<String>>` in `TomlProject` - Added `rust-version` to `TomlWorkspace` to allow for inheritance - Added `rust-version` to `InheritableFields1 and a method to get it as needed - Updated tests to include `rust-version` Remaining implementation work for the RFC - Switch the inheritance source from `workspace` to `workspace.package`, see [epage's comment](#8415 (comment)) - Add `include` and `exclude` now that `workspace.package` is done, see [epage's comment](#8415 (comment)) - `cargo-add` support, see [epage's comment](#8415 (comment)) - Optimizations, as needed

Part 6 of RFC2906 - Switch the inheritance source from `workspace` to… Tracking issue: #8415 RFC: rust-lang/rfcs#2906 Part 1: #10497 Part 2: #10517 Part 3: #10538 Part 4: #10548 Part 5: #10563 This PR focuses on switching the inheritance source from `workspace` to `workspace.package`. The RFC specified `workspace` but it was decided that it should be changed to `workspace.package` per [epage's comment](#8415 (comment)). - Moved fields that can be inherited to ` package: Option<InheritableFields>` in `TomlWorkspace` - Making `package` `Option<InheritableFields>` was done to remove duplication of types and better signify what fields you can inherit from in one place - Added `#[serde(skip)]` to `ws_root` and `dependencies` in `InheritableFields` since they will never be present when deserializing and we don't want them present when serializing - Added a method to update `ws_root` and `dependencies` in `InheritableFields` - Added for `ws_root` since it will never be present in a `Cargo.toml` and is needed to resolve relative paths - Added for `dependencies` since they are under `workspace.dependencies` not `workspace.package.dependencies` - `workspace.dependencies` was left out of `workspace.package` to better match `package` and `package.dependencies` - Updated error messages from `workspace._` to `workspace.package._` - Updated `unstable.md` to reflect change to `workspace.package` - Updated tests to use `workspace.package` Remaining implementation work for the RFC - Add `include` and `exclude` now that `workspace.package` is done, see [epage's comment](#8415 (comment)) - `cargo-add` support, see [epage's comment](#8415 (comment)) - Optimizations, as needed

Part 7 of RFC2906 - Add support for inheriting `exclude` and `include` Tracking issue: #8415 RFC: rust-lang/rfcs#2906 Part 1: #10497 Part 2: #10517 Part 3: #10538 Part 4: #10548 Part 5: #10563 Part 6: #10564 This PR focuses on adding support for inheriting `include` and `exclude`. While they were not in the original RFC it was decided that they should be added per [epage's comment](#8415 (comment)). - Changed `include` and `exclude` into `Option<MaybeWorkspace<Vec<String>>>` inside `TomlProject` - Added `include` and `exclude` to `InheritbaleFields` - Updated tests to verify `include` and `exclude` are inheriting correctly Remaining implementation work for the RFC - `cargo-add` support, see [epage's comment](#8415 (comment)) - Optimizations, as needed

Part 8 of RFC2906 - Keep `InheritableFields` in a `LazyCell` inside `… Tracking issue: #8415 RFC: rust-lang/rfcs#2906 Part 1: #10497 Part 2: #10517 Part 3: #10538 Part 4: #10548 Part 5: #10563 Part 6: #10564 Part 7: #10565 This PR focuses on optimizations surrounding `InheritabeFields` and searching for a `WorkspaceRootConfig`: - Keep `InheritableFields` in a `LazyCell` inside `to_real_manifest` so we only search for a workspace root once per `to_real_manifest` - Changed calls for getting a workspace root to use `inherit_cell.try_borrow_with()` [Testing Framework](https://gist.github.com/Muscraft/14f6879af27500e34584296edb468d15) Test Results: nest=1, runs=7, members=75, step=25 - 25 Members: - Optimized: 0.145s - Not Optimized: 0.181s - Normal: 0.141s - Percent change from Normal: 2.837% - 50 Members - Optimized: 0.152s - Not Optimized: 0.267s - Normal: 0.141s - Percent change from Normal: 7.801% nest=2, runs=7, members=75, step=25 - 25 Members - Optimized: 0.147s, - Not Optimized: 0.437s - Normal: 0.14s - Percent change from Normal: 5.0% - 50 Members - Optimized: 0.159s - Not Optimized: 1.023s - Normal: 0.141s - Percent change from Normal: 12.766% Using a `LazyCell` for `InheritableFields` brought down runtimes to more acceptable levels. It is worth noting that this is a very harsh test case and not one that represents real-world scenarios. That being said there are still some optimizations that could be done if we need to. The biggest one is making sure we only parse a `Cargo.toml` once. Remaining implementation work for the RFC - `cargo-add` support, see [epage's comment](#8415 (comment)) - More optimizations, as needed

Prefer `key.workspace = true` to `key = { workspace = true }` Tracking issue: #8415 RFC: rust-lang/rfcs#2906 Part 1: #10497 Part 2: #10517 Part 3: #10538 Part 4: #10548 Part 5: #10563 Part 6: #10564 Part 7: #10565 Part 8: #10568 This PR fell out of [this discussion](#10497 (comment)) regarding if `key.workspace = true` is better than `key = { workspace = true }`. Changes: - Made all fields into `field.workspace = true` - Made dependencies into `dep.workspace = true` when a they only specify `{ workspace = true }` Remaining implementation work for the RFC - `cargo-add` support, see [epage's comment](#8415 (comment)) - Optimizations as needed

rust-highfive assigned ehuss Mar 29, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 29, 2022

rust-highfive assigned epage and unassigned ehuss Mar 29, 2022

epage reviewed Apr 1, 2022

View reviewed changes

Muscraft added 3 commits April 4, 2022 14:58

Fixed bug on TomlDependncy::Simple where it would not inherit `opti…

b127a4c

…onal` or `features` correctly

Extracted loop over ancestor paths when searching for a workspace roo…

035cb09

…t to its own function

Part 2 of RFC2906 - allow inheriting from parent workspaces

0cd5546

Muscraft force-pushed the rfc2906-part2 branch from 100c69f to 0cd5546 Compare April 4, 2022 20:33

epage reviewed Apr 4, 2022

View reviewed changes

src/cargo/core/workspace.rs Outdated Show resolved Hide resolved

src/cargo/core/workspace.rs Outdated Show resolved Hide resolved

src/cargo/core/workspace.rs Outdated Show resolved Hide resolved

src/cargo/util/toml/mod.rs Show resolved Hide resolved

Change searching for a workspace root from find_map() to explicit loop

8b3ce98

Muscraft force-pushed the rfc2906-part2 branch from 5a3c399 to 8b3ce98 Compare April 5, 2022 16:29

epage approved these changes Apr 5, 2022

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 5, 2022

epage mentioned this pull request Apr 5, 2022

Tracking issue for Deduplicate Cargo workspace information RFC #8415

Closed

12 tasks

bors merged commit e2e2ddd into rust-lang:master Apr 5, 2022

ehuss mentioned this pull request Apr 6, 2022

Update cargo rust-lang/rust#95720

Merged

Muscraft mentioned this pull request Apr 6, 2022

Part 3 of RFC2906 - Add support for inheriting license-path, and depednency.path #10538

Merged

ehuss added this to the 1.62.0 milestone Apr 7, 2022

Muscraft mentioned this pull request Apr 8, 2022

Part 4 of RFC2906 - Add support for inheriting readme #10548

Merged

Muscraft mentioned this pull request Apr 13, 2022

Part 5 of RFC2906 - Add support for inheriting rust-version #10563

Merged

Muscraft mentioned this pull request Apr 13, 2022

Part 6 of RFC2906 - Switch the inheritance source from workspace to… #10564

Merged

Muscraft mentioned this pull request Apr 13, 2022

Part 7 of RFC2906 - Add support for inheriting exclude and include #10565

Merged

Muscraft mentioned this pull request Apr 14, 2022

Part 8 of RFC2906 - Keep InheritableFields in a LazyCell inside `… #10568

Merged

Muscraft mentioned this pull request Apr 20, 2022

Prefer key.workspace = true to key = { workspace = true } #10584

Merged

This was referenced Apr 27, 2022

Support overwriting a foo.workspace = true with a dependency from a… Muscraft/cargo#4

Merged

Cargo add support for workspace inheritance #10606

Merged

Muscraft deleted the rfc2906-part2 branch May 27, 2022 01:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Part 2 of RFC2906 -- allow inheriting from a different `Cargo.toml` #10517

Part 2 of RFC2906 -- allow inheriting from a different `Cargo.toml` #10517

Muscraft commented Mar 29, 2022 •

edited

Loading

rust-highfive commented Mar 29, 2022

Muscraft commented Mar 29, 2022

epage left a comment

epage left a comment

epage commented Apr 5, 2022

bors commented Apr 5, 2022

bors commented Apr 5, 2022

bors commented Apr 5, 2022

Part 2 of RFC2906 -- allow inheriting from a different Cargo.toml #10517

Part 2 of RFC2906 -- allow inheriting from a different Cargo.toml #10517

Conversation

Muscraft commented Mar 29, 2022 • edited Loading

rust-highfive commented Mar 29, 2022

Muscraft commented Mar 29, 2022

epage left a comment

Choose a reason for hiding this comment

epage left a comment

Choose a reason for hiding this comment

epage commented Apr 5, 2022

bors commented Apr 5, 2022

bors commented Apr 5, 2022

bors commented Apr 5, 2022

Part 2 of RFC2906 -- allow inheriting from a different `Cargo.toml` #10517

Part 2 of RFC2906 -- allow inheriting from a different `Cargo.toml` #10517

Muscraft commented Mar 29, 2022 •

edited

Loading