-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Allow sysroots to only consist of the source root dir #17287
Merged
Merged
Conversation
This file contains 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
rustbot
added
the
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
label
May 23, 2024
@bors r+ |
bors
added a commit
that referenced
this pull request
May 23, 2024
Allow sysroots to only consist of the source root dir Fixes #17159 This PR encodes the `None` case of an optional sysroot into `Sysroot` itself. This simplifies a lot of things and allows us to have sysroots that consist of nothing, only standard library sources, everything but the standard library sources or everything. This makes things a lot more flexible. Additionally, this removes the workspace status bar info again, as it turns out that that can be too much information for the status bar to handle (this is better rendered somewhere else, like in the status view).
💔 Test failed - checks-actions |
Veykril
force-pushed
the
sysroot-encode-empty
branch
from
May 23, 2024 18:12
30fa6aa
to
bd37e27
Compare
@bors r+ |
☀️ Test successful - checks-actions |
This was referenced May 23, 2024
This was referenced May 28, 2024
This was referenced Jun 11, 2024
ojeda
pushed a commit
to Rust-for-Linux/linux
that referenced
this pull request
Aug 6, 2024
Sets the `sysroot` field in rust-project.json which is now needed in newer versions of rust-analyzer instead of the `sysroot_src` field. Till [1] `rust-analyzer` used to guess the `sysroot` based on the `sysroot_src` at [2]. Now `sysroot` is a required parameter for a `rust-project.json` file. It is required because `rust-analyzer` need it to find the proc-macro server [3]. In the current version of `rust-analyzer` the `sysroot_src` is only used to include the inbuilt library crates (std, core, alloc, etc) [4]. Since we already specify the core library to be included in the `rust-project.json` we don't need to define the `sysroot_src`. Code editors like VS Code try to use the latest version of rust-analyzer (which is updated every week) instead of the version of rust-analyzer that comes with the rustup toolchain (which is updated every six weeks along with the rust version). Without this change `rust-analyzer` is breaking for anyone using VS Code. As they are getting the latest version of `rust-analyzer` with the changes made in [1]. `rust-analyzer` will also start breaking for other developers as they update their rust version (assuming that also updates the rust-analyzer version on their system). This patch should work with every setup as there is no more guess work being done by `rust-analyzer`. [ Lukas, who leads the rust-analyzer team, says: `sysroot_src` is required now if you want to have the sysroot source libraries be loaded. I think we used to infer it as `{sysroot}/lib/rustlib/src/rust/library` before when only the `sysroot` field was given but that was since changed to make it possible in having a sysroot without the standard library sources (that is only have the binaries available). So if you want the library sources to be loaded by rust-analyzer you will have to set that field as well now. - Miguel ] Link: rust-lang/rust-analyzer#17287 [1] Link: https://github.com/rust-lang/rust-analyzer/blob/f372a8a1176ff8dd5f45ab2ddd45f3530db0374f/crates/project-model/src/workspace.rs#L367-L374 [2] Link: https://github.com/rust-lang/rust-analyzer/blob/eeb192b79aeac47b40add66347022af17a74fbaf/crates/project-model/src/sysroot.rs#L180-L192 [3] Link: https://github.com/search?q=repo%3AVeykril%2Frust-analyzer%20src_root()&type=code [4] Tested-by: Dirk Behme <dirk.behme@de.bosch.com> Signed-off-by: Sarthak Singh <sarthak.singh99@gmail.com> Link: https://lore.kernel.org/r/20240724172713.899399-1-sarthak.singh99@gmail.com [ Formatted comment, fixed typo and removed spurious empty line. - Miguel ] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda
pushed a commit
to Rust-for-Linux/linux
that referenced
this pull request
Aug 6, 2024
Sets the `sysroot` field in rust-project.json which is now needed in newer versions of rust-analyzer instead of the `sysroot_src` field. Till [1] `rust-analyzer` used to guess the `sysroot` based on the `sysroot_src` at [2]. Now `sysroot` is a required parameter for a `rust-project.json` file. It is required because `rust-analyzer` need it to find the proc-macro server [3]. In the current version of `rust-analyzer` the `sysroot_src` is only used to include the inbuilt library crates (std, core, alloc, etc) [4]. Since we already specify the core library to be included in the `rust-project.json` we don't need to define the `sysroot_src`. Code editors like VS Code try to use the latest version of rust-analyzer (which is updated every week) instead of the version of rust-analyzer that comes with the rustup toolchain (which is updated every six weeks along with the rust version). Without this change `rust-analyzer` is breaking for anyone using VS Code. As they are getting the latest version of `rust-analyzer` with the changes made in [1]. `rust-analyzer` will also start breaking for other developers as they update their rust version (assuming that also updates the rust-analyzer version on their system). This patch should work with every setup as there is no more guess work being done by `rust-analyzer`. [ Lukas, who leads the rust-analyzer team, says: `sysroot_src` is required now if you want to have the sysroot source libraries be loaded. I think we used to infer it as `{sysroot}/lib/rustlib/src/rust/library` before when only the `sysroot` field was given but that was since changed to make it possible in having a sysroot without the standard library sources (that is only have the binaries available). So if you want the library sources to be loaded by rust-analyzer you will have to set that field as well now. - Miguel ] Link: rust-lang/rust-analyzer#17287 [1] Link: https://github.com/rust-lang/rust-analyzer/blob/f372a8a1176ff8dd5f45ab2ddd45f3530db0374f/crates/project-model/src/workspace.rs#L367-L374 [2] Link: https://github.com/rust-lang/rust-analyzer/blob/eeb192b79aeac47b40add66347022af17a74fbaf/crates/project-model/src/sysroot.rs#L180-L192 [3] Link: https://github.com/search?q=repo%3AVeykril%2Frust-analyzer%20src_root()&type=code [4] Tested-by: Dirk Behme <dirk.behme@de.bosch.com> Signed-off-by: Sarthak Singh <sarthak.singh99@gmail.com> Link: https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/How.20to.20rust-analyzer.20correctly.20working Link: https://lore.kernel.org/r/20240724172713.899399-1-sarthak.singh99@gmail.com [ Formatted comment, fixed typo and removed spurious empty line. - Miguel ] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Mr-Bossman
pushed a commit
to Mr-Bossman/linux
that referenced
this pull request
Sep 16, 2024
Sets the `sysroot` field in rust-project.json which is now needed in newer versions of rust-analyzer instead of the `sysroot_src` field. Till [1] `rust-analyzer` used to guess the `sysroot` based on the `sysroot_src` at [2]. Now `sysroot` is a required parameter for a `rust-project.json` file. It is required because `rust-analyzer` need it to find the proc-macro server [3]. In the current version of `rust-analyzer` the `sysroot_src` is only used to include the inbuilt library crates (std, core, alloc, etc) [4]. Since we already specify the core library to be included in the `rust-project.json` we don't need to define the `sysroot_src`. Code editors like VS Code try to use the latest version of rust-analyzer (which is updated every week) instead of the version of rust-analyzer that comes with the rustup toolchain (which is updated every six weeks along with the rust version). Without this change `rust-analyzer` is breaking for anyone using VS Code. As they are getting the latest version of `rust-analyzer` with the changes made in [1]. `rust-analyzer` will also start breaking for other developers as they update their rust version (assuming that also updates the rust-analyzer version on their system). This patch should work with every setup as there is no more guess work being done by `rust-analyzer`. [ Lukas, who leads the rust-analyzer team, says: `sysroot_src` is required now if you want to have the sysroot source libraries be loaded. I think we used to infer it as `{sysroot}/lib/rustlib/src/rust/library` before when only the `sysroot` field was given but that was since changed to make it possible in having a sysroot without the standard library sources (that is only have the binaries available). So if you want the library sources to be loaded by rust-analyzer you will have to set that field as well now. - Miguel ] Link: rust-lang/rust-analyzer#17287 [1] Link: https://github.com/rust-lang/rust-analyzer/blob/f372a8a1176ff8dd5f45ab2ddd45f3530db0374f/crates/project-model/src/workspace.rs#L367-L374 [2] Link: https://github.com/rust-lang/rust-analyzer/blob/eeb192b79aeac47b40add66347022af17a74fbaf/crates/project-model/src/sysroot.rs#L180-L192 [3] Link: https://github.com/search?q=repo%3AVeykril%2Frust-analyzer%20src_root()&type=code [4] Tested-by: Dirk Behme <dirk.behme@de.bosch.com> Signed-off-by: Sarthak Singh <sarthak.singh99@gmail.com> Link: https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/How.20to.20rust-analyzer.20correctly.20working Link: https://lore.kernel.org/r/20240724172713.899399-1-sarthak.singh99@gmail.com [ Formatted comment, fixed typo and removed spurious empty line. - Miguel ] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #17159
This PR encodes the
None
case of an optional sysroot intoSysroot
itself. This simplifies a lot of things and allows us to have sysroots that consist of nothing, only standard library sources, everything but the standard library sources or everything. This makes things a lot more flexible. Additionally, this removes the workspace status bar info again, as it turns out that that can be too much information for the status bar to handle (this is better rendered somewhere else, like in the status view).