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.

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

Set RUSTC_BOOTSTRAP=1 consistently #120096

Merged
merged 1 commit into from
Jan 19, 2024
Merged

Conversation

onur-ozkan
Copy link
Member

@rustbot
Copy link
Collaborator

rustbot commented Jan 18, 2024

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 Jan 18, 2024
@Mark-Simulacrum
Copy link
Member

I'm not sure we should do this. Users can bypass passing flags to this part of the build with RUSTFLAGS_NOT_BOOTSTRAP and similar. If we want to always pass this, I think we should also set -Zallow-features or otherwise make sure bootstrap doesn't encounter problems across stable versions. Distros rely on being able to build with a different version than our CI and this is effectively our only (implicit) way of providing that.

@onur-ozkan
Copy link
Member Author

RUSTFLAGS_NOT_BOOTSTRAP has no effect on building bootstrap? This is only for bootstrap compilation.

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

I think Mark's point was that whoever is setting RUSTFLAGS=-Zthreads=8 RUSTC_BOOTSTRAP=1 to get the failure in #120001 (comment) should instead be setting RUSTFLAGS_NOT_BOOTSTRAP=-Zthreads=8 RUSTC_BOOTSTRAP=1.

That may not be adequate as a path forward because I'd guess they have that configured globally on their system, such as they put those flags in ~/.bashrc or similar, and want it to apply to all projects, not just rustc. So they do want RUSTFLAGS=-Zthreads=8 present, and there is no way using RUSTFLAGS_BOOTSTRAP= to cancel that out from applying to bootstrap too.

But I agree with Mark's impression that the change currently in this PR (always setting RUSTC_BOOTSTRAP=1) is not what I would want.

Unconditionally inserting -Zallow-features= into RUSTFLAGS would also not be good, because letting users opt in to -Zthreads=8 still seems valuable, and is the entire motivation for this PR anyway.

How about this?— For the purpose of validating that bootstrap can build with the most vanilla toolchain possible for the benefit of distros (i.e. we don't introduce unstable language features into bootstrap code over time), it should be sufficient to set -Zallow-features= only if RUSTFLAGS and RUSTFLAGS_BOOTSTRAP are both absent or empty.

This way RUSTFLAGS=-Zthreads=8 RUSTC_BOOTSTRAP=1 works again, but we still get some validation that bootstrap works without features.

Comment on lines 933 to 937
if 'RUSTC_BOOTSTRAP' not in env:
env['RUSTC_BOOTSTRAP'] = '1'

# preserve existing RUSTFLAGS
env.setdefault("RUSTFLAGS", "")
Copy link
Member

Choose a reason for hiding this comment

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

Something like this:

Suggested change
if 'RUSTC_BOOTSTRAP' not in env:
env['RUSTC_BOOTSTRAP'] = '1'
# preserve existing RUSTFLAGS
env.setdefault("RUSTFLAGS", "")
env["RUSTC_BOOTSTRAP"] = "1"
default_rustflags = "" if env.get("RUSTFLAGS_BOOTSTRAP", "") \
else "-Zallow-features="
env.setdefault("RUSTFLAGS", default_rustflags)

@Mark-Simulacrum
Copy link
Member

I think that's a good suggestion, I like that approach.

@onur-ozkan onur-ozkan changed the title Set RUSTC_BOOTSTRAP=1 consistently unless manually configured Set RUSTC_BOOTSTRAP=1 consistently Jan 18, 2024
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@dtolnay
Copy link
Member

dtolnay commented Jan 18, 2024

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Jan 18, 2024

📌 Commit 21b4fe2 has been approved by dtolnay

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 Jan 18, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 18, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#119582 (bootstrap: handle vendored sources when remapping crate paths)
 - rust-lang#119730 (docs: fix typos)
 - rust-lang#119828 (Improved collapse_debuginfo attribute, added command-line flag)
 - rust-lang#119869 (replace `track_errors` usages with bubbling up `ErrorGuaranteed`)
 - rust-lang#120037 (Remove `next_root_ty_var`)
 - rust-lang#120094 (tests/ui/asm/inline-syntax: adapt for LLVM 18)
 - rust-lang#120096 (Set RUSTC_BOOTSTRAP=1 consistently)
 - rust-lang#120101 (change `.unwrap()` to `?` on write where `fmt::Result` is returned)
 - rust-lang#120102 (Fix typo in munmap_partial.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 19, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#119582 (bootstrap: handle vendored sources when remapping crate paths)
 - rust-lang#119730 (docs: fix typos)
 - rust-lang#119828 (Improved collapse_debuginfo attribute, added command-line flag)
 - rust-lang#119869 (replace `track_errors` usages with bubbling up `ErrorGuaranteed`)
 - rust-lang#120037 (Remove `next_root_ty_var`)
 - rust-lang#120094 (tests/ui/asm/inline-syntax: adapt for LLVM 18)
 - rust-lang#120096 (Set RUSTC_BOOTSTRAP=1 consistently)
 - rust-lang#120101 (change `.unwrap()` to `?` on write where `fmt::Result` is returned)
 - rust-lang#120102 (Fix typo in munmap_partial.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c6d25cf into rust-lang:master Jan 19, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 19, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 19, 2024
Rollup merge of rust-lang#120096 - onur-ozkan:rustc_bootstrap, r=dtolnay

Set RUSTC_BOOTSTRAP=1 consistently

Fixes https://internals.rust-lang.org/t/rust-compiler-with-parallel-build/20099 which is a regression from rust-lang#120001

cc `@dtolnay` `@petrochenkov`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 21, 2024
Capture the rationale for `-Zallow-features=` in bootstrap.py

Based on the discussion in rust-lang#120096.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 21, 2024
Capture the rationale for `-Zallow-features=` in bootstrap.py

Based on the discussion in rust-lang#120096.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 22, 2024
Rollup merge of rust-lang#120167 - dtolnay:bootstrap, r=clubby789

Capture the rationale for `-Zallow-features=` in bootstrap.py

Based on the discussion in rust-lang#120096.
@dtolnay dtolnay assigned dtolnay and unassigned Mark-Simulacrum Mar 24, 2024
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
Capture the rationale for `-Zallow-features=` in bootstrap.py

Based on the discussion in rust-lang/rust#120096.
@onur-ozkan onur-ozkan deleted the rustc_bootstrap branch April 13, 2024 07:15
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
Capture the rationale for `-Zallow-features=` in bootstrap.py

Based on the discussion in rust-lang/rust#120096.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

5 participants