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 4 of RFC2906 - Add support for inheriting readme #10548

Merged
merged 2 commits into from
Apr 8, 2022

Conversation

Muscraft
Copy link
Member

@Muscraft Muscraft commented Apr 8, 2022

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

Part 1
Part 2
Part 3

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)

@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 8, 2022
@Muscraft
Copy link
Member Author

Muscraft commented Apr 8, 2022

r? @epage

Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

src/cargo/ops/cargo_package.rs Show resolved Hide resolved
src/cargo/ops/cargo_package.rs Outdated Show resolved Hide resolved
src/cargo/util/toml/mod.rs Show resolved Hide resolved
Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@epage
Copy link
Contributor

epage commented Apr 8, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Apr 8, 2022

📌 Commit 3bbb781 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 8, 2022
@bors
Copy link
Contributor

bors commented Apr 8, 2022

⌛ Testing commit 3bbb781 with merge fc5035d...

@bors
Copy link
Contributor

bors commented Apr 8, 2022

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

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

11 commits in e2e2dddebe66dfc1403a312653557e332445308b..dba5baf4345858c591517b24801902a062c399f8
2022-04-05 17:04:53 +0000 to 2022-04-13 21:58:27 +0000
- Part 6 of RFC2906 - Switch the inheritance source from `workspace` to… (rust-lang/cargo#10564)
- Part 5 of RFC2906 - Add support for inheriting `rust-version` (rust-lang/cargo#10563)
- Add support for rustc --check-cfg well known names and values (rust-lang/cargo#10486)
- Reserve filename `Cargo.toml.orig` in `cargo package` (rust-lang/cargo#10551)
- Retry command invocation with argfile (rust-lang/cargo#10546)
- Add a progress indicator for `cargo clean` (rust-lang/cargo#10236)
- Ensure host units don't depend on Docscrape units, fixes rust-lang/cargo#10545 (rust-lang/cargo#10549)
- Fix docs: Bindeps env vars are passed to build script at runtime (rust-lang/cargo#10550)
- Part 4 of RFC2906 - Add support for inheriting `readme` (rust-lang/cargo#10548)
- Part 3 of RFC2906 - Add support for inheriting `license-path`, and `depednency.path` (rust-lang/cargo#10538)
- Bump to 0.63.0, update changelog (rust-lang/cargo#10544)
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 14, 2022
Update cargo

11 commits in e2e2dddebe66dfc1403a312653557e332445308b..dba5baf4345858c591517b24801902a062c399f8
2022-04-05 17:04:53 +0000 to 2022-04-13 21:58:27 +0000
- Part 6 of RFC2906 - Switch the inheritance source from `workspace` to… (rust-lang/cargo#10564)
- Part 5 of RFC2906 - Add support for inheriting `rust-version` (rust-lang/cargo#10563)
- Add support for rustc --check-cfg well known names and values (rust-lang/cargo#10486)
- Reserve filename `Cargo.toml.orig` in `cargo package` (rust-lang/cargo#10551)
- Retry command invocation with argfile (rust-lang/cargo#10546)
- Add a progress indicator for `cargo clean` (rust-lang/cargo#10236)
- Ensure host units don't depend on Docscrape units, fixes rust-lang/cargo#10545 (rust-lang/cargo#10549)
- Fix docs: Bindeps env vars are passed to build script at runtime (rust-lang/cargo#10550)
- Part 4 of RFC2906 - Add support for inheriting `readme` (rust-lang/cargo#10548)
- Part 3 of RFC2906 - Add support for inheriting `license-path`, and `depednency.path` (rust-lang/cargo#10538)
- Bump to 0.63.0, update changelog (rust-lang/cargo#10544)
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 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-part4 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