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

### What does this PR try to resolve?

Just found we can simplify `human_readable_bytes` a bit.
This also remove the `bytesize` dependency,
since we already have a hand-rolled function.
(It is a good crate, though)

### How should we test and review this PR?

Should have no real functional difference.
`cargo package` starts reporting IEC (KiB) instead of SI (KB).
So do some thresholds of `cargo package`.

Could also switch entirely to `bytesize` if that is preferred.

### Additional information
This commit is contained in:
Ed Page 2025-02-28 14:32:42 +00:00 committed by GitHub
commit 8686e24c72
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 73 additions and 78 deletions

7
Cargo.lock generated
View File

@ -279,12 +279,6 @@ version = "1.10.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f61dac84819c6588b558454b194026eb1f09c293b9036ae9b159e74e73ab6cf9"
[[package]]
name = "bytesize"
version = "1.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a3e368af43e418a04d52505cf3dbc23dda4e3407ae2fa99fd0e4f308ce546acc"
[[package]]
name = "camino"
version = "1.1.9"
@ -314,7 +308,6 @@ dependencies = [
"anyhow",
"base64",
"blake3",
"bytesize",
"cargo-credential",
"cargo-credential-libsecret",
"cargo-credential-macos-keychain",

View File

@ -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" }
@ -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

View File

@ -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};
@ -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.
@ -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()))?;
@ -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)));
}
}
}
@ -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.

View File

@ -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};
@ -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

View File

@ -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;
@ -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));

View File

@ -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};
@ -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,

View File

@ -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;
@ -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)

View File

@ -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;
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);
(bytes / 1024_f32.powi(i as i32), UNITS[i])
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 {
@ -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"));
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 * 256),
(1.25, "MiB")
&format!("{:.3}", HumanBytes((1024. * 1.23456) as u64)),
"1.234KiB"
);
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")
);
assert_eq!(
human_readable_bytes(1024 * 1024 * 1024 * 1024 * 1024 * 1024),
(1., "EiB")
);
assert_eq!(human_readable_bytes(u64::MAX), (16., "EiB"));
}
}

View File

@ -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
@ -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);
}

View File

@ -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
...