diff mbox series

scripts: rust-analyzer: Skip crate module directories

Message ID 20230307120736.75492-1-nmi@metaspace.dk
State New
Headers
Series scripts: rust-analyzer: Skip crate module directories |

Commit Message

Andreas Hindborg March 7, 2023, 12:07 p.m. UTC 
  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 <a.hindborg@samsung.com>
---
 scripts/generate_rust_analyzer.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 8c20eb7e6a27b2c493b0bbb435e75cae7135634f

Comments

Miguel Ojeda March 7, 2023, 12:38 p.m. UTC | #1 
On Tue, Mar 7, 2023 at 1:08 PM Andreas Hindborg <nmi@metaspace.dk> 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/20230118160220.776302-1-varmavinaym@gmail.com/.

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
Andreas Hindborg March 7, 2023, 12:53 p.m. UTC | #2 
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> writes:

> On Tue, Mar 7, 2023 at 1:08 PM Andreas Hindborg <nmi@metaspace.dk> 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/20230118160220.776302-1-varmavinaym@gmail.com/.

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
Gary Guo March 7, 2023, 4:32 p.m. UTC | #3 
On Tue, 7 Mar 2023 13:38:10 +0100
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Tue, Mar 7, 2023 at 1:08 PM Andreas Hindborg <nmi@metaspace.dk> 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/20230118160220.776302-1-varmavinaym@gmail.com/.
> 
> 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
Miguel Ojeda March 7, 2023, 5:14 p.m. UTC | #4 
On Tue, Mar 7, 2023 at 5:32 PM Gary Guo <gary@garyguo.net> 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
Miguel Ojeda April 6, 2023, 10:33 p.m. UTC | #5 
On Tue, Mar 7, 2023 at 1:08 PM Andreas Hindborg <nmi@metaspace.dk> 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
Miguel Ojeda April 6, 2023, 10:33 p.m. UTC | #6 
On Tue, Mar 7, 2023 at 6:14 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> 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
diff mbox series

Patch

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)