Add explicit download methods to download module in bootstrap #144482

Shourya742 · 2025-07-26T04:35:57Z

This PR attempts to decouple the default initialization of the config object from parse_inner. It moves specific download methods, previously used during the initial config setup, into standalone functions outside the config implementation.

r? @Kobzol

Kobzol · 2025-07-26T07:38:01Z

Hi, thanks for taking a look at this? Just to clarify, did you separate just the Config download (and other related) methods that were needed for parse_inner, or everything? It seems to me that you moved a lot of methods to functions, but maybe the download functionality just needs all of that..

I took a look at the download functions, and they require mostly the same stuff. Rather than sending so many parameters to each function, could you create some DownloadContext or something like that, which will contain all the basic stuff needed by all downloads, and pass that to the download functions?

Shourya742 · 2025-07-26T13:36:39Z

Hi, thanks for taking a look at this? Just to clarify, did you separate just the Config download (and other related) methods that were needed for parse_inner, or everything? It seems to me that you moved a lot of methods to functions, but maybe the download functionality just needs all of that..

I took a look at the download functions, and they require mostly the same stuff. Rather than sending so many parameters to each function, could you create some DownloadContext or something like that, which will contain all the basic stuff needed by all downloads, and pass that to the download functions?

Yes, only the one's used during config parsing,(clippy and fmt as well, that I gonna revert, I thought they were used, but apparently not) I moved a couple of helpers methods as well. And ya they all required almost same set of arguments, I was nudging towards adding them. Just wanted concept acknowledgement from you, I will introduce a new structure for common fields.

…itialization

…ation

Kobzol

Looks good. Let's use the functions in the config parsing, to get rid of the config method calls.

src/bootstrap/src/core/download.rs

…nd during config parsing directly invoke cdownload methods

rustbot · 2025-07-28T13:54:35Z

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

Shourya742 · 2025-07-28T14:01:17Z

Looks good. Let's use the functions in the config parsing, to get rid of the config method calls.

Did that here: db42d5b
We can't currently define a single DownloadContext at the top and reuse it across all explicit download methods. In this PR, I had to construct a DownloadContext explicitly before each download call because the ExecutionContext inside it requires a shared reference to the config, whereas other method calls in parse_inner require an exclusive reference to the config.

Kobzol · 2025-07-28T14:15:53Z

We will hopefully get rid of that config variable soon, so shouldn't be matter.

Kobzol · 2025-07-28T16:00:30Z

config.patch_binaries_for_nix = patch_binaries_for_nix; is configured at line 889, but the first download ctx is created at 806.
config.llvm_assertions = ... is configured at 898.
config.bootstrap_cache_path is configured at 870.

The first and third thing should be easy to fix, not sure about the second one. Funnily enough, it looks like the code was already calling these download methods before even though the LLVM assertions field was not configured (?). In that case it should be probably fine. All the more reason to get rid of Default for Config...

…ation in config parsing

Shourya742 · 2025-07-28T16:57:39Z

config.patch_binaries_for_nix = patch_binaries_for_nix; is configured at line 889, but the first download ctx is created at 806.

config.llvm_assertions = ... is configured at 898.

config.bootstrap_cache_path is configured at 870.

The first and third thing should be easy to fix, not sure about the second one. Funnily enough, it looks like the code was already calling these download methods before even though the LLVM assertions field was not configured (?). In that case it should be probably fine. All the more reason to get rid of Default for Config...

Yeah, I noticed that too. I was initially planning to leave them out of this PR since we were going to fix them in the next one anyway, but I ended up making the changes here: c071101 :)

Kobzol · 2025-07-29T08:18:54Z

Thanks, let's try!

@bors r+

bors · 2025-07-29T08:18:56Z

📌 Commit c071101 has been approved by Kobzol

It is now in the queue for this repository.

…t-download-methods, r=Kobzol Add explicit download methods to download module in bootstrap This PR attempts to decouple the default initialization of the config object from parse_inner. It moves specific download methods, previously used during the initial config setup, into standalone functions outside the config implementation. r? `@Kobzol`

Rollup of 14 pull requests Successful merges: - #144022 (Implementation: `#[feature(sync_nonpoison)]`, `#[feature(nonpoison_mutex)]`) - #144167 (Document why `Range*<&T> as RangeBounds<T>` impls are not `T: ?Sized`, and give an alternative.) - #144407 (fix(debuginfo): disable overflow check for recursive non-enum types) - #144451 (fix: Reject upvar scrutinees for `loop_match`) - #144482 (Add explicit download methods to download module in bootstrap) - #144500 (thread name in stack overflow message) - #144511 (tidy: increase performance of auto extra checks feature) - #144586 (Update wasi-sdk to 27.0 in CI) - #144599 (bootstrap: enable tidy auto extra checks on tools profile) - #144600 (Ensure external paths passed via flags end up in rustdoc depinfo) - #144609 (feat: Right align line numbers) - #144623 (miri subtree update) - #144626 (cc dependencies: clarify comment) - #144627 (Add a test case for the issue #129882) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 13 pull requests Successful merges: - #144022 (Implementation: `#[feature(sync_nonpoison)]`, `#[feature(nonpoison_mutex)]`) - #144167 (Document why `Range*<&T> as RangeBounds<T>` impls are not `T: ?Sized`, and give an alternative.) - #144407 (fix(debuginfo): disable overflow check for recursive non-enum types) - #144451 (fix: Reject upvar scrutinees for `loop_match`) - #144482 (Add explicit download methods to download module in bootstrap) - #144500 (thread name in stack overflow message) - #144511 (tidy: increase performance of auto extra checks feature) - #144599 (bootstrap: enable tidy auto extra checks on tools profile) - #144600 (Ensure external paths passed via flags end up in rustdoc depinfo) - #144609 (feat: Right align line numbers) - #144623 (miri subtree update) - #144626 (cc dependencies: clarify comment) - #144627 (Add a test case for the issue #129882) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #144482 - Shourya742:2025-07-24-have-explicit-download-methods, r=Kobzol Add explicit download methods to download module in bootstrap This PR attempts to decouple the default initialization of the config object from parse_inner. It moves specific download methods, previously used during the initial config setup, into standalone functions outside the config implementation. r? ``@Kobzol``

bors · 2025-07-29T13:40:13Z

⌛ Testing commit c071101 with merge 7278554...

Rollup of 13 pull requests Successful merges: - rust-lang/rust#144022 (Implementation: `#[feature(sync_nonpoison)]`, `#[feature(nonpoison_mutex)]`) - rust-lang/rust#144167 (Document why `Range*<&T> as RangeBounds<T>` impls are not `T: ?Sized`, and give an alternative.) - rust-lang/rust#144407 (fix(debuginfo): disable overflow check for recursive non-enum types) - rust-lang/rust#144451 (fix: Reject upvar scrutinees for `loop_match`) - rust-lang/rust#144482 (Add explicit download methods to download module in bootstrap) - rust-lang/rust#144500 (thread name in stack overflow message) - rust-lang/rust#144511 (tidy: increase performance of auto extra checks feature) - rust-lang/rust#144599 (bootstrap: enable tidy auto extra checks on tools profile) - rust-lang/rust#144600 (Ensure external paths passed via flags end up in rustdoc depinfo) - rust-lang/rust#144609 (feat: Right align line numbers) - rust-lang/rust#144623 (miri subtree update) - rust-lang/rust#144626 (cc dependencies: clarify comment) - rust-lang/rust#144627 (Add a test case for the issue rust-lang/rust#129882) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned Kobzol Jul 26, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jul 26, 2025

Shourya742 added 4 commits July 27, 2025 12:21

add download context

3d3534d

move utility methods out of config impl

861de92

move download_beta_toolchain out of impl as its used during config in…

8facd06

…itialization

move download_rustfmt out of impl as its used during config initializ…

19dc195

…ation

Shourya742 force-pushed the 2025-07-24-have-explicit-download-methods branch from d2df958 to 19dc195 Compare July 27, 2025 06:52

Kobzol reviewed Jul 28, 2025

View reviewed changes

src/bootstrap/src/core/download.rs Outdated Show resolved Hide resolved

Shourya742 added 2 commits July 28, 2025 18:53

use dry_run and verbose directly from exec_ctx

f63f212

remove config wrappers of download_toolchain and maybe_download_fmt a…

db42d5b

…nd during config parsing directly invoke cdownload methods

Shourya742 requested a review from Kobzol July 28, 2025 15:18

make sure to populate DownloadState dependencies before its initializ…

c071101

…ation in config parsing

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 29, 2025

Zalathar mentioned this pull request Jul 29, 2025

Rollup of 14 pull requests #144628

Closed

Zalathar mentioned this pull request Jul 29, 2025

Rollup of 13 pull requests #144633

Merged

bors merged commit 047304b into rust-lang:master Jul 29, 2025
10 checks passed

rustbot added this to the 1.90.0 milestone Jul 29, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add explicit download methods to download module in bootstrap #144482

Add explicit download methods to download module in bootstrap #144482

Uh oh!

Shourya742 commented Jul 26, 2025

Uh oh!

Kobzol commented Jul 26, 2025

Uh oh!

Shourya742 commented Jul 26, 2025 •

edited

Loading

Uh oh!

Kobzol left a comment

Uh oh!

Uh oh!

rustbot commented Jul 28, 2025

Uh oh!

Shourya742 commented Jul 28, 2025

Uh oh!

Kobzol commented Jul 28, 2025

Uh oh!

Kobzol commented Jul 28, 2025 •

edited

Loading

Uh oh!

Shourya742 commented Jul 28, 2025

Uh oh!

Kobzol commented Jul 29, 2025

Uh oh!

bors commented Jul 29, 2025

Uh oh!

Uh oh!

bors commented Jul 29, 2025

Uh oh!

Uh oh!

Add explicit download methods to download module in bootstrap #144482

Add explicit download methods to download module in bootstrap #144482

Uh oh!

Conversation

Shourya742 commented Jul 26, 2025

Uh oh!

Kobzol commented Jul 26, 2025

Uh oh!

Shourya742 commented Jul 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kobzol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rustbot commented Jul 28, 2025

Uh oh!

Shourya742 commented Jul 28, 2025

Uh oh!

Kobzol commented Jul 28, 2025

Uh oh!

Kobzol commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Shourya742 commented Jul 28, 2025

Uh oh!

Kobzol commented Jul 29, 2025

Uh oh!

bors commented Jul 29, 2025

Uh oh!

Uh oh!

bors commented Jul 29, 2025

Uh oh!

Uh oh!

Shourya742 commented Jul 26, 2025 •

edited

Loading

Kobzol commented Jul 28, 2025 •

edited

Loading