Skip to content

ensure compiler existance of tools on the dist step #140006

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

ensure compiler existance of tools on the dist step #140006

wants to merge 2 commits into from

Conversation

onur-ozkan
Copy link
Member

@onur-ozkan onur-ozkan commented Apr 18, 2025 •
edited by Kobzol
Loading

Fixes #138778 with a coverage on #138123 and #138004.

try-job: dist-powerpc64le-linux

@rustbot
Copy link
Collaborator

rustbot commented Apr 18, 2025

r? @clubby789

rustbot has assigned @clubby789.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Apr 18, 2025
Kobzol
Kobzol reviewed Apr 18, 2025
View reviewed changes
src/bootstrap/src/core/build_steps/dist.rs
@@ -421,13 +421,13 @@ impl Step for Rustc {
builder.install(&rustdoc, &image.join("bin"), FileType::Executable);
}

let ra_proc_macro_srv_compiler =
builder.compiler_for(compiler.stage, builder.config.build, compiler.host);
builder.ensure(compile::Rustc::new(ra_proc_macro_srv_compiler, compiler.host));
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this now build the compiler for the proc macro server even if tool::RustAnalyzerProcMacroSrv should not actually be built? 🤔 It's not always enabled.

Copy link
Member Author

Choose a reason for hiding this comment

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

Compiler is passed before the condition here so I'm not sure how to guard against that without applying ugly hacks. Also, in practice, I don't think we ever hit the case of "trying to build RustAnalyzerProcMacroSrv while the compiler isn't compiled/ready for use".

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, fair enough.

@Kobzol
Copy link
Contributor

Kobzol commented Apr 20, 2025 •
edited
Loading

@bors try

1 similar comment
@Kobzol
Copy link
Contributor

Kobzol commented Apr 20, 2025

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 20, 2025
@bors
Auto merge of rust-lang#140006 - onur-ozkan:138778, r=<try>
927bf00 
ensure compiler existance of tools on the dist step

Fixes rust-lang#138778 with a coverage on rust-lang#138123 and rust-lang#138004.

try-job: dist-powerpc64le-linux
@bors
Copy link
Collaborator

bors commented Apr 20, 2025

⌛ Trying commit 4ba9fff with merge 927bf00b29ad8261c60384c24fcac128268adfcb...

@bors
Copy link
Collaborator

bors commented Apr 20, 2025

☀️ Try build successful - checks-actions
Build commit: 927bf00 (927bf00b29ad8261c60384c24fcac128268adfcb)

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

@Kobzol Kobzol Kobzol left review comments

Assignees

@clubby789 clubby789

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

./x dist --stage 2 $tool broken after #138224
5 participants
@onur-ozkan @rustbot @Kobzol @bors @clubby789