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.

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

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

Merged
merged 1 commit into from
Apr 14, 2022

Conversation

Muscraft
Copy link
Member

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.

  • 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

@rust-highfive
Copy link

r? @ehuss

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 13, 2022
@Muscraft
Copy link
Member Author

r? @epage

@epage
Copy link
Contributor

epage commented Apr 14, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Apr 14, 2022

📌 Commit 114f1e5 has been approved by epage

@bors 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 14, 2022
@bors
Copy link
Contributor

bors commented Apr 14, 2022

⌛ Testing commit 114f1e5 with merge ec83f8d...

@bors
Copy link
Contributor

bors commented Apr 14, 2022

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

@bors bors merged commit ec83f8d into rust-lang:master Apr 14, 2022
bors added a commit that referenced this pull request Apr 14, 2022
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
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 20, 2022
Update cargo

7 commits in dba5baf4345858c591517b24801902a062c399f8..edffc4ada3d77799e5a04eeafd9b2f843d29fc23
2022-04-13 21:58:27 +0000 to 2022-04-19 17:38:29 +0000
- Document cargo-add (rust-lang/cargo#10578)
- feat: Support '-F' as an alias for '--features' (rust-lang/cargo#10576)
- Completion support for `cargo-add` (rust-lang/cargo#10577)
- Add a link to the document in the timings report (rust-lang/cargo#10492)
- feat: Import cargo-add into cargo (rust-lang/cargo#10472)
- Part 8 of RFC2906 - Keep `InheritableFields` in a `LazyCell` inside `… (rust-lang/cargo#10568)
- Part 7 of RFC2906 - Add support for inheriting `exclude` and `include` (rust-lang/cargo#10565)
bors added a commit that referenced this pull request Apr 20, 2022
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
@ehuss ehuss added this to the 1.62.0 milestone Apr 22, 2022
@Muscraft Muscraft deleted the rfc2906-part7 branch May 27, 2022 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants