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

rustfmt Not Downloaded Until Manual x.py fmt #101306

Closed
asquared31415 opened this issue Sep 2, 2022 · 7 comments · Fixed by #101969
Closed

rustfmt Not Downloaded Until Manual x.py fmt #101306

asquared31415 opened this issue Sep 2, 2022 · 7 comments · Fixed by #101969
Assignees
Labels
C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@asquared31415
Copy link
Contributor

asquared31415 commented Sep 2, 2022

As of #97507 rustfmt is not downloaded until the user runs x.py fmt. The rustc dev guide suggests setting the formatter to the rustfmt that is downloaded for stage0 in the build directory, but that is no longer downloaded with any other x.py commands.

@jyn514 (sorry for the delay in making this issue)

@asquared31415 asquared31415 added the C-bug Category: This is a bug. label Sep 2, 2022
@jyn514 jyn514 added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Sep 2, 2022
@reez12g
Copy link
Contributor

reez12g commented Sep 8, 2022

@rustbot claim

@jyn514
Copy link
Member

jyn514 commented Sep 14, 2022

Hi @reez12g, have you had time to work on this? Let me know if anything is confusing and I'm happy to help out :)

@reez12g
Copy link
Contributor

reez12g commented Sep 15, 2022

@jyn514
Thank you!!
Sorry for the delay in catching up.

Am I correct that this Issue is about "at some point, make sure to automatically download rustfmt"?
If so, I'm still not sure which file to fix.

I think I somehow understood that this Issue is related to this (i.e., no dependence on python scripts?)

@jyn514
Copy link
Member

jyn514 commented Sep 15, 2022

I think I somehow understood that this Issue is related to this (i.e., no dependence on python scripts?)

That was the motivation for #97507, right, we want to move all the logic out of the python script eventually.

Am I correct that this Issue is about "at some point, make sure to automatically download rustfmt"?

Not just at some point - it should be done on every invocation of bootstrap (assuming it hasn't been downloaded yet). Right now we only download it when someone runs x.py fmt.

If so, I'm still not sure which file to fix.

If you look at the PR I linked, it's currently in format.rs - I think moving it a line early to

self.maybe_update_submodules();
would be a good approach.

@zsfVishnu
Copy link

Is this picked up or shall i work on it? @jyn514

@jyn514
Copy link
Member

jyn514 commented Sep 18, 2022

@zsfVishnu no, @reez12g is already working on it.

@zsfVishnu
Copy link

cool

@bors bors closed this as completed in c5b105d Oct 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants