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

Use rustc_private and bump nightly #1174

Merged
merged 1 commit into from
Jan 14, 2022
Merged

Conversation

flip1995
Copy link
Contributor

This moves away from the rustc-ap crates and uses rustc-dev instead. It also bumps this crate to use the latest nightly.

I just experimented with this. Since the diff was so small, I decided to submit it as a PR. Feel free to close this, if you don't want to do this.

This would help with rust-lang/rust#91543

@calebcartwright
Copy link
Contributor

If we were to go this route (and fwiw I'd like to see this change too) I think it might be helpful to update the top of the readme file and/or installation section to detail the extra requirements.

There's historically been duplicative issues (e.g. #1173) with folks attempting to install from crates.io using an unsupported toolchain, so I think emphasizing that in documentation along with the newly required rustc-dev component would be helpful.

One thing we heard after making the same switch in rustfmt is that the rustc-dev component isn't available for every target which was problematic for at least one case where someone was attempting to consume rustfmt as a lib. I doubt that'll be a problem here though given the nature of racer

@flip1995
Copy link
Contributor Author

I can adapt the documentation, if this should move forward.

One thing we heard after making the same switch in rustfmt is that the rustc-dev component isn't available for every target

Oh, we didn't hear anything like that in Clippy 🤔 Going through the components history, it is available on ever Tier 1 target and by random clicking on Tier 2 targets it is also available on every Tier 2 target, where also tools like rustfmt, clippy, ... are available. So I'm not that concerned here.

@calebcartwright
Copy link
Contributor

I think someone was trying to use rustfmt in a wasm context perhaps? Can't recall the exact specifics but there was some edge case that was surfaced to us

@flip1995
Copy link
Contributor Author

flip1995 commented Dec 13, 2021

Ok, I've updated the README with a note that rustc-dev has to be installed to compile Racer.

I'm not that concerned, that something like this will happen to Racer. Especially since it has a note that it is not actively maintaineddeveloped.

@calebcartwright
Copy link
Contributor

@kngwyu what are your thoughts on this? The impacts of the current AP versions are starting to snowball, so think we'll either need to go this route or at least get the AP crate versions bumped

Copy link
Collaborator

@kngwyu kngwyu left a comment

Choose a reason for hiding this comment

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

Thank you!

@calebcartwright
This way is better since we don't have much maintenance ability now...

@flip1995
Copy link
Contributor Author

Should I bump this PR to the latest nightly before merging? Is anything missing, before this can get merged?

@Xanewok
Copy link
Collaborator

Xanewok commented Jan 13, 2022

@flip1995 if that would not be a problem, please update to the newest nightly; I'll merge this immediately after. Thanks for all the work here!

This moves away from the rustc-ap crates and uses rustc-dev instead. It
also bumps this crate to use the latest nightly.
@flip1995
Copy link
Contributor Author

@Xanewok Just required a rust-toolchain.toml bump and a Cargo.lock update.

I can also do the RLS update once a new Racer version is released 👍

@Xanewok Xanewok merged commit cbca5c6 into racer-rust:master Jan 14, 2022
@flip1995 flip1995 deleted the no-ap branch January 14, 2022 12:43
@flip1995
Copy link
Contributor Author

@Xanewok not sure if you're already at it, but are you planning to bump the Racer version soon, so that we can update Racer in RLS?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants