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

Author: epage

Cargo.lock

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

Cargo.toml

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

src/cargo/core/package.rs

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

src/cargo/ops/cargo_clean.rs

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

src/cargo/ops/cargo_package/mod.rs

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

src/cargo/sources/git/oxide.rs

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

src/cargo/sources/git/utils.rs

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

src/cargo/util/mod.rs

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

tests/testsuite/package.rs

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