Skip to content

refactor(network): convert .crate downloads to use http_async#16902

Merged
weihanglo merged 4 commits intorust-lang:masterfrom
arlosi:async/pkgset
Apr 20, 2026
Merged

refactor(network): convert .crate downloads to use http_async#16902
weihanglo merged 4 commits intorust-lang:masterfrom
arlosi:async/pkgset

Conversation

@arlosi
Copy link
Copy Markdown
Contributor

@arlosi arlosi commented Apr 17, 2026

What does this PR try to resolve?

  • Makes the existing download and finish_download methods async. It may be possible to do a future refactor PR that combines these two methods together, since they no longer need to be separated to get parallel downloads. This could be a nice future simplification.
  • Remove the direct use of curl::Multi for downloading crates in parallel, and replace it with http_async.
  • Add .unordered() to several test file references. The async downloader is less deterministic in the order that it performs downloads, which would otherwise make the tests flaky.

cc #16845

How to test and review this PR?

Commit by commit. All commits pass tests.

r? @weihanglo

@rustbot rustbot added A-directory-source Area: directory sources (vendoring) A-git Area: anything dealing with git A-local-registry-source Area: local registry sources (vendoring) A-networking Area: networking issues, curl, etc. A-overrides Area: general issues with overriding dependencies (patch, replace, paths) A-registries Area: registries A-source-replacement Area: [source] replacement A-sparse-registry Area: http sparse registries S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 17, 2026
@arlosi arlosi force-pushed the async/pkgset branch 2 times, most recently from 9f8d227 to cbbd5c4 Compare April 17, 2026 17:27
@arlosi arlosi mentioned this pull request Apr 17, 2026
6 tasks
Comment thread src/cargo/util/network/http_async.rs
Comment thread src/cargo/core/package.rs
Comment thread src/cargo/util/network/http_async.rs Outdated
Comment thread src/cargo/util/network/http_async.rs Outdated
Comment thread src/cargo/util/network/http_async.rs
@arlosi
Copy link
Copy Markdown
Contributor Author

arlosi commented Apr 17, 2026

Split off part of this change into #16903. Will wait until that one is in, then rebase this one.

@rustbot

This comment has been minimized.

arlosi added 4 commits April 17, 2026 19:22
Makes the two crate downloading methods async
Removes the existing Multi-based interface to use the http_async
abstraction for downloading crates.
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 18, 2026

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Comment thread src/cargo/core/package.rs
.get(&id)
.ok_or_else(|| internal(format!("couldn't find `{}` in package set", id)))?;
if let Some(pkg) = slot.get() {
return CargoResult::Ok(pkg);
Copy link
Copy Markdown
Member

@weihanglo weihanglo Apr 20, 2026

Choose a reason for hiding this comment

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

nits, and ditto in this file

Suggested change
return CargoResult::Ok(pkg);
return Ok(pkg);

View changes since the review

Comment thread src/cargo/core/package.rs
/// Returns `None` if the package is queued up for download and will
/// eventually be returned from `wait_for_download`. Returns `Some(pkg)` if
/// the package is ready and doesn't need to be downloaded.
#[tracing::instrument(skip_all)]
Copy link
Copy Markdown
Member

@weihanglo weihanglo Apr 20, 2026

Choose a reason for hiding this comment

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

We lose this instrument. Do we have alternative?

(This is not a blocker)

View changes since the review

Copy link
Copy Markdown
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Actually. This is good.

View changes since this review

@weihanglo weihanglo added this pull request to the merge queue Apr 20, 2026
Merged via the queue into rust-lang:master with commit 72d80ec Apr 20, 2026
31 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 20, 2026
rust-bors Bot pushed a commit to rust-lang/rust that referenced this pull request Apr 21, 2026
Update cargo submodule

9 commits in 7ecf0285ebb408d596e4a8ac76a0980d8edb7005..06ac0e7c05770a8c7bbf67bdd12fa1a1eefdc8ae
2026-04-18 15:34:11 +0000 to 2026-04-21 15:33:56 +0000
- test(git): add regression test for full-hash GitHub fast path (rust-lang/cargo#16919)
- fix(help): add `.1` extension to man page temp file (rust-lang/cargo#16917)
- Fix flaky test: sparse_blocking_count (rust-lang/cargo#16916)
- Fix flaky test compile_offline_while_transitive_dep_not_cached (rust-lang/cargo#16915)
- Fix test fetch_all_platform_dependencies_when_no_target_is_given (rust-lang/cargo#16914)
- chore(ci): Use `actions/deploy-pages` for Cargo Contributor Guide deployment (rust-lang/cargo#16876)
- Convert GitHub fast path to use http_async (rust-lang/cargo#16912)
- refactor(network): convert .crate downloads to use http_async (rust-lang/cargo#16902)
- fix(tests): flaky test local_poll_adapter deferred_success (rust-lang/cargo#16909)

r? ghost
@rustbot rustbot added this to the 1.97.0 milestone Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-directory-source Area: directory sources (vendoring) A-git Area: anything dealing with git A-local-registry-source Area: local registry sources (vendoring) A-networking Area: networking issues, curl, etc. A-overrides Area: general issues with overriding dependencies (patch, replace, paths) A-registries Area: registries A-source-replacement Area: [source] replacement A-sparse-registry Area: http sparse registries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants