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

x.py fix #53896

Closed
zackmdavis opened this issue Sep 2, 2018 · 2 comments · Fixed by #56595
Closed

x.py fix #53896

zackmdavis opened this issue Sep 2, 2018 · 2 comments · Fixed by #56595
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. 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

@zackmdavis
Copy link
Member

#53816 (migrating part of the codebase to use '_ lifetimes in type names) ended up involving quite a lot of manual text-editing labor because, unfortunately, rustfix doesn't work with x.py. (You might think that you could run cargo fix on an individual crate, but after supplying missing feature flags and environment variables (RUSTC_ERROR_METADATA_DST, CFG_COMPILER_HOST_TRIPLE), you end up triggering an assertion that I don't know how to hack around.) It would be nice if there was an x.py fix that worked (even if the need won't come up very often).

This issue should probably receive the A-rustbuild, C-feature-request, T-infra, and P-low labels.

@Mark-Simulacrum
Copy link
Member

Seeing as I believe we'd like to migrate the compiler over to 2018 edition (and probably std?), I think p-medium might be more accurate.

@Havvy Havvy 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. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Sep 2, 2018
zackmdavis added a commit to zackmdavis/rust that referenced this issue Sep 30, 2018
This seemed like a good way to kick the tires on the
elided-lifetimes-in-paths lint (rust-lang#52069)—seems to work! This was also
pretty tedious—it sure would be nice if `cargo fix` worked on this
codebase (rust-lang#53896)!
@ljedrz
Copy link
Contributor

ljedrz commented Dec 5, 2018

I've started working on this.

bors added a commit that referenced this issue May 25, 2019
Add clippy and fix commands to x.py

Since they are kind of similar in nature, I have used the same approach as for `cargo check`. At least some of the boilerplate could probably be shared, but I'd prefer to gather some feedback before I decide to merge them more aggressively.

This works reasonably well for `clippy`; with `-A clippy::all` and some extra `#![feature(rustc_private)]`s almost the whole codebase can be processed. There are some concerns, though:
- unlike `check`, in order to be able to traverse all the crates, some of them need to be marked with the `#![feature(rustc_private)]` attribute
- `-W clippy::all` breaks on any error. Is there a way to produce errors but not have them break the progress?
- I'm not sure how to redirect the errors in a way that would show colors; for now I was able to de-jsonize and print them (something not needed for `check`)

`cargo fix` is much more stubborn; it refuses to acknowledge crates like `core` and `std`, so it doesn't progress much at all.

Since this is a bit more tricky than I have envisioned, I need some guidance:
- is this the right approach or am I doing something very wrong ^^?
- why are the extra `rustc_private` features necessary? I was hoping for the same treatment as `check`
- are changes in `clippy` and `cargo fix` needed e.g. in order to produce errors in the same manner as `check` or did I miss something?
- do we need this level of file granularity (e.g. for futureproofing) or can `check`, `clippy` and `fix` files be condensed?

Hopes-to-fix: #53896

Cc @alexcrichton, @zackmdavis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. 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.

4 participants