Skip to content

x setup: ask if warnings should (not) be denied - #165461

Open
WaffleLapkin wants to merge 4 commits into
rust-lang:mainfrom
WaffleLapkin:x-setup
Open

WaffleLapkin wants to merge 4 commits into
rust-lang:mainfrom
WaffleLapkin:x-setup

Conversation

@WaffleLapkin

@WaffleLapkin WaffleLapkin commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

Alternative to #124439 that I feel easier about.

Refactors how we handle x setup and adds a question about denying warnings.

r? @Kobzol

Move `Profile` into it's own module for clarity, make the `bootstrap.toml`
setup step a separate struct (`BootstrapToml`) *containing* `Profile`.
This makes it much clearer which steps use which helper functions, etc.
Previous version was hardcoding yes/print/no/default as the options,
which is silly given that it's used in many places where "print" is not
*actually* an option. IMO the previous version made it quite confusing...

Now it accepts a closure which decides which asnwers are accepted.
@rustbot 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 Nov 4, 2026
@rustbot

rustbot commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Kobzol is not on the review rotation at the moment.
They may take a while to respond.

Comment on lines +90 to +102
let deny_warnings = {
println!(
"\nRust CI fails when there are any warnings. It is possible to deny warnings locally too, \
but that can hurt iteration cycle (forcing you to rebuild everything when there are warnings).\n"
);

prompt_user("Deny warnings? [y/N]", |input| match input {
"y" => Break(true),
"n" | "" => Break(false),
_ => Continue(()),
})
.unwrap()
};

@WaffleLapkin WaffleLapkin Nov 4, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This makes x setup library/etc be interactive. Is that a problem?

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think so, other setup commands are already interactive, and I assume that it doesn't really make sense to use those commands non-interactively 🤔 If yes, people can just stuff yes into it.

@rust-log-analyzer

Copy link
Copy Markdown
Collaborator

The job test-aarch64-gnu-llvm-22-2 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
cargo:warning=Compiler family detection failed due to error: ToolNotFound: failed to find tool "x86_64-linux-gnu-g++": No such file or directory (os error 2)


error: expect test failed
   --> snapshots/x_setup.snap

You can update all `expect!` tests by running:

    env UPDATE_EXPECT=1 cargo test

To update a single test, place the cursor on `expect` token and use `run` feature of rust-analyzer.

Expect:
----
---
source: src/bootstrap/src/core/builder/cli_paths/tests.rs
expression: setup
---
[Setup] setup::Profile
    targets: [aarch64-unknown-linux-gnu]
    - Set({compiler})
    - Set({dist})
    - Set({library})
    - Set({none})
    - Set({tools})
[Setup] setup::Hook
    targets: [aarch64-unknown-linux-gnu]
    - Set({hook})
[Setup] setup::Link
    targets: [aarch64-unknown-linux-gnu]
    - Set({link})
[Setup] setup::Editor
    targets: [aarch64-unknown-linux-gnu]
    - Set({editor})

----

Actual:
----
---
source: src/bootstrap/src/core/builder/cli_paths/tests.rs
expression: setup
---
[Setup] setup::bootstrap_toml::BootstrapToml
    targets: [aarch64-unknown-linux-gnu]
    - Set({compiler})
    - Set({dist})
    - Set({library})
    - Set({none})
    - Set({tools})
[Setup] setup::hook::Hook
    targets: [aarch64-unknown-linux-gnu]
    - Set({hook})
[Setup] setup::link::Link
    targets: [aarch64-unknown-linux-gnu]
    - Set({link})
[Setup] setup::editor::Editor
    targets: [aarch64-unknown-linux-gnu]
    - Set({editor})

----

Diff:
----
---
source: src/bootstrap/src/core/builder/cli_paths/tests.rs
expression: setup
---
[Setup] setup::Profilebootstrap_toml::BootstrapToml
    targets: [aarch64-unknown-linux-gnu]
    - Set({compiler})
    - Set({dist})
    - Set({library})
    - Set({none})
    - Set({tools})
[Setup] setup::hook::Hook
    targets: [aarch64-unknown-linux-gnu]
    - Set({hook})
[Setup] setup::link::Link
    targets: [aarch64-unknown-linux-gnu]
    - Set({link})
[Setup] setup::editor::Editor
    targets: [aarch64-unknown-linux-gnu]
    - Set({editor})

----

---- core::builder::cli_paths::tests::x_setup stdout end ----

@Kobzol Kobzol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good! Left some comments. GitHub is not allowing me to comment inline atm, so:

I was a bit confused reading the "Rust CI fails when there are any warnings." sentence. The "forces you to rebuild" is a bit misleading, I think. The real problem is that it forces you to fix the warnings before you can get a working compiler build, right?

let rust_section = if deny_warnings { "" } else { "\n[rust]\ndeny-warnings = false\n" };

let settings = format!(
"# See bootstrap.example.toml for documentation of available options\n\

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we just use raw strings (r#"..."#) here and in rust_section? 😆 It would be much more readable.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants