2023-03-07 12:08:09

by Andreas Hindborg

[permalink] [raw]
Subject: [PATCH] scripts: rust-analyzer: Skip crate module directories

When generating rust-analyzer configuration, skip module directories. This fixes
an issue that occur if we have

- drivers/block/driver.rs
- drivers/block/driver_mod/mod.rs

If `driver_mod` is a module of the crate `driver`, the directory `driver_mod`
may not contain `Makefile`, and `generate_rust_analyzer.py` will fail.

Signed-off-by: Andreas Hindborg <[email protected]>
---
scripts/generate_rust_analyzer.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index ecc7ea9a4dcf..e8c643fb2488 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -104,7 +104,7 @@ def generate_crates(srctree, objtree, sysroot_src):
name = path.name.replace(".rs", "")

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

logging.info("Adding %s", name)

base-commit: 8c20eb7e6a27b2c493b0bbb435e75cae7135634f
--
2.39.2



2023-03-07 12:38:29

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] scripts: rust-analyzer: Skip crate module directories

On Tue, Mar 7, 2023 at 1:08 PM Andreas Hindborg <[email protected]> wrote:
>
> When generating rust-analyzer configuration, skip module directories.

This is https://github.com/Rust-for-Linux/linux/pull/883, also handled
by Vinay's patch
https://lore.kernel.org/rust-for-linux/[email protected]/.

Lina's approach is arguably a bit more idiomatic in Python in that it
is usually encouraged to follow the "Easier to ask for forgiveness
than permission" approach.

Lina, would you like to submit yours? Or do you prefer a `Link: ` /
`Reported-by: ` / `Co-developed-by: ` here?

> If `driver_mod` is a module of the crate `driver`, the directory `driver_mod`
> may not contain `Makefile`, and `generate_rust_analyzer.py` will fail.

By the way, note that in the kernel crate we are avoiding `mod.rs`
files, instead using `name.rs` in the parent folder, in other to make
it easier to find the files. I will add a note about it in the docs.

Cheers,
Miguel

2023-03-07 13:00:07

by Andreas Hindborg

[permalink] [raw]
Subject: Re: [PATCH] scripts: rust-analyzer: Skip crate module directories


Miguel Ojeda <[email protected]> writes:

> On Tue, Mar 7, 2023 at 1:08 PM Andreas Hindborg <[email protected]> wrote:
>>
>> When generating rust-analyzer configuration, skip module directories.
>
> This is https://github.com/Rust-for-Linux/linux/pull/883, also handled
> by Vinay's patch
> https://lore.kernel.org/rust-for-linux/[email protected]/.

Awesome, three solutions to the same problem ????

>
> Lina's approach is arguably a bit more idiomatic in Python in that it
> is usually encouraged to follow the "Easier to ask for forgiveness
> than permission" approach.
>
> Lina, would you like to submit yours? Or do you prefer a `Link: ` /
> `Reported-by: ` / `Co-developed-by: ` here?
>
>> If `driver_mod` is a module of the crate `driver`, the directory `driver_mod`
>> may not contain `Makefile`, and `generate_rust_analyzer.py` will fail.
>
> By the way, note that in the kernel crate we are avoiding `mod.rs`
> files, instead using `name.rs` in the parent folder, in other to make
> it easier to find the files. I will add a note about it in the docs.

Thanks for pointing that out :)

BR Andreas

2023-03-07 16:35:40

by Gary Guo

[permalink] [raw]
Subject: Re: [PATCH] scripts: rust-analyzer: Skip crate module directories

On Tue, 7 Mar 2023 13:38:10 +0100
Miguel Ojeda <[email protected]> wrote:

> On Tue, Mar 7, 2023 at 1:08 PM Andreas Hindborg <[email protected]> wrote:
> >
> > When generating rust-analyzer configuration, skip module directories.
>
> This is https://github.com/Rust-for-Linux/linux/pull/883, also handled
> by Vinay's patch
> https://lore.kernel.org/rust-for-linux/[email protected]/.
>
> Lina's approach is arguably a bit more idiomatic in Python in that it
> is usually encouraged to follow the "Easier to ask for forgiveness
> than permission" approach.
>
> Lina, would you like to submit yours? Or do you prefer a `Link: ` /
> `Reported-by: ` / `Co-developed-by: ` here?
>
> > If `driver_mod` is a module of the crate `driver`, the directory `driver_mod`
> > may not contain `Makefile`, and `generate_rust_analyzer.py` will fail.
>
> By the way, note that in the kernel crate we are avoiding `mod.rs`
> files, instead using `name.rs` in the parent folder, in other to make
> it easier to find the files. I will add a note about it in the docs.

I personally think mod.rs makes it easier for me to find files because
all related stuff are contained inside a single directory, especially
the parent modules and submodules are closed related.

That's just personal opinion though.

Best,
Gary

2023-03-07 17:19:33

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] scripts: rust-analyzer: Skip crate module directories

On Tue, Mar 7, 2023 at 5:32 PM Gary Guo <[email protected]> wrote:
>
> I personally think mod.rs makes it easier for me to find files because
> all related stuff are contained inside a single directory, especially
> the parent modules and submodules are closed related.
>
> That's just personal opinion though.

I don't have a strong opinion either way -- this was originally done
to improve fuzzy searching, see commit 829c2df153d7 ("rust: move `net`
and `sync` modules to uniquely-named files") upstream:

This is so that each file in the module has a unique name instead of the
generic `mod.rs` name. It makes it easier to open files when using fuzzy
finders like `fzf` once names are unique.

Cheers,
Miguel

2023-04-06 22:54:58

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] scripts: rust-analyzer: Skip crate module directories

On Tue, Mar 7, 2023 at 6:14 PM Miguel Ojeda
<[email protected]> wrote:
>
> I don't have a strong opinion either way -- this was originally done
> to improve fuzzy searching, see commit 829c2df153d7 ("rust: move `net`
> and `sync` modules to uniquely-named files") upstream:
>
> This is so that each file in the module has a unique name instead of the
> generic `mod.rs` name. It makes it easier to open files when using fuzzy
> finders like `fzf` once names are unique.

Apparently the "encouraged" way is using `name.rs`:

https://doc.rust-lang.org/stable/reference/items/modules.html#module-source-filenames
https://doc.rust-lang.org/edition-guide/rust-2018/path-changes.html#no-more-modrs

Another argument I saw for `name.rs` is that one can easily the name
of the file in editor's tabs/titles, and some of the editors can add
part of the path to disambiguate, which may take more space in the UI.

Cheers,
Miguel

2023-04-06 23:21:37

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] scripts: rust-analyzer: Skip crate module directories

On Tue, Mar 7, 2023 at 1:08 PM Andreas Hindborg <[email protected]> wrote:
>
> When generating rust-analyzer configuration, skip module directories. This fixes
> an issue that occur if we have
>
> - drivers/block/driver.rs
> - drivers/block/driver_mod/mod.rs
>
> If `driver_mod` is a module of the crate `driver`, the directory `driver_mod`
> may not contain `Makefile`, and `generate_rust_analyzer.py` will fail.

I picked Lina's for `rust-fixes` from
https://github.com/Rust-for-Linux/linux/pull/883.

Cheers,
Miguel