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

refactor: control byte display precision with std::fmt options #15246

Merged
merged 2 commits into from
Feb 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ anyhow = "1.0.95"
base64 = "0.22.1"
blake3 = "1.5.5"
build-rs = { version = "0.3.0", path = "crates/build-rs" }
bytesize = "1.3"
cargo = { path = "" }
cargo-credential = { version = "0.4.2", path = "credential/cargo-credential" }
cargo-credential-libsecret = { version = "0.4.13", path = "credential/cargo-credential-libsecret" }
Expand Down Expand Up @@ -157,7 +156,6 @@ anstyle.workspace = true
anyhow.workspace = true
base64.workspace = true
blake3.workspace = true
bytesize.workspace = true
cargo-credential.workspace = true
cargo-platform.workspace = true
cargo-util-schemas.workspace = true
Expand Down
18 changes: 10 additions & 8 deletions src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use std::rc::Rc;
use std::time::{Duration, Instant};

use anyhow::Context as _;
use bytesize::ByteSize;
use cargo_util_schemas::manifest::RustVersion;
use curl::easy::Easy;
use curl::multi::{EasyHandle, Multi};
Expand All @@ -35,6 +34,7 @@ use crate::util::network::http::http_handle_and_timeout;
use crate::util::network::http::HttpTimeout;
use crate::util::network::retry::{Retry, RetryResult};
use crate::util::network::sleep::SleepTracker;
use crate::util::HumanBytes;
use crate::util::{self, internal, GlobalContext, Progress, ProgressStyle};

/// Information about a package that is available somewhere in the file system.
Expand Down Expand Up @@ -914,7 +914,8 @@ impl<'a, 'gctx> Downloads<'a, 'gctx> {
// have a great view into the progress of the extraction. Let's prepare
// the user for this CPU-heavy step if it looks like it'll take some
// time to do so.
if dl.total.get() < ByteSize::kb(400).0 {
let kib_400 = 1024 * 400;
if dl.total.get() < kib_400 {
self.tick(WhyTick::DownloadFinished)?;
} else {
self.tick(WhyTick::Extracting(&dl.id.name()))?;
Expand Down Expand Up @@ -1113,7 +1114,7 @@ impl<'a, 'gctx> Downloads<'a, 'gctx> {
}
}
if remaining > 0 && dur > Duration::from_millis(500) {
msg.push_str(&format!(", remaining bytes: {}", ByteSize(remaining)));
msg.push_str(&format!(", remaining bytes: {:.1}", HumanBytes(remaining)));
}
}
}
Expand Down Expand Up @@ -1153,20 +1154,21 @@ impl<'a, 'gctx> Drop for Downloads<'a, 'gctx> {
"crates"
};
let mut status = format!(
"{} {} ({}) in {}",
"{} {} ({:.1}) in {}",
self.downloads_finished,
crate_string,
ByteSize(self.downloaded_bytes),
HumanBytes(self.downloaded_bytes),
util::elapsed(self.start.elapsed())
);
// print the size of largest crate if it was >1mb
// however don't print if only a single crate was downloaded
// because it is obvious that it will be the largest then
if self.largest.0 > ByteSize::mb(1).0 && self.downloads_finished > 1 {
let mib_1 = 1024 * 1024;
if self.largest.0 > mib_1 && self.downloads_finished > 1 {
status.push_str(&format!(
" (largest was `{}` at {})",
" (largest was `{}` at {:.1})",
self.largest.1,
ByteSize(self.largest.0),
HumanBytes(self.largest.0),
));
}
// Clear progress before displaying final summary.
Expand Down
12 changes: 4 additions & 8 deletions src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use crate::ops;
use crate::util::edit_distance;
use crate::util::errors::CargoResult;
use crate::util::interning::InternedString;
use crate::util::{human_readable_bytes, GlobalContext, Progress, ProgressStyle};
use crate::util::HumanBytes;
use crate::util::{GlobalContext, Progress, ProgressStyle};
use anyhow::bail;
use cargo_util::paths;
use std::collections::{HashMap, HashSet};
Expand Down Expand Up @@ -457,13 +458,8 @@ impl<'gctx> CleanContext<'gctx> {
let byte_count = if self.total_bytes_removed == 0 {
String::new()
} else {
// Don't show a fractional number of bytes.
if self.total_bytes_removed < 1024 {
format!(", {}B total", self.total_bytes_removed)
} else {
let (bytes, unit) = human_readable_bytes(self.total_bytes_removed);
format!(", {bytes:.1}{unit} total")
}
let bytes = HumanBytes(self.total_bytes_removed);
format!(", {bytes:.1} total")
};
// I think displaying the number of directories removed isn't
// particularly interesting to the user. However, if there are 0
Expand Down
11 changes: 4 additions & 7 deletions src/cargo/ops/cargo_package/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ use crate::sources::{PathSource, CRATES_IO_REGISTRY};
use crate::util::cache_lock::CacheLockMode;
use crate::util::context::JobsConfig;
use crate::util::errors::CargoResult;
use crate::util::human_readable_bytes;
use crate::util::restricted_names;
use crate::util::toml::prepare_for_publish;
use crate::util::FileLock;
use crate::util::Filesystem;
use crate::util::GlobalContext;
use crate::util::Graph;
use crate::util::HumanBytes;
use crate::{drop_println, ops};
use anyhow::{bail, Context as _};
use cargo_util::paths;
Expand Down Expand Up @@ -129,13 +129,10 @@ fn create_package(
.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 uncompressed = HumanBytes(uncompressed_size);
let compressed = HumanBytes(compressed_size);

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

Expand Down
7 changes: 4 additions & 3 deletions src/cargo/sources/git/oxide.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
//! `utils` closely for now. One day it can be renamed into `utils` once `git2` isn't required anymore.

use crate::util::network::http::HttpTimeout;
use crate::util::{human_readable_bytes, network, MetricsCounter, Progress};
use crate::util::HumanBytes;
use crate::util::{network, MetricsCounter, Progress};
use crate::{CargoResult, GlobalContext};
use cargo_util::paths;
use gix::bstr::{BString, ByteSlice};
Expand Down Expand Up @@ -148,8 +149,8 @@ fn translate_progress_to_bar(
counter.add(received_bytes, now);
last_percentage_update = now;
}
let (rate, unit) = human_readable_bytes(counter.rate() as u64);
let msg = format!(", {rate:.2}{unit}/s");
let rate = HumanBytes(counter.rate() as u64);
let msg = format!(", {rate:.2}/s");

progress_bar.tick(
(total_objects * (num_phases - 2)) + objects,
Expand Down
9 changes: 4 additions & 5 deletions src/cargo/sources/git/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ use crate::sources::git::fetch::RemoteKind;
use crate::sources::git::oxide;
use crate::sources::git::oxide::cargo_config_to_gitoxide_overrides;
use crate::util::errors::CargoResult;
use crate::util::{
human_readable_bytes, network, GlobalContext, IntoUrl, MetricsCounter, Progress,
};
use crate::util::HumanBytes;
use crate::util::{network, GlobalContext, IntoUrl, MetricsCounter, Progress};
use anyhow::{anyhow, Context as _};
use cargo_util::{paths, ProcessBuilder};
use curl::easy::List;
Expand Down Expand Up @@ -914,8 +913,8 @@ pub fn with_fetch_options(
counter.add(stats.received_bytes(), now);
last_update = now;
}
let (rate, unit) = human_readable_bytes(counter.rate() as u64);
format!(", {:.2}{}/s", rate, unit)
let rate = HumanBytes(counter.rate() as u64);
format!(", {rate:.2}/s")
};
progress
.tick(stats.indexed_objects(), stats.total_objects(), &msg)
Expand Down
70 changes: 41 additions & 29 deletions src/cargo/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,26 @@ 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 struct HumanBytes(pub u64);

impl std::fmt::Display for HumanBytes {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
const UNITS: [&str; 7] = ["B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB"];
let bytes = self.0 as f32;
let i = ((bytes.log2() / 10.0) as usize).min(UNITS.len() - 1);
let unit = UNITS[i];
let size = bytes / 1024_f32.powi(i as i32);

// Don't show a fractional number of bytes.
if i == 0 {
return write!(f, "{size}{unit}");
}

let Some(precision) = f.precision() else {
return write!(f, "{size}{unit}");
};
write!(f, "{size:.precision$}{unit}",)
}
}

pub fn indented_lines(text: &str) -> String {
Expand Down Expand Up @@ -162,32 +176,30 @@ pub fn get_umask() -> u32 {
mod test {
use super::*;

#[track_caller]
fn t(bytes: u64, expected: &str) {
assert_eq!(&HumanBytes(bytes).to_string(), expected);
}

#[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. * 1.2345) as u64),
(1.2345, "GiB")
);
assert_eq!(human_readable_bytes(1024 * 1024 * 1024 * 1024), (1., "TiB"));
assert_eq!(
human_readable_bytes(1024 * 1024 * 1024 * 1024 * 1024),
(1., "PiB")
);
t(0, "0B");
t(8, "8B");
t(1000, "1000B");
t(1024, "1KiB");
t(1024 * 420 + 512, "420.5KiB");
t(1024 * 1024, "1MiB");
t(1024 * 1024 + 1024 * 256, "1.25MiB");
t(1024 * 1024 * 1024, "1GiB");
t((1024. * 1024. * 1024. * 1.2345) as u64, "1.2345GiB");
t(1024 * 1024 * 1024 * 1024, "1TiB");
t(1024 * 1024 * 1024 * 1024 * 1024, "1PiB");
t(1024 * 1024 * 1024 * 1024 * 1024 * 1024, "1EiB");
t(u64::MAX, "16EiB");

assert_eq!(
human_readable_bytes(1024 * 1024 * 1024 * 1024 * 1024 * 1024),
(1., "EiB")
&format!("{:.3}", HumanBytes((1024. * 1.23456) as u64)),
"1.234KiB"
);
assert_eq!(human_readable_bytes(u64::MAX), (16., "EiB"));
}
}
11 changes: 4 additions & 7 deletions tests/testsuite/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3424,7 +3424,7 @@ fn verify_packaged_status_line(
uncompressed_size: u64,
compressed_size: u64,
) {
use cargo::util::human_readable_bytes;
use cargo::util::HumanBytes;

let stderr = String::from_utf8(output.stderr).unwrap();
let mut packaged_lines = stderr
Expand All @@ -3438,12 +3438,9 @@ fn verify_packaged_status_line(
"Only one `Packaged` status line should appear in stderr"
);
let size_info = packaged_line.trim().trim_start_matches("Packaged").trim();
let uncompressed = human_readable_bytes(uncompressed_size);
let compressed = human_readable_bytes(compressed_size);
let expected = format!(
"{} files, {:.1}{} ({:.1}{} compressed)",
num_files, uncompressed.0, uncompressed.1, compressed.0, compressed.1
);
let uncompressed = HumanBytes(uncompressed_size);
let compressed = HumanBytes(compressed_size);
let expected = format!("{num_files} files, {uncompressed:.1} ({compressed:.1} compressed)");
assert_eq!(size_info, expected);
}

Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/progress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ fn always_shows_progress() {
p.cargo("check")
.with_stderr_data(
str![[r#"
[DOWNLOADING] [..] crate
[DOWNLOADED] 3 crates ([..]KB) in [..]s
[DOWNLOADING] [..] crate [..]
[DOWNLOADED] 3 crates ([..]) in [..]s
[BUILDING] [..] [..]/4: [..]
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
...
Expand Down