[PATCH v2 0/2] Makefile: rust-analyzer better error handling, documentation

[PATCH v2 0/2] Makefile: rust-analyzer better error handling, documentation

[John Hubbard]

Changes since v1:

1) Thanks to Alice Ryhl for testing and reviewing v1. I had to drop both
   of those tags for this v2, because even though it is small, it's
   different enough to invalidate both, unfortunately.

2) Adopted Miguel Ojeda's suggestion of letting rust_is_available.sh
   report its results directly, thus improving the output, while also
   simplifying the patch.

3) As requested by Miguel, split out some new comment documentation
   for the rust-analyzer target into its own patch.

John Hubbard (2):
  Makefile: rust-analyzer target: better error handling
  Makefile: improve comment documentation for the rust-analyzer target

 Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)


base-commit: e5b3efbe1ab1793bb49ae07d56d0973267e65112
-- 
2.45.2

[PATCH v2 1/2] Makefile: rust-analyzer target: better error handling

[John Hubbard]

1) Provide a better error message for the "Rust not available" case.
Without this patch, one gets various misleading messages, such as:

    "No rule to make target 'rust-analyzer'"

Instead, run scripts/rust_is_available.sh directly, as a prerequisite,
and let that script report the cause of any problems, as well as
providing a link to the documentation. Thanks to Miguel Ojeda for the
idea of just letting rust_is_available.sh report its results directly.

The new output in the failure case looks like this:

$ make rust-analyzer
***
*** Rust compiler 'rustc' could not be found.
***
***
*** Please see Documentation/rust/quick-start.rst for details
*** on how to set up the Rust support.
***
make[1]: *** [/kernel_work/linux-github/Makefile:1975: rust-analyzer] Error 1
make: *** [Makefile:240: __sub-make] Error 2

Cc: Alice Ryhl <aliceryhl@google.com>
Acked-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 14427547dc1e..07308277a6f5 100644
--- a/Makefile
+++ b/Makefile
@@ -1970,6 +1970,7 @@ tags TAGS cscope gtags: FORCE
 # IDE support targets
 PHONY += rust-analyzer
 rust-analyzer:
+	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh
 	$(Q)$(MAKE) $(build)=rust $@
 
 # Script to generate missing namespace dependencies
-- 
2.45.2

Re: [PATCH v2 1/2] Makefile: rust-analyzer target: better error handling

[Finn Behrens]

On 20 Jun 2024, at 22:54, John Hubbard wrote:

> 1) Provide a better error message for the "Rust not available" case.
> Without this patch, one gets various misleading messages, such as:
>
>     "No rule to make target 'rust-analyzer'"
>
> Instead, run scripts/rust_is_available.sh directly, as a prerequisite,
> and let that script report the cause of any problems, as well as
> providing a link to the documentation. Thanks to Miguel Ojeda for the
> idea of just letting rust_is_available.sh report its results directly.
>
> The new output in the failure case looks like this:
>
> $ make rust-analyzer
> ***
> *** Rust compiler 'rustc' could not be found.
> ***
> ***
> *** Please see Documentation/rust/quick-start.rst for details
> *** on how to set up the Rust support.
> ***
> make[1]: *** [/kernel_work/linux-github/Makefile:1975: rust-analyzer] Error 1
> make: *** [Makefile:240: __sub-make] Error 2
>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Acked-by: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  Makefile | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Makefile b/Makefile
> index 14427547dc1e..07308277a6f5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1970,6 +1970,7 @@ tags TAGS cscope gtags: FORCE
>  # IDE support targets
>  PHONY += rust-analyzer
>  rust-analyzer:
> +	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh
>  	$(Q)$(MAKE) $(build)=rust $@
>
>  # Script to generate missing namespace dependencies
> -- 
> 2.45.2

Reviewed-by: Finn Behrens <me@kloenk.dev>

Re: [PATCH v2 1/2] Makefile: rust-analyzer target: better error handling

[Alice Ryhl]

On Thu, Jun 20, 2024 at 10:55 PM John Hubbard <jhubbard@nvidia.com> wrote:
>
> 1) Provide a better error message for the "Rust not available" case.
> Without this patch, one gets various misleading messages, such as:
>
>     "No rule to make target 'rust-analyzer'"
>
> Instead, run scripts/rust_is_available.sh directly, as a prerequisite,
> and let that script report the cause of any problems, as well as
> providing a link to the documentation. Thanks to Miguel Ojeda for the
> idea of just letting rust_is_available.sh report its results directly.
>
> The new output in the failure case looks like this:
>
> $ make rust-analyzer
> ***
> *** Rust compiler 'rustc' could not be found.
> ***
> ***
> *** Please see Documentation/rust/quick-start.rst for details
> *** on how to set up the Rust support.
> ***
> make[1]: *** [/kernel_work/linux-github/Makefile:1975: rust-analyzer] Error 1
> make: *** [Makefile:240: __sub-make] Error 2
>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Acked-by: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Tested-by: Alice Ryhl <aliceryhl@google.com>

[PATCH v2 2/2] Makefile: improve comment documentation for the rust-analyzer target

[John Hubbard]

Replace the cryptic phrase ("IDE support targets") that initially
appears to be about how to support old hard drives, with a few sentences
that explain what "make rust-analyzer" provides.

Cc: Alice Ryhl <aliceryhl@google.com>
Cc: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 07308277a6f5..d22491184af6 100644
--- a/Makefile
+++ b/Makefile
@@ -1967,7 +1967,9 @@ quiet_cmd_tags = GEN     $@
 tags TAGS cscope gtags: FORCE
 	$(call cmd,tags)
 
-# IDE support targets
+# Generate rust-project.json, which does for Rust what clangd's
+# compile_commands.json does for C/C++: provides a symbol browsing database for
+# code editors and IDEs (Integrated Development Environments).
 PHONY += rust-analyzer
 rust-analyzer:
 	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh
-- 
2.45.2

Re: [PATCH v2 2/2] Makefile: improve comment documentation for the rust-analyzer target

[Finn Behrens]

On 20 Jun 2024, at 22:54, John Hubbard wrote:

> Replace the cryptic phrase ("IDE support targets") that initially
> appears to be about how to support old hard drives, with a few sentences
> that explain what "make rust-analyzer" provides.
>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 07308277a6f5..d22491184af6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1967,7 +1967,9 @@ quiet_cmd_tags = GEN     $@
>  tags TAGS cscope gtags: FORCE
>  	$(call cmd,tags)
>
> -# IDE support targets
> +# Generate rust-project.json, which does for Rust what clangd's
> +# compile_commands.json does for C/C++: provides a symbol browsing database for
> +# code editors and IDEs (Integrated Development Environments).
>  PHONY += rust-analyzer
>  rust-analyzer:
>  	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh
> -- 
> 2.45.2

Reviewed-by: Finn Behrens <me@kloenk.dev>

Re: [PATCH v2 2/2] Makefile: improve comment documentation for the rust-analyzer target

[Alice Ryhl]

On Thu, Jun 20, 2024 at 10:55 PM John Hubbard <jhubbard@nvidia.com> wrote:
>
> Replace the cryptic phrase ("IDE support targets") that initially
> appears to be about how to support old hard drives, with a few sentences
> that explain what "make rust-analyzer" provides.
>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 07308277a6f5..d22491184af6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1967,7 +1967,9 @@ quiet_cmd_tags = GEN     $@
>  tags TAGS cscope gtags: FORCE
>         $(call cmd,tags)
>
> -# IDE support targets
> +# Generate rust-project.json, which does for Rust what clangd's
> +# compile_commands.json does for C/C++: provides a symbol browsing database for
> +# code editors and IDEs (Integrated Development Environments).

Is "symbol browsing database" the right word here? It's not actually a
list of symbols, but instructions for how to compile the code.

