Skip to content

Commit 9b9a0fc

Browse files
committed
make util::Progress threadsafe
Only then it will be possible to easily obtain just-in-time progress information from `gitoxide`. This also fixes a long-standing issue of transfer speeds never heading to zero if `git2` stops calling the progress function.
1 parent 2d3537d commit 9b9a0fc

File tree

9 files changed

+49
-40
lines changed

9 files changed

+49
-40
lines changed

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ libgit2-sys = "0.14.0"
4747
memchr = "2.1.3"
4848
opener = "0.5"
4949
os_info = "3.5.0"
50+
parking_lot = "0.12.1"
5051
pathdiff = "0.2"
5152
percent-encoding = "2.0"
5253
rustfix = "0.6.0"

src/cargo/core/compiler/job_queue.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ struct DrainState<'cfg> {
136136
documented: HashSet<PackageId>,
137137
scraped: HashSet<PackageId>,
138138
counts: HashMap<PackageId, usize>,
139-
progress: Progress<'cfg>,
139+
progress: Progress,
140140
next_id: u32,
141141
timings: Timings<'cfg>,
142142

src/cargo/core/package.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ pub struct Downloads<'a, 'cfg> {
329329
/// The next ID to use for creating a token (see `Download::token`).
330330
next: usize,
331331
/// Progress bar.
332-
progress: RefCell<Option<Progress<'cfg>>>,
332+
progress: RefCell<Option<Progress>>,
333333
/// Number of downloads that have successfully finished.
334334
downloads_finished: usize,
335335
/// Total bytes for all successfully downloaded packages.

src/cargo/core/shell.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ impl fmt::Debug for Shell {
7272
/// A `Write`able object, either with or without color support
7373
enum ShellOut {
7474
/// A plain write object without color support
75-
Write(Box<dyn Write>),
75+
Write(Box<dyn Write + Send>),
7676
/// Color-enabled stdio, with information on whether color should be used
7777
Stream {
7878
stdout: StandardStream,
@@ -111,7 +111,7 @@ impl Shell {
111111
}
112112

113113
/// Creates a shell from a plain writable object, with no color, and max verbosity.
114-
pub fn from_write(out: Box<dyn Write>) -> Shell {
114+
pub fn from_write(out: Box<dyn Write + Send>) -> Shell {
115115
Shell {
116116
output: ShellOut::Write(out),
117117
verbosity: Verbosity::Verbose,

src/cargo/ops/cargo_clean.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -315,14 +315,14 @@ trait CleaningProgressBar {
315315
fn on_clean(&mut self) -> CargoResult<()>;
316316
}
317317

318-
struct CleaningFolderBar<'cfg> {
319-
bar: Progress<'cfg>,
318+
struct CleaningFolderBar {
319+
bar: Progress,
320320
max: usize,
321321
cur: usize,
322322
}
323323

324-
impl<'cfg> CleaningFolderBar<'cfg> {
325-
fn new(cfg: &'cfg Config, max: usize) -> Self {
324+
impl CleaningFolderBar {
325+
fn new(cfg: &Config, max: usize) -> Self {
326326
Self {
327327
bar: Progress::with_style("Cleaning", ProgressStyle::Percentage, cfg),
328328
max,
@@ -335,7 +335,7 @@ impl<'cfg> CleaningFolderBar<'cfg> {
335335
}
336336
}
337337

338-
impl<'cfg> CleaningProgressBar for CleaningFolderBar<'cfg> {
338+
impl CleaningProgressBar for CleaningFolderBar {
339339
fn display_now(&mut self) -> CargoResult<()> {
340340
self.bar.tick_now(self.cur_progress(), self.max, "")
341341
}
@@ -346,16 +346,16 @@ impl<'cfg> CleaningProgressBar for CleaningFolderBar<'cfg> {
346346
}
347347
}
348348

349-
struct CleaningPackagesBar<'cfg> {
350-
bar: Progress<'cfg>,
349+
struct CleaningPackagesBar {
350+
bar: Progress,
351351
max: usize,
352352
cur: usize,
353353
num_files_folders_cleaned: usize,
354354
package_being_cleaned: String,
355355
}
356356

357-
impl<'cfg> CleaningPackagesBar<'cfg> {
358-
fn new(cfg: &'cfg Config, max: usize) -> Self {
357+
impl CleaningPackagesBar {
358+
fn new(cfg: &Config, max: usize) -> Self {
359359
Self {
360360
bar: Progress::with_style("Cleaning", ProgressStyle::Ratio, cfg),
361361
max,
@@ -384,7 +384,7 @@ impl<'cfg> CleaningPackagesBar<'cfg> {
384384
}
385385
}
386386

387-
impl<'cfg> CleaningProgressBar for CleaningPackagesBar<'cfg> {
387+
impl CleaningProgressBar for CleaningPackagesBar {
388388
fn display_now(&mut self) -> CargoResult<()> {
389389
self.bar
390390
.tick_now(self.cur_progress(), self.max, &self.format_message())

src/cargo/sources/git/oxide.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,15 @@ pub fn with_retry_and_progress(
5050
.unwrap_or_default()
5151
.auto_deregister();
5252
let should_interrupt = AtomicBool::new(false);
53-
let _progress_bar = Progress::new("Fetch", config);
53+
let mut progress_bar = Progress::new("Fetch", config);
5454
std::thread::scope(move |s| {
5555
s.spawn({
5656
let root = Arc::downgrade(&progress_root);
5757
move || -> CargoResult<()> {
5858
let mut tasks = Vec::with_capacity(10);
5959
while let Some(root) = root.upgrade() {
6060
root.sorted_snapshot(&mut tasks);
61+
progress_bar.tick(0, 10, "TBD")?;
6162
// dbg!(&tasks);
6263
std::thread::sleep(Duration::from_millis(300));
6364
}

src/cargo/sources/registry/http_remote.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ pub struct Downloads<'cfg> {
105105
/// The next ID to use for creating a token (see `Download::token`).
106106
next: usize,
107107
/// Progress bar.
108-
progress: RefCell<Option<Progress<'cfg>>>,
108+
progress: RefCell<Option<Progress>>,
109109
/// Number of downloads that have successfully finished.
110110
downloads_finished: usize,
111111
/// Number of times the caller has requested blocking. This is used for

src/cargo/util/config/mod.rs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
//! translate from `ConfigValue` and environment variables to the caller's
5050
//! desired type.
5151
52+
use parking_lot::{Mutex, MutexGuard};
5253
use std::borrow::Cow;
5354
use std::cell::{RefCell, RefMut};
5455
use std::collections::hash_map::Entry::{Occupied, Vacant};
@@ -62,7 +63,7 @@ use std::io::{self, SeekFrom};
6263
use std::mem;
6364
use std::path::{Path, PathBuf};
6465
use std::str::FromStr;
65-
use std::sync::Once;
66+
use std::sync::{Arc, Once};
6667
use std::time::Instant;
6768

6869
use self::ConfigValue as CV;
@@ -143,7 +144,7 @@ pub struct Config {
143144
/// The location of the user's Cargo home directory. OS-dependent.
144145
home_path: Filesystem,
145146
/// Information about how to write messages to the shell
146-
shell: RefCell<Shell>,
147+
shell: Arc<Mutex<Shell>>,
147148
/// A collection of configuration options
148149
values: LazyCell<HashMap<String, ConfigValue>>,
149150
/// A collection of configuration options from the credentials file
@@ -269,7 +270,7 @@ impl Config {
269270

270271
Config {
271272
home_path: Filesystem::new(homedir),
272-
shell: RefCell::new(shell),
273+
shell: Arc::new(Mutex::new(shell)),
273274
cwd,
274275
search_stop_path: None,
275276
values: LazyCell::new(),
@@ -368,8 +369,13 @@ impl Config {
368369
}
369370

370371
/// Gets a reference to the shell, e.g., for writing error messages.
371-
pub fn shell(&self) -> RefMut<'_, Shell> {
372-
self.shell.borrow_mut()
372+
pub fn shell(&self) -> MutexGuard<'_, Shell> {
373+
self.shell.lock()
374+
}
375+
376+
/// Gets a shared reference to the shell, e.g., for writing error messages, for use when writing from threads.
377+
pub fn shell_detached(&self) -> Arc<Mutex<Shell>> {
378+
Arc::clone(&self.shell)
373379
}
374380

375381
/// Gets the path to the `rustdoc` executable.
@@ -1261,9 +1267,7 @@ impl Config {
12611267
// --config path_to_file
12621268
let str_path = arg_as_path
12631269
.to_str()
1264-
.ok_or_else(|| {
1265-
anyhow::format_err!("config path {:?} is not utf-8", arg_as_path)
1266-
})?
1270+
.ok_or_else(|| format_err!("config path {:?} is not utf-8", arg_as_path))?
12671271
.to_string();
12681272
self._load_file(&self.cwd().join(&str_path), &mut seen, true, WhyLoad::Cli)
12691273
.with_context(|| format!("failed to load config from `{}`", str_path))?

src/cargo/util/progress.rs

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
1+
use parking_lot::Mutex;
12
use std::cmp;
23
use std::env;
4+
use std::sync::Arc;
35
use std::time::{Duration, Instant};
46

57
use crate::core::shell::Verbosity;
8+
use crate::core::Shell;
69
use crate::util::config::ProgressWhen;
710
use crate::util::{CargoResult, Config};
811
use cargo_util::is_ci;
912
use unicode_width::UnicodeWidthChar;
1013

11-
pub struct Progress<'cfg> {
12-
state: Option<State<'cfg>>,
14+
pub struct Progress {
15+
state: Option<State>,
1316
}
1417

1518
pub enum ProgressStyle {
@@ -23,8 +26,8 @@ struct Throttle {
2326
last_update: Instant,
2427
}
2528

26-
struct State<'cfg> {
27-
config: &'cfg Config,
29+
struct State {
30+
shell: Arc<Mutex<Shell>>,
2831
format: Format,
2932
name: String,
3033
done: bool,
@@ -39,8 +42,8 @@ struct Format {
3942
max_print: usize,
4043
}
4144

42-
impl<'cfg> Progress<'cfg> {
43-
pub fn with_style(name: &str, style: ProgressStyle, cfg: &'cfg Config) -> Progress<'cfg> {
45+
impl Progress {
46+
pub fn with_style(name: &str, style: ProgressStyle, cfg: &Config) -> Progress {
4447
// report no progress when -q (for quiet) or TERM=dumb are set
4548
// or if running on Continuous Integration service like Travis where the
4649
// output logs get mangled.
@@ -60,15 +63,15 @@ impl<'cfg> Progress<'cfg> {
6063
Progress::new_priv(name, style, cfg)
6164
}
6265

63-
fn new_priv(name: &str, style: ProgressStyle, cfg: &'cfg Config) -> Progress<'cfg> {
66+
fn new_priv(name: &str, style: ProgressStyle, cfg: &Config) -> Progress {
6467
let progress_config = cfg.progress_config();
6568
let width = progress_config
6669
.width
6770
.or_else(|| cfg.shell().err_width().progress_max_width());
6871

6972
Progress {
7073
state: width.map(|n| State {
71-
config: cfg,
74+
shell: cfg.shell_detached(),
7275
format: Format {
7376
style,
7477
max_width: n,
@@ -93,7 +96,7 @@ impl<'cfg> Progress<'cfg> {
9396
self.state.is_some()
9497
}
9598

96-
pub fn new(name: &str, cfg: &'cfg Config) -> Progress<'cfg> {
99+
pub fn new(name: &str, cfg: &Config) -> Progress {
97100
Self::with_style(name, ProgressStyle::Percentage, cfg)
98101
}
99102

@@ -180,7 +183,7 @@ impl Throttle {
180183
}
181184
}
182185

183-
impl<'cfg> State<'cfg> {
186+
impl State {
184187
fn tick(&mut self, cur: usize, max: usize, msg: &str) -> CargoResult<()> {
185188
if self.done {
186189
return Ok(());
@@ -215,8 +218,8 @@ impl<'cfg> State<'cfg> {
215218
}
216219

217220
// Only update if the line has changed.
218-
if self.config.shell().is_cleared() || self.last_line.as_ref() != Some(&line) {
219-
let mut shell = self.config.shell();
221+
if self.shell.lock().is_cleared() || self.last_line.as_ref() != Some(&line) {
222+
let mut shell = self.shell.lock();
220223
shell.set_needs_clear(false);
221224
shell.status_header(&self.name)?;
222225
write!(shell.err(), "{}\r", line)?;
@@ -229,15 +232,15 @@ impl<'cfg> State<'cfg> {
229232

230233
fn clear(&mut self) {
231234
// No need to clear if the progress is not currently being displayed.
232-
if self.last_line.is_some() && !self.config.shell().is_cleared() {
233-
self.config.shell().err_erase_line();
235+
if self.last_line.is_some() && !self.shell.lock().is_cleared() {
236+
self.shell.lock().err_erase_line();
234237
self.last_line = None;
235238
}
236239
}
237240

238241
fn try_update_max_width(&mut self) {
239242
if self.fixed_width.is_none() {
240-
if let Some(n) = self.config.shell().err_width().progress_max_width() {
243+
if let Some(n) = self.shell.lock().err_width().progress_max_width() {
241244
self.format.max_width = n;
242245
}
243246
}
@@ -323,7 +326,7 @@ impl Format {
323326
}
324327
}
325328

326-
impl<'cfg> Drop for State<'cfg> {
329+
impl Drop for State {
327330
fn drop(&mut self) {
328331
self.clear();
329332
}

0 commit comments

Comments
 (0)