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

opt-dist: propagate channel info to bootstrap #134528

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

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Dec 19, 2024

Fixes #133503.

Previously, tests/ui/bootstrap/rustc_bootstap.rs [sic] failed during beta bump in opt-dist tests. This is because:

  • opt-dist tried to run ./x test against beta-channel dist rustc through bootstrap.
  • The dist build produced during the beta bump produces a rustc which correctly thinks that it is a beta compiler based on src/ci/channel info.
  • opt-dist tries to run ./x test on the beta rustc from the dist build, but without specifying channel through a synthetic config.toml, so bootstrap tells compiletest that we're on the nightly channel (by default).
  • Now there's a channel mismatch: compiletest believes the rustc under test is a nightly rustc, but the rustc under test actually considers itself a beta rustc. This means that //@ only-nightly will be satisfied yet the test will fail as the beta rustc is not a nightly rustc.

This PR:

  • Fixes the test failure during beta bump (i.e. tests/ui/bootstrap/rustc_bootstap.rs fails in post-PGO/opt-dist tests #133503) by having opt-dist faithfully report the channel of the dist rustc being tested (i.e. "beta" in a beta bump PR). This will properly make the test be ignored during beta bump as the rustc under test is not a nightly rustc.
  • Fixes the test name rustc_bootstap.rs -> rustc_bootstrap.rs. No more stapping.
  • Slightly adjusts the doc comment in the test to make it more clear.

I ran a try-job against the beta branch (explicitly running the opt-dist tests by modifying the job definition) with these changes in #134131, and it appears that the try-job was successful. The two commits in this PR are cherry-picked from #134131, with the test commit slightly modified (to also adjust the test comments).

r? @Kobzol (or compiler or bootstrap or infra I guess?)

jieyouxu and others added 2 commits December 20, 2024 02:32
- Fixed test name, it should've been `rustc_bootstrap.rs`, oops.
- Slightly reworded test comment to make it more clear.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 19, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 19, 2024

Some changes occurred in src/tools/opt-dist

cc @Kobzol

Comment on lines +126 to +128
/// Roughly convert a version string (`nightly`, `beta`, or `1.XY.Z`) to channel string (`nightly`,
/// `beta` or `stable`).
fn version_to_channel(version_str: &str) -> &'static str {
Copy link
Member Author

Choose a reason for hiding this comment

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

The dist version extraction above this is slightly hacky, and so is this, but oh well.

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-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests/ui/bootstrap/rustc_bootstap.rs fails in post-PGO/opt-dist tests
3 participants