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

Report crate size on package and publish #11270

Merged
merged 4 commits into from Oct 29, 2022

Conversation

antonok-edm
Copy link
Contributor

@antonok-edm antonok-edm commented Oct 21, 2022

Motivation

Fixes #11251.

This adds a line like Packaged 42 files, 727.0KiB (143.8KiB compressed) to the output of cargo package and cargo publish. See the associated issue for more details.

Test info

I've updated associated tests to account for the new line in the output, including the file count where relevant. I've also added a few additional tests specifically to address the uncompressed and compressed file sizes.

If you'd like to test this manually, simply run cargo package or cargo publish within a project of your choice. The new Packaged line will appear at the end of the stderr output, or directly before the Uploaded line, respectively.

Review info

This PR touches many test files to account for the additional line of stderr output. For convenience, I've split the PR into 4 commits.

  1. contains the actual implementation
  2. updates existing tests unrelated to cargo package and cargo publish
  3. updates existing tests related to cargo package and cargo publish, including file counts where relevant
  4. adds new tests specifically for the file sizes, which are generally not covered by existing tests

Additional information

The feature was discussed on Zulip prior to implementation.

Potential future extensions to explore include:

  • Report size per file when using --verbose
  • Compare to the size of the previously uploaded version to warn if the increase is sufficiently large
  • Consider designs that can draw more attention to the most important information
  • Warn if large binary files are included (#9058)

@rust-highfive
Copy link

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 rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 21, 2022
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request. This seems generally good to me.

Cargo.toml Outdated Show resolved Hide resolved
src/cargo/ops/cargo_package.rs Outdated Show resolved Hide resolved
@antonok-edm antonok-edm force-pushed the report-packagesize branch 3 times, most recently from fb671f1 to 7c538b9 Compare October 21, 2022 09:34
src/cargo/util/mod.rs Outdated Show resolved Hide resolved
tests/testsuite/package.rs Show resolved Hide resolved
src/cargo/ops/cargo_package.rs Outdated Show resolved Hide resolved
src/cargo/sources/git/utils.rs Outdated Show resolved Hide resolved
src/cargo/ops/cargo_package.rs Show resolved Hide resolved
@weihanglo weihanglo added the T-cargo Team: Cargo label Oct 22, 2022
@weihanglo
Copy link
Member

weihanglo commented Oct 22, 2022

The PR description summarizes the changes and future extensions very well, so I won't repeat it here.

This shouldn't introduce any breaking change, and is easy to revert/update if one doesn't like it. However, a UI enhancement is always a bikeshedding problem, and we often go through an FCP for UI changes. I am going to seek a consensus here.

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Oct 22, 2022

Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Oct 22, 2022
@rfcbot rfcbot added the final-comment-period FCP — a period for last comments before action is taken label Oct 27, 2022
@rfcbot
Copy link
Collaborator

rfcbot commented Oct 27, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period An FCP proposal has started, but not yet signed off. label Oct 27, 2022
@bors
Copy link
Collaborator

bors commented Oct 27, 2022

☔ The latest upstream changes (presumably #11062) made this pull request unmergeable. Please resolve the merge conflicts.

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Thank you for resolving conflicts so fast! I am going to merge this sooner or later after we check the last item.

src/cargo/util/mod.rs Outdated Show resolved Hide resolved
@weihanglo
Copy link
Member

It's fantastic to collaborate on this! Thank you for the contribution.

Going to merge it, and please feel free to do any discussion and/or follow-up patch.

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 29, 2022

📌 Commit 2450035 has been approved by weihanglo

It is now in the queue for this repository.

@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 Oct 29, 2022
@bors
Copy link
Collaborator

bors commented Oct 29, 2022

⌛ Testing commit 2450035 with merge da20496...

@jonhoo
Copy link
Contributor

jonhoo commented Oct 29, 2022

This feels like it's worthy of mentioning in the Cargo release notes too when it lands!

@bors
Copy link
Collaborator

bors commented Oct 29, 2022

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing da20496 to master...

@bors bors merged commit da20496 into rust-lang:master Oct 29, 2022
@antonok-edm antonok-edm deleted the report-packagesize branch October 29, 2022 02:32
@antonok-edm
Copy link
Contributor Author

Awesome, thanks all for a very smooth process!

weihanglo added a commit to weihanglo/rust that referenced this pull request Nov 2, 2022
14 commits in 7e484fc1a766f56dbc95380f45719698e0c82749..810cbad9a123ad4ee0a55a96171b8f8478ff1c03
2022-10-27 15:20:57 +0000 to 2022-11-02 21:04:31 +0000
- Update curl-sys (rust-lang/cargo#11326)
- Mention fix on build script deadlock (rust-lang/cargo#11325)
- Make cargo forward pre-existing CARGO if set (rust-lang/cargo#11285)
- Clean up workspace dependencies after cargo remove (rust-lang/cargo#11242)
- Update the outdated link for rust-semverver (rust-lang/cargo#11322)
- Fix broken link to compilation entry point (rust-lang/cargo#11317)
- Only remove fingerprints and build script artifacts of the requested package (rust-lang/cargo#10621)
- Newer anyhow features are required (rust-lang/cargo#11316)
- Clean stale git temp files (rust-lang/cargo#11308)
- Report crate size on package and publish (rust-lang/cargo#11270)
- add a note that some warnings (and/or errors) can be auto-fixed (rust-lang/cargo#10989)
- Update libcurl (rust-lang/cargo#11307)
- artifact deps shoud works when target field specified coexists with `optional = true` (rust-lang/cargo#11183)
- Fix singular verb in tests page (rust-lang/cargo#11300)
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 3, 2022
Update cargo

14 commits in 7e484fc1a766f56dbc95380f45719698e0c82749..810cbad9a123ad4ee0a55a96171b8f8478ff1c03
2022-10-27 15:20:57 +0000 to 2022-11-02 21:04:31 +0000
- Update curl-sys (rust-lang/cargo#11326)
- Mention fix on build script deadlock (rust-lang/cargo#11325)
- Make cargo forward pre-existing CARGO if set (rust-lang/cargo#11285)
- Clean up workspace dependencies after cargo remove (rust-lang/cargo#11242)
- Update the outdated link for rust-semverver (rust-lang/cargo#11322)
- Fix broken link to compilation entry point (rust-lang/cargo#11317)
- Only remove fingerprints and build script artifacts of the requested package (rust-lang/cargo#10621)
- Newer anyhow features are required (rust-lang/cargo#11316)
- Clean stale git temp files (rust-lang/cargo#11308)
- Report crate size on package and publish (rust-lang/cargo#11270)
- add a note that some warnings (and/or errors) can be auto-fixed (rust-lang/cargo#10989)
- Update libcurl (rust-lang/cargo#11307)
- artifact deps shoud works when target field specified coexists with `optional = true` (rust-lang/cargo#11183)
- Fix singular verb in tests page (rust-lang/cargo#11300)

r? `@ghost`
bors pushed a commit to rust-lang-ci/rust that referenced this pull request Nov 5, 2022
20 commits in 7e484fc1a766f56dbc95380f45719698e0c82749..9286a1beba5b28b115bad67de2ae91fb1c61eb0b
2022-10-27 15:20:57 +0000 to 2022-11-04 06:41:49 +0000
- chore: Upgrade dependencies (rust-lang/cargo#11328)
- Clean more aggressively in CI (rust-lang/cargo#11335)
- Remove remove_dir_all (rust-lang/cargo#11333)
- test(publish): Cover more wait-for-publish cases (rust-lang/cargo#11327)
- Revert rust-lang/cargo#11183 (rust-lang/cargo#11331)
- fix(semver-check): adapt to a different error for variant not covered (rust-lang/cargo#11332)
- Update curl-sys (rust-lang/cargo#11326)
- Mention fix on build script deadlock (rust-lang/cargo#11325)
- Make cargo forward pre-existing CARGO if set (rust-lang/cargo#11285)
- Clean up workspace dependencies after cargo remove (rust-lang/cargo#11242)
- Update the outdated link for rust-semverver (rust-lang/cargo#11322)
- Fix broken link to compilation entry point (rust-lang/cargo#11317)
- Only remove fingerprints and build script artifacts of the requested package (rust-lang/cargo#10621)
- Newer anyhow features are required (rust-lang/cargo#11316)
- Clean stale git temp files (rust-lang/cargo#11308)
- Report crate size on package and publish (rust-lang/cargo#11270)
- add a note that some warnings (and/or errors) can be auto-fixed (rust-lang/cargo#10989)
- Update libcurl (rust-lang/cargo#11307)
- artifact deps shoud works when target field specified coexists with `optional = true` (rust-lang/cargo#11183)
- Fix singular verb in tests page (rust-lang/cargo#11300)
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 5, 2022
Update cargo

20 commits in 7e484fc1a766f56dbc95380f45719698e0c82749..9286a1beba5b28b115bad67de2ae91fb1c61eb0b 2022-10-27 15:20:57 +0000 to 2022-11-04 06:41:49 +0000
- chore: Upgrade dependencies (rust-lang/cargo#11328)
- Clean more aggressively in CI (rust-lang/cargo#11335)
- Remove remove_dir_all (rust-lang/cargo#11333)
- test(publish): Cover more wait-for-publish cases (rust-lang/cargo#11327)
- Revert rust-lang/cargo#11183 (rust-lang/cargo#11331)
- fix(semver-check): adapt to a different error for variant not covered (rust-lang/cargo#11332)
- Update curl-sys (rust-lang/cargo#11326)
- Mention fix on build script deadlock (rust-lang/cargo#11325)
- Make cargo forward pre-existing CARGO if set (rust-lang/cargo#11285)
- Clean up workspace dependencies after cargo remove (rust-lang/cargo#11242)
- Update the outdated link for rust-semverver (rust-lang/cargo#11322)
- Fix broken link to compilation entry point (rust-lang/cargo#11317)
- Only remove fingerprints and build script artifacts of the requested package (rust-lang/cargo#10621)
- Newer anyhow features are required (rust-lang/cargo#11316)
- Clean stale git temp files (rust-lang/cargo#11308)
- Report crate size on package and publish (rust-lang/cargo#11270)
- add a note that some warnings (and/or errors) can be auto-fixed (rust-lang/cargo#10989)
- Update libcurl (rust-lang/cargo#11307)
- artifact deps shoud works when target field specified coexists with `optional = true` (rust-lang/cargo#11183)
- Fix singular verb in tests page (rust-lang/cargo#11300)

r? `@ghost`
@rfcbot rfcbot added finished-final-comment-period FCP complete to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels Nov 6, 2022
@ehuss ehuss added this to the 1.67.0 milestone Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge FCP with intent to merge finished-final-comment-period FCP complete S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo to-announce
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Report crate size in cargo publish
7 participants