Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

./x.py fmt uses incorrect rustfmt with download-ci-llvm=true and custom rustc (not stage0/beta) #81155

Closed
tesuji opened this issue Jan 18, 2021 · 4 comments · Fixed by #108212
Assignees
Labels
C-bug Category: This is a bug. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Comments

@tesuji
Copy link
Contributor

tesuji commented Jan 18, 2021

On master branch with commit c4df63f, replace RUSTUP_HOME
with actual value and apply this diff:

% diff config.toml.example config.toml
--- config.toml.example 2021-01-18 02:40:45.127363851 +0000
+++ config.toml 2021-01-18 05:34:16.052659202 +0000
@@ -45,7 +45,7 @@
 #
 # Note that many of the LLVM options are not currently supported for
 # downloading. Currently only the "assertions" option can be toggled.
-#download-ci-llvm = false
+download-ci-llvm = true
 
 # Indicates whether LLVM rebuild should be skipped when running bootstrap. If
 # this is `false` then the compiler's LLVM will be rebuilt whenever the built
@@ -193,11 +193,11 @@
 
 # Instead of downloading the src/stage0.txt version of Cargo specified, use
 # this Cargo binary instead to build all Rust code
-#cargo = "/path/to/bin/cargo"
+cargo = "$RUSTUP_HOME/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo"
 
 # Instead of downloading the src/stage0.txt version of the compiler
 # specified, use this rustc binary instead as the stage0 snapshot compiler.
-#rustc = "/path/to/bin/rustc"
+rustc = "$RUSTUP_HOME/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rustc"
 
 # Instead of download the src/stage0.txt version of rustfmt specified,
 # use this rustfmt binary instead as the stage0 snapshot rustfmt.

Run ./x.py fmt after that. Then run git status to check status of the repo.

I expected to see this happen: Nothing should change.

Instead, this happened: Many files changed dues to using wrong rustfmt.

cc @Mark-Simulacrum as in #81152 (comment)

@rustbot label T-infra

Meta

% rustc -Vv
rustc 1.51.0-nightly (4253153db 2021-01-17)
binary: rustc
commit-hash: 4253153db205251f72ea4493687a31e04a2a8ca0
commit-date: 2021-01-17
host: x86_64-unknown-linux-gnu
release: 1.51.0-nightly
LLVM version: 11.0.1
@tesuji tesuji added the C-bug Category: This is a bug. label Jan 18, 2021
@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jan 18, 2021
@Mark-Simulacrum
Copy link
Member

I think custom rustc also overrides rustfmt, though probably shouldn't. Will investigate fixing.

@Mark-Simulacrum Mark-Simulacrum self-assigned this Jan 18, 2021
@Mark-Simulacrum
Copy link
Member

Yeah, if you configure a rustc we currently default to the sibling rustfmt - that's probably a bug, we should use the python-downloaded rustfmt.

The likely correct thing here is to move to setting RUSTFMT from bootstrap.py on download, and then detecting that in config.rs similarly to rustc/cargo. We should probably treat it as an optional env variable for now though.

@Mark-Simulacrum Mark-Simulacrum removed their assignment Jan 18, 2021
@jyn514 jyn514 added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. labels Feb 3, 2023
@KittyBorgX
Copy link
Member

@rustbot claim
If no one has claimed it yet :)

@KittyBorgX
Copy link
Member

Any starting point that I can get? Like the right file or something similar? :>

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-bug Category: This is a bug. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants