[PATCH v3] rust: Support latest version of `rust-analyzer`

[PATCH v3] rust: Support latest version of `rust-analyzer`

[Sarthak Singh]

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 thier 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`.

Link: https://github.com/rust-lang/rust-analyzer/pull/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>
---
Hello,

This version contains more a more detailed justification for this patch.
Also fixes some issues with using the wrong email amoung others.

Previous Version:
  v2: Fixed typos
  v1: Initial patch

 rust/Makefile                     | 2 +-
 scripts/generate_rust_analyzer.py | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/rust/Makefile b/rust/Makefile
index bf05e65365da..e3d0ba099b18 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -350,7 +350,7 @@ rust-analyzer:
 	$(Q)$(srctree)/scripts/generate_rust_analyzer.py \
 		--cfgs='core=$(core-cfgs)' --cfgs='alloc=$(alloc-cfgs)' \
 		$(realpath $(srctree)) $(realpath $(objtree)) \
-		$(RUST_LIB_SRC) $(KBUILD_EXTMOD) > \
+		$(rustc_sysroot) $(RUST_LIB_SRC) $(KBUILD_EXTMOD) > \
 		$(if $(KBUILD_EXTMOD),$(extmod_prefix),$(objtree))/rust-project.json
 
 redirect-intrinsics = \
diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index f270c7b0cf34..8529c3940136 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -145,6 +145,7 @@ def main():
     parser.add_argument('--cfgs', action='append', default=[])
     parser.add_argument("srctree", type=pathlib.Path)
     parser.add_argument("objtree", type=pathlib.Path)
+    parser.add_argument("sysroot", type=pathlib.Path)
     parser.add_argument("sysroot_src", type=pathlib.Path)
     parser.add_argument("exttree", type=pathlib.Path, nargs="?")
     args = parser.parse_args()
@@ -154,9 +155,12 @@ def main():
         level=logging.INFO if args.verbose else logging.WARNING
     )
 
+    # Making sure that the sysroot and sysroot_src belong to the same toolchain
+    assert args.sysroot in args.sysroot_src.parents
+
     rust_project = {
         "crates": generate_crates(args.srctree, args.objtree, args.sysroot_src, args.exttree, args.cfgs),
-        "sysroot_src": str(args.sysroot_src),
+        "sysroot": str(args.sysroot),
     }
 
     json.dump(rust_project, sys.stdout, sort_keys=True, indent=4)
-- 
2.45.2

Re: [PATCH v3] rust: Support latest version of `rust-analyzer`

[Sarthak Singh]

A polite reminder to please take a look at this patch.

Re: [PATCH v3] rust: Support latest version of `rust-analyzer`

[Dirk Behme]

On 31.07.2024 10:00, Sarthak Singh wrote:
> A polite reminder to please take a look at this patch.

Would it make sense to apply this to rust-fixes and/or send it to Linus 
as fix for inclusion into 6.11-rcX?

Best regards

Dirk

Re: [PATCH v3] rust: Support latest version of `rust-analyzer`

[Miguel Ojeda]

On Mon, Aug 5, 2024 at 12:41 PM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>
> Would it make sense to apply this to rust-fixes and/or send it to Linus
> as fix for inclusion into 6.11-rcX?

Yes, that is the plan -- it would be nice if someone else confirms it
fixes it for them, though (especially if they use another setup than
yours, e.g. no VS Code) and gives a Tested-by tag for that.

Thanks!

Cheers,
Miguel

Re: [PATCH v3] rust: Support latest version of `rust-analyzer`

[Miguel Ojeda]

On Wed, Jul 31, 2024 at 9:53 AM Sarthak Singh <sarthak.singh99@gmail.com> wrote:
>
> A polite reminder to please take a look at this patch.

It is not forgotten :)

I think you didn't reply to my v2 message? The changelog does not
really explain what changed, so to understand what is going on one
needs to recall all the details, diff your patch messages and try to
understand if there is still something missing from the v2 discussion,
e.g.

    So I assume things could break (or be confusing) if someone provides
    (intentionally) a `RUST_LIB_SRC` that is different than the one in the
    actual sysroot, no?

Please do not get me wrong, the patch contents are probably good, I am
just saying this because it helps others keep up so that you get
feedback sooner.

In this commit message, you say:

    Now `sysroot` is a required parameter for a `rust-project.json` file.

But that does not seem to be true given their  docs. I guess you mean
for combination in particular, so I guess it it is fine.

(Cc'd Lukas)

Cheers,
Miguel

Re: [PATCH v3] rust: Support latest version of `rust-analyzer`

[Miguel Ojeda]

On Mon, Aug 5, 2024 at 12:44 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> But that does not seem to be true given their  docs. I guess you mean
> for combination in particular, so I guess it it is fine.

It seems I had a brain shortcircuit here given the several typos.

In particular, I meant: "for this/our combination in particular"
(rather than in general).

Cheers,
Miguel

Re: [PATCH v3] rust: Support latest version of `rust-analyzer`

[Miguel Ojeda]

On Mon, Aug 5, 2024 at 1:43 PM Lukas Wirth <lukastw97@gmail.com> wrote:
>
> `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.

Thanks a lot Lukas for confirming that.

Cheers,
Miguel

Re: [PATCH v3] rust: Support latest version of `rust-analyzer`

[Miguel Ojeda]

On Wed, Jul 24, 2024 at 7:27 PM Sarthak Singh <sarthak.singh99@gmail.com> wrote:
>
> Sets the `sysroot` field in rust-project.json which is now needed in
> newer versions of rust-analyzer instead of the `sysroot_src` field.

Applied to `rust-fixes` -- thanks everyone! I tested it in a vim +
coc.nvim setup and that error goes away for me too.

    [ 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 ]

    [ Formatted comment, fixed typo and removed spurious empty line. - Miguel ]

Cheers,
Miguel

Re: [PATCH v3] rust: Support latest version of `rust-analyzer`

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1394 bytes --]

On Wed, Aug 07, 2024 at 01:19:11AM +0200, Miguel Ojeda wrote:
> On Wed, Jul 24, 2024 at 7:27 PM Sarthak Singh <sarthak.singh99@gmail.com> wrote:
> >
> > Sets the `sysroot` field in rust-project.json which is now needed in
> > newer versions of rust-analyzer instead of the `sysroot_src` field.
> 
> Applied to `rust-fixes` -- thanks everyone! I tested it in a vim +
> coc.nvim setup and that error goes away for me too.
> 
>     [ 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 ]
> 
>     [ Formatted comment, fixed typo and removed spurious empty line. - Miguel ]

Why is it not possible to fall back to the inferred path when the
argument is not provided? It's rather annoying to require different
incantations for the script while hoping around between different
kernels.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3] rust: Support latest version of `rust-analyzer`

[Miguel Ojeda]

On Tue, Aug 27, 2024 at 5:07 PM Conor Dooley <conor@kernel.org> wrote:
>
> Why is it not possible to fall back to the inferred path when the
> argument is not provided? It's rather annoying to require different
> incantations for the script while hoping around between different
> kernels.

Why do you need to call the script directly?

Cheers,
Miguel