remove deprecated tool rls by onur-ozkan · Pull Request #126856 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation36 Commits4 Checks6 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

onur-ozkan

@rustbot

r? @clubby789

rustbot has assigned @clubby789.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-testsuite

Area: The testsuite used to check the correctness of rustc

S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

T-bootstrap

Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

labels

Jun 23, 2024

@rustbot

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@rust-log-analyzer

This comment has been minimized.

@onur-ozkan

For the issue 2, we will have to handle this (and any other deprecated components) from the rustup side.

I will change the PR label as blocked until rustup have this.

@onur-ozkan onur-ozkan added S-blocked

Status: Blocked on something else such as an RFC or other implementation work.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Jun 23, 2024

@rami3l

@rbtcollins and I see no obvious downsides/blockers on Rustup's side regarding the removal of the "dummy" RLS.

The removal of RLS' shim created by Rustup itself is another issue though, which will be discussed in detail at rust-lang/rustup#3316 instead. Before that, you're still expected to see an rls-named binary on the PATH, and running it will cause Rustup to warn you about a not-yet-installed component.

Just tried the following on my machine:

rls error: 'rls' is not installed for the toolchain 'stable-aarch64-apple-darwin'. To install, run rustup component add rls

eza -lah (which rls) Permissions Size User Date Modified Name lrwxr-xr-x@ - rami3l 13 Feb 2023 ~/.cargo/bin/rls -> /opt/homebrew/bin/rustup-init

@onur-ozkan

When we remove the dummy rls (by deleting the rls source code from the repository) and stop including it in the release builds, what will users get when they invoke rustup component add rls ?

@rami3l

When we remove the dummy rls (by deleting the rls source code from the repository) and stop including it in the release builds, what will users get when they invoke rustup component add rls ?

@onur-ozkan The same thing has happened before when clippy was missing from several nightlies. No big deal.

This time, just imagine clippy is missing from all the nightlies starting from a certain date, and that's exactly what it'll be like1. Those who want to install rls will get an error when rls is not present in that channel, but we should also not break the rare use case where people really want to install from a channel before rust-analyzer was a thing...

I don't think a new deprecation warning in Rustup is worth our while: the dummy RLS has been a thing for such a long time after all.

Footnotes

  1. I'm simplifying a bit: for nightlies this is not a hard error, and the updater will happily update the current toolchain to the latest one with the component(s) in question; for betas/stables however, this is expected to be a hard error. ↩

@bors

@onur-ozkan

@onur-ozkan The same thing has happened before when clippy was missing from several nightlies. No big deal.

This time, just imagine clippy is missing from all the nightlies starting from a certain date, and that's exactly what it'll be like. Those who want to install rls will get an error when rls is not present in that channel, but we should also not break the rare use case where people really want to install from a channel before rust-analyzer was a thing...

I don't think a new deprecation warning in Rustup is worth our while: the dummy RLS has been a thing for such a long time after all.

Some of us were concerned that when people attempt to do rustup upgrades with rls component in their toolchain (even if they don't use it), this will cause a break. So, if rustup could provide a clear warning (e.g., "rls is no longer available; it will be skipped") without breaking the process, that would be useful and we could ship this PR without any hesitation.

@rami3l

@onur-ozkan I've made rust-lang/rustup#3920 trying to address this concern. Where do you think we should document the removed components though? I initially proposed the Rustup Book but it turned out that the book is updated per Rustup version so that might still cause problems if you remove components in the middle of a release cycle.

https://rust-lang.github.io/rustup-components-history looks better although I have no idea who actually controls it 👀

@onur-ozkan

@rami3l

@rbtcollins

I think we're overthinking this.

The original concerns were:

1- If they are building rls with bootstrap, the build will fail.
2- Having rls in their toolchain will break the toolchain update.

For 1) that is a source-only concern.
For 2) rustup won't error, and by definition they must be upgrading on a channel (specific version toolchains won't get updates like this) - which means that to have this suddenly break, the user must have:

The problem I see with depending on rustup to show a message is that rustup updates are often done headless - e.g. underneath vscode, or in github actions.

Sure, we can do it, but is this particular case going to show a message to the right people at the right time - and we'll have the complexity of that message in the system forever, since rustup supports installing very rust version ever.

And also, not all rust users use rustup - adding a message to rustup won't inform people using rpm or deb packages about the removal at all.

We've advertised this removal for years now....

IMO: just ship the removal. Add a clear note to the version release notes for the subsequent release.

@rbtcollins

Oh, one other issue - the approach of blocking this PR on rustup showing announcements is that rustup is self-updated after toolchains are updated. So people that run 'rustup update' when they realise they are missing the current version, will get the version, with the rls removal, and the existing message ... and then rustup will update.

As we have no daemon or cron-check of rustup versions, we have no way to ensure that people are likely to have updated it before updating rust. (We can of course do a new release in advance, but with the behaviour pattern above, this probably needs a full rust release cycle to have a good chance of being distributed in advance).

And relatedly, homebrew, snaps, and other distribution managed versions of rustup will update on their own schedule, so we'd need to coordinate with them to do a backport to their stable versions of their distributions, before rustup will be updated for their users. (Ignoring the related question of -when- their users will update rustup)

@rami3l

@ehuss

or 2) rustup won't error, and by definition they must be upgrading on a channel (specific version toolchains won't get updates like this) - which means that to have this suddenly break, the user must have:

* been using stable, and not upgraded their toolchain for > 2 years

I'm not quite understanding what this is trying to say. If we remove the rls component, and If someone has the rls component installed, and they try to update, it should error, correct? Why do you say they must not have updated in >2 years?

@rami3l

@onur-ozkan

@rami3l

@onur-ozkan

bors added a commit to rust-lang-ci/rust that referenced this pull request

Mar 11, 2025

@bors

@bors

@jieyouxu

D'oh it's the book update PR. I can't cancel this try-job, please requeue once the try-job comes back.

@bors

@Kobzol

@bors

📌 Commit a657aeb has been approved by clubby789

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Mar 11, 2025

@bors

@bors bors added S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

and removed S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

labels

Mar 12, 2025

@onur-ozkan

Signed-off-by: onur-ozkan work@onurozkan.dev

@onur-ozkan

Signed-off-by: onur-ozkan work@onurozkan.dev

@onur-ozkan

Signed-off-by: onur-ozkan work@onurozkan.dev

@onur-ozkan

Signed-off-by: onur-ozkan work@onurozkan.dev

@onur-ozkan

@bors

📌 Commit 707d4b7 has been approved by clubby789

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Mar 12, 2025

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Mar 13, 2025

@matthiaskrgr

bors added a commit to rust-lang-ci/rust that referenced this pull request

Mar 13, 2025

@bors

…iaskrgr

Rollup of 9 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

Mar 13, 2025

@bors

…iaskrgr

Rollup of 8 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Mar 13, 2025

@rust-timer

Labels

A-testsuite

Area: The testsuite used to check the correctness of rustc

A-tidy

Area: The tidy tool

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

T-bootstrap

Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)