rust-analyzer for out-of-tree modules #914

vvarma · 2022-10-17T15:41:20Z

adding support for running make rust-analyzer from an out-of-tree module.

make LLVM=1 -C <path_to_kdir> M=`pwd` rust-analyzer

ojeda · 2022-10-18T11:57:34Z

Thanks!

Please read Documentation/process/submitting-patches.rst. In particular, you will need to reword your commit messages to explain why each patch is needed, fix the title with a prefix (see previous commits to get a feeling for what is the right one), and also sign your work with your real name to certify you that you wrote it or otherwise have the right to pass it on as an open-source patch (Developer's Certificate of Origin).

scripts/generate_rust_analyzer.py

rust/Makefile

Makefile

ojeda · 2022-12-02T11:37:02Z

rust/Makefile

@@ -392,6 +392,10 @@ rust-analyzer:
 	$(Q)$(srctree)/scripts/generate_rust_analyzer.py $(srctree) $(objtree) \
 		$(RUST_LIB_SRC) > $(objtree)/rust-project.json

+rust-analyzer-extmod:
+	$(Q)$(srctree)/scripts/generate_rust_analyzer.py $(abs_srctree) $(abs_objtree) \


Why are absolute paths required compared to the existing target? It would be good to mention the reason in a comment -- I did something similar in our other rustfmt target for find:

# We match using absolute paths since `find` does not resolve them # when matching, which is a problem when e.g. `srctree` is `..`.

If it is due to some detail in the Python script, then it may make sense to resolve it / expand them / explain it in the Python script. Or, if the targets are merged (please see my other comment on that), then maybe we can use absolute paths for both here.

rust/Makefile

ojeda · 2023-01-04T11:41:50Z

Related: https://lore.kernel.org/rust-for-linux/20230103210219.5690-1-apantykhin@gmail.com/

vvarma · 2023-01-16T11:16:23Z

Hi @ojeda ! Is there anything else that I need to do to get this PR merged?

ojeda · 2023-01-16T21:08:13Z

Now that the targets are merged, would the existing target work for both cases (in-tree and out-of-tree) with abs_ paths, so that we simplify the code? Or not? i.e. this part of my comment above:

Or, if the targets are merged (please see my other comment on that), then maybe we can use absolute paths for both here.

Also, could you please submit this as a patch to the mailing list? Thanks!

ojeda · 2023-01-19T17:02:01Z

Submitted at: https://lore.kernel.org/rust-for-linux/20230118160220.776302-1-varmavinaym@gmail.com/

ojeda · 2023-01-19T19:03:59Z

@bjorn3 @aliceinwire Since you reviewed this patch previously, you may want to send the Reviewed-bys into the mailing list (the patch changed a fair bit since then, though). Most importantly, if you have time to give it a try and confirm it works in your setup (in- and/or out-of-tree) to give a Tested-by, it would be great. Thanks!

thepacketgeek · 2023-01-21T00:37:58Z

scripts/generate_rust_analyzer.py

        for path in (srctree / folder).rglob("*.rs"):
            logging.info("Checking %s", path)
            name = path.name.replace(".rs", "")

            # Skip those that are not crate roots.
-            if f"{name}.o" not in open(path.parent / "Makefile").read():
+            if os.path.exists(path.parent / "Makefile") and f"{name}.o" not in open(path.parent / "Makefile").read():


As I was testing this with the rust-out-of-tree-module example, I found that the crate was getting skipped because of this logic. I believe this will also need to search a possible Kbuild file (E.g. the example)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py index 1546b80db554..f5567825c94c 100755 --- a/scripts/generate_rust_analyzer.py +++ b/scripts/generate_rust_analyzer.py @@ -107,7 +107,15 @@ def generate_crates(srctree, objtree, sysroot_src, external_src): name = path.name.replace(".rs", "") # Skip those that are not crate roots. - if os.path.exists(path.parent / "Makefile") and f"{name}.o" not in open(path.parent / "Makefile").read(): + def is_crate_root(*filenames):+ for filename in filenames: + filepath = path.parent / filename + if os.path.exists(filepath) and f"{name}.o" in open(filepath).read(): + return True + return False + + if not is_crate_root("Makefile", "Kbuild"): + logging.warning(f"skipping") continue logging.info("Adding %s", name)

thepacketgeek

Found an issue where .o files referenced in Kbuild files are causing the script to skip the crate addition in rust-project.json

ojeda · 2023-01-21T00:41:59Z

Indeed, @bjorn3 also reported it in the mailing list today, and I agree it should be supported (and potentially a smarter way provided) -- please see the thread at https://lore.kernel.org/rust-for-linux/20230118160220.776302-1-varmavinaym@gmail.com/

thepacketgeek · 2023-01-21T00:44:15Z

Indeed, @bjorn3 also reported it in the mailing list today, and I agree it should be supported (and potentially a smarter way provided) -- please see the thread at https://lore.kernel.org/rust-for-linux/20230118160220.776302-1-varmavinaym@gmail.com/

Ah, sorry I missed it in the mailing list, will follow along there now (Y)

ojeda · 2023-01-21T01:23:50Z

No need to apologize! :)

