Skip to content

Commit

Permalink
Auto merge of #11270 - antonok-edm:report-packagesize, r=weihanglo
Browse files Browse the repository at this point in the history
Report crate size on package and publish

### 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](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Report.20crate.20size.20when.20packaging) 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](#9058))
  • Loading branch information
bors committed Oct 29, 2022
2 parents 2d04bcd + 2450035 commit da20496
Show file tree
Hide file tree
Showing 14 changed files with 441 additions and 13 deletions.
1 change: 1 addition & 0 deletions crates/cargo-test-support/src/compare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ fn substitute_macros(input: &str) -> String {
("[REMOVING]", " Removing"),
("[DOCTEST]", " Doc-tests"),
("[PACKAGING]", " Packaging"),
("[PACKAGED]", " Packaged"),
("[DOWNLOADING]", " Downloading"),
("[DOWNLOADED]", " Downloaded"),
("[UPLOADING]", " Uploading"),
Expand Down
33 changes: 29 additions & 4 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::core::{Package, PackageId, PackageSet, Resolve, SourceId};
use crate::sources::PathSource;
use crate::util::errors::CargoResult;
use crate::util::toml::TomlManifest;
use crate::util::{self, restricted_names, Config, FileLock};
use crate::util::{self, human_readable_bytes, restricted_names, Config, FileLock};
use crate::{drop_println, ops};
use anyhow::Context as _;
use cargo_util::paths;
Expand Down Expand Up @@ -110,6 +110,8 @@ pub fn package_one(

let ar_files = build_ar_list(ws, pkg, src_files, vcs_info)?;

let filecount = ar_files.len();

if opts.list {
for ar_file in ar_files {
drop_println!(config, "{}", ar_file.rel_str);
Expand Down Expand Up @@ -138,7 +140,7 @@ pub fn package_one(
.shell()
.status("Packaging", pkg.package_id().to_string())?;
dst.file().set_len(0)?;
tar(ws, pkg, ar_files, dst.file(), &filename)
let uncompressed_size = tar(ws, pkg, ar_files, dst.file(), &filename)
.with_context(|| "failed to prepare local package for uploading")?;
if opts.verify {
dst.seek(SeekFrom::Start(0))?;
Expand All @@ -151,6 +153,22 @@ pub fn package_one(
fs::rename(&src_path, &dst_path)
.with_context(|| "failed to move temporary tarball into final location")?;

let dst_metadata = dst
.file()
.metadata()
.with_context(|| format!("could not learn metadata for: `{}`", dst_path.display()))?;
let compressed_size = dst_metadata.len();

let uncompressed = human_readable_bytes(uncompressed_size);
let compressed = human_readable_bytes(compressed_size);

let message = format!(
"{} files, {:.1}{} ({:.1}{} compressed)",
filecount, uncompressed.0, uncompressed.1, compressed.0, compressed.1,
);
// It doesn't really matter if this fails.
drop(config.shell().status("Packaged", message));

return Ok(Some(dst));
}

Expand Down Expand Up @@ -567,13 +585,16 @@ fn check_repo_state(
}
}

/// Compresses and packages a list of [`ArchiveFile`]s and writes into the given file.
///
/// Returns the uncompressed size of the contents of the new archive file.
fn tar(
ws: &Workspace<'_>,
pkg: &Package,
ar_files: Vec<ArchiveFile>,
dst: &File,
filename: &str,
) -> CargoResult<()> {
) -> CargoResult<u64> {
// Prepare the encoder and its header.
let filename = Path::new(filename);
let encoder = GzBuilder::new()
Expand All @@ -586,6 +607,8 @@ fn tar(

let base_name = format!("{}-{}", pkg.name(), pkg.version());
let base_path = Path::new(&base_name);

let mut uncompressed_size = 0;
for ar_file in ar_files {
let ArchiveFile {
rel_path,
Expand All @@ -611,6 +634,7 @@ fn tar(
.with_context(|| {
format!("could not archive source file `{}`", disk_path.display())
})?;
uncompressed_size += metadata.len() as u64;
}
FileContents::Generated(generated_kind) => {
let contents = match generated_kind {
Expand All @@ -626,13 +650,14 @@ fn tar(
header.set_cksum();
ar.append_data(&mut header, &ar_path, contents.as_bytes())
.with_context(|| format!("could not archive source file `{}`", rel_str))?;
uncompressed_size += contents.len() as u64;
}
}
}

let encoder = ar.into_inner()?;
encoder.finish()?;
Ok(())
Ok(uncompressed_size)
}

/// Generate warnings when packaging Cargo.lock, and the resolve have changed.
Expand Down
11 changes: 3 additions & 8 deletions src/cargo/sources/git/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use crate::core::GitReference;
use crate::util::errors::CargoResult;
use crate::util::{network, Config, IntoUrl, MetricsCounter, Progress};
use crate::util::{human_readable_bytes, network, Config, IntoUrl, MetricsCounter, Progress};
use anyhow::{anyhow, Context as _};
use cargo_util::{paths, ProcessBuilder};
use curl::easy::List;
Expand Down Expand Up @@ -755,13 +755,8 @@ pub fn with_fetch_options(
counter.add(stats.received_bytes(), now);
last_update = now;
}
fn format_bytes(bytes: f32) -> (&'static str, f32) {
static UNITS: [&str; 5] = ["", "Ki", "Mi", "Gi", "Ti"];
let i = (bytes.log2() / 10.0).min(4.0) as usize;
(UNITS[i], bytes / 1024_f32.powi(i as i32))
}
let (unit, rate) = format_bytes(counter.rate());
format!(", {:.2}{}B/s", rate, unit)
let (rate, unit) = human_readable_bytes(counter.rate() as u64);
format!(", {:.2}{}/s", rate, unit)
};
progress
.tick(stats.indexed_objects(), stats.total_objects(), &msg)
Expand Down
43 changes: 43 additions & 0 deletions src/cargo/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ pub fn elapsed(duration: Duration) -> String {
}
}

/// Formats a number of bytes into a human readable SI-prefixed size.
/// Returns a tuple of `(quantity, units)`.
pub fn human_readable_bytes(bytes: u64) -> (f32, &'static str) {
static UNITS: [&str; 7] = ["B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB"];
let bytes = bytes as f32;
let i = ((bytes.log2() / 10.0) as usize).min(UNITS.len() - 1);
(bytes / 1024_f32.powi(i as i32), UNITS[i])
}

pub fn iter_join_onto<W, I, T>(mut w: W, iter: I, delim: &str) -> fmt::Result
where
W: fmt::Write,
Expand Down Expand Up @@ -122,3 +131,37 @@ pub fn truncate_with_ellipsis(s: &str, max_width: usize) -> String {
}
prefix
}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn test_human_readable_bytes() {
assert_eq!(human_readable_bytes(0), (0., "B"));
assert_eq!(human_readable_bytes(8), (8., "B"));
assert_eq!(human_readable_bytes(1000), (1000., "B"));
assert_eq!(human_readable_bytes(1024), (1., "KiB"));
assert_eq!(human_readable_bytes(1024 * 420 + 512), (420.5, "KiB"));
assert_eq!(human_readable_bytes(1024 * 1024), (1., "MiB"));
assert_eq!(
human_readable_bytes(1024 * 1024 + 1024 * 256),
(1.25, "MiB")
);
assert_eq!(human_readable_bytes(1024 * 1024 * 1024), (1., "GiB"));
assert_eq!(
human_readable_bytes((1024. * 1024. * 1024. * 3.1415) as u64),
(3.1415, "GiB")
);
assert_eq!(human_readable_bytes(1024 * 1024 * 1024 * 1024), (1., "TiB"));
assert_eq!(
human_readable_bytes(1024 * 1024 * 1024 * 1024 * 1024),
(1., "PiB")
);
assert_eq!(
human_readable_bytes(1024 * 1024 * 1024 * 1024 * 1024 * 1024),
(1., "EiB")
);
assert_eq!(human_readable_bytes(u64::MAX), (16., "EiB"));
}
}
1 change: 1 addition & 0 deletions tests/testsuite/artifact_dep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1919,6 +1919,7 @@ fn publish_artifact_dep() {
"\
[UPDATING] [..]
[PACKAGING] foo v0.1.0 [..]
[PACKAGED] [..]
[UPLOADING] foo v0.1.0 [..]
[UPDATING] [..]
",
Expand Down
2 changes: 2 additions & 0 deletions tests/testsuite/credential_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ Only one of these values may be set, remove one or the other to proceed.
"\
[UPDATING] [..]
[PACKAGING] foo v0.1.0 [..]
[PACKAGED] [..]
[UPLOADING] foo v0.1.0 [..]
[UPDATING] [..]
",
Expand Down Expand Up @@ -208,6 +209,7 @@ fn publish() {
"\
[UPDATING] [..]
[PACKAGING] foo v0.1.0 [..]
[PACKAGED] [..]
[UPLOADING] foo v0.1.0 [..]
[UPDATING] [..]
",
Expand Down
2 changes: 2 additions & 0 deletions tests/testsuite/cross_publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ fn simple_cross_package() {
[VERIFYING] foo v0.0.0 ([CWD])
[COMPILING] foo v0.0.0 ([CWD]/target/package/foo-0.0.0)
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
[PACKAGED] 4 files, [..] ([..] compressed)
",
)
.run();
Expand Down Expand Up @@ -109,6 +110,7 @@ fn publish_with_target() {
[VERIFYING] foo v0.0.0 ([CWD])
[COMPILING] foo v0.0.0 ([CWD]/target/package/foo-0.0.0)
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
[PACKAGED] [..]
[UPLOADING] foo v0.0.0 ([CWD])
[UPDATING] crates.io index
",
Expand Down
2 changes: 2 additions & 0 deletions tests/testsuite/features_namespaced.rs
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,7 @@ fn publish_no_implicit() {
"\
[UPDATING] [..]
[PACKAGING] foo v0.1.0 [..]
[PACKAGED] [..]
[UPLOADING] foo v0.1.0 [..]
[UPDATING] [..]
",
Expand Down Expand Up @@ -1029,6 +1030,7 @@ fn publish() {
[VERIFYING] foo v0.1.0 [..]
[COMPILING] foo v0.1.0 [..]
[FINISHED] [..]
[PACKAGED] [..]
[UPLOADING] foo v0.1.0 [..]
[UPDATING] [..]
",
Expand Down
Loading

0 comments on commit da20496

Please sign in to comment.