Skip to content
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

refactor fn bootstrap::builder::Builder::compiler_for logic #109295

Merged
merged 1 commit into from
Mar 22, 2023
Merged

refactor fn bootstrap::builder::Builder::compiler_for logic #109295

merged 1 commit into from
Mar 22, 2023

Conversation

onur-ozkan
Copy link
Member

@onur-ozkan onur-ozkan commented Mar 18, 2023 •
edited
Loading

  • check compiler stage before forcing for stage2.
  • check if download_rustc is not set before forcing for stage1.

resolves #109286

@rustbot
Copy link
Collaborator

rustbot commented Mar 18, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@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 Mar 18, 2023
@onur-ozkan
Copy link
Member Author

r? @jyn514

@rustbot rustbot assigned jyn514 and unassigned Mark-Simulacrum Mar 18, 2023
@onur-ozkan onur-ozkan changed the title refactor fn bootstrap::builder::Builder::compiler_for refactor fn bootstrap::builder::Builder::compiler_for logic Mar 18, 2023
@jyn514
Copy link
Member

jyn514 commented Mar 18, 2023

I don't have time for reviews, sorry.

r? @Mark-Simulacrum

@rustbot rustbot assigned Mark-Simulacrum and unassigned jyn514 Mar 18, 2023
@onur-ozkan
Copy link
Member Author

I don't have time for reviews, sorry.

r? @Mark-Simulacrum

I think Mark is quite busy too.

Can you review this ? @albertlarsan68

Mark-Simulacrum
Mark-Simulacrum reviewed Mar 18, 2023
View reviewed changes
src/bootstrap/builder.rs Outdated
@@ -919,7 +919,7 @@ impl<'a> Builder<'a> {
host: TargetSelection,
target: TargetSelection,
) -> Compiler {
if self.build.force_use_stage2() {
if self.build.force_use_stage2(Compiler { stage, host }) {
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer that we avoid creating Compiler structs if they don't correspond to an already-built compiler (i.e., the only constructor should be self.compiler).

Can we refactor both force_use functions to take stage and host as separate values?

@Mark-Simulacrum
Copy link
Member

r=me with nit fixed

I'm generally not sure this is the right logic - the force-use functions feel pretty iffy to me - but I'm OK patching it for now.

@onur-ozkan onur-ozkan added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 18, 2023
@onur-ozkan
refactor fn bootstrap::builder::Builder::compiler_for
2ec7f6c 
- check compiler stage before forcing for stage2.
- check if download_rustc is not set before forcing for stage1.

Signed-off-by: ozkanonur <work@onurozkan.dev>
@onur-ozkan
Copy link
Member Author

@Mark-Simulacrum fixed

@onur-ozkan onur-ozkan added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 18, 2023
@onur-ozkan
Copy link
Member Author

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 21, 2023

📌 Commit 2ec7f6c has been approved by ozkanonur

It is now in the queue for this repository.

@bors 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 Mar 21, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 21, 2023
@matthiaskrgr
Rollup merge of rust-lang#109295 - ozkanonur:issue-109286, r=ozkanonur
69d989b 
refactor `fn bootstrap::builder::Builder::compiler_for` logic

- check compiler stage before forcing for stage2.
- check if download_rustc is not set before forcing for stage1.

resolves rust-lang#109286
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 21, 2023
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 22, 2023
@Dylan-DPC
Rollup merge of rust-lang#109295 - ozkanonur:issue-109286, r=ozkanonur
46a3442 
refactor `fn bootstrap::builder::Builder::compiler_for` logic

- check compiler stage before forcing for stage2.
- check if download_rustc is not set before forcing for stage1.

resolves rust-lang#109286
@Dylan-DPC Dylan-DPC mentioned this pull request Mar 22, 2023
Closed
@Dylan-DPC Dylan-DPC mentioned this pull request Mar 22, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2023
@bors
Auto merge of rust-lang#109496 - Dylan-DPC:rollup-u8rsi3h, r=Dylan-DPC
a266f11 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#100311 (Fix handling of trailing bare CR in str::lines)
 - rust-lang#108997 (Change text -> rust highlighting in sanitizer.md)
 - rust-lang#109179 (move Option::as_slice to intrinsic)
 - rust-lang#109187 (Render source page layout with Askama)
 - rust-lang#109280 (Remove `VecMap`)
 - rust-lang#109295 (refactor `fn bootstrap::builder::Builder::compiler_for` logic)
 - rust-lang#109312 (rustdoc: Cleanup parent module tracking for doc links)
 - rust-lang#109317 (Update links for custom discriminants.)
 - rust-lang#109405 (RPITITs are `DefKind::Opaque` with new lowering strategy)
 - rust-lang#109414 (Do not consider synthesized RPITITs on missing items checks)
 - rust-lang#109435 (Detect uninhabited types early in const eval)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7a57d88 into rust-lang:master Mar 22, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 22, 2023
@onur-ozkan onur-ozkan deleted the issue-109286 branch March 22, 2023 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mark-Simulacrum Mark-Simulacrum Mark-Simulacrum left review comments

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
1.70.0
Development

Successfully merging this pull request may close these issues.

bootstrap: Error: Unable to find the stamp file
5 participants
@onur-ozkan @rustbot @jyn514 @Mark-Simulacrum @bors