Adds support for out-of-tree rust modules to use the `rust-analyzer` make target to generate the rust-project.json file. The change involves adding an optional parameter `external_src` to the `generate_rust_analyzer.py` which expects the path to the out-of-tree module's source directory. When this parameter is passed, I have chosen not to add the non-core modules (samples and drivers) into the result since these are not expected to be used in third party modules. Related changes are also made to the Makefile and rust/Makefile allowing the `rust-analyzer` target to be used for out-of-tree modules as well. Link: Rust-for-Linux#914 Link: Rust-for-Linux/rust-out-of-tree-module#2 Signed-off-by: Vinay Varma <varmavinaym@gmail.com>

Adds support for out-of-tree rust modules to use the `rust-analyzer` make target to generate the rust-project.json file. The change involves adding an optional parameter `external_src` to the `generate_rust_analyzer.py` which expects the path to the out-of-tree module's source directory. When this parameter is passed, I have chosen not to add the non-core modules (samples and drivers) into the result since these are not expected to be used in third party modules. Related changes are also made to the Makefile and rust/Makefile allowing the `rust-analyzer` target to be used for out-of-tree modules as well. Link: #914 Link: Rust-for-Linux/rust-out-of-tree-module#2 Signed-off-by: Vinay Varma <varmavinaym@gmail.com> Link: https://lore.kernel.org/r/20230411091714.130525-1-varmavinaym@gmail.com Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Adds support for out-of-tree rust modules to use the `rust-analyzer` make target to generate the rust-project.json file. The change involves adding an optional parameter `external_src` to the `generate_rust_analyzer.py` which expects the path to the out-of-tree module's source directory. When this parameter is passed, I have chosen not to add the non-core modules (samples and drivers) into the result since these are not expected to be used in third party modules. Related changes are also made to the Makefile and rust/Makefile allowing the `rust-analyzer` target to be used for out-of-tree modules as well. Link: Rust-for-Linux#914 Link: Rust-for-Linux/rust-out-of-tree-module#2 Signed-off-by: Vinay Varma <varmavinaym@gmail.com> Link: https://lore.kernel.org/r/20230411091714.130525-1-varmavinaym@gmail.com Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Adds support for out-of-tree rust modules to use the `rust-analyzer` make target to generate the rust-project.json file. The change involves adding an optional parameter `external_src` to the `generate_rust_analyzer.py` which expects the path to the out-of-tree module's source directory. When this parameter is passed, I have chosen not to add the non-core modules (samples and drivers) into the result since these are not expected to be used in third party modules. Related changes are also made to the Makefile and rust/Makefile allowing the `rust-analyzer` target to be used for out-of-tree modules as well. Link: Rust-for-Linux#914 Link: Rust-for-Linux/rust-out-of-tree-module#2

bjorn3 mentioned this pull request Oct 18, 2022

Fix rust-analyzer's complaint about not finding proc macro library #918

Closed

vvarma force-pushed the extmod/rustanalyzer branch from d812f19 to c688922 Compare October 23, 2022 15:44

bjorn3 suggested changes Oct 23, 2022

View reviewed changes

scripts/generate_rust_analyzer.py Show resolved Hide resolved

bjorn3 approved these changes Oct 24, 2022

View reviewed changes

aliceinwire approved these changes Dec 2, 2022

View reviewed changes

ojeda reviewed Dec 2, 2022

View reviewed changes

vvarma force-pushed the extmod/rustanalyzer branch from c688922 to 96f2342 Compare December 3, 2022 11:39

vvarma force-pushed the extmod/rustanalyzer branch 2 times, most recently from 23d263e to 2fca591 Compare January 18, 2023 15:12

ojeda mentioned this pull request Jan 19, 2023

Adding support to generate rust-project.json Rust-for-Linux/rust-out-of-tree-module#2

Closed

thepacketgeek mentioned this pull request Jan 20, 2023

Adding section about rust-analyzer in README Rust-for-Linux/rust-out-of-tree-module#4

Merged

thepacketgeek reviewed Jan 21, 2023

View reviewed changes

vvarma force-pushed the extmod/rustanalyzer branch from 2fca591 to 33102ff Compare January 21, 2023 04:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rust-analyzer for out-of-tree modules #914

rust-analyzer for out-of-tree modules #914

vvarma commented Oct 17, 2022

ojeda commented Oct 18, 2022

ojeda Dec 2, 2022 •

edited

Loading

ojeda commented Jan 4, 2023

vvarma commented Jan 16, 2023

ojeda commented Jan 16, 2023

ojeda commented Jan 19, 2023

ojeda commented Jan 19, 2023 •

edited

Loading

thepacketgeek Jan 21, 2023

thepacketgeek left a comment

ojeda commented Jan 21, 2023

thepacketgeek commented Jan 21, 2023

ojeda commented Jan 21, 2023

rust-analyzer for out-of-tree modules #914

Are you sure you want to change the base?

rust-analyzer for out-of-tree modules #914

Conversation

vvarma commented Oct 17, 2022

ojeda commented Oct 18, 2022

ojeda Dec 2, 2022 • edited Loading

Choose a reason for hiding this comment

ojeda commented Jan 4, 2023

vvarma commented Jan 16, 2023

ojeda commented Jan 16, 2023

ojeda commented Jan 19, 2023

ojeda commented Jan 19, 2023 • edited Loading

thepacketgeek Jan 21, 2023

Choose a reason for hiding this comment

thepacketgeek left a comment

Choose a reason for hiding this comment

ojeda commented Jan 21, 2023

thepacketgeek commented Jan 21, 2023

ojeda commented Jan 21, 2023

ojeda Dec 2, 2022 •

edited

Loading

ojeda commented Jan 19, 2023 •

edited

Loading