Skip to content

Commit 5ea27ba

Browse files
committedSep 9, 2022
Change progress indicator for sparse registries
It's now based on an estimate of the depth of the dependency tree, so the progress bar will never go backwards.
1 parent 9467f81 commit 5ea27ba

File tree

2 files changed

+30
-29
lines changed

2 files changed

+30
-29
lines changed
 

‎src/cargo/sources/registry/http_remote.rs

+28-29
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use cargo_util::paths;
1414
use curl::easy::{HttpVersion, List};
1515
use curl::multi::{EasyHandle, Multi};
1616
use log::{debug, trace};
17-
use std::cell::{Cell, RefCell};
17+
use std::cell::RefCell;
1818
use std::collections::{HashMap, HashSet};
1919
use std::fs::{self, File};
2020
use std::path::{Path, PathBuf};
@@ -98,6 +98,9 @@ pub struct Downloads<'cfg> {
9898
progress: RefCell<Option<Progress<'cfg>>>,
9999
/// Number of downloads that have successfully finished.
100100
downloads_finished: usize,
101+
/// Number of times the caller has requested blocking. This is used for
102+
/// an estimate of progress.
103+
blocking_calls: usize,
101104
}
102105

103106
struct Download {
@@ -113,10 +116,6 @@ struct Download {
113116

114117
/// ETag or Last-Modified header received from the server (if any).
115118
index_version: RefCell<Option<String>>,
116-
117-
/// Statistics updated from the progress callback in libcurl.
118-
total: Cell<u64>,
119-
current: Cell<u64>,
120119
}
121120

122121
struct CompletedDownload {
@@ -159,10 +158,11 @@ impl<'cfg> HttpRegistry<'cfg> {
159158
results: HashMap::new(),
160159
progress: RefCell::new(Some(Progress::with_style(
161160
"Fetch",
162-
ProgressStyle::Ratio,
161+
ProgressStyle::Indeterminate,
163162
config,
164163
))),
165164
downloads_finished: 0,
165+
blocking_calls: 0,
166166
},
167167
fresh: HashSet::new(),
168168
requested_update: false,
@@ -457,15 +457,6 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
457457
Ok(buf.len())
458458
})?;
459459

460-
// Same goes for the progress function -- it goes through thread-local storage.
461-
handle.progress(true)?;
462-
handle.progress_function(move |dl_total, dl_cur, _, _| {
463-
tls::with(|downloads| match downloads {
464-
Some(d) => d.progress(token, dl_total as u64, dl_cur as u64),
465-
None => false,
466-
})
467-
})?;
468-
469460
// And ditto for the header function.
470461
handle.header_function(move |buf| {
471462
if let Some((tag, value)) = Self::handle_http_header(buf) {
@@ -494,8 +485,6 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
494485
data: RefCell::new(Vec::new()),
495486
path: path.to_path_buf(),
496487
index_version: RefCell::new(None),
497-
total: Cell::new(0),
498-
current: Cell::new(0),
499488
};
500489

501490
// Finally add the request we've lined up to the pool of requests that cURL manages.
@@ -588,11 +577,11 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
588577
}
589578

590579
fn block_until_ready(&mut self) -> CargoResult<()> {
591-
let initial_pending_count = self.downloads.pending.len();
592580
trace!(
593581
"block_until_ready: {} transfers pending",
594-
initial_pending_count
582+
self.downloads.pending.len()
595583
);
584+
self.downloads.blocking_calls += 1;
596585

597586
loop {
598587
self.handle_completed_downloads()?;
@@ -622,21 +611,31 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
622611
}
623612

624613
impl<'cfg> Downloads<'cfg> {
625-
fn progress(&self, token: usize, total: u64, cur: u64) -> bool {
626-
let dl = &self.pending[&token].0;
627-
dl.total.set(total);
628-
dl.current.set(cur);
629-
true
630-
}
631-
632614
fn tick(&self) -> CargoResult<()> {
633615
let mut progress = self.progress.borrow_mut();
634616
let progress = progress.as_mut().unwrap();
635617

618+
// Since the sparse protocol discovers dependencies as it goes,
619+
// it's not possible to get an accurate progress indication.
620+
//
621+
// As an approximation, we assume that the depth of the dependency graph
622+
// is fixed, and base the progress on how many times the caller has asked
623+
// for blocking. If there are actually additional dependencies, the progress
624+
// bar will get stuck. If there are fewer dependencies, it will disappear
625+
// early. It will never go backwards.
626+
//
627+
// The status text also contains the number of completed & pending requests, which
628+
// gives an better indication of forward progress.
629+
let approximate_tree_depth = 10;
630+
636631
progress.tick(
637-
self.downloads_finished,
638-
self.downloads_finished + self.pending.len(),
639-
"",
632+
self.blocking_calls.min(approximate_tree_depth),
633+
approximate_tree_depth + 1,
634+
&format!(
635+
" {} complete; {} pending",
636+
self.downloads_finished,
637+
self.pending.len()
638+
),
640639
)
641640
}
642641
}

‎src/cargo/util/progress.rs

+2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ pub struct Progress<'cfg> {
1515
pub enum ProgressStyle {
1616
Percentage,
1717
Ratio,
18+
Indeterminate,
1819
}
1920

2021
struct Throttle {
@@ -253,6 +254,7 @@ impl Format {
253254
let stats = match self.style {
254255
ProgressStyle::Percentage => format!(" {:6.02}%", pct * 100.0),
255256
ProgressStyle::Ratio => format!(" {}/{}", cur, max),
257+
ProgressStyle::Indeterminate => String::new(),
256258
};
257259
let extra_len = stats.len() + 2 /* [ and ] */ + 15 /* status header */;
258260
let display_width = match self.width().checked_sub(extra_len) {

0 commit comments

Comments
 (0)
Please sign in to comment.