Alice

Re: [PATCH v2 2/2] Makefile: improve comment documentation for the rust-analyzer target

[Miguel Ojeda]

On Wed, Jun 26, 2024 at 10:08 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Is "symbol browsing database" the right word here? It's not actually a
> list of symbols, but instructions for how to compile the code.

Yeah, I think the sentence mixes a bit what the file is with what
(some of) the users of the file do with it.

What about something like (getting inspiration from the official documentation):

    # Generate `rust-project.json` (a file that describes the
structure of non-Cargo Rust projects) for `rust-analyzer` (an
implementation of the Language Server Protocol).

I would avoid mentioning `compile_commands.json`, since they are
slightly different the Rust one does not really contain the compile
commands.

As for "IDE", I am happy either way (i.e. removing it or not). Another
alternative that may clarify by giving context could be "Editor / IDE"
(since one may use LSP with "simple editors" and not "full IDEs"
anyway).

With that changed, if Masahiro wants to pick these two up:

    Acked-by: Miguel Ojeda <ojeda@kernel.org>

Otherwise I am happy to take them too.

Cheers,
Miguel

Re: [PATCH v2 2/2] Makefile: improve comment documentation for the rust-analyzer target

[John Hubbard]

On 6/26/24 1:42 AM, Miguel Ojeda wrote:
> On Wed, Jun 26, 2024 at 10:08 AM Alice Ryhl <aliceryhl@google.com> wrote:
>>
>> Is "symbol browsing database" the right word here? It's not actually a
>> list of symbols, but instructions for how to compile the code.
> 
> Yeah, I think the sentence mixes a bit what the file is with what
> (some of) the users of the file do with it.
> 
> What about something like (getting inspiration from the official documentation):
> 
>      # Generate `rust-project.json` (a file that describes the
> structure of non-Cargo Rust projects) for `rust-analyzer` (an
> implementation of the Language Server Protocol).
> 
> I would avoid mentioning `compile_commands.json`, since they are
> slightly different the Rust one does not really contain the compile
> commands.
> 
> As for "IDE", I am happy either way (i.e. removing it or not). Another
> alternative that may clarify by giving context could be "Editor / IDE"
> (since one may use LSP with "simple editors" and not "full IDEs"
> anyway).
> 

OK, with those changes applied (minus the backticks, which don't want to
be in this particular Makefile), we have:

Author: John Hubbard <jhubbard@nvidia.com>
Date:   Thu Jun 20 13:54:53 2024 -0700

     Makefile: improve comment documentation for the rust-analyzer target
     
     Replace the cryptic phrase ("IDE support targets") that initially
     appears to be about how to support old hard drives, with a few sentences
     that explain what "make rust-analyzer" provides.
     
     Cc: Alice Ryhl <aliceryhl@google.com>
     Reviewed-by: Finn Behrens <me@kloenk.dev>
     Acked-by: Miguel Ojeda <ojeda@kernel.org>
     Signed-off-by: John Hubbard <jhubbard@nvidia.com>

diff --git a/Makefile b/Makefile
index 204e9be0e010..7db597bdb09d 100644
--- a/Makefile
+++ b/Makefile
@@ -1967,7 +1967,9 @@ quiet_cmd_tags = GEN     $@
  tags TAGS cscope gtags: FORCE
         $(call cmd,tags)
  
-# IDE support targets
+# Generate rust-project.json (a file that describes the structure of non-Cargo
+# Rust projects) for rust-analyzer (an implementation of the Language Server
+# Protocol).
  PHONY += rust-analyzer
  rust-analyzer:
         $(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh


> With that changed, if Masahiro wants to pick these two up:
> 
>      Acked-by: Miguel Ojeda <ojeda@kernel.org>
> 
> Otherwise I am happy to take them too.
> 
> Cheers,
> Miguel

I can send out a v3, I'll wait to see if there is any additional feedback
first, though.


thanks,
-- 
John Hubbard
NVIDIA