Skip to content

build: migrate the codebase to the 2024 Language Edition #4191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ members = ["download"]

[workspace.package]
version = "1.28.0"
edition = "2021"
edition = "2024"
license = "MIT OR Apache-2.0"

[workspace.dependencies]
Expand Down
38 changes: 24 additions & 14 deletions download/tests/read-proxy-env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,30 @@ use url::Url;

static SERIALISE_TESTS: LazyLock<Mutex<()>> = LazyLock::new(|| Mutex::new(()));

fn scrub_env() {
remove_var("http_proxy");
remove_var("https_proxy");
remove_var("HTTPS_PROXY");
remove_var("ftp_proxy");
remove_var("FTP_PROXY");
remove_var("all_proxy");
remove_var("ALL_PROXY");
remove_var("no_proxy");
remove_var("NO_PROXY");
unsafe fn scrub_env() {
unsafe {
remove_var("http_proxy");
remove_var("https_proxy");
remove_var("HTTPS_PROXY");
remove_var("ftp_proxy");
remove_var("FTP_PROXY");
remove_var("all_proxy");
remove_var("ALL_PROXY");
remove_var("no_proxy");
remove_var("NO_PROXY");
}
}

// Tests for correctly retrieving the proxy (host, port) tuple from $https_proxy
#[tokio::test]
async fn read_basic_proxy_params() {
let _guard = SERIALISE_TESTS.lock().await;
scrub_env();
set_var("https_proxy", "http://proxy.example.com:8080");
// SAFETY: We are setting environment variables when `SERIALISE_TESTS` is locked,
// and those environment variables in question are not relevant elsewhere in the test suite.
unsafe {
scrub_env();
set_var("https_proxy", "http://proxy.example.com:8080");
}
let u = Url::parse("https://www.example.org").ok().unwrap();
assert_eq!(
for_url(&u).host_port(),
Expand All @@ -46,8 +52,12 @@ async fn socks_proxy_request() {
static CALL_COUNT: AtomicUsize = AtomicUsize::new(0);
let _guard = SERIALISE_TESTS.lock().await;

scrub_env();
set_var("all_proxy", "socks5://127.0.0.1:1080");
// SAFETY: We are setting environment variables when `SERIALISE_TESTS` is locked,
// and those environment variables in question are not relevant elsewhere in the test suite.
unsafe {
scrub_env();
set_var("all_proxy", "socks5://127.0.0.1:1080");
}

thread::spawn(move || {
let listener = TcpListener::bind("127.0.0.1:1080").unwrap();
Expand Down
1 change: 0 additions & 1 deletion rustfmt.toml

This file was deleted.

80 changes: 41 additions & 39 deletions src/cli/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,47 +58,49 @@ mod imp {
}

pub(crate) unsafe fn setup() -> Option<Setup> {
// Creates a new job object for us to use and then adds ourselves to it.
// Note that all errors are basically ignored in this function,
// intentionally. Job objects are "relatively new" in Windows,
// particularly the ability to support nested job objects. Older
// Windows installs don't support this ability. We probably don't want
// to force Cargo to abort in this situation or force others to *not*
// use job objects, so we instead just ignore errors and assume that
// we're otherwise part of someone else's job object in this case.

let job = CreateJobObjectW(ptr::null_mut(), ptr::null());
if job.is_null() {
return None;
}
let job = Handle { inner: job };

// Indicate that when all handles to the job object are gone that all
// process in the object should be killed. Note that this includes our
// entire process tree by default because we've added ourselves and
// our children will reside in the job once we spawn a process.
let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION;
info = mem::zeroed();
info.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
let r = SetInformationJobObject(
job.inner,
JobObjectExtendedLimitInformation,
&mut info as *mut _ as *const std::ffi::c_void,
mem::size_of_val(&info) as u32,
);
if r == 0 {
return None;
}
unsafe {
// Creates a new job object for us to use and then adds ourselves to it.
// Note that all errors are basically ignored in this function,
// intentionally. Job objects are "relatively new" in Windows,
// particularly the ability to support nested job objects. Older
// Windows installs don't support this ability. We probably don't want
// to force Cargo to abort in this situation or force others to *not*
// use job objects, so we instead just ignore errors and assume that
// we're otherwise part of someone else's job object in this case.

let job = CreateJobObjectW(ptr::null_mut(), ptr::null());
if job.is_null() {
return None;
}
let job = Handle { inner: job };

// Indicate that when all handles to the job object are gone that all
// process in the object should be killed. Note that this includes our
// entire process tree by default because we've added ourselves and
// our children will reside in the job once we spawn a process.
let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION;
info = mem::zeroed();
info.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
let r = SetInformationJobObject(
job.inner,
JobObjectExtendedLimitInformation,
&mut info as *mut _ as *const std::ffi::c_void,
mem::size_of_val(&info) as u32,
);
if r == 0 {
return None;
}

// Assign our process to this job object, meaning that our children will
// now live or die based on our existence.
let me = GetCurrentProcess();
let r = AssignProcessToJobObject(job.inner, me);
if r == 0 {
return None;
}
// Assign our process to this job object, meaning that our children will
// now live or die based on our existence.
let me = GetCurrentProcess();
let r = AssignProcessToJobObject(job.inner, me);
if r == 0 {
return None;
}

Some(Setup { job })
Some(Setup { job })
}
}

impl Drop for Setup {
Expand Down
6 changes: 3 additions & 3 deletions src/cli/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::{process::Process, utils::notify::NotificationLevel};
pub fn tracing_subscriber(
process: &Process,
) -> (
impl tracing::Subscriber,
impl tracing::Subscriber + use<>,
reload::Handle<EnvFilter, Registry>,
) {
#[cfg(feature = "otel")]
Expand All @@ -45,7 +45,7 @@ pub fn tracing_subscriber(
/// When the `RUSTUP_LOG` environment variable is present, a standard [`tracing_subscriber`]
/// formatter will be used according to the filtering directives set in its value.
/// Otherwise, this logger will use [`EventFormatter`] to mimic "classic" Rustup `stderr` output.
fn console_logger<S>(process: &Process) -> (impl Layer<S>, reload::Handle<EnvFilter, S>)
fn console_logger<S>(process: &Process) -> (impl Layer<S> + use<S>, reload::Handle<EnvFilter, S>)
where
S: Subscriber + for<'span> LookupSpan<'span>,
{
Expand Down Expand Up @@ -129,7 +129,7 @@ impl NotificationLevel {
/// A [`tracing::Subscriber`] [`Layer`][`tracing_subscriber::Layer`] that corresponds to Rustup's
/// optional `opentelemetry` (a.k.a. `otel`) feature.
#[cfg(feature = "otel")]
fn telemetry<S>(process: &Process) -> impl Layer<S>
fn telemetry<S>(process: &Process) -> impl Layer<S> + use<S>
Copy link
Member Author

@rami3l rami3l Feb 22, 2025

Choose a reason for hiding this comment

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

@djc So basically impl Layer<S> + use<S> means "this impl Layer<S> has nothing to do with 'process" (this function is generic over both 'process and S, so we use<S> to exclude 'process), and the same thing goes for other fixes in this commit.

where
S: Subscriber + for<'span> LookupSpan<'span>,
{
Expand Down
69 changes: 32 additions & 37 deletions src/diskio/immediate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ impl Executor for ImmediateUnpacker {
fn dispatch(&self, mut item: Item) -> Box<dyn Iterator<Item = CompletedIo> + '_> {
item.result = match &mut item.kind {
super::Kind::Directory => super::create_dir(&item.full_path),
super::Kind::File(ref contents) => {
if let super::FileBuffer::Immediate(ref contents) = &contents {
super::Kind::File(contents) => {
if let super::FileBuffer::Immediate(contents) = &contents {
super::write_file(&item.full_path, contents, item.mode)
} else {
unreachable!()
Expand All @@ -81,21 +81,20 @@ impl Executor for ImmediateUnpacker {
// If there is a pending error, return it, otherwise stash the
// Item for eventual return when the file is finished.
let mut guard = self.incremental_state.lock().unwrap();
if let Some(ref mut state) = *guard {
if state.err.is_some() {
let err = state.err.take().unwrap();
item.result = err;
item.finish = item
.start
.map(|s| Instant::now().saturating_duration_since(s));
*guard = None;
Box::new(Some(CompletedIo::Item(item)).into_iter())
} else {
state.item = Some(item);
Box::new(None.into_iter())
}
let Some(ref mut state) = *guard else {
unreachable!()
};
if state.err.is_some() {
let err = state.err.take().unwrap();
item.result = err;
item.finish = item
.start
.map(|s| Instant::now().saturating_duration_since(s));
*guard = None;
Box::new(Some(CompletedIo::Item(item)).into_iter())
} else {
unreachable!();
state.item = Some(item);
Box::new(None.into_iter())
}
};
}
Expand Down Expand Up @@ -181,9 +180,7 @@ impl IncrementalFileWriter {
if (self.state.lock().unwrap()).is_none() {
return false;
}
let chunk = if let FileBuffer::Immediate(v) = chunk {
v
} else {
let FileBuffer::Immediate(chunk) = chunk else {
unreachable!()
};
match self.write(chunk) {
Expand All @@ -203,25 +200,23 @@ impl IncrementalFileWriter {

fn write(&mut self, chunk: Vec<u8>) -> std::result::Result<bool, io::Error> {
let mut state = self.state.lock().unwrap();
if let Some(ref mut state) = *state {
if let Some(ref mut file) = self.file.as_mut() {
// Length 0 vector is used for clean EOF signalling.
if chunk.is_empty() {
trace_scoped!("close", "name:": self.path_display);
drop(std::mem::take(&mut self.file));
state.finished = true;
} else {
trace_scoped!("write_segment", "name": self.path_display, "len": chunk.len());
file.write_all(&chunk)?;

state.completed_chunks.push(chunk.len());
}
Ok(true)
} else {
Ok(false)
}
let Some(ref mut state) = *state else {
unreachable!()
};
let Some(ref mut file) = self.file.as_mut() else {
return Ok(false);
};
// Length 0 vector is used for clean EOF signalling.
if chunk.is_empty() {
trace_scoped!("close", "name:": self.path_display);
drop(std::mem::take(&mut self.file));
state.finished = true;
} else {
unreachable!();
trace_scoped!("write_segment", "name": self.path_display, "len": chunk.len());
file.write_all(&chunk)?;

state.completed_chunks.push(chunk.len());
}
Ok(true)
}
}
18 changes: 7 additions & 11 deletions src/diskio/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,14 @@ pub(crate) enum FileBuffer {
impl FileBuffer {
/// All the buffers space to be re-used when the last reference to it is dropped.
pub(crate) fn clear(&mut self) {
if let FileBuffer::Threaded(ref mut contents) = self {
if let FileBuffer::Threaded(contents) = self {
contents.clear()
}
}

pub(crate) fn len(&self) -> usize {
match self {
FileBuffer::Immediate(ref vec) => vec.len(),
FileBuffer::Immediate(vec) => vec.len(),
FileBuffer::Threaded(PoolReference::Owned(owned, _)) => owned.len(),
FileBuffer::Threaded(PoolReference::Mut(mutable, _)) => mutable.len(),
}
Expand All @@ -109,7 +109,7 @@ impl Deref for FileBuffer {

fn deref(&self) -> &Self::Target {
match self {
FileBuffer::Immediate(ref vec) => vec,
FileBuffer::Immediate(vec) => vec,
FileBuffer::Threaded(PoolReference::Owned(owned, _)) => owned,
FileBuffer::Threaded(PoolReference::Mut(mutable, _)) => mutable,
}
Expand All @@ -119,7 +119,7 @@ impl Deref for FileBuffer {
impl DerefMut for FileBuffer {
fn deref_mut(&mut self) -> &mut Self::Target {
match self {
FileBuffer::Immediate(ref mut vec) => vec,
FileBuffer::Immediate(vec) => vec,
FileBuffer::Threaded(PoolReference::Owned(_, _)) => {
unimplemented!()
}
Expand Down Expand Up @@ -337,15 +337,11 @@ pub(crate) fn perform<F: Fn(usize)>(item: &mut Item, chunk_complete_callback: F)
// Files, write them.
item.result = match &mut item.kind {
Kind::Directory => create_dir(&item.full_path),
Kind::File(ref mut contents) => {
Kind::File(contents) => {
contents.clear();
match contents {
FileBuffer::Immediate(ref contents) => {
write_file(&item.full_path, contents, item.mode)
}
FileBuffer::Threaded(ref mut contents) => {
write_file(&item.full_path, contents, item.mode)
}
FileBuffer::Immediate(contents) => write_file(&item.full_path, contents, item.mode),
FileBuffer::Threaded(contents) => write_file(&item.full_path, contents, item.mode),
}
}
Kind::IncrementalFile(incremental_file) => write_file_incremental(
Expand Down
Loading