Skip to content
Merged
9 changes: 5 additions & 4 deletions src/cli/download_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ impl DownloadTracker {
struct Duration(f64);

impl fmt::Display for Duration {
#[allow(clippy::many_single_char_names)]
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// repurposing the alternate mode for ETA
let sec = self.0;
Expand Down Expand Up @@ -258,13 +259,13 @@ mod tests {

assert_eq!(DownloadTracker::from_seconds(60), (0, 0, 1, 0));

assert_eq!(DownloadTracker::from_seconds(3600), (0, 1, 0, 0));
assert_eq!(DownloadTracker::from_seconds(3_600), (0, 1, 0, 0));

assert_eq!(DownloadTracker::from_seconds(3600 * 24), (1, 0, 0, 0));
assert_eq!(DownloadTracker::from_seconds(3_600 * 24), (1, 0, 0, 0));

assert_eq!(DownloadTracker::from_seconds(52292), (0, 14, 31, 32));
assert_eq!(DownloadTracker::from_seconds(52_292), (0, 14, 31, 32));

assert_eq!(DownloadTracker::from_seconds(222292), (2, 13, 44, 52));
assert_eq!(DownloadTracker::from_seconds(222_292), (2, 13, 44, 52));
}

}
1 change: 1 addition & 0 deletions src/cli/errors.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(clippy::large_enum_variant)]
#![allow(dead_code)]

use crate::rustup_mode::CompletionCommand;
Expand Down
2 changes: 1 addition & 1 deletion src/cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn run_rustup() -> Result<()> {
open_trace_file!(dir)?;
}
let result = run_rustup_inner();
if let Ok(_) = env::var("RUSTUP_TRACE_DIR") {
if env::var("RUSTUP_TRACE_DIR").is_ok() {
close_trace_file!();
}
result
Expand Down
2 changes: 1 addition & 1 deletion src/cli/rustup_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1146,7 +1146,7 @@ pub enum CompletionCommand {
Cargo,
}

static COMPLETIONS: &[(&'static str, CompletionCommand)] = &[
static COMPLETIONS: &[(&str, CompletionCommand)] = &[
("rustup", CompletionCommand::Rustup),
("cargo", CompletionCommand::Cargo),
];
Expand Down
2 changes: 1 addition & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl Cfg {
);
let dist_root = dist_root_server.clone() + "/dist";

let cfg = Cfg {
let cfg = Self {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a new lint I was previously unaware of?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just enabled the lint in this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aah I see. This one is a pedantic lint and I'm not sure I completely care for it. I won't say "no" this time, but it feels a bit overly pedantic for my liking. Can make it harder to tell what something is if it's overused IMO.

rustup_dir,
settings_file,
toolchains_dir,
Expand Down
7 changes: 4 additions & 3 deletions src/diskio/immediate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@
/// threaded code paths.
use super::{perform, Executor, Item};

#[derive(Default)]
pub struct ImmediateUnpacker {}
impl ImmediateUnpacker {
pub fn new<'a>() -> ImmediateUnpacker {
ImmediateUnpacker {}
pub fn new() -> Self {
Self {}
}
}

impl Executor for ImmediateUnpacker {
fn dispatch(&mut self, mut item: Item) -> Box<dyn '_ + Iterator<Item = Item>> {
fn dispatch(&mut self, mut item: Item) -> Box<dyn Iterator<Item = Item> + '_> {
perform(&mut item);
Box::new(Some(item).into_iter())
}
Expand Down
26 changes: 12 additions & 14 deletions src/diskio/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub struct Item {

impl Item {
pub fn make_dir(full_path: PathBuf, mode: u32) -> Self {
Item {
Self {
full_path,
kind: Kind::Directory,
start: 0.0,
Expand All @@ -102,7 +102,7 @@ impl Item {

pub fn write_file(full_path: PathBuf, content: Vec<u8>, mode: u32) -> Self {
let len = content.len();
Item {
Self {
full_path,
kind: Kind::File(content),
start: 0.0,
Expand All @@ -122,24 +122,24 @@ pub trait Executor {
/// During overload situations previously queued items may
/// need to be completed before the item is accepted:
/// consume the returned iterator.
fn execute(&mut self, mut item: Item) -> Box<dyn '_ + Iterator<Item = Item>> {
fn execute(&mut self, mut item: Item) -> Box<dyn Iterator<Item = Item> + '_> {
item.start = precise_time_s();
self.dispatch(item)
}

/// Actually dispatch a operation.
/// This is called by the default execute() implementation and
/// should not be called directly.
fn dispatch(&mut self, item: Item) -> Box<dyn '_ + Iterator<Item = Item>>;
fn dispatch(&mut self, item: Item) -> Box<dyn Iterator<Item = Item> + '_>;

/// Wrap up any pending operations and iterate over them.
/// All operations submitted before the join will have been
/// returned either through ready/complete or join once join
/// returns.
fn join(&mut self) -> Box<dyn '_ + Iterator<Item = Item>>;
fn join(&mut self) -> Box<dyn Iterator<Item = Item> + '_>;

/// Iterate over completed items.
fn completed(&mut self) -> Box<dyn '_ + Iterator<Item = Item>>;
fn completed(&mut self) -> Box<dyn Iterator<Item = Item> + '_>;
}

/// Trivial single threaded IO to be used from executors.
Expand Down Expand Up @@ -200,15 +200,13 @@ pub fn get_executor<'a>(
if let Ok(thread_str) = env::var("RUSTUP_IO_THREADS") {
if thread_str == "disabled" {
Box::new(immediate::ImmediateUnpacker::new())
} else if let Ok(thread_count) = thread_str.parse::<usize>() {
Box::new(threaded::Threaded::new_with_threads(
notify_handler,
thread_count,
))
} else {
if let Ok(thread_count) = thread_str.parse::<usize>() {
Box::new(threaded::Threaded::new_with_threads(
notify_handler,
thread_count,
))
} else {
Box::new(threaded::Threaded::new(notify_handler))
}
Box::new(threaded::Threaded::new(notify_handler))
}
} else {
Box::new(threaded::Threaded::new(notify_handler))
Expand Down
31 changes: 15 additions & 16 deletions src/diskio/threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ impl<'a> Threaded<'a> {
}

impl<'a> Executor for Threaded<'a> {
fn dispatch(&mut self, item: Item) -> Box<dyn '_ + Iterator<Item = Item>> {
fn dispatch(&mut self, item: Item) -> Box<dyn Iterator<Item = Item> + '_> {
// Yield any completed work before accepting new work - keep memory
// pressure under control
// - return an iterator that runs until we can submit and then submits
Expand All @@ -100,7 +100,7 @@ impl<'a> Executor for Threaded<'a> {
})
}

fn join(&mut self) -> Box<dyn '_ + Iterator<Item = Item>> {
fn join(&mut self) -> Box<dyn Iterator<Item = Item> + '_> {
// Some explanation is in order. Even though the tar we are reading from (if
// any) will have had its FileWithProgress download tracking
// completed before we hit drop, that is not true if we are unwinding due to a
Expand All @@ -115,16 +115,14 @@ impl<'a> Executor for Threaded<'a> {
// items, and the download tracker's progress is confounded with
// actual handling of data today, we synthesis a data buffer and
// pretend to have bytes to deliver.
self.notify_handler
.map(|handler| handler(Notification::DownloadFinished));
self.notify_handler
.map(|handler| handler(Notification::DownloadPushUnits("iops")));
let mut prev_files = self.n_files.load(Ordering::Relaxed);
self.notify_handler.map(|handler| {
if let Some(handler) = self.notify_handler {
handler(Notification::DownloadFinished);
handler(Notification::DownloadPushUnits("iops"));
handler(Notification::DownloadContentLengthReceived(
prev_files as u64,
))
});
));
}
Comment thread
tesuji marked this conversation as resolved.
if prev_files > 50 {
println!("{} deferred IO operations", prev_files);
}
Expand All @@ -139,14 +137,15 @@ impl<'a> Executor for Threaded<'a> {
prev_files = current_files;
current_files = self.n_files.load(Ordering::Relaxed);
let step_count = prev_files - current_files;
self.notify_handler
.map(|handler| handler(Notification::DownloadDataReceived(&buf[0..step_count])));
if let Some(handler) = self.notify_handler {
handler(Notification::DownloadDataReceived(&buf[0..step_count]));
}
}
self.pool.join();
self.notify_handler
.map(|handler| handler(Notification::DownloadFinished));
self.notify_handler
.map(|handler| handler(Notification::DownloadPopUnits));
if let Some(handler) = self.notify_handler {
handler(Notification::DownloadFinished);
handler(Notification::DownloadPopUnits);
}
Comment thread
tesuji marked this conversation as resolved.
// close the feedback channel so that blocking reads on it can
// complete. send is atomic, and we know the threads completed from the
// pool join, so this is race-free. It is possible that try_iter is safe
Expand All @@ -167,7 +166,7 @@ impl<'a> Executor for Threaded<'a> {
})
}

fn completed(&mut self) -> Box<dyn '_ + Iterator<Item = Item>> {
fn completed(&mut self) -> Box<dyn Iterator<Item = Item> + '_> {
Box::new(JoinIterator {
iter: self.rx.try_iter(),
consume_sentinel: true,
Expand Down
4 changes: 2 additions & 2 deletions src/dist/component/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub struct Components {

impl Components {
pub fn open(prefix: InstallPrefix) -> Result<Self> {
let c = Components { prefix };
let c = Self { prefix };

// Validate that the metadata uses a format we know
if let Some(v) = c.read_version()? {
Expand Down Expand Up @@ -152,7 +152,7 @@ impl ComponentPart {
}
pub fn decode(line: &str) -> Option<Self> {
line.find(':')
.map(|pos| ComponentPart(line[0..pos].to_owned(), PathBuf::from(&line[(pos + 1)..])))
.map(|pos| Self(line[0..pos].to_owned(), PathBuf::from(&line[(pos + 1)..])))
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/dist/component/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl DirectoryPackage {
.lines()
.map(std::borrow::ToOwned::to_owned)
.collect();
Ok(DirectoryPackage {
Ok(Self {
path,
components,
copy,
Expand Down Expand Up @@ -165,7 +165,7 @@ struct MemoryBudget {

// Probably this should live in diskio but ¯\_(ツ)_/¯
impl MemoryBudget {
fn new(max_file_size: usize) -> MemoryBudget {
fn new(max_file_size: usize) -> Self {
const DEFAULT_UNPACK_RAM: usize = 400 * 1024 * 1024;
let unpack_ram = if let Ok(budget_str) = env::var("RUSTUP_UNPACK_RAM") {
if let Ok(budget) = budget_str.parse::<usize>() {
Expand All @@ -179,7 +179,7 @@ impl MemoryBudget {
if max_file_size > unpack_ram {
panic!("RUSTUP_UNPACK_RAM must be larger than {}", max_file_size);
}
MemoryBudget {
Self {
limit: unpack_ram,
used: 0,
}
Expand Down
4 changes: 2 additions & 2 deletions src/dist/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl Config {
let components =
Self::toml_to_components(components, &format!("{}{}.", path, "components"))?;

Ok(Config {
Ok(Self {
config_version: version,
components,
})
Expand Down Expand Up @@ -77,7 +77,7 @@ impl Config {

impl Default for Config {
fn default() -> Self {
Config {
Self {
config_version: DEFAULT_CONFIG_VERSION.to_owned(),
components: Vec::new(),
}
Expand Down
16 changes: 8 additions & 8 deletions src/dist/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,14 @@ static TRIPLE_MIPS64_UNKNOWN_LINUX_GNUABI64: &str = "mips64el-unknown-linux-gnua

impl TargetTriple {
pub fn new(name: &str) -> Self {
TargetTriple(name.to_string())
Self(name.to_string())
}

pub fn from_build() -> Self {
if let Some(triple) = option_env!("RUSTUP_OVERRIDE_BUILD_TRIPLE") {
TargetTriple::new(triple)
Self::new(triple)
} else {
TargetTriple::new(env!("TARGET"))
Self::new(env!("TARGET"))
}
}

Expand Down Expand Up @@ -207,7 +207,7 @@ impl TargetTriple {
}

if let Ok(triple) = env::var("RUSTUP_OVERRIDE_HOST_TRIPLE") {
Some(TargetTriple(triple))
Some(Self(triple))
} else {
inner()
}
Expand All @@ -221,7 +221,7 @@ impl TargetTriple {
impl PartialTargetTriple {
pub fn new(name: &str) -> Option<Self> {
if name.is_empty() {
return Some(PartialTargetTriple {
return Some(Self {
arch: None,
os: None,
env: None,
Expand Down Expand Up @@ -250,7 +250,7 @@ impl PartialTargetTriple {
}
}

PartialTargetTriple {
Self {
arch: c.get(1).map(|s| s.as_str()).and_then(fn_map),
os: c.get(2).map(|s| s.as_str()).and_then(fn_map),
env: c.get(3).map(|s| s.as_str()).and_then(fn_map),
Expand Down Expand Up @@ -288,7 +288,7 @@ impl FromStr for PartialToolchainDesc {

let trip = c.get(3).map(|c| c.as_str()).unwrap_or("");
let trip = PartialTargetTriple::new(&trip);
trip.map(|t| PartialToolchainDesc {
trip.map(|t| Self {
channel: c.get(1).unwrap().as_str().to_owned(),
date: c.get(2).map(|s| s.as_str()).and_then(fn_map),
target: t,
Expand Down Expand Up @@ -382,7 +382,7 @@ impl FromStr for ToolchainDesc {
}
}

ToolchainDesc {
Self {
channel: c.get(1).unwrap().as_str().to_owned(),
date: c.get(2).map(|s| s.as_str()).and_then(fn_map),
target: TargetTriple(c.get(3).unwrap().as_str().to_owned()),
Expand Down
Loading