[PATCH v5 00/13] rust: generate_rust_analyzer.py: define host crates and scripts

[PATCH v5 00/13] rust: generate_rust_analyzer.py: define host crates and scripts

[Tamir Duberstein]

This series updates rust-project.json to differentiate between host and
target crates, where the former are used as dependencies of the `macros`
crate. Please see individual commit messages for details.

The first 3 commits contain mechanical formatting changes and are
optional. The series can be taken without them.

I avoided more significant formatting or changes where possible to
reduce the diff. Unfortunately `scripts/generate_rust_analyzer.py` is
not consistently formatted before nor after this series.

The 5th commit ("scripts: generate_rust_analyzer.py: use
str(pathlib.Path)") can also be considered optional. It removes an
inconsistency I noticed while working on this series and which occurs on
a line which churns in this series anyway.

The trailing commits can also be considered optional, as they can be
submitted separately. I included them in this series because they depend
on it, but they can be split out if this is preferred.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v5:
- Split remove `"is_proc_macro": false` into separate patch. (Daniel
  Almeida)
- Move whitespace change from "add type hints" to "add missing
  whitespace".
- Add patch to avoid default arguments and force keyword arguments.
- Add patch to add type hints for CLI parameters.
- Replace some instances of `str.replace` with `str.{l,r}strip`.
- Rebase on next-20250325. This must be rooted on linux-next because
  it's the only place rust-next and rust-fixes have been merged.
- Link to v4: https://lore.kernel.org/r/20250322-rust-analyzer-host-v4-0-1f51f9c907eb@gmail.com

Changes in v4:
- Fix typo (s/generate/generated/).
- Pull Trevor's suggested change into a separate patch with a
  Suggested-by tag.
- Add patch to avoid file descriptor leak.
- Add patch to generate rust-project.json entries for scripts/*.rs.
- Add patch to use `cfg_groups` to reduce size of `rust-project.json` by
  90%.
- Link to v3: https://lore.kernel.org/r/20250319-rust-analyzer-host-v3-0-311644ee23d2@gmail.com

Changes in v3:
- Rebase on linux-next. This is needed to pick up all the conflicts from
  both rust-next and rust-fixes.
- Drop `uv` from `mypy` command. (Trevor Gross)
- Add `--python-version 3.8` to `mypy` command. (Trevor Gross)
- `from typings import ...` directly. (Trevor Gross)
- Extract `build_crate` and `register_crate` to avoid peeking into
  `crates[-1]`. (Trevor Gross)
- Link to v2: https://lore.kernel.org/r/20250311-rust-analyzer-host-v2-0-30220e116511@gmail.com

Changes in v2:
- Rebased on "rust: fix rust-analyzer configuration for generated files" [1]
  Link: https://lore.kernel.org/all/CANiq72nv7nQ+1BinCHe2qsvwdUb-y9t7x=RGSppi_n9TBXNHpw@mail.gmail.com/ [1]
- Link to v1: https://lore.kernel.org/r/20250209-rust-analyzer-host-v1-0-a2286a2a2fa3@gmail.com

---
Tamir Duberstein (13):
      scripts: generate_rust_analyzer.py: add missing whitespace
      scripts: generate_rust_analyzer.py: use double quotes
      scripts: generate_rust_analyzer.py: add trailing comma
      scripts: generate_rust_analyzer.py: extract `{build,register}_crate`
      scripts: generate_rust_analyzer.py: drop `"is_proc_macro": false`
      scripts: generate_rust_analyzer.py: add type hints
      scripts: generate_rust_analyzer.py: avoid optional arguments
      scripts: generate_rust_analyzer.py: use str(pathlib.Path)
      scripts: generate_rust_analyzer.py: identify crates explicitly
      scripts: generate_rust_analyzer.py: define host crates
      scripts: generate_rust_analyzer.py: avoid FD leak
      scripts: generate_rust_analyzer.py: define scripts
      scripts: generate_rust_analyzer.py: use `cfg_groups`

 scripts/generate_rust_analyzer.py | 330 ++++++++++++++++++++++++++++----------
 1 file changed, 243 insertions(+), 87 deletions(-)
---
base-commit: eb4bc4b07f66f01618d9cb1aa4eaef59b1188415
change-id: 20250209-rust-analyzer-host-43b108655578

Best regards,
-- 
Tamir Duberstein <tamird@gmail.com>

[PATCH v5 01/13] scripts: generate_rust_analyzer.py: add missing whitespace

[Tamir Duberstein]

Add a space before the `/` operator for consistency with surrounding
code and code formatting tools. Add a second newline between top-level
items in accordance with PEP 8[1]:

> Surround top-level function and class definitions with two blank
lines.

This change was made by a code formatting tool.

Link: https://peps.python.org/pep-0008/ [1]
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 scripts/generate_rust_analyzer.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index a0e5a0aef444..7bd7f7ef2529 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -118,7 +118,7 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
     ):
         append_crate(
             display_name,
-            srctree / "rust"/ display_name / "lib.rs",
+            srctree / "rust" / display_name / "lib.rs",
             deps,
             cfg=cfg,
         )
@@ -167,6 +167,7 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
 
     return crates
 
+
 def main():
     parser = argparse.ArgumentParser()
     parser.add_argument('--verbose', '-v', action='store_true')
@@ -193,5 +194,6 @@ def main():
 
     json.dump(rust_project, sys.stdout, sort_keys=True, indent=4)
 
+
 if __name__ == "__main__":
     main()

-- 
2.49.0

Re: [PATCH v5 01/13] scripts: generate_rust_analyzer.py: add missing whitespace

[Trevor Gross]

On Tue, Mar 25, 2025 at 3:06 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Add a space before the `/` operator for consistency with surrounding
> code and code formatting tools. Add a second newline between top-level
> items in accordance with PEP 8[1]:
>
> > Surround top-level function and class definitions with two blank
> lines.
>
> This change was made by a code formatting tool.
>
> Link: https://peps.python.org/pep-0008/ [1]
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  scripts/generate_rust_analyzer.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH v5 02/13] scripts: generate_rust_analyzer.py: use double quotes

[Tamir Duberstein]

Replace inconsistent use of single quotes with double quotes.

This change was made by a code formatting tool.

Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 scripts/generate_rust_analyzer.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index 7bd7f7ef2529..7d4274ee90ab 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -170,8 +170,8 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
 
 def main():
     parser = argparse.ArgumentParser()
-    parser.add_argument('--verbose', '-v', action='store_true')
-    parser.add_argument('--cfgs', action='append', default=[])
+    parser.add_argument("--verbose", "-v", action="store_true")
+    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)

-- 
2.49.0

Re: [PATCH v5 02/13] scripts: generate_rust_analyzer.py: use double quotes

[Trevor Gross]

On Tue, Mar 25, 2025 at 3:06 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Replace inconsistent use of single quotes with double quotes.
>
> This change was made by a code formatting tool.
>
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  scripts/generate_rust_analyzer.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH v5 03/13] scripts: generate_rust_analyzer.py: add trailing comma

[Tamir Duberstein]

Add missing trailing comma on multi-line function call as suggested by
PEP-8:

> The pattern is to put each value (etc.) on a line by itself, always
> adding a trailing comma, and add the close parenthesis/bracket/brace
> on the next line.

This change was made by a code formatting tool.

Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 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 7d4274ee90ab..1b6b69501131 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -181,7 +181,7 @@ def main():
 
     logging.basicConfig(
         format="[%(asctime)s] [%(levelname)s] %(message)s",
-        level=logging.INFO if args.verbose else logging.WARNING
+        level=logging.INFO if args.verbose else logging.WARNING,
     )
 
     # Making sure that the `sysroot` and `sysroot_src` belong to the same toolchain.

-- 
2.49.0

Re: [PATCH v5 03/13] scripts: generate_rust_analyzer.py: add trailing comma

[Trevor Gross]

On Tue, Mar 25, 2025 at 3:06 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Add missing trailing comma on multi-line function call as suggested by
> PEP-8:
>
> > The pattern is to put each value (etc.) on a line by itself, always
> > adding a trailing comma, and add the close parenthesis/bracket/brace
> > on the next line.
>
> This change was made by a code formatting tool.
>
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  scripts/generate_rust_analyzer.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH v5 04/13] scripts: generate_rust_analyzer.py: extract `{build,register}_crate`

[Tamir Duberstein]

Extract helpers from `append_crate` to avoid the need to peek into
`crates[-1]`. This improves readability.

Suggested-by: Trevor Gross <tmgross@umich.edu>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 scripts/generate_rust_analyzer.py | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index 1b6b69501131..71e6038964f6 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -35,7 +35,14 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
     crates_indexes = {}
     crates_cfgs = args_crates_cfgs(cfgs)
 
-    def append_crate(display_name, root_module, deps, cfg=[], is_workspace_member=True, is_proc_macro=False):
+    def build_crate(
+        display_name,
+        root_module,
+        deps,
+        cfg=[],
+        is_workspace_member=True,
+        is_proc_macro=False,
+    ):
         crate = {
             "display_name": display_name,
             "root_module": str(root_module),
@@ -54,9 +61,26 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
                 stdin=subprocess.DEVNULL,
             ).decode('utf-8').strip()
             crate["proc_macro_dylib_path"] = f"{objtree}/rust/{proc_macro_dylib_name}"
-        crates_indexes[display_name] = len(crates)
+        return crate
+
+    def register_crate(crate):
+        crates_indexes[crate["display_name"]] = len(crates)
         crates.append(crate)
 
+    def append_crate(
+        display_name,
+        root_module,
+        deps,
+        cfg=[],
+        is_workspace_member=True,
+        is_proc_macro=False,
+    ):
+        register_crate(
+            build_crate(
+                display_name, root_module, deps, cfg, is_workspace_member, is_proc_macro
+            )
+        )
+
     def append_sysroot_crate(
         display_name,
         deps,
@@ -116,20 +140,21 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
         display_name,
         deps,
     ):
-        append_crate(
+        crate = build_crate(
             display_name,
             srctree / "rust" / display_name / "lib.rs",
             deps,
             cfg=cfg,
         )
-        crates[-1]["env"]["OBJTREE"] = str(objtree.resolve(True))
-        crates[-1]["source"] = {
+        crate["env"]["OBJTREE"] = str(objtree.resolve(True))
+        crate["source"] = {
             "include_dirs": [
                 str(srctree / "rust" / display_name),
                 str(objtree / "rust")
             ],
             "exclude_dirs": [],
         }
+        register_crate(crate)
 
     append_crate_with_generated("bindings", ["core"])
     append_crate_with_generated("uapi", ["core"])

-- 
2.49.0

Re: [PATCH v5 04/13] scripts: generate_rust_analyzer.py: extract `{build,register}_crate`

[Trevor Gross]

On Tue, Mar 25, 2025 at 3:06 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Extract helpers from `append_crate` to avoid the need to peek into
> `crates[-1]`. This improves readability.
>
> Suggested-by: Trevor Gross <tmgross@umich.edu>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  scripts/generate_rust_analyzer.py | 35 ++++++++++++++++++++++++++++++-----
>  1 file changed, 30 insertions(+), 5 deletions(-)

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH v5 05/13] scripts: generate_rust_analyzer.py: drop `"is_proc_macro": false`

[Tamir Duberstein]

Add a dedicated `append_proc_macro_crate` function to reduce overloading
in `append_crate`. This has the effect of removing `"is_proc_macro":
false` from the output; this field is interpreted as false if absent[1]
so this doesn't change the behavior of rust-analyzer.

Link: https://github.com/rust-lang/rust-analyzer/blob/8d01570b5e812a49daa1f08404269f6ea5dd73a1/crates/project-model/src/project_json.rs#L372-L373 [1]
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 scripts/generate_rust_analyzer.py | 55 +++++++++++++++++++++++++++------------
 1 file changed, 39 insertions(+), 16 deletions(-)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index 71e6038964f6..80eb21c0d082 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -41,13 +41,11 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
         deps,
         cfg=[],
         is_workspace_member=True,
-        is_proc_macro=False,
     ):
-        crate = {
+        return {
             "display_name": display_name,
             "root_module": str(root_module),
             "is_workspace_member": is_workspace_member,
-            "is_proc_macro": is_proc_macro,
             "deps": [{"crate": crates_indexes[dep], "name": dep} for dep in deps],
             "cfg": cfg,
             "edition": "2021",
@@ -55,13 +53,6 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
                 "RUST_MODFILE": "This is only for rust-analyzer"
             }
         }
-        if is_proc_macro:
-            proc_macro_dylib_name = subprocess.check_output(
-                [os.environ["RUSTC"], "--print", "file-names", "--crate-name", display_name, "--crate-type", "proc-macro", "-"],
-                stdin=subprocess.DEVNULL,
-            ).decode('utf-8').strip()
-            crate["proc_macro_dylib_path"] = f"{objtree}/rust/{proc_macro_dylib_name}"
-        return crate
 
     def register_crate(crate):
         crates_indexes[crate["display_name"]] = len(crates)
@@ -73,14 +64,48 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
         deps,
         cfg=[],
         is_workspace_member=True,
-        is_proc_macro=False,
     ):
         register_crate(
             build_crate(
-                display_name, root_module, deps, cfg, is_workspace_member, is_proc_macro
+                display_name,
+                root_module,
+                deps,
+                cfg,
+                is_workspace_member,
             )
         )
 
+    def append_proc_macro_crate(
+        display_name,
+        root_module,
+        deps,
+        cfg=[],
+    ):
+        crate = build_crate(display_name, root_module, deps, cfg)
+        proc_macro_dylib_name = (
+            subprocess.check_output(
+                [
+                    os.environ["RUSTC"],
+                    "--print",
+                    "file-names",
+                    "--crate-name",
+                    display_name,
+                    "--crate-type",
+                    "proc-macro",
+                    "-",
+                ],
+                stdin=subprocess.DEVNULL,
+            )
+            .decode("utf-8")
+            .strip()
+        )
+        proc_macro_crate = {
+            **crate,
+            "is_proc_macro": True,
+            "proc_macro_dylib_path": f"{objtree}/rust/{proc_macro_dylib_name}",
+        }
+        register_crate(proc_macro_crate)
+
     def append_sysroot_crate(
         display_name,
         deps,
@@ -108,11 +133,10 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
         [],
     )
 
-    append_crate(
+    append_proc_macro_crate(
         "macros",
         srctree / "rust" / "macros" / "lib.rs",
         ["std", "proc_macro"],
-        is_proc_macro=True,
     )
 
     append_crate(
@@ -121,12 +145,11 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
         ["core", "compiler_builtins"],
     )
 
-    append_crate(
+    append_proc_macro_crate(
         "pin_init_internal",
         srctree / "rust" / "pin-init" / "internal" / "src" / "lib.rs",
         [],
         cfg=["kernel"],
-        is_proc_macro=True,
     )
 
     append_crate(

-- 
2.49.0

Re: [PATCH v5 05/13] scripts: generate_rust_analyzer.py: drop `"is_proc_macro": false`

[Daniel Almeida]

Hi Tamir

> On 25 Mar 2025, at 17:06, Tamir Duberstein <tamird@gmail.com> wrote:
> 
> Add a dedicated `append_proc_macro_crate` function to reduce overloading
> in `append_crate`. This has the effect of removing `"is_proc_macro":
> false` from the output; this field is interpreted as false if absent[1]
> so this doesn't change the behavior of rust-analyzer.
> 
> Link: https://github.com/rust-lang/rust-analyzer/blob/8d01570b5e812a49daa1f08404269f6ea5dd73a1/crates/project-model/src/project_json.rs#L372-L373 [1]
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> scripts/generate_rust_analyzer.py | 55 +++++++++++++++++++++++++++------------
> 1 file changed, 39 insertions(+), 16 deletions(-)
> 
> diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
> index 71e6038964f6..80eb21c0d082 100755
> --- a/scripts/generate_rust_analyzer.py
> +++ b/scripts/generate_rust_analyzer.py
> @@ -41,13 +41,11 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
>         deps,
>         cfg=[],
>         is_workspace_member=True,
> -        is_proc_macro=False,
>     ):
> -        crate = {
> +        return {
>             "display_name": display_name,
>             "root_module": str(root_module),
>             "is_workspace_member": is_workspace_member,
> -            "is_proc_macro": is_proc_macro,
>             "deps": [{"crate": crates_indexes[dep], "name": dep} for dep in deps],
>             "cfg": cfg,
>             "edition": "2021",
> @@ -55,13 +53,6 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
>                 "RUST_MODFILE": "This is only for rust-analyzer"
>             }
>         }
> -        if is_proc_macro:
> -            proc_macro_dylib_name = subprocess.check_output(
> -                [os.environ["RUSTC"], "--print", "file-names", "--crate-name", display_name, "--crate-type", "proc-macro", "-"],
> -                stdin=subprocess.DEVNULL,
> -            ).decode('utf-8').strip()
> -            crate["proc_macro_dylib_path"] = f"{objtree}/rust/{proc_macro_dylib_name}"
> -        return crate
> 
>     def register_crate(crate):
>         crates_indexes[crate["display_name"]] = len(crates)
> @@ -73,14 +64,48 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
>         deps,
>         cfg=[],
>         is_workspace_member=True,
> -        is_proc_macro=False,
>     ):
>         register_crate(
>             build_crate(
> -                display_name, root_module, deps, cfg, is_workspace_member, is_proc_macro
> +                display_name,
> +                root_module,
> +                deps,
> +                cfg,
> +                is_workspace_member,
>             )
>         )
> 
> +    def append_proc_macro_crate(
> +        display_name,
> +        root_module,
> +        deps,
> +        cfg=[],
> +    ):
> +        crate = build_crate(display_name, root_module, deps, cfg)
> +        proc_macro_dylib_name = (
> +            subprocess.check_output(
> +                [
> +                    os.environ["RUSTC"],
> +                    "--print",
> +                    "file-names",
> +                    "--crate-name",
> +                    display_name,
> +                    "--crate-type",
> +                    "proc-macro",
> +                    "-",
> +                ],
> +                stdin=subprocess.DEVNULL,
> +            )
> +            .decode("utf-8")
> +            .strip()
> +        )
> +        proc_macro_crate = {
> +            **crate,
> +            "is_proc_macro": True,
> +            "proc_macro_dylib_path": f"{objtree}/rust/{proc_macro_dylib_name}",
> +        }
> +        register_crate(proc_macro_crate)
> +
>     def append_sysroot_crate(
>         display_name,
>         deps,
> @@ -108,11 +133,10 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
>         [],
>     )
> 
> -    append_crate(
> +    append_proc_macro_crate(
>         "macros",
>         srctree / "rust" / "macros" / "lib.rs",
>         ["std", "proc_macro"],
> -        is_proc_macro=True,
>     )
> 
>     append_crate(
> @@ -121,12 +145,11 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
>         ["core", "compiler_builtins"],
>     )
> 
> -    append_crate(
> +    append_proc_macro_crate(
>         "pin_init_internal",
>         srctree / "rust" / "pin-init" / "internal" / "src" / "lib.rs",
>         [],
>         cfg=["kernel"],
> -        is_proc_macro=True,
>     )
> 
>     append_crate(
> 
> -- 
> 2.49.0
> 
> 

In terms of testing, a diff shows that the only change to the output is the removal of `is_proc_macro: false`
as indicated by the commit message.

This looks good to me.

Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

— Daniel

Re: [PATCH v5 05/13] scripts: generate_rust_analyzer.py: drop `"is_proc_macro": false`

[Trevor Gross]

On Tue, Mar 25, 2025 at 3:06 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Add a dedicated `append_proc_macro_crate` function to reduce overloading
> in `append_crate`. This has the effect of removing `"is_proc_macro":
> false` from the output; this field is interpreted as false if absent[1]
> so this doesn't change the behavior of rust-analyzer.
>
> Link: https://github.com/rust-lang/rust-analyzer/blob/8d01570b5e812a49daa1f08404269f6ea5dd73a1/crates/project-model/src/project_json.rs#L372-L373 [1]
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  scripts/generate_rust_analyzer.py | 55 +++++++++++++++++++++++++++------------
>  1 file changed, 39 insertions(+), 16 deletions(-)

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH v5 06/13] scripts: generate_rust_analyzer.py: add type hints

[Tamir Duberstein]

Python type hints allow static analysis tools like mypy to detect type
errors during development, improving the developer experience.

Python type hints have been present in the kernel since 2019 at the
latest; see commit 6ebf5866f2e8 ("kunit: tool: add Python wrappers for
running KUnit tests").

Add a subclass of `argparse.Namespace` to get type checking on the CLI
arguments. Move parsing of `cfg` out of `generate_crates` to reduce the
number of variables in scope with `cfg` in their name. Use a defaultdict
to avoid `.get("key", [])`.

Run `mypy --strict scripts/generate_rust_analyzer.py --python-version
3.8` to verify. Note that `mypy` no longer supports python < 3.8.

Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 scripts/generate_rust_analyzer.py | 166 +++++++++++++++++++++++++-------------
 1 file changed, 109 insertions(+), 57 deletions(-)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index 80eb21c0d082..b37d8345486a 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -10,16 +10,48 @@ import os
 import pathlib
 import subprocess
 import sys
+from collections import defaultdict
+from typing import DefaultDict, Dict, Iterable, List, Literal, Optional, TypedDict
 
-def args_crates_cfgs(cfgs):
-    crates_cfgs = {}
-    for cfg in cfgs:
-        crate, vals = cfg.split("=", 1)
-        crates_cfgs[crate] = vals.replace("--cfg", "").split()
 
-    return crates_cfgs
+class Dependency(TypedDict):
+    crate: int
+    name: str
 
-def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
+
+class Source(TypedDict):
+    include_dirs: List[str]
+    exclude_dirs: List[str]
+
+
+class Crate(TypedDict):
+    display_name: str
+    root_module: str
+    is_workspace_member: bool
+    deps: List[Dependency]
+    cfg: List[str]
+    edition: Literal["2021"]
+    env: Dict[str, str]
+
+
+# `NotRequired` fields on `Crate` would be better but `NotRequired` was added in 3.11.
+class ProcMacroCrate(Crate):
+    is_proc_macro: Literal[True]
+    proc_macro_dylib_path: Optional[str]  # `pathlib.Path` is not JSON serializable.
+
+
+# `NotRequired` fields on `Crate` would be better but `NotRequired` was added in 3.11.
+class CrateWithGenerated(Crate):
+    source: Optional[Source]
+
+
+def generate_crates(
+    srctree: pathlib.Path,
+    objtree: pathlib.Path,
+    sysroot_src: pathlib.Path,
+    external_src: pathlib.Path,
+    crates_cfgs: DefaultDict[str, List[str]],
+) -> List[Crate]:
     # Generate the configuration list.
     cfg = []
     with open(objtree / "include" / "generated" / "rustc_cfg") as fd:
@@ -31,17 +63,16 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
     # Now fill the crates list -- dependencies need to come first.
     #
     # Avoid O(n^2) iterations by keeping a map of indexes.
-    crates = []
-    crates_indexes = {}
-    crates_cfgs = args_crates_cfgs(cfgs)
+    crates: List[Crate] = []
+    crates_indexes: Dict[str, int] = {}
 
     def build_crate(
-        display_name,
-        root_module,
-        deps,
-        cfg=[],
-        is_workspace_member=True,
-    ):
+        display_name: str,
+        root_module: pathlib.Path,
+        deps: List[str],
+        cfg: List[str] = [],
+        is_workspace_member: bool = True,
+    ) -> Crate:
         return {
             "display_name": display_name,
             "root_module": str(root_module),
@@ -51,36 +82,30 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
             "edition": "2021",
             "env": {
                 "RUST_MODFILE": "This is only for rust-analyzer"
-            }
+            },
         }
 
-    def register_crate(crate):
+    def register_crate(crate: Crate) -> None:
         crates_indexes[crate["display_name"]] = len(crates)
         crates.append(crate)
 
     def append_crate(
-        display_name,
-        root_module,
-        deps,
-        cfg=[],
-        is_workspace_member=True,
-    ):
+        display_name: str,
+        root_module: pathlib.Path,
+        deps: List[str],
+        cfg: List[str] = [],
+        is_workspace_member: bool = True,
+    ) -> None:
         register_crate(
-            build_crate(
-                display_name,
-                root_module,
-                deps,
-                cfg,
-                is_workspace_member,
-            )
+            build_crate(display_name, root_module, deps, cfg, is_workspace_member)
         )
 
     def append_proc_macro_crate(
-        display_name,
-        root_module,
-        deps,
-        cfg=[],
-    ):
+        display_name: str,
+        root_module: pathlib.Path,
+        deps: List[str],
+        cfg: List[str] = [],
+    ) -> None:
         crate = build_crate(display_name, root_module, deps, cfg)
         proc_macro_dylib_name = (
             subprocess.check_output(
@@ -99,7 +124,7 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
             .decode("utf-8")
             .strip()
         )
-        proc_macro_crate = {
+        proc_macro_crate: ProcMacroCrate = {
             **crate,
             "is_proc_macro": True,
             "proc_macro_dylib_path": f"{objtree}/rust/{proc_macro_dylib_name}",
@@ -107,10 +132,10 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
         register_crate(proc_macro_crate)
 
     def append_sysroot_crate(
-        display_name,
-        deps,
-        cfg=[],
-    ):
+        display_name: str,
+        deps: List[str],
+        cfg: List[str] = [],
+    ) -> None:
         append_crate(
             display_name,
             sysroot_src / display_name / "src" / "lib.rs",
@@ -122,7 +147,7 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
     # NB: sysroot crates reexport items from one another so setting up our transitive dependencies
     # here is important for ensuring that rust-analyzer can resolve symbols. The sources of truth
     # for this dependency graph are `(sysroot_src / crate / "Cargo.toml" for crate in crates)`.
-    append_sysroot_crate("core", [], cfg=crates_cfgs.get("core", []))
+    append_sysroot_crate("core", [], cfg=crates_cfgs["core"])
     append_sysroot_crate("alloc", ["core"])
     append_sysroot_crate("std", ["alloc", "core"])
     append_sysroot_crate("proc_macro", ["core", "std"])
@@ -160,9 +185,9 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
     )
 
     def append_crate_with_generated(
-        display_name,
-        deps,
-    ):
+        display_name: str,
+        deps: List[str],
+    ) -> None:
         crate = build_crate(
             display_name,
             srctree / "rust" / display_name / "lib.rs",
@@ -170,20 +195,23 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
             cfg=cfg,
         )
         crate["env"]["OBJTREE"] = str(objtree.resolve(True))
-        crate["source"] = {
-            "include_dirs": [
-                str(srctree / "rust" / display_name),
-                str(objtree / "rust")
-            ],
-            "exclude_dirs": [],
+        crate_with_generated: CrateWithGenerated = {
+            **crate,
+            "source": {
+                "include_dirs": [
+                    str(srctree / "rust" / display_name),
+                    str(objtree / "rust")
+                ],
+                "exclude_dirs": [],
+            }
         }
-        register_crate(crate)
+        register_crate(crate_with_generated)
 
     append_crate_with_generated("bindings", ["core"])
     append_crate_with_generated("uapi", ["core"])
     append_crate_with_generated("kernel", ["core", "macros", "build_error", "bindings", "pin_init", "uapi"])
 
-    def is_root_crate(build_file, target):
+    def is_root_crate(build_file: pathlib.Path, target: str) -> bool:
         try:
             return f"{target}.o" in open(build_file).read()
         except FileNotFoundError:
@@ -192,7 +220,9 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
     # Then, the rest outside of `rust/`.
     #
     # We explicitly mention the top-level folders we want to cover.
-    extra_dirs = map(lambda dir: srctree / dir, ("samples", "drivers"))
+    extra_dirs: Iterable[pathlib.Path] = map(
+        lambda dir: srctree / dir, ("samples", "drivers")
+    )
     if external_src is not None:
         extra_dirs = [external_src]
     for folder in extra_dirs:
@@ -216,7 +246,7 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
     return crates
 
 
-def main():
+def main() -> None:
     parser = argparse.ArgumentParser()
     parser.add_argument("--verbose", "-v", action="store_true")
     parser.add_argument("--cfgs", action="append", default=[])
@@ -225,7 +255,17 @@ def main():
     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()
+
+    class Args(argparse.Namespace):
+        verbose: bool
+        cfgs: List[str]
+        srctree: pathlib.Path
+        objtree: pathlib.Path
+        sysroot: pathlib.Path
+        sysroot_src: pathlib.Path
+        exttree: pathlib.Path
+
+    args = parser.parse_args(namespace=Args())
 
     logging.basicConfig(
         format="[%(asctime)s] [%(levelname)s] %(message)s",
@@ -236,7 +276,19 @@ def main():
     assert args.sysroot in args.sysroot_src.parents
 
     rust_project = {
-        "crates": generate_crates(args.srctree, args.objtree, args.sysroot_src, args.exttree, args.cfgs),
+        "crates": generate_crates(
+            args.srctree,
+            args.objtree,
+            args.sysroot_src,
+            args.exttree,
+            defaultdict(
+                list,
+                {
+                    crate: vals.lstrip("--cfg").split()
+                    for crate, vals in map(lambda cfg: cfg.split("=", 1), args.cfgs)
+                },
+            ),
+        ),
         "sysroot": str(args.sysroot),
     }
 

-- 
2.49.0

Re: [PATCH v5 06/13] scripts: generate_rust_analyzer.py: add type hints

[Daniel Almeida]

Hi Tamir,

[snip]

>     rust_project = {
> -        "crates": generate_crates(args.srctree, args.objtree, args.sysroot_src, args.exttree, args.cfgs),
> +        "crates": generate_crates(
> +            args.srctree,
> +            args.objtree,
> +            args.sysroot_src,
> +            args.exttree,
> +            defaultdict(
> +                list,
> +                {
> +                    crate: vals.lstrip("--cfg").split()
> +                    for crate, vals in map(lambda cfg: cfg.split("=", 1), args.cfgs)
> +                },
> +            ),
> +        ),
>         "sysroot": str(args.sysroot),
>     }
> 
> 
> -- 
> 2.49.0
> 

I found `args_crates_cfgs()` a lot easier to understand, but I guess this is a
matter of taste. I also find that this `defaultdict()` call slightly pollutes
the surrounding code, but again, that might be just me.

Regardless, running `mypy` still passes, and there is no change to the output.

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

— Daniel

Re: [PATCH v5 06/13] scripts: generate_rust_analyzer.py: add type hints

[Tamir Duberstein]

On Mon, Mar 31, 2025 at 1:09 PM Daniel Almeida
<daniel.almeida@collabora.com> wrote:
>
> Hi Tamir,
>
> [snip]
>
> >     rust_project = {
> > -        "crates": generate_crates(args.srctree, args.objtree, args.sysroot_src, args.exttree, args.cfgs),
> > +        "crates": generate_crates(
> > +            args.srctree,
> > +            args.objtree,
> > +            args.sysroot_src,
> > +            args.exttree,
> > +            defaultdict(
> > +                list,
> > +                {
> > +                    crate: vals.lstrip("--cfg").split()
> > +                    for crate, vals in map(lambda cfg: cfg.split("=", 1), args.cfgs)
> > +                },
> > +            ),
> > +        ),
> >         "sysroot": str(args.sysroot),
> >     }
> >
> >
> > --
> > 2.49.0
> >
>
> I found `args_crates_cfgs()` a lot easier to understand, but I guess this is a
> matter of taste. I also find that this `defaultdict()` call slightly pollutes
> the surrounding code, but again, that might be just me.

Would extracting a local variable suffice?

> Regardless, running `mypy` still passes, and there is no change to the output.
>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

Thanks!

Re: [PATCH v5 06/13] scripts: generate_rust_analyzer.py: add type hints

[Daniel Almeida]

Hi Tamir,

>> 
>> I found `args_crates_cfgs()` a lot easier to understand, but I guess this is a
>> matter of taste. I also find that this `defaultdict()` call slightly pollutes
>> the surrounding code, but again, that might be just me.
> 
> Would extracting a local variable suffice?
> 

Yeah, that would make it less crowded.

— Daniel

Re: [PATCH v5 06/13] scripts: generate_rust_analyzer.py: add type hints

[Trevor Gross]

On Tue, Mar 25, 2025 at 3:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Python type hints allow static analysis tools like mypy to detect type
> errors during development, improving the developer experience.
>
> Python type hints have been present in the kernel since 2019 at the
> latest; see commit 6ebf5866f2e8 ("kunit: tool: add Python wrappers for
> running KUnit tests").
>
> Add a subclass of `argparse.Namespace` to get type checking on the CLI
> arguments. Move parsing of `cfg` out of `generate_crates` to reduce the
> number of variables in scope with `cfg` in their name. Use a defaultdict
> to avoid `.get("key", [])`.
>
> Run `mypy --strict scripts/generate_rust_analyzer.py --python-version
> 3.8` to verify. Note that `mypy` no longer supports python < 3.8.
>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  scripts/generate_rust_analyzer.py | 166 +++++++++++++++++++++++++-------------
>  1 file changed, 109 insertions(+), 57 deletions(-)

> +                {
> +                    crate: vals.lstrip("--cfg").split()
> +                    for crate, vals in map(lambda cfg: cfg.split("=", 1), args.cfgs)
> +                },

Tiny nit only if you wind up touching this again, generators or
comprehension are a bit more canonical than `map` and `filter`

     for crate, _, vals in (cfg.partition("=") for cfg in args.cfg)

The rest looks good to me, with or without that

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH v5 07/13] scripts: generate_rust_analyzer.py: avoid optional arguments

[Tamir Duberstein]

Make all arguments required to reduce the probability of incorrect
implicit behavior. Use keyword arguments for clarity.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 scripts/generate_rust_analyzer.py | 90 +++++++++++++++++++++++++++------------
 1 file changed, 62 insertions(+), 28 deletions(-)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index b37d8345486a..badcef4126cf 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -69,9 +69,10 @@ def generate_crates(
     def build_crate(
         display_name: str,
         root_module: pathlib.Path,
+        *,
         deps: List[str],
-        cfg: List[str] = [],
-        is_workspace_member: bool = True,
+        cfg: List[str],
+        is_workspace_member: bool,
     ) -> Crate:
         return {
             "display_name": display_name,
@@ -92,21 +93,34 @@ def generate_crates(
     def append_crate(
         display_name: str,
         root_module: pathlib.Path,
+        *,
         deps: List[str],
-        cfg: List[str] = [],
-        is_workspace_member: bool = True,
+        cfg: List[str],
     ) -> None:
         register_crate(
-            build_crate(display_name, root_module, deps, cfg, is_workspace_member)
+            build_crate(
+                display_name,
+                root_module,
+                deps=deps,
+                cfg=cfg,
+                is_workspace_member=True,
+            )
         )
 
     def append_proc_macro_crate(
         display_name: str,
         root_module: pathlib.Path,
+        *,
         deps: List[str],
-        cfg: List[str] = [],
+        cfg: List[str],
     ) -> None:
-        crate = build_crate(display_name, root_module, deps, cfg)
+        crate = build_crate(
+            display_name,
+            root_module,
+            deps=deps,
+            cfg=cfg,
+            is_workspace_member=True,
+        )
         proc_macro_dylib_name = (
             subprocess.check_output(
                 [
@@ -133,66 +147,75 @@ def generate_crates(
 
     def append_sysroot_crate(
         display_name: str,
+        *,
         deps: List[str],
-        cfg: List[str] = [],
+        cfg: List[str],
     ) -> None:
-        append_crate(
-            display_name,
-            sysroot_src / display_name / "src" / "lib.rs",
-            deps,
-            cfg,
-            is_workspace_member=False,
+        register_crate(
+            build_crate(
+                display_name,
+                sysroot_src / display_name / "src" / "lib.rs",
+                deps=deps,
+                cfg=cfg,
+                is_workspace_member=False,
+            )
         )
 
     # NB: sysroot crates reexport items from one another so setting up our transitive dependencies
     # here is important for ensuring that rust-analyzer can resolve symbols. The sources of truth
     # for this dependency graph are `(sysroot_src / crate / "Cargo.toml" for crate in crates)`.
-    append_sysroot_crate("core", [], cfg=crates_cfgs["core"])
-    append_sysroot_crate("alloc", ["core"])
-    append_sysroot_crate("std", ["alloc", "core"])
-    append_sysroot_crate("proc_macro", ["core", "std"])
+    append_sysroot_crate("core", deps=[], cfg=crates_cfgs["core"])
+    append_sysroot_crate("alloc", deps=["core"], cfg=[])
+    append_sysroot_crate("std", deps=["alloc", "core"], cfg=[])
+    append_sysroot_crate("proc_macro", deps=["core", "std"], cfg=[])
 
     append_crate(
         "compiler_builtins",
         srctree / "rust" / "compiler_builtins.rs",
-        [],
+        deps=[],
+        cfg=[],
     )
 
     append_proc_macro_crate(
         "macros",
         srctree / "rust" / "macros" / "lib.rs",
-        ["std", "proc_macro"],
+        deps=["std", "proc_macro"],
+        cfg=[],
     )
 
     append_crate(
         "build_error",
         srctree / "rust" / "build_error.rs",
-        ["core", "compiler_builtins"],
+        deps=["core", "compiler_builtins"],
+        cfg=[],
     )
 
     append_proc_macro_crate(
         "pin_init_internal",
         srctree / "rust" / "pin-init" / "internal" / "src" / "lib.rs",
-        [],
+        deps=[],
         cfg=["kernel"],
     )
 
     append_crate(
         "pin_init",
         srctree / "rust" / "pin-init" / "src" / "lib.rs",
-        ["core", "pin_init_internal", "macros"],
+        deps=["core", "pin_init_internal", "macros"],
         cfg=["kernel"],
     )
 
     def append_crate_with_generated(
         display_name: str,
+        *,
         deps: List[str],
+        cfg: List[str],
     ) -> None:
         crate = build_crate(
             display_name,
             srctree / "rust" / display_name / "lib.rs",
-            deps,
+            deps=deps,
             cfg=cfg,
+            is_workspace_member=True,
         )
         crate["env"]["OBJTREE"] = str(objtree.resolve(True))
         crate_with_generated: CrateWithGenerated = {
@@ -207,9 +230,20 @@ def generate_crates(
         }
         register_crate(crate_with_generated)
 
-    append_crate_with_generated("bindings", ["core"])
-    append_crate_with_generated("uapi", ["core"])
-    append_crate_with_generated("kernel", ["core", "macros", "build_error", "bindings", "pin_init", "uapi"])
+    append_crate_with_generated("bindings", deps=["core"], cfg=[])
+    append_crate_with_generated("uapi", deps=["core"], cfg=[])
+    append_crate_with_generated(
+        "kernel",
+        deps=[
+            "core",
+            "macros",
+            "build_error",
+            "bindings",
+            "pin_init",
+            "uapi",
+        ],
+        cfg=[],
+    )
 
     def is_root_crate(build_file: pathlib.Path, target: str) -> bool:
         try:
@@ -239,7 +273,7 @@ def generate_crates(
             append_crate(
                 name,
                 path,
-                ["core", "kernel"],
+                deps=["core", "kernel"],
                 cfg=cfg,
             )
 

-- 
2.49.0

Re: [PATCH v5 07/13] scripts: generate_rust_analyzer.py: avoid optional arguments

[Daniel Almeida]

Hi Tamir,

This patch looks good to me but,

> On 25 Mar 2025, at 17:06, Tamir Duberstein <tamird@gmail.com> wrote:
> 
> Make all arguments required to reduce the probability of incorrect
> implicit behavior. Use keyword arguments for clarity.
> 
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> scripts/generate_rust_analyzer.py | 90 +++++++++++++++++++++++++++------------
> 1 file changed, 62 insertions(+), 28 deletions(-)
> 
> diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
> index b37d8345486a..badcef4126cf 100755
> --- a/scripts/generate_rust_analyzer.py
> +++ b/scripts/generate_rust_analyzer.py
> @@ -69,9 +69,10 @@ def generate_crates(
>     def build_crate(
>         display_name: str,
>         root_module: pathlib.Path,
> +        *,
>         deps: List[str],
> -        cfg: List[str] = [],
> -        is_workspace_member: bool = True,
> +        cfg: List[str],
> +        is_workspace_member: bool,
>     ) -> Crate:
>         return {
>             "display_name": display_name,
> @@ -92,21 +93,34 @@ def generate_crates(
>     def append_crate(
>         display_name: str,
>         root_module: pathlib.Path,
> +        *,
>         deps: List[str],
> -        cfg: List[str] = [],
> -        is_workspace_member: bool = True,
> +        cfg: List[str],
>     ) -> None:
>         register_crate(
> -            build_crate(display_name, root_module, deps, cfg, is_workspace_member)
> +            build_crate(
> +                display_name,
> +                root_module,
> +                deps=deps,
> +                cfg=cfg,
> +                is_workspace_member=True,
> +            )
>         )
> 
>     def append_proc_macro_crate(
>         display_name: str,
>         root_module: pathlib.Path,
> +        *,
>         deps: List[str],
> -        cfg: List[str] = [],
> +        cfg: List[str],
>     ) -> None:
> -        crate = build_crate(display_name, root_module, deps, cfg)
> +        crate = build_crate(
> +            display_name,
> +            root_module,
> +            deps=deps,
> +            cfg=cfg,
> +            is_workspace_member=True,
> +        )
>         proc_macro_dylib_name = (
>             subprocess.check_output(
>                 [
> @@ -133,66 +147,75 @@ def generate_crates(
> 
>     def append_sysroot_crate(
>         display_name: str,
> +        *,
>         deps: List[str],
> -        cfg: List[str] = [],
> +        cfg: List[str],
>     ) -> None:
> -        append_crate(
> -            display_name,
> -            sysroot_src / display_name / "src" / "lib.rs",
> -            deps,
> -            cfg,
> -            is_workspace_member=False,
> +        register_crate(
> +            build_crate(
> +                display_name,
> +                sysroot_src / display_name / "src" / "lib.rs",
> +                deps=deps,
> +                cfg=cfg,
> +                is_workspace_member=False,
> +            )
>         )

Why the change from append to register+build here? Maybe this change
should be in another patch?

— Daniel

Re: [PATCH v5 07/13] scripts: generate_rust_analyzer.py: avoid optional arguments

[Tamir Duberstein]

On Tue, Apr 1, 2025 at 8:59 AM Daniel Almeida
<daniel.almeida@collabora.com> wrote:
>
> Hi Tamir,
>
> This patch looks good to me but,
>
> > On 25 Mar 2025, at 17:06, Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > Make all arguments required to reduce the probability of incorrect
> > implicit behavior. Use keyword arguments for clarity.
> >
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > ---
> > scripts/generate_rust_analyzer.py | 90 +++++++++++++++++++++++++++------------
> > 1 file changed, 62 insertions(+), 28 deletions(-)
> >
> > diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
> > index b37d8345486a..badcef4126cf 100755
> > --- a/scripts/generate_rust_analyzer.py
> > +++ b/scripts/generate_rust_analyzer.py
> > @@ -69,9 +69,10 @@ def generate_crates(
> >     def build_crate(
> >         display_name: str,
> >         root_module: pathlib.Path,
> > +        *,
> >         deps: List[str],
> > -        cfg: List[str] = [],
> > -        is_workspace_member: bool = True,
> > +        cfg: List[str],
> > +        is_workspace_member: bool,
> >     ) -> Crate:
> >         return {
> >             "display_name": display_name,
> > @@ -92,21 +93,34 @@ def generate_crates(
> >     def append_crate(
> >         display_name: str,
> >         root_module: pathlib.Path,
> > +        *,
> >         deps: List[str],
> > -        cfg: List[str] = [],
> > -        is_workspace_member: bool = True,
> > +        cfg: List[str],
> >     ) -> None:
> >         register_crate(
> > -            build_crate(display_name, root_module, deps, cfg, is_workspace_member)
> > +            build_crate(
> > +                display_name,
> > +                root_module,
> > +                deps=deps,
> > +                cfg=cfg,
> > +                is_workspace_member=True,
> > +            )
> >         )
> >
> >     def append_proc_macro_crate(
> >         display_name: str,
> >         root_module: pathlib.Path,
> > +        *,
> >         deps: List[str],
> > -        cfg: List[str] = [],
> > +        cfg: List[str],
> >     ) -> None:
> > -        crate = build_crate(display_name, root_module, deps, cfg)
> > +        crate = build_crate(
> > +            display_name,
> > +            root_module,
> > +            deps=deps,
> > +            cfg=cfg,
> > +            is_workspace_member=True,
> > +        )
> >         proc_macro_dylib_name = (
> >             subprocess.check_output(
> >                 [
> > @@ -133,66 +147,75 @@ def generate_crates(
> >
> >     def append_sysroot_crate(
> >         display_name: str,
> > +        *,
> >         deps: List[str],
> > -        cfg: List[str] = [],
> > +        cfg: List[str],
> >     ) -> None:
> > -        append_crate(
> > -            display_name,
> > -            sysroot_src / display_name / "src" / "lib.rs",
> > -            deps,
> > -            cfg,
> > -            is_workspace_member=False,
> > +        register_crate(
> > +            build_crate(
> > +                display_name,
> > +                sysroot_src / display_name / "src" / "lib.rs",
> > +                deps=deps,
> > +                cfg=cfg,
> > +                is_workspace_member=False,
> > +            )
> >         )
>
> Why the change from append to register+build here? Maybe this change
> should be in another patch?

The reason is that `append_crate` has lost its `is_workspace_member`
parameter, so the only way to pass `False` now is to use
`build_crate`. I chose to remove the argument from `append_crate`
because the alternative would have introduced many instances of
`is_workspace_member=True`, which I judged to be less-good than this.

Cheers.
Tamir

Re: [PATCH v5 07/13] scripts: generate_rust_analyzer.py: avoid optional arguments

[Daniel Almeida]

Hi Tamir,

> On 1 Apr 2025, at 10:34, Tamir Duberstein <tamird@gmail.com> wrote:
> 
> On Tue, Apr 1, 2025 at 8:59 AM Daniel Almeida
> <daniel.almeida@collabora.com> wrote:
>> 
>> Hi Tamir,
>> 
>> This patch looks good to me but,
>> 
>>> On 25 Mar 2025, at 17:06, Tamir Duberstein <tamird@gmail.com> wrote:
>>> 
>>> Make all arguments required to reduce the probability of incorrect
>>> implicit behavior. Use keyword arguments for clarity.
>>> 
>>> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>>> ---
>>> scripts/generate_rust_analyzer.py | 90 +++++++++++++++++++++++++++------------
>>> 1 file changed, 62 insertions(+), 28 deletions(-)
>>> 
>>> diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
>>> index b37d8345486a..badcef4126cf 100755
>>> --- a/scripts/generate_rust_analyzer.py
>>> +++ b/scripts/generate_rust_analyzer.py
>>> @@ -69,9 +69,10 @@ def generate_crates(
>>>    def build_crate(
>>>        display_name: str,
>>>        root_module: pathlib.Path,
>>> +        *,
>>>        deps: List[str],
>>> -        cfg: List[str] = [],
>>> -        is_workspace_member: bool = True,
>>> +        cfg: List[str],
>>> +        is_workspace_member: bool,
>>>    ) -> Crate:
>>>        return {
>>>            "display_name": display_name,
>>> @@ -92,21 +93,34 @@ def generate_crates(
>>>    def append_crate(
>>>        display_name: str,
>>>        root_module: pathlib.Path,
>>> +        *,
>>>        deps: List[str],
>>> -        cfg: List[str] = [],
>>> -        is_workspace_member: bool = True,
>>> +        cfg: List[str],
>>>    ) -> None:
>>>        register_crate(
>>> -            build_crate(display_name, root_module, deps, cfg, is_workspace_member)
>>> +            build_crate(
>>> +                display_name,
>>> +                root_module,
>>> +                deps=deps,
>>> +                cfg=cfg,
>>> +                is_workspace_member=True,
>>> +            )
>>>        )
>>> 
>>>    def append_proc_macro_crate(
>>>        display_name: str,
>>>        root_module: pathlib.Path,
>>> +        *,
>>>        deps: List[str],
>>> -        cfg: List[str] = [],
>>> +        cfg: List[str],
>>>    ) -> None:
>>> -        crate = build_crate(display_name, root_module, deps, cfg)
>>> +        crate = build_crate(
>>> +            display_name,
>>> +            root_module,
>>> +            deps=deps,
>>> +            cfg=cfg,
>>> +            is_workspace_member=True,
>>> +        )
>>>        proc_macro_dylib_name = (
>>>            subprocess.check_output(
>>>                [
>>> @@ -133,66 +147,75 @@ def generate_crates(
>>> 
>>>    def append_sysroot_crate(
>>>        display_name: str,
>>> +        *,
>>>        deps: List[str],
>>> -        cfg: List[str] = [],
>>> +        cfg: List[str],
>>>    ) -> None:
>>> -        append_crate(
>>> -            display_name,
>>> -            sysroot_src / display_name / "src" / "lib.rs",
>>> -            deps,
>>> -            cfg,
>>> -            is_workspace_member=False,
>>> +        register_crate(
>>> +            build_crate(
>>> +                display_name,
>>> +                sysroot_src / display_name / "src" / "lib.rs",
>>> +                deps=deps,
>>> +                cfg=cfg,
>>> +                is_workspace_member=False,
>>> +            )
>>>        )
>> 
>> Why the change from append to register+build here? Maybe this change
>> should be in another patch?
> 
> The reason is that `append_crate` has lost its `is_workspace_member`
> parameter, so the only way to pass `False` now is to use
> `build_crate`. I chose to remove the argument from `append_crate`
> because the alternative would have introduced many instances of
> `is_workspace_member=True`, which I judged to be less-good than this.

Fine with me.

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

> 
> Cheers.
> Tamir

— Daniel

Re: [PATCH v5 07/13] scripts: generate_rust_analyzer.py: avoid optional arguments

[Trevor Gross]

On Tue, Mar 25, 2025 at 3:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Make all arguments required to reduce the probability of incorrect
> implicit behavior. Use keyword arguments for clarity.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  scripts/generate_rust_analyzer.py | 90 +++++++++++++++++++++++++++------------
>  1 file changed, 62 insertions(+), 28 deletions(-)

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH v5 08/13] scripts: generate_rust_analyzer.py: use str(pathlib.Path)

[Tamir Duberstein]

Use the `/` operator on `pathlib.Path` rather than directly crafting a
string. This is consistent with all other path manipulation in this
script.

Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 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 badcef4126cf..5370563688eb 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -141,7 +141,7 @@ def generate_crates(
         proc_macro_crate: ProcMacroCrate = {
             **crate,
             "is_proc_macro": True,
-            "proc_macro_dylib_path": f"{objtree}/rust/{proc_macro_dylib_name}",
+            "proc_macro_dylib_path": str(objtree / "rust" / proc_macro_dylib_name),
         }
         register_crate(proc_macro_crate)
 

-- 
2.49.0

Re: [PATCH v5 08/13] scripts: generate_rust_analyzer.py: use str(pathlib.Path)

[Trevor Gross]

On Tue, Mar 25, 2025 at 3:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Use the `/` operator on `pathlib.Path` rather than directly crafting a
> string. This is consistent with all other path manipulation in this
> script.
>
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  scripts/generate_rust_analyzer.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH v5 09/13] scripts: generate_rust_analyzer.py: identify crates explicitly

[Tamir Duberstein]

Use the return of `append_crate` to declare dependency on that crate.
This allows multiple crates with the same display_name be defined, which
we'll use to define host crates separately from target crates.

Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 scripts/generate_rust_analyzer.py | 82 +++++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 42 deletions(-)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index 5370563688eb..09f5f40d689a 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -60,17 +60,14 @@ def generate_crates(
             line = line.replace("\n", "")
             cfg.append(line)
 
-    # Now fill the crates list -- dependencies need to come first.
-    #
-    # Avoid O(n^2) iterations by keeping a map of indexes.
+    # Now fill the crates list.
     crates: List[Crate] = []
-    crates_indexes: Dict[str, int] = {}
 
     def build_crate(
         display_name: str,
         root_module: pathlib.Path,
         *,
-        deps: List[str],
+        deps: List[Dependency],
         cfg: List[str],
         is_workspace_member: bool,
     ) -> Crate:
@@ -78,7 +75,7 @@ def generate_crates(
             "display_name": display_name,
             "root_module": str(root_module),
             "is_workspace_member": is_workspace_member,
-            "deps": [{"crate": crates_indexes[dep], "name": dep} for dep in deps],
+            "deps": deps,
             "cfg": cfg,
             "edition": "2021",
             "env": {
@@ -86,18 +83,19 @@ def generate_crates(
             },
         }
 
-    def register_crate(crate: Crate) -> None:
-        crates_indexes[crate["display_name"]] = len(crates)
+    def register_crate(crate: Crate) -> Dependency:
+        index = len(crates)
         crates.append(crate)
+        return {"crate": index, "name": crate["display_name"]}
 
     def append_crate(
         display_name: str,
         root_module: pathlib.Path,
         *,
-        deps: List[str],
+        deps: List[Dependency],
         cfg: List[str],
-    ) -> None:
-        register_crate(
+    ) -> Dependency:
+        return register_crate(
             build_crate(
                 display_name,
                 root_module,
@@ -111,9 +109,9 @@ def generate_crates(
         display_name: str,
         root_module: pathlib.Path,
         *,
-        deps: List[str],
+        deps: List[Dependency],
         cfg: List[str],
-    ) -> None:
+    ) -> Dependency:
         crate = build_crate(
             display_name,
             root_module,
@@ -143,15 +141,15 @@ def generate_crates(
             "is_proc_macro": True,
             "proc_macro_dylib_path": str(objtree / "rust" / proc_macro_dylib_name),
         }
-        register_crate(proc_macro_crate)
+        return register_crate(proc_macro_crate)
 
     def append_sysroot_crate(
         display_name: str,
         *,
-        deps: List[str],
+        deps: List[Dependency],
         cfg: List[str],
-    ) -> None:
-        register_crate(
+    ) -> Dependency:
+        return register_crate(
             build_crate(
                 display_name,
                 sysroot_src / display_name / "src" / "lib.rs",
@@ -164,52 +162,52 @@ def generate_crates(
     # NB: sysroot crates reexport items from one another so setting up our transitive dependencies
     # here is important for ensuring that rust-analyzer can resolve symbols. The sources of truth
     # for this dependency graph are `(sysroot_src / crate / "Cargo.toml" for crate in crates)`.
-    append_sysroot_crate("core", deps=[], cfg=crates_cfgs["core"])
-    append_sysroot_crate("alloc", deps=["core"], cfg=[])
-    append_sysroot_crate("std", deps=["alloc", "core"], cfg=[])
-    append_sysroot_crate("proc_macro", deps=["core", "std"], cfg=[])
+    core = append_sysroot_crate("core", deps=[], cfg=crates_cfgs["core"])
+    alloc = append_sysroot_crate("alloc", deps=[core], cfg=[])
+    std = append_sysroot_crate("std", deps=[alloc, core], cfg=[])
+    proc_macro = append_sysroot_crate("proc_macro", deps=[core, std], cfg=[])
 
-    append_crate(
+    compiler_builtins = append_crate(
         "compiler_builtins",
         srctree / "rust" / "compiler_builtins.rs",
         deps=[],
         cfg=[],
     )
 
-    append_proc_macro_crate(
+    macros = append_proc_macro_crate(
         "macros",
         srctree / "rust" / "macros" / "lib.rs",
-        deps=["std", "proc_macro"],
+        deps=[std, proc_macro],
         cfg=[],
     )
 
-    append_crate(
+    build_error = append_crate(
         "build_error",
         srctree / "rust" / "build_error.rs",
-        deps=["core", "compiler_builtins"],
+        deps=[core, compiler_builtins],
         cfg=[],
     )
 
-    append_proc_macro_crate(
+    pin_init_internal = append_proc_macro_crate(
         "pin_init_internal",
         srctree / "rust" / "pin-init" / "internal" / "src" / "lib.rs",
         deps=[],
         cfg=["kernel"],
     )
 
-    append_crate(
+    pin_init = append_crate(
         "pin_init",
         srctree / "rust" / "pin-init" / "src" / "lib.rs",
-        deps=["core", "pin_init_internal", "macros"],
+        deps=[core, pin_init_internal, macros],
         cfg=["kernel"],
     )
 
     def append_crate_with_generated(
         display_name: str,
         *,
-        deps: List[str],
+        deps: List[Dependency],
         cfg: List[str],
-    ) -> None:
+    ) -> Dependency:
         crate = build_crate(
             display_name,
             srctree / "rust" / display_name / "lib.rs",
@@ -228,19 +226,19 @@ def generate_crates(
                 "exclude_dirs": [],
             }
         }
-        register_crate(crate_with_generated)
+        return register_crate(crate_with_generated)
 
-    append_crate_with_generated("bindings", deps=["core"], cfg=[])
-    append_crate_with_generated("uapi", deps=["core"], cfg=[])
-    append_crate_with_generated(
+    bindings = append_crate_with_generated("bindings", deps=[core], cfg=[])
+    uapi = append_crate_with_generated("uapi", deps=[core], cfg=[])
+    kernel = append_crate_with_generated(
         "kernel",
         deps=[
-            "core",
-            "macros",
-            "build_error",
-            "bindings",
-            "pin_init",
-            "uapi",
+            core,
+            macros,
+            build_error,
+            bindings,
+            pin_init,
+            uapi,
         ],
         cfg=[],
     )
@@ -273,7 +271,7 @@ def generate_crates(
             append_crate(
                 name,
                 path,
-                deps=["core", "kernel"],
+                deps=[core, kernel],
                 cfg=cfg,
             )
 

-- 
2.49.0

Re: [PATCH v5 09/13] scripts: generate_rust_analyzer.py: identify crates explicitly

[Trevor Gross]

On Tue, Mar 25, 2025 at 3:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Use the return of `append_crate` to declare dependency on that crate.
> This allows multiple crates with the same display_name be defined, which
> we'll use to define host crates separately from target crates.
>
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  scripts/generate_rust_analyzer.py | 82 +++++++++++++++++++--------------------
>  1 file changed, 40 insertions(+), 42 deletions(-)

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH v5 10/13] scripts: generate_rust_analyzer.py: define host crates

[Tamir Duberstein]

Define host crates used by the `macros` crate separately from target
crates, now that we can uniquely identify crates with the same name.

This avoids rust-analyzer thinking the host `core` crate has our target
configs applied to it.

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/all/CANiq72mw83RmLYeFFoJW6mUUygoyiA_f1ievSC2pmBESsQew+w@mail.gmail.com/
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 scripts/generate_rust_analyzer.py | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index 09f5f40d689a..ea4dd68d0dd9 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -162,10 +162,16 @@ def generate_crates(
     # NB: sysroot crates reexport items from one another so setting up our transitive dependencies
     # here is important for ensuring that rust-analyzer can resolve symbols. The sources of truth
     # for this dependency graph are `(sysroot_src / crate / "Cargo.toml" for crate in crates)`.
+    host_core = append_sysroot_crate("core", deps=[], cfg=[])
+    host_alloc = append_sysroot_crate("alloc", deps=[host_core], cfg=[])
+    host_std = append_sysroot_crate("std", deps=[host_alloc, host_core], cfg=[])
+    host_proc_macro = append_sysroot_crate(
+        "proc_macro",
+        deps=[host_core, host_std],
+        cfg=[],
+    )
+
     core = append_sysroot_crate("core", deps=[], cfg=crates_cfgs["core"])
-    alloc = append_sysroot_crate("alloc", deps=[core], cfg=[])
-    std = append_sysroot_crate("std", deps=[alloc, core], cfg=[])
-    proc_macro = append_sysroot_crate("proc_macro", deps=[core, std], cfg=[])
 
     compiler_builtins = append_crate(
         "compiler_builtins",
@@ -177,7 +183,7 @@ def generate_crates(
     macros = append_proc_macro_crate(
         "macros",
         srctree / "rust" / "macros" / "lib.rs",
-        deps=[std, proc_macro],
+        deps=[host_std, host_proc_macro],
         cfg=[],
     )
 

-- 
2.49.0

Re: [PATCH v5 10/13] scripts: generate_rust_analyzer.py: define host crates

[Trevor Gross]

On Tue, Mar 25, 2025 at 3:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Define host crates used by the `macros` crate separately from target
> crates, now that we can uniquely identify crates with the same name.
>
> This avoids rust-analyzer thinking the host `core` crate has our target
> configs applied to it.
>
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://lore.kernel.org/all/CANiq72mw83RmLYeFFoJW6mUUygoyiA_f1ievSC2pmBESsQew+w@mail.gmail.com/
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  scripts/generate_rust_analyzer.py | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH v5 11/13] scripts: generate_rust_analyzer.py: avoid FD leak

[Tamir Duberstein]

Use a context manager to avoid leaking file descriptors.

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 scripts/generate_rust_analyzer.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index ea4dd68d0dd9..e3f1ec856ecf 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -251,7 +251,8 @@ def generate_crates(
 
     def is_root_crate(build_file: pathlib.Path, target: str) -> bool:
         try:
-            return f"{target}.o" in open(build_file).read()
+            with open(build_file) as f:
+                return f"{target}.o" in f.read()
         except FileNotFoundError:
             return False
 

-- 
2.49.0

Re: [PATCH v5 11/13] scripts: generate_rust_analyzer.py: avoid FD leak

[Trevor Gross]

On Tue, Mar 25, 2025 at 3:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Use a context manager to avoid leaking file descriptors.
>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  scripts/generate_rust_analyzer.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH v5 12/13] scripts: generate_rust_analyzer.py: define scripts

[Tamir Duberstein]

Generate rust-project.json entries for scripts written in Rust. This is
possible now that we have a definition for `std` built for the host.

Use `str::rstrip` for consistency.

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 scripts/generate_rust_analyzer.py | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index e3f1ec856ecf..5c0056c265bb 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -249,6 +249,20 @@ def generate_crates(
         cfg=[],
     )
 
+    scripts = srctree / "scripts"
+    with open(scripts / "Makefile") as f:
+        makefile = f.read()
+    for path in scripts.glob("*.rs"):
+        name = path.name.rstrip(".rs")
+        if f"{name}-rust" not in makefile:
+            continue
+        _script = append_crate(
+            name,
+            path,
+            deps=[host_std],
+            cfg=[],
+        )
+
     def is_root_crate(build_file: pathlib.Path, target: str) -> bool:
         try:
             with open(build_file) as f:
@@ -267,7 +281,7 @@ def generate_crates(
     for folder in extra_dirs:
         for path in folder.rglob("*.rs"):
             logging.info("Checking %s", path)
-            name = path.name.replace(".rs", "")
+            name = path.name.rstrip(".rs")
 
             # Skip those that are not crate roots.
             if not is_root_crate(path.parent / "Makefile", name) and \

-- 
2.49.0

Re: [PATCH v5 12/13] scripts: generate_rust_analyzer.py: define scripts

[Trevor Gross]

On Tue, Mar 25, 2025 at 3:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Generate rust-project.json entries for scripts written in Rust. This is
> possible now that we have a definition for `std` built for the host.
>
> Use `str::rstrip` for consistency.
>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  scripts/generate_rust_analyzer.py | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)

> +    with open(scripts / "Makefile") as f:
> +        makefile = f.read()

Since we are using pathlib, this can be `makefile = (scripts /
"Makefile").open().read()`

> +    for path in scripts.glob("*.rs"):
> +        name = path.name.rstrip(".rs")

`stem` should provide the file name without extension, so

    name = path.stem

> @@ -267,7 +281,7 @@ def generate_crates(
>      for folder in extra_dirs:
>          for path in folder.rglob("*.rs"):
>              logging.info("Checking %s", path)
> -            name = path.name.replace(".rs", "")
> +            name = path.name.rstrip(".rs")

The above applies here too.

A couple of optional nits if you resend but nothing to block on:

Reviewed-by: Trevor Gross <tmgross@umich.edu>

Re: [PATCH v5 12/13] scripts: generate_rust_analyzer.py: define scripts

[Tamir Duberstein]

On Thu, Apr 17, 2025 at 3:25 AM Trevor Gross <tmgross@umich.edu> wrote:
>
> On Tue, Mar 25, 2025 at 3:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > Generate rust-project.json entries for scripts written in Rust. This is
> > possible now that we have a definition for `std` built for the host.
> >
> > Use `str::rstrip` for consistency.
> >
> > Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> > Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> > Reviewed-by: Fiona Behrens <me@kloenk.dev>
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > ---
> >  scripts/generate_rust_analyzer.py | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
>
> > +    with open(scripts / "Makefile") as f:
> > +        makefile = f.read()
>
> Since we are using pathlib, this can be `makefile = (scripts /
> "Makefile").open().read()`

I assume you mean `makefile = (scripts / "Makefile").read_text()`;
`.open().read()` would leak a file descriptor.

>
> > +    for path in scripts.glob("*.rs"):
> > +        name = path.name.rstrip(".rs")
>
> `stem` should provide the file name without extension, so
>
>     name = path.stem
>
> > @@ -267,7 +281,7 @@ def generate_crates(
> >      for folder in extra_dirs:
> >          for path in folder.rglob("*.rs"):
> >              logging.info("Checking %s", path)
> > -            name = path.name.replace(".rs", "")
> > +            name = path.name.rstrip(".rs")
>
> The above applies here too.
>
> A couple of optional nits if you resend but nothing to block on:
>
> Reviewed-by: Trevor Gross <tmgross@umich.edu>

[PATCH v5 13/13] scripts: generate_rust_analyzer.py: use `cfg_groups`

[Tamir Duberstein]

Declare common `cfg`s just once to reduce the size of rust-analyzer.json
from 30619 to 2624 lines.

Link: https://github.com/rust-lang/rust-analyzer/commit/2607c09fddef36da0d6f0a84625db5e20a5ebde3
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 scripts/generate_rust_analyzer.py | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
index 5c0056c265bb..4faf01e2cbc6 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -29,6 +29,7 @@ class Crate(TypedDict):
     root_module: str
     is_workspace_member: bool
     deps: List[Dependency]
+    cfg_groups: List[str]
     cfg: List[str]
     edition: Literal["2021"]
     env: Dict[str, str]
@@ -51,15 +52,8 @@ def generate_crates(
     sysroot_src: pathlib.Path,
     external_src: pathlib.Path,
     crates_cfgs: DefaultDict[str, List[str]],
+    cfg_groups: List[str],
 ) -> List[Crate]:
-    # Generate the configuration list.
-    cfg = []
-    with open(objtree / "include" / "generated" / "rustc_cfg") as fd:
-        for line in fd:
-            line = line.replace("--cfg=", "")
-            line = line.replace("\n", "")
-            cfg.append(line)
-
     # Now fill the crates list.
     crates: List[Crate] = []
 
@@ -76,6 +70,7 @@ def generate_crates(
             "root_module": str(root_module),
             "is_workspace_member": is_workspace_member,
             "deps": deps,
+            "cfg_groups": cfg_groups if is_workspace_member else [],
             "cfg": cfg,
             "edition": "2021",
             "env": {
@@ -293,7 +288,7 @@ def generate_crates(
                 name,
                 path,
                 deps=[core, kernel],
-                cfg=cfg,
+                cfg=[],
             )
 
     return crates
@@ -328,6 +323,10 @@ def main() -> None:
     # Making sure that the `sysroot` and `sysroot_src` belong to the same toolchain.
     assert args.sysroot in args.sysroot_src.parents
 
+    # Generate the configuration list.
+    with open(args.objtree / "include" / "generated" / "rustc_cfg") as fd:
+        cfg_groups = {"rustc_cfg": [line.lstrip("--cfg=").rstrip("\n") for line in fd]}
+
     rust_project = {
         "crates": generate_crates(
             args.srctree,
@@ -341,8 +340,10 @@ def main() -> None:
                     for crate, vals in map(lambda cfg: cfg.split("=", 1), args.cfgs)
                 },
             ),
+            list(cfg_groups.keys()),
         ),
         "sysroot": str(args.sysroot),
+        "cfg_groups": cfg_groups,
     }
 
     json.dump(rust_project, sys.stdout, sort_keys=True, indent=4)

-- 
2.49.0

Re: [PATCH v5 13/13] scripts: generate_rust_analyzer.py: use `cfg_groups`

[Trevor Gross]

On Tue, Mar 25, 2025 at 3:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Declare common `cfg`s just once to reduce the size of rust-analyzer.json
> from 30619 to 2624 lines.
>
> Link: https://github.com/rust-lang/rust-analyzer/commit/2607c09fddef36da0d6f0a84625db5e20a5ebde3
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  scripts/generate_rust_analyzer.py | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)

> @@ -76,6 +70,7 @@ def generate_crates(
>              "root_module": str(root_module),
>              "is_workspace_member": is_workspace_member,
>              "deps": deps,
> +            "cfg_groups": cfg_groups if is_workspace_member else [],

Does r-a just expect cfg_groups for workspace members or is there
logic here? Maybe worth a comment.

Reviewed-by: Trevor Gross <tmgross@umich.edu>

Re: [PATCH v5 13/13] scripts: generate_rust_analyzer.py: use `cfg_groups`

[Tamir Duberstein]

On Thu, Apr 17, 2025 at 3:29 AM Trevor Gross <tmgross@umich.edu> wrote:
>
> On Tue, Mar 25, 2025 at 3:07 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > Declare common `cfg`s just once to reduce the size of rust-analyzer.json
> > from 30619 to 2624 lines.
> >
> > Link: https://github.com/rust-lang/rust-analyzer/commit/2607c09fddef36da0d6f0a84625db5e20a5ebde3
> > Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> > Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > ---
> >  scripts/generate_rust_analyzer.py | 19 ++++++++++---------
> >  1 file changed, 10 insertions(+), 9 deletions(-)
>
> > @@ -76,6 +70,7 @@ def generate_crates(
> >              "root_module": str(root_module),
> >              "is_workspace_member": is_workspace_member,
> >              "deps": deps,
> > +            "cfg_groups": cfg_groups if is_workspace_member else [],
>
> Does r-a just expect cfg_groups for workspace members or is there
> logic here? Maybe worth a comment.

r-a doesn't have opinions, but our configs apply only to our crates as
opposed to sysroot crates. That's not entirely true for core, but the
configs we apply there are narrow, whereas our workspace crates get
all of KConfig, which is huge.

I'll add a comment.

> Reviewed-by: Trevor Gross <tmgross@umich.edu>

Thanks for the reviews! I need to rebase on rust-fixes which picked up
the ffi crate anyway, so I picked up your suggestions:
- generators instead of `map`.
- `Pathlib.path.stem`.
- `Pathlib.path.read_text`.
- Add a comment about cfg_groups.

Cheers.

Tamir