[RFC PATCH 00/11] rust: improved integration with cargo

[RFC PATCH 00/11] rust: improved integration with cargo

[Paolo Bonzini]

While we're not sure where we'll be going in the future, for now
using cargo remains an important part of developing QEMU Rust code.
This is because cargo is the easiest way to run clippy, rustfmt,
rustdoc.  Cargo also allows working with doc tests, though there are
pretty much none yet, and provides tools such as "cargo expand".

This series aims at improving the integration with cargo and
cargo-based tooling.

First, while it is currently possible to run cargo on the rust/ directory,
but it has the issue that the bindings.rs must be placed by hand in
the build directory.  Therefore, this series starts by allowing
cargo to "just work" when run in a "meson devenv" environment:

    meson devenv -w ../rust cargo clippy --tests
    meson devenv -w ../rust cargo fmt

If you are going to use cargo repeatedly, invoking just 'meson devenv'
will put you in a shell where commands like 'cargo clippy' just work.
For simplicity, I am also adding two targets 'make clippy' and 'make
rustfmt'.

Secondly, one problem with mixing Cargo and meson is having to redo the
configuration of "lints" in both sides.  This series standardizes
on using Cargo.toml to configure the build, and bringing the flags
over to build.ninja with extensions to the existing rustc_args.py script.
I admit that these additions to the script are pretty large and therefore
I'm open to scrapping the idea.  I tried to organize the changes so that
the changes are split over multiple patches.

Finally, this series adds a CI job that runs rustfmt, clippy, and
rustdoc, including running doctests.

Please send comments!

Paolo

Paolo Bonzini (11):
  rust: qemu_api: do not disable lints outside bindgen-generated code
  rust: build: move rustc_args.py invocation to individual crates
  rust: build: restrict --cfg generation to only required symbols
  rust: build: generate warning flags from Cargo.toml
  rust: cargo: store desired warning levels in workspace Cargo.toml
  rust: build: move strict lints handling to rustc_args.py
  rust: fix a couple style issues from clippy
  rust: build: establish a baseline of lints across all crates
  rust: build: add "make clippy", "make rustfmt"
  rust: fix doc test syntax
  rust: ci: add job that runs Rust tools

 meson.build                                   |  56 +++---
 .gitlab-ci.d/buildtest-template.yml           |  14 ++
 .gitlab-ci.d/buildtest.yml                    |  14 ++
 rust/Cargo.toml                               |  80 ++++++++
 rust/hw/char/pl011/Cargo.toml                 |   3 +
 rust/hw/char/pl011/src/device.rs              |   6 +-
 rust/hw/char/pl011/src/lib.rs                 |  18 +-
 rust/hw/char/pl011/src/memory_ops.rs          |   4 +-
 rust/meson.build                              |  14 ++
 rust/qemu-api-macros/Cargo.toml               |   3 +
 rust/qemu-api/.gitignore                      |   2 +-
 rust/qemu-api/Cargo.toml                      |   5 +-
 rust/qemu-api/build.rs                        |  24 ++-
 rust/qemu-api/meson.build                     |   5 +
 rust/qemu-api/src/bindings.rs                 |  29 +++
 rust/qemu-api/src/lib.rs                      |  22 ---
 rust/qemu-api/src/zeroable.rs                 |   6 +-
 rust/qemu-api/tests/tests.rs                  |   2 +-
 scripts/rust/rustc_args.py                    | 178 ++++++++++++++++--
 .../dockerfiles/fedora-rust-nightly.docker    |   4 +
 tests/lcitool/refresh                         |   4 +
 21 files changed, 391 insertions(+), 102 deletions(-)
 create mode 100644 rust/qemu-api/src/bindings.rs

-- 
2.47.0

[RFC PATCH 01/11] rust: qemu_api: do not disable lints outside bindgen-generated code

[Paolo Bonzini]

rust/qemu-api/src/lib.rs is disabling lints that cause problems
with code generated by bindgen.  Instead, include the bindgen
code via include!(...) and move the #![allow()] directives
into the bindings module.

Add MESON_BUILD_ROOT to the devenv, so that it's easy for
build.rs to find the include file.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                   |  4 +++-
 rust/qemu-api/.gitignore      |  2 +-
 rust/qemu-api/build.rs        | 20 ++++++++++++++------
 rust/qemu-api/meson.build     |  1 +
 rust/qemu-api/src/bindings.rs | 29 +++++++++++++++++++++++++++++
 rust/qemu-api/src/lib.rs      | 22 ----------------------
 6 files changed, 48 insertions(+), 30 deletions(-)
 create mode 100644 rust/qemu-api/src/bindings.rs

diff --git a/meson.build b/meson.build
index e0b880e4e13..a7342c6edbd 100644
--- a/meson.build
+++ b/meson.build
@@ -3,6 +3,8 @@ project('qemu', ['c'], meson_version: '>=1.5.0',
                           'b_staticpic=false', 'stdsplit=false', 'optimization=2', 'b_pie=true'],
         version: files('VERSION'))
 
+meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() })
+
 add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true)
 add_test_setup('slow', exclude_suites: ['thorough'], env: ['G_TEST_SLOW=1', 'SPEED=slow'])
 add_test_setup('thorough', env: ['G_TEST_SLOW=1', 'SPEED=thorough'])
@@ -4089,7 +4091,7 @@ if have_rust
   bindings_rs = rust.bindgen(
     input: 'rust/wrapper.h',
     dependencies: common_ss.all_dependencies(),
-    output: 'bindings.rs',
+    output: 'bindings.rs.inc',
     include_directories: include_directories('.', 'include'),
     bindgen_version: ['>=0.60.0'],
     args: bindgen_args,
diff --git a/rust/qemu-api/.gitignore b/rust/qemu-api/.gitignore
index b9e7e004c86..2accb8745dc 100644
--- a/rust/qemu-api/.gitignore
+++ b/rust/qemu-api/.gitignore
@@ -1,2 +1,2 @@
 # Ignore generated bindings file overrides.
-src/bindings.rs
+/src/bindings.rs.inc
diff --git a/rust/qemu-api/build.rs b/rust/qemu-api/build.rs
index 20f8f718b90..e4eab718553 100644
--- a/rust/qemu-api/build.rs
+++ b/rust/qemu-api/build.rs
@@ -2,18 +2,26 @@
 // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::path::Path;
+use std::{env, path::Path};
 
 use version_check as rustc;
 
 fn main() {
-    if !Path::new("src/bindings.rs").exists() {
-        panic!(
-            "No generated C bindings found! Either build them manually with bindgen or with meson \
-             (`ninja bindings.rs`) and copy them to src/bindings.rs, or build through meson."
-        );
+    // Placing bindings.rs.inc in the source directory is supported
+    // but not documented or encouraged.
+    let path = env::var("MESON_BUILD_ROOT")
+        .unwrap_or_else(|_| format!("{}/src", env!("CARGO_MANIFEST_DIR")));
+
+    let file = format!("{}/bindings.rs.inc", path);
+    if !Path::new(&file).exists() {
+        panic!(concat!(
+            "No generated C bindings found! If you want to run `cargo`, start a subshell\n",
+            "with `meson devenv`, or point MESON_BUILD_ROOT to the top of the build tree."
+        ));
     }
 
+    println!("cargo:rustc-env=BINDINGS_RS_INC={}", file);
+
     // Check for available rustc features
     if rustc::is_min_version("1.77.0").unwrap_or(false) {
         println!("cargo:rustc-cfg=has_offset_of");
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 6f637af7b1b..e3870e901e3 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -9,6 +9,7 @@ _qemu_api_rs = static_library(
   structured_sources(
     [
       'src/lib.rs',
+      'src/bindings.rs',
       'src/c_str.rs',
       'src/definitions.rs',
       'src/device_class.rs',
diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
new file mode 100644
index 00000000000..1dac310594d
--- /dev/null
+++ b/rust/qemu-api/src/bindings.rs
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#![allow(
+    dead_code,
+    improper_ctypes_definitions,
+    improper_ctypes,
+    non_camel_case_types,
+    non_snake_case,
+    non_upper_case_globals,
+    unsafe_op_in_unsafe_fn,
+    clippy::missing_const_for_fn,
+    clippy::too_many_arguments,
+    clippy::approx_constant,
+    clippy::use_self,
+    clippy::useless_transmute,
+    clippy::missing_safety_doc
+)]
+
+#[cfg(MESON)]
+include!("bindings.rs.inc");
+
+#[cfg(not(MESON))]
+include!(env!("BINDINGS_RS_INC"));
+
+unsafe impl Send for Property {}
+unsafe impl Sync for Property {}
+unsafe impl Sync for TypeInfo {}
+unsafe impl Sync for VMStateDescription {}
+unsafe impl Sync for VMStateField {}
+unsafe impl Sync for VMStateInfo {}
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index aa8d16ec94b..440aff3817d 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -4,31 +4,9 @@
 
 #![cfg_attr(not(MESON), doc = include_str!("../README.md"))]
 
-#[allow(
-    dead_code,
-    improper_ctypes_definitions,
-    improper_ctypes,
-    non_camel_case_types,
-    non_snake_case,
-    non_upper_case_globals,
-    unsafe_op_in_unsafe_fn,
-    clippy::missing_const_for_fn,
-    clippy::too_many_arguments,
-    clippy::approx_constant,
-    clippy::use_self,
-    clippy::useless_transmute,
-    clippy::missing_safety_doc,
-)]
 #[rustfmt::skip]
 pub mod bindings;
 
-unsafe impl Send for bindings::Property {}
-unsafe impl Sync for bindings::Property {}
-unsafe impl Sync for bindings::TypeInfo {}
-unsafe impl Sync for bindings::VMStateDescription {}
-unsafe impl Sync for bindings::VMStateField {}
-unsafe impl Sync for bindings::VMStateInfo {}
-
 pub mod c_str;
 pub mod definitions;
 pub mod device_class;
-- 
2.47.0

[RFC PATCH 02/11] rust: build: move rustc_args.py invocation to individual crates

[Paolo Bonzini]

Only qemu-api needs access to the symbols in config-host.h.  Remove
the temptation to use them by limiting the --cfg arguments to the
qemu-api crate.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build               | 54 +++++++++++++++++----------------------
 rust/qemu-api/meson.build |  4 +++
 2 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/meson.build b/meson.build
index a7342c6edbd..7a9a523467b 100644
--- a/meson.build
+++ b/meson.build
@@ -120,7 +120,29 @@ if have_rust
 endif
 
 if have_rust
+  rustc_args = find_program('scripts/rust/rustc_args.py')
   rustfmt = find_program('rustfmt', required: false)
+
+  # Prohibit code that is forbidden in Rust 2024
+  rustc_lint_args = ['-D', 'unsafe_op_in_unsafe_fn']
+
+  # Occasionally, we may need to silence warnings and clippy lints that
+  # were only introduced in newer Rust compiler versions.  Do not croak
+  # in that case; a CI job with rust_strict_lints == true ensures that
+  # we do not have misspelled allow() attributes.
+  if not get_option('strict_rust_lints')
+    rustc_lint_args += ['-A', 'unknown_lints']
+  endif
+
+  # Apart from procedural macros, our Rust executables will often link
+  # with C code, so include all the libraries that C code needs.  This
+  # is safe; https://github.com/rust-lang/rust/pull/54675 says that
+  # passing -nodefaultlibs to the linker "was more ideological to
+  # start with than anything".
+  add_project_arguments(rustc_lint_args + ['-C', 'default-linker-libraries'],
+      native: false, language: 'rust')
+
+  add_project_arguments(rustc_lint_args, native: true, language: 'rust')
 endif
 
 dtrace = not_found
@@ -3399,36 +3421,8 @@ endif
 # Generated sources #
 #####################
 
-genh += configure_file(output: 'config-host.h', configuration: config_host_data)
-
-if have_rust
-  rustc_args = run_command(
-    find_program('scripts/rust/rustc_args.py'),
-    '--config-headers', meson.project_build_root() / 'config-host.h',
-    capture : true,
-    check: true).stdout().strip().split()
-
-  # Prohibit code that is forbidden in Rust 2024
-  rustc_args += ['-D', 'unsafe_op_in_unsafe_fn']
-
-  # Occasionally, we may need to silence warnings and clippy lints that
-  # were only introduced in newer Rust compiler versions.  Do not croak
-  # in that case; a CI job with rust_strict_lints == true ensures that
-  # we do not have misspelled allow() attributes.
-  if not get_option('strict_rust_lints')
-    rustc_args += ['-A', 'unknown_lints']
-  endif
-
-  # Apart from procedural macros, our Rust executables will often link
-  # with C code, so include all the libraries that C code needs.  This
-  # is safe; https://github.com/rust-lang/rust/pull/54675 says that
-  # passing -nodefaultlibs to the linker "was more ideological to
-  # start with than anything".
-  add_project_arguments(rustc_args + ['-C', 'default-linker-libraries'],
-      native: false, language: 'rust')
-
-  add_project_arguments(rustc_args, native: true, language: 'rust')
-endif
+config_host_h = configure_file(output: 'config-host.h', configuration: config_host_data)
+genh += config_host_h
 
 hxtool = find_program('scripts/hxtool')
 shaderinclude = find_program('scripts/shaderinclude.py')
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index e3870e901e3..f84f85b88c6 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -4,6 +4,10 @@ if rustc.version().version_compare('>=1.77.0')
   _qemu_api_cfg += ['--cfg', 'has_offset_of']
 endif
 
+_qemu_api_cfg += run_command(rustc_args,
+  '--config-headers', config_host_h,
+  capture: true, check: true).stdout().strip().split()
+
 _qemu_api_rs = static_library(
   'qemu_api',
   structured_sources(
-- 
2.47.0

[RFC PATCH 03/11] rust: build: restrict --cfg generation to only required symbols

[Paolo Bonzini]

Parse the Cargo.toml file, looking for the unexpected_cfgs
configuration.  When generating --cfg options from the
config-host.h file, only use those that are included in the
configuration.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/meson.build  |  2 +-
 scripts/rust/rustc_args.py | 61 ++++++++++++++++++++++++++++----------
 2 files changed, 46 insertions(+), 17 deletions(-)

diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index f84f85b88c6..bb2ed2844dc 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -5,7 +5,7 @@ if rustc.version().version_compare('>=1.77.0')
 endif
 
 _qemu_api_cfg += run_command(rustc_args,
-  '--config-headers', config_host_h,
+  '--config-headers', config_host_h, files('Cargo.toml'),
   capture: true, check: true).stdout().strip().split()
 
 _qemu_api_rs = static_library(
diff --git a/scripts/rust/rustc_args.py b/scripts/rust/rustc_args.py
index e4cc9720e16..942dd2b2bab 100644
--- a/scripts/rust/rustc_args.py
+++ b/scripts/rust/rustc_args.py
@@ -26,30 +26,51 @@
 
 import argparse
 import logging
+from pathlib import Path
+from typing import Any, Iterable, Mapping, Optional, Set
 
-from typing import List
+try:
+    import tomllib
+except ImportError:
+    import tomli as tomllib
 
 
-def generate_cfg_flags(header: str) -> List[str]:
+class CargoTOML:
+    tomldata: Mapping[Any, Any]
+    check_cfg: Set[str]
+
+    def __init__(self, path: str):
+        with open(path, 'rb') as f:
+            self.tomldata = tomllib.load(f)
+
+        self.check_cfg = set(self.find_check_cfg())
+
+    def find_check_cfg(self) -> Iterable[str]:
+        toml_lints = self.lints
+        rust_lints = toml_lints.get("rust", {})
+        cfg_lint = rust_lints.get("unexpected_cfgs", {})
+        return cfg_lint.get("check-cfg", [])
+
+    @property
+    def lints(self) -> Mapping[Any, Any]:
+        return self.get_table("lints")
+
+    def get_table(self, key: str) -> Mapping[Any, Any]:
+        table = self.tomldata.get(key, {})
+
+        return table
+
+
+def generate_cfg_flags(header: str, cargo_toml: CargoTOML) -> Iterable[str]:
     """Converts defines from config[..].h headers to rustc --cfg flags."""
 
-    def cfg_name(name: str) -> str:
-        """Filter function for C #defines"""
-        if (
-            name.startswith("CONFIG_")
-            or name.startswith("TARGET_")
-            or name.startswith("HAVE_")
-        ):
-            return name
-        return ""
-
     with open(header, encoding="utf-8") as cfg:
         config = [l.split()[1:] for l in cfg if l.startswith("#define")]
 
     cfg_list = []
     for cfg in config:
-        name = cfg_name(cfg[0])
-        if not name:
+        name = cfg[0]
+        if f'cfg({name})' not in cargo_toml.check_cfg:
             continue
         if len(cfg) >= 2 and cfg[1] != "1":
             continue
@@ -59,7 +80,6 @@ def cfg_name(name: str) -> str:
 
 
 def main() -> None:
-    # pylint: disable=missing-function-docstring
     parser = argparse.ArgumentParser()
     parser.add_argument("-v", "--verbose", action="store_true")
     parser.add_argument(
@@ -71,12 +91,21 @@ def main() -> None:
         required=False,
         default=[],
     )
+    parser.add_argument(
+        metavar="TOML_FILE",
+        action="store",
+        dest="cargo_toml",
+        help="path to Cargo.toml file",
+    )
     args = parser.parse_args()
     if args.verbose:
         logging.basicConfig(level=logging.DEBUG)
     logging.debug("args: %s", args)
+
+    cargo_toml = CargoTOML(args.cargo_toml)
+
     for header in args.config_headers:
-        for tok in generate_cfg_flags(header):
+        for tok in generate_cfg_flags(header, cargo_toml):
             print(tok)
 
 
-- 
2.47.0

[RFC PATCH 04/11] rust: build: generate warning flags from Cargo.toml

[Paolo Bonzini]

Cargo.toml makes it possible to describe the desired lint level settings
in a nice format.  We can extend this to Meson-built crates, by teaching
rustc_args.py to fetch lint and --check-cfg arguments from Cargo.toml.

Start with qemu-api, since it already has a [lints.rust] table and
an invocation of rustc_args.py.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/meson.build  |  4 +-
 scripts/rust/rustc_args.py | 83 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index bb2ed2844dc..4ba5607d66b 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -5,8 +5,8 @@ if rustc.version().version_compare('>=1.77.0')
 endif
 
 _qemu_api_cfg += run_command(rustc_args,
-  '--config-headers', config_host_h, files('Cargo.toml'),
-  capture: true, check: true).stdout().strip().split()
+  '--config-headers', config_host_h, '--features', '--lints', files('Cargo.toml'),
+  capture: true, check: true).stdout().strip().splitlines()
 
 _qemu_api_rs = static_library(
   'qemu_api',
diff --git a/scripts/rust/rustc_args.py b/scripts/rust/rustc_args.py
index 942dd2b2bab..9b9778a1cac 100644
--- a/scripts/rust/rustc_args.py
+++ b/scripts/rust/rustc_args.py
@@ -25,9 +25,10 @@
 """
 
 import argparse
+from dataclasses import dataclass
 import logging
 from pathlib import Path
-from typing import Any, Iterable, Mapping, Optional, Set
+from typing import Any, Iterable, List, Mapping, Optional, Set
 
 try:
     import tomllib
@@ -61,6 +62,45 @@ def get_table(self, key: str) -> Mapping[Any, Any]:
         return table
 
 
+@dataclass
+class LintFlag:
+    flags: List[str]
+    priority: int
+
+
+def generate_lint_flags(cargo_toml: CargoTOML) -> Iterable[str]:
+    """Converts Cargo.toml lints to rustc -A/-D/-F/-W flags."""
+
+    toml_lints = cargo_toml.lints
+
+    lint_list = []
+    for k, v in toml_lints.items():
+        prefix = "" if k == "rust" else k + "::"
+        for lint, data in v.items():
+            level = data if isinstance(data, str) else data["level"]
+            priority = 0 if isinstance(data, str) else data.get("priority", 0)
+            if level == "deny":
+                flag = "-D"
+            elif level == "allow":
+                flag = "-A"
+            elif level == "warn":
+                flag = "-W"
+            elif level == "forbid":
+                flag = "-F"
+            else:
+                raise Exception(f"invalid level {level} for {prefix}{lint}")
+
+            # This may change if QEMU ever invokes clippy-driver or rustdoc by
+            # hand.  For now, check the syntax but do not add non-rustc lints to
+            # the command line.
+            if k == "rust":
+                lint_list.append(LintFlag(flags=[flag, prefix + lint], priority=priority))
+
+    lint_list.sort(key=lambda x: x.priority)
+    for lint in lint_list:
+        yield from lint.flags
+
+
 def generate_cfg_flags(header: str, cargo_toml: CargoTOML) -> Iterable[str]:
     """Converts defines from config[..].h headers to rustc --cfg flags."""
 
@@ -97,13 +137,54 @@ def main() -> None:
         dest="cargo_toml",
         help="path to Cargo.toml file",
     )
+    parser.add_argument(
+        "--features",
+        action="store_true",
+        dest="features",
+        help="generate --check-cfg arguments for features",
+        required=False,
+        default=None,
+    )
+    parser.add_argument(
+        "--lints",
+        action="store_true",
+        dest="lints",
+        help="generate arguments from [lints] table",
+        required=False,
+        default=None,
+    )
+    parser.add_argument(
+        "--rustc-version",
+        metavar="VERSION",
+        dest="rustc_version",
+        action="store",
+        help="version of rustc",
+        required=False,
+        default="1.0.0",
+    )
     args = parser.parse_args()
     if args.verbose:
         logging.basicConfig(level=logging.DEBUG)
     logging.debug("args: %s", args)
 
+    rustc_version = tuple((int(x) for x in args.rustc_version.split('.')[0:2]))
     cargo_toml = CargoTOML(args.cargo_toml)
 
+    if args.lints:
+        for tok in generate_lint_flags(cargo_toml):
+            print(tok)
+
+    if rustc_version >= (1, 80):
+        if args.lints:
+            for cfg in sorted(cargo_toml.check_cfg):
+                print("--check-cfg")
+                print(cfg)
+        if args.features:
+            for feature in cargo_toml.get_table("features"):
+                if feature != "default":
+                    print("--check-cfg")
+                    print(f'cfg(feature,values("{feature}"))')
+
     for header in args.config_headers:
         for tok in generate_cfg_flags(header, cargo_toml):
             print(tok)
-- 
2.47.0

[RFC PATCH 05/11] rust: cargo: store desired warning levels in workspace Cargo.toml

[Paolo Bonzini]

An extra benefit of workspaces is that they allow to place lint level
settings in a single Cargo.toml; the settings are then inherited by
packages in the workspace.

Correspondingly, teach rustc_build_args.py to get the unexpected_cfgs
configuration from the workspace Cargo.toml.

Note that it is still possible to allow or deny warnings per crate or
module, via the #![] attribute syntax.  The rust/qemu-api/src/bindings.rs
file is an example.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                     |  8 +++++---
 rust/Cargo.toml                 |  8 ++++++++
 rust/hw/char/pl011/Cargo.toml   |  3 +++
 rust/qemu-api-macros/Cargo.toml |  3 +++
 rust/qemu-api/Cargo.toml        |  5 ++---
 rust/qemu-api/meson.build       |  2 +-
 scripts/rust/rustc_args.py      | 31 +++++++++++++++++++++++++++----
 7 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/meson.build b/meson.build
index 7a9a523467b..5726135b324 100644
--- a/meson.build
+++ b/meson.build
@@ -120,11 +120,13 @@ if have_rust
 endif
 
 if have_rust
-  rustc_args = find_program('scripts/rust/rustc_args.py')
+  rustc_args = [find_program('scripts/rust/rustc_args.py'),
+    '--workspace', meson.project_source_root() / 'rust']
   rustfmt = find_program('rustfmt', required: false)
 
-  # Prohibit code that is forbidden in Rust 2024
-  rustc_lint_args = ['-D', 'unsafe_op_in_unsafe_fn']
+  rustc_lint_args = run_command(rustc_args,
+     '--lints', files('rust/Cargo.toml'),
+     capture: true, check: true).stdout().strip().splitlines()
 
   # Occasionally, we may need to silence warnings and clippy lints that
   # were only introduced in newer Rust compiler versions.  Do not croak
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 0c94d5037da..0230b92a9fa 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -5,3 +5,11 @@ members = [
     "qemu-api",
     "hw/char/pl011",
 ]
+
+[workspace.lints.rust]
+unexpected_cfgs = { level = "warn", check-cfg = [
+    'cfg(MESON)', 'cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)',
+    'cfg(has_offset_of)'] }
+
+# Prohibit code that is forbidden in Rust 2024
+unsafe_op_in_unsafe_fn = "deny"
diff --git a/rust/hw/char/pl011/Cargo.toml b/rust/hw/char/pl011/Cargo.toml
index a373906b9fb..58f3e859f7e 100644
--- a/rust/hw/char/pl011/Cargo.toml
+++ b/rust/hw/char/pl011/Cargo.toml
@@ -21,3 +21,6 @@ bilge = { version = "0.2.0" }
 bilge-impl = { version = "0.2.0" }
 qemu_api = { path = "../../../qemu-api" }
 qemu_api_macros = { path = "../../../qemu-api-macros" }
+
+[lints]
+workspace = true
diff --git a/rust/qemu-api-macros/Cargo.toml b/rust/qemu-api-macros/Cargo.toml
index a8f7377106b..5a27b52ee6e 100644
--- a/rust/qemu-api-macros/Cargo.toml
+++ b/rust/qemu-api-macros/Cargo.toml
@@ -20,3 +20,6 @@ proc-macro = true
 proc-macro2 = "1"
 quote = "1"
 syn = { version = "2", features = ["extra-traits"] }
+
+[lints]
+workspace = true
diff --git a/rust/qemu-api/Cargo.toml b/rust/qemu-api/Cargo.toml
index cc716d75d46..669f288d1cb 100644
--- a/rust/qemu-api/Cargo.toml
+++ b/rust/qemu-api/Cargo.toml
@@ -23,6 +23,5 @@ version_check = "~0.9"
 default = []
 allocator = []
 
-[lints.rust]
-unexpected_cfgs = { level = "warn", check-cfg = ['cfg(MESON)', 'cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)',
-                                                 'cfg(has_offset_of)'] }
+[lints]
+workspace = true
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 4ba5607d66b..8013911a348 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -5,7 +5,7 @@ if rustc.version().version_compare('>=1.77.0')
 endif
 
 _qemu_api_cfg += run_command(rustc_args,
-  '--config-headers', config_host_h, '--features', '--lints', files('Cargo.toml'),
+  '--config-headers', config_host_h, '--features', files('Cargo.toml'),
   capture: true, check: true).stdout().strip().splitlines()
 
 _qemu_api_rs = static_library(
diff --git a/scripts/rust/rustc_args.py b/scripts/rust/rustc_args.py
index 9b9778a1cac..26733439ed4 100644
--- a/scripts/rust/rustc_args.py
+++ b/scripts/rust/rustc_args.py
@@ -38,12 +38,19 @@
 
 class CargoTOML:
     tomldata: Mapping[Any, Any]
+    workspace_data: Mapping[Any, Any]
     check_cfg: Set[str]
 
-    def __init__(self, path: str):
+    def __init__(self, path: str, workspace: Optional[str]):
         with open(path, 'rb') as f:
             self.tomldata = tomllib.load(f)
 
+        if workspace is not None:
+            with open(workspace, 'rb') as f:
+                self.workspace_data = tomllib.load(f)
+            if "workspace" not in self.workspace_data:
+                self.workspace_data["workspace"] = {}
+
         self.check_cfg = set(self.find_check_cfg())
 
     def find_check_cfg(self) -> Iterable[str]:
@@ -54,10 +61,12 @@ def find_check_cfg(self) -> Iterable[str]:
 
     @property
     def lints(self) -> Mapping[Any, Any]:
-        return self.get_table("lints")
+        return self.get_table("lints", True)
 
-    def get_table(self, key: str) -> Mapping[Any, Any]:
+    def get_table(self, key: str, can_be_workspace: bool = False) -> Mapping[Any, Any]:
         table = self.tomldata.get(key, {})
+        if can_be_workspace and table.get("workspace", False) is True:
+            table = self.workspace_data["workspace"].get(key, {})
 
         return table
 
@@ -136,6 +145,16 @@ def main() -> None:
         action="store",
         dest="cargo_toml",
         help="path to Cargo.toml file",
+        nargs='?',
+    )
+    parser.add_argument(
+        "--workspace",
+        metavar="DIR",
+        action="store",
+        dest="workspace",
+        help="path to root of the workspace",
+        required=False,
+        default=None,
     )
     parser.add_argument(
         "--features",
@@ -168,7 +187,11 @@ def main() -> None:
     logging.debug("args: %s", args)
 
     rustc_version = tuple((int(x) for x in args.rustc_version.split('.')[0:2]))
-    cargo_toml = CargoTOML(args.cargo_toml)
+    if args.workspace:
+        workspace_cargo_toml = Path(args.workspace, "Cargo.toml").resolve()
+        cargo_toml = CargoTOML(args.cargo_toml, str(workspace_cargo_toml))
+    else:
+        cargo_toml = CargoTOML(args.cargo_toml, None)
 
     if args.lints:
         for tok in generate_lint_flags(cargo_toml):
-- 
2.47.0

[RFC PATCH 06/11] rust: build: move strict lints handling to rustc_args.py

[Paolo Bonzini]

Make Cargo use unknown_lints = "allow" as well.  This is more future
proof as we might add new lints to rust/Cargo.toml that are not supported
by older versions of rustc or clippy.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                | 12 ++++--------
 rust/Cargo.toml            |  6 ++++++
 scripts/rust/rustc_args.py | 19 ++++++++++++++++---
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/meson.build b/meson.build
index 5726135b324..1239f5c48c1 100644
--- a/meson.build
+++ b/meson.build
@@ -122,20 +122,16 @@ endif
 if have_rust
   rustc_args = [find_program('scripts/rust/rustc_args.py'),
     '--workspace', meson.project_source_root() / 'rust']
+  if get_option('strict_rust_lints')
+    rustc_args += ['--strict-lints']
+  endif
+
   rustfmt = find_program('rustfmt', required: false)
 
   rustc_lint_args = run_command(rustc_args,
      '--lints', files('rust/Cargo.toml'),
      capture: true, check: true).stdout().strip().splitlines()
 
-  # Occasionally, we may need to silence warnings and clippy lints that
-  # were only introduced in newer Rust compiler versions.  Do not croak
-  # in that case; a CI job with rust_strict_lints == true ensures that
-  # we do not have misspelled allow() attributes.
-  if not get_option('strict_rust_lints')
-    rustc_lint_args += ['-A', 'unknown_lints']
-  endif
-
   # Apart from procedural macros, our Rust executables will often link
   # with C code, so include all the libraries that C code needs.  This
   # is safe; https://github.com/rust-lang/rust/pull/54675 says that
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 0230b92a9fa..1ff8f5c2781 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -11,5 +11,11 @@ unexpected_cfgs = { level = "warn", check-cfg = [
     'cfg(MESON)', 'cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)',
     'cfg(has_offset_of)'] }
 
+# Occasionally, we may need to silence warnings and clippy lints that
+# were only introduced in newer Rust compiler versions.  Do not croak
+# in that case; a CI job with rust_strict_lints == true disables this
+# and ensures that we do not have misspelled allow() attributes.
+unknown_lints = "allow"
+
 # Prohibit code that is forbidden in Rust 2024
 unsafe_op_in_unsafe_fn = "deny"
diff --git a/scripts/rust/rustc_args.py b/scripts/rust/rustc_args.py
index 26733439ed4..1f8b05b8001 100644
--- a/scripts/rust/rustc_args.py
+++ b/scripts/rust/rustc_args.py
@@ -35,6 +35,8 @@
 except ImportError:
     import tomli as tomllib
 
+STRICT_LINTS = {"unknown_lints", "warnings"}
+
 
 class CargoTOML:
     tomldata: Mapping[Any, Any]
@@ -77,7 +79,7 @@ class LintFlag:
     priority: int
 
 
-def generate_lint_flags(cargo_toml: CargoTOML) -> Iterable[str]:
+def generate_lint_flags(cargo_toml: CargoTOML, strict_lints: bool) -> Iterable[str]:
     """Converts Cargo.toml lints to rustc -A/-D/-F/-W flags."""
 
     toml_lints = cargo_toml.lints
@@ -102,9 +104,13 @@ def generate_lint_flags(cargo_toml: CargoTOML) -> Iterable[str]:
             # This may change if QEMU ever invokes clippy-driver or rustdoc by
             # hand.  For now, check the syntax but do not add non-rustc lints to
             # the command line.
-            if k == "rust":
+            if k == "rust" and not (strict_lints and lint in STRICT_LINTS):
                 lint_list.append(LintFlag(flags=[flag, prefix + lint], priority=priority))
 
+    if strict_lints:
+        for lint in STRICT_LINTS:
+            lint_list.append(LintFlag(flags=["-D", lint], priority=1000000))
+
     lint_list.sort(key=lambda x: x.priority)
     for lint in lint_list:
         yield from lint.flags
@@ -181,6 +187,13 @@ def main() -> None:
         required=False,
         default="1.0.0",
     )
+    parser.add_argument(
+        "--strict-lints",
+        action="store_true",
+        dest="strict_lints",
+        help="apply stricter checks (for nightly Rust)",
+        default=False,
+    )
     args = parser.parse_args()
     if args.verbose:
         logging.basicConfig(level=logging.DEBUG)
@@ -194,7 +207,7 @@ def main() -> None:
         cargo_toml = CargoTOML(args.cargo_toml, None)
 
     if args.lints:
-        for tok in generate_lint_flags(cargo_toml):
+        for tok in generate_lint_flags(cargo_toml, args.strict_lints):
             print(tok)
 
     if rustc_version >= (1, 80):
-- 
2.47.0

[RFC PATCH 07/11] rust: fix a couple style issues from clippy

[Paolo Bonzini]

These are reported as clippy::semicolon_inside_block and clippy::as_ptr_cast_mut.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs     | 6 ++++--
 rust/hw/char/pl011/src/memory_ops.rs | 4 +++-
 rust/qemu-api/tests/tests.rs         | 2 +-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 2a85960b81f..7f40e7f71fa 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -499,7 +499,9 @@ pub fn update(&self) {
         let flags = self.int_level & self.int_enabled;
         for (irq, i) in self.interrupts.iter().zip(IRQMASK) {
             // SAFETY: self.interrupts have been initialized in init().
-            unsafe { qemu_set_irq(*irq, i32::from(flags & i != 0)) };
+            unsafe {
+                qemu_set_irq(*irq, i32::from(flags & i != 0));
+            }
         }
     }
 
@@ -601,7 +603,7 @@ pub fn post_load(&mut self, _version_id: u32) -> Result<(), ()> {
         let sysbus: *mut SysBusDevice = dev.cast::<SysBusDevice>();
 
         qdev_prop_set_chr(dev, c_str!("chardev").as_ptr(), chr);
-        sysbus_realize_and_unref(sysbus, addr_of!(error_fatal) as *mut *mut Error);
+        sysbus_realize_and_unref(sysbus, addr_of!(error_fatal).cast_mut());
         sysbus_mmio_map(sysbus, 0, addr);
         sysbus_connect_irq(sysbus, 0, irq);
         dev
diff --git a/rust/hw/char/pl011/src/memory_ops.rs b/rust/hw/char/pl011/src/memory_ops.rs
index 169d485a4d2..c4e8599ba43 100644
--- a/rust/hw/char/pl011/src/memory_ops.rs
+++ b/rust/hw/char/pl011/src/memory_ops.rs
@@ -33,7 +33,9 @@
             // SAFETY: self.char_backend is a valid CharBackend instance after it's been
             // initialized in realize().
             let cb_ptr = unsafe { core::ptr::addr_of_mut!(state.as_mut().char_backend) };
-            unsafe { qemu_chr_fe_accept_input(cb_ptr) };
+            unsafe {
+                qemu_chr_fe_accept_input(cb_ptr);
+            }
 
             val
         }
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 43a4827de12..925f5a3c77b 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -74,6 +74,6 @@ impl Class for DummyClass {
 
     unsafe {
         module_call_init(module_init_type::MODULE_INIT_QOM);
-        object_unref(object_new(DummyState::TYPE_NAME.as_ptr()) as *mut _);
+        object_unref(object_new(DummyState::TYPE_NAME.as_ptr()).cast());
     }
 }
-- 
2.47.0

[RFC PATCH 08/11] rust: build: establish a baseline of lints across all crates

[Paolo Bonzini]

Many lints that default to allow can be helpful in detecting bugs or
keeping the code style homogeneous.  Add them liberally, though perhaps
not as liberally as in hw/char/pl011/src/lib.rs.  In particular, enabling
entire groups can be problematic because of bitrot when new links are
added in the future.

For Clippy, this is actually a feature that is only present in Cargo
1.74.0 but, since we are not using Cargo to *build* QEMU, only developers
will need a new-enough cargo and only to run tools such as clippy.
The requirement does not apply to distros that are building QEMU.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/Cargo.toml               | 66 +++++++++++++++++++++++++++++++++++
 rust/hw/char/pl011/src/lib.rs | 18 ++--------
 rust/qemu-api/src/bindings.rs |  6 ++--
 3 files changed, 71 insertions(+), 19 deletions(-)

diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 1ff8f5c2781..43cca33a8d8 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -19,3 +19,69 @@ unknown_lints = "allow"
 
 # Prohibit code that is forbidden in Rust 2024
 unsafe_op_in_unsafe_fn = "deny"
+
+[workspace.lints.rustdoc]
+broken_intra_doc_links = "deny"
+invalid_html_tags = "deny"
+invalid_rust_codeblocks = "deny"
+bare_urls = "deny"
+unescaped_backticks = "deny"
+redundant_explicit_links = "deny"
+
+[workspace.lints.clippy]
+# default-warn lints
+result_unit_err = "allow"
+too_many_arguments = "allow"
+upper_case_acronyms = "allow"
+
+# default-allow lints
+as_ptr_cast_mut = "deny"
+as_underscore = "deny"
+assertions_on_result_states = "deny"
+bool_to_int_with_if = "deny"
+borrow_as_ptr = "deny"
+cast_lossless = "deny"
+dbg_macro = "deny"
+debug_assert_with_mut_call = "deny"
+derive_partial_eq_without_eq = "deny"
+doc_markdown = "deny"
+empty_structs_with_brackets = "deny"
+ignored_unit_patterns = "deny"
+implicit_clone = "deny"
+macro_use_imports = "deny"
+missing_const_for_fn = "deny"
+missing_safety_doc = "deny"
+multiple_crate_versions = "deny"
+mut_mut = "deny"
+needless_bitwise_bool = "deny"
+needless_pass_by_ref_mut = "deny"
+no_effect_underscore_binding = "deny"
+option_option = "deny"
+or_fun_call = "deny"
+ptr_as_ptr = "deny"
+ptr_cast_constness = "deny"
+pub_underscore_fields = "deny"
+redundant_clone = "deny"
+redundant_closure_for_method_calls = "deny"
+redundant_else = "deny"
+redundant_pub_crate = "deny"
+ref_binding_to_reference = "deny"
+ref_option_ref = "deny"
+return_self_not_must_use = "deny"
+same_name_method = "deny"
+semicolon_inside_block = "deny"
+significant_drop_in_scrutinee = "deny"
+significant_drop_tightening = "deny"
+suspicious_operation_groupings = "deny"
+transmute_ptr_to_ptr = "deny"
+transmute_undefined_repr = "deny"
+type_repetition_in_bounds = "deny"
+unused_self = "deny"
+used_underscore_binding = "deny"
+
+# nice to have, but cannot be enabled yet
+#wildcard_imports = "deny"
+
+# these may have false positives
+#option_if_let_else = "deny"
+cognitive_complexity = "deny"
diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs
index cd0a49acb91..3fa178cded0 100644
--- a/rust/hw/char/pl011/src/lib.rs
+++ b/rust/hw/char/pl011/src/lib.rs
@@ -14,28 +14,14 @@
 //! the [`registers`] module for register types.
 
 #![deny(
-    rustdoc::broken_intra_doc_links,
-    rustdoc::redundant_explicit_links,
     clippy::correctness,
     clippy::suspicious,
     clippy::complexity,
     clippy::perf,
     clippy::cargo,
     clippy::nursery,
-    clippy::style,
-    // restriction group
-    clippy::dbg_macro,
-    clippy::as_underscore,
-    clippy::assertions_on_result_states,
-    // pedantic group
-    clippy::doc_markdown,
-    clippy::borrow_as_ptr,
-    clippy::cast_lossless,
-    clippy::option_if_let_else,
-    clippy::missing_const_for_fn,
-    clippy::cognitive_complexity,
-    clippy::missing_safety_doc,
-    )]
+    clippy::style
+)]
 #![allow(clippy::result_unit_err)]
 
 extern crate bilge;
diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
index 1dac310594d..972b1f1ee90 100644
--- a/rust/qemu-api/src/bindings.rs
+++ b/rust/qemu-api/src/bindings.rs
@@ -7,10 +7,10 @@
     non_snake_case,
     non_upper_case_globals,
     unsafe_op_in_unsafe_fn,
+    clippy::pedantic,
+    clippy::restriction,
+    clippy::style,
     clippy::missing_const_for_fn,
-    clippy::too_many_arguments,
-    clippy::approx_constant,
-    clippy::use_self,
     clippy::useless_transmute,
     clippy::missing_safety_doc
 )]
-- 
2.47.0

[RFC PATCH 09/11] rust: build: add "make clippy", "make rustfmt"

[Paolo Bonzini]

Abstract common invocations of "cargo", that do not require copying
the generated bindgen file or setting up MESON_BUILD_ROOT.

In the future these could also do completely without cargo and invoke
the underlying programs directly.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/meson.build       | 14 ++++++++++++++
 rust/qemu-api/build.rs |  8 ++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/rust/meson.build b/rust/meson.build
index def77389cdd..6fa0fd54527 100644
--- a/rust/meson.build
+++ b/rust/meson.build
@@ -2,3 +2,17 @@ subdir('qemu-api-macros')
 subdir('qemu-api')
 
 subdir('hw')
+
+cargo = find_program('cargo')
+
+run_target('clippy',
+  command: [config_host['MESON'], 'devenv',
+            '--workdir', '@CURRENT_SOURCE_DIR@',
+            cargo, 'clippy', '--tests'],
+  depends: bindings_rs)
+
+run_target('rustfmt',
+  command: [config_host['MESON'], 'devenv',
+            '--workdir', '@CURRENT_SOURCE_DIR@',
+            cargo, 'fmt'],
+  depends: bindings_rs)
diff --git a/rust/qemu-api/build.rs b/rust/qemu-api/build.rs
index e4eab718553..d7b6d76828b 100644
--- a/rust/qemu-api/build.rs
+++ b/rust/qemu-api/build.rs
@@ -15,8 +15,12 @@ fn main() {
     let file = format!("{}/bindings.rs.inc", path);
     if !Path::new(&file).exists() {
         panic!(concat!(
-            "No generated C bindings found! If you want to run `cargo`, start a subshell\n",
-            "with `meson devenv`, or point MESON_BUILD_ROOT to the top of the build tree."
+            "\n",
+            "    No generated C bindings found! To run clippy or rustfmt, you can use\n",
+            "    `make clippy` or `make rustfmt`.\n",
+            "\n",
+            "    For other uses of `cargo`, start a subshell with `meson devenv`, or\n",
+            "    point MESON_BUILD_ROOT to the top of the build tree."
         ));
     }
 
-- 
2.47.0

[RFC PATCH 10/11] rust: fix doc test syntax

[Paolo Bonzini]

Allow "cargo test --doc" to pass.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/src/zeroable.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs
index 13cdb2ccba5..6125aeed8b4 100644
--- a/rust/qemu-api/src/zeroable.rs
+++ b/rust/qemu-api/src/zeroable.rs
@@ -7,9 +7,9 @@
 /// behavior.  This trait in principle could be implemented as just:
 ///
 /// ```
-///     const ZERO: Self = unsafe {
-///         ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init()
-///     },
+/// pub unsafe trait Zeroable: Default {
+///     const ZERO: Self = unsafe { ::core::mem::MaybeUninit::<Self>::zeroed().assume_init() };
+/// }
 /// ```
 ///
 /// The need for a manual implementation is only because `zeroed()` cannot
-- 
2.47.0

[RFC PATCH 11/11] rust: ci: add job that runs Rust tools

[Paolo Bonzini]

Code checks, as well as documentation generation, are not yet tied
to "make check" because they need new version of the Rust toolchain
(even nightly in the case of "rustfmt").  Run them in CI using the
existing nightly-Rust container.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 .gitlab-ci.d/buildtest-template.yml                | 14 ++++++++++++++
 .gitlab-ci.d/buildtest.yml                         | 14 ++++++++++++++
 .../docker/dockerfiles/fedora-rust-nightly.docker  |  4 ++++
 tests/lcitool/refresh                              |  4 ++++
 4 files changed, 36 insertions(+)

diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml
index 39da7698b09..612e968ff19 100644
--- a/.gitlab-ci.d/buildtest-template.yml
+++ b/.gitlab-ci.d/buildtest-template.yml
@@ -79,6 +79,20 @@
     - $MAKE NINJA=":" $MAKE_CHECK_ARGS
     - section_end test
 
+.rust_test_job_template:
+  extends: .base_job_template
+  stage: test
+  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:$QEMU_CI_CONTAINER_TAG
+  script:
+    - source scripts/ci/gitlab-ci-section
+    - section_start test "Running Rust code checks"
+    - cd build
+    - pyvenv/bin/meson devenv -w ../rust ${CARGO-cargo} fmt --check
+    - make clippy
+    - pyvenv/bin/meson devenv -w ../rust ${CARGO-cargo} doc --no-deps
+    - pyvenv/bin/meson devenv -w ../rust ${CARGO-cargo} test --doc
+    - section_end test
+
 .native_test_job_template:
   extends: .common_test_job_template
   artifacts:
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 336223484d8..5250b61f089 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -142,6 +142,20 @@ check-system-fedora:
     IMAGE: fedora
     MAKE_CHECK_ARGS: check
 
+check-rust-fedora-rust-nightly:
+  extends: .rust_test_job_template
+  needs:
+    - job: build-system-fedora-rust-nightly
+      artifacts: true
+  variables:
+    IMAGE: fedora-rust-nightly
+    MAKE_CHECK_ARGS: check
+  artifacts:
+    when: on_success
+    expire_in: 2 days
+    paths:
+      - rust/target/doc
+
 functional-system-fedora:
   extends: .functional_test_job_template
   needs:
diff --git a/tests/docker/dockerfiles/fedora-rust-nightly.docker b/tests/docker/dockerfiles/fedora-rust-nightly.docker
index 9180c8b5222..a8e4fb279a7 100644
--- a/tests/docker/dockerfiles/fedora-rust-nightly.docker
+++ b/tests/docker/dockerfiles/fedora-rust-nightly.docker
@@ -155,6 +155,7 @@ ENV PYTHON "/usr/bin/python3"
 RUN dnf install -y wget
 ENV RUSTUP_HOME=/usr/local/rustup CARGO_HOME=/usr/local/cargo
 ENV RUSTC=/usr/local/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rustc
+ENV CARGO=/usr/local/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo
 RUN set -eux && \
   rustArch='x86_64-unknown-linux-gnu' && \
   rustupSha256='6aeece6993e902708983b209d04c0d1dbb14ebb405ddb87def578d41f920f56d' && \
@@ -165,10 +166,13 @@ RUN set -eux && \
   ./rustup-init -y --no-modify-path --profile default --default-toolchain nightly --default-host ${rustArch} && \
   chmod -R a+w $RUSTUP_HOME $CARGO_HOME && \
   /usr/local/cargo/bin/rustup --version && \
+  /usr/local/cargo/bin/rustup run nightly cargo --version && \
   /usr/local/cargo/bin/rustup run nightly rustc --version && \
+  test "$CARGO" = "$(/usr/local/cargo/bin/rustup +nightly which cargo)" && \
   test "$RUSTC" = "$(/usr/local/cargo/bin/rustup +nightly which rustc)"
 ENV PATH=$CARGO_HOME/bin:$PATH
 RUN /usr/local/cargo/bin/rustup run nightly cargo install bindgen-cli
+RUN $CARGO --list
 # As a final step configure the user (if env is defined)
 ARG USER
 ARG UID
diff --git a/tests/lcitool/refresh b/tests/lcitool/refresh
index 51012783c0f..6720516b942 100755
--- a/tests/lcitool/refresh
+++ b/tests/lcitool/refresh
@@ -121,6 +121,7 @@ fedora_rustup_nightly_extras = [
     "RUN dnf install -y wget\n",
     "ENV RUSTUP_HOME=/usr/local/rustup CARGO_HOME=/usr/local/cargo\n",
     "ENV RUSTC=/usr/local/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rustc\n",
+    "ENV CARGO=/usr/local/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo\n",
     "RUN set -eux && \\\n",
     "  rustArch='x86_64-unknown-linux-gnu' && \\\n",
     "  rustupSha256='6aeece6993e902708983b209d04c0d1dbb14ebb405ddb87def578d41f920f56d' && \\\n",
@@ -131,10 +132,13 @@ fedora_rustup_nightly_extras = [
     "  ./rustup-init -y --no-modify-path --profile default --default-toolchain nightly --default-host ${rustArch} && \\\n",
     "  chmod -R a+w $RUSTUP_HOME $CARGO_HOME && \\\n",
     "  /usr/local/cargo/bin/rustup --version && \\\n",
+    "  /usr/local/cargo/bin/rustup run nightly cargo --version && \\\n",
     "  /usr/local/cargo/bin/rustup run nightly rustc --version && \\\n",
+    '  test "$CARGO" = "$(/usr/local/cargo/bin/rustup +nightly which cargo)" && \\\n',
     '  test "$RUSTC" = "$(/usr/local/cargo/bin/rustup +nightly which rustc)"\n',
     'ENV PATH=$CARGO_HOME/bin:$PATH\n',
     'RUN /usr/local/cargo/bin/rustup run nightly cargo install bindgen-cli\n',
+    'RUN $CARGO --list\n',
 ]
 
 ubuntu2204_bindgen_extras = [
-- 
2.47.0

Re: [RFC PATCH 11/11] rust: ci: add job that runs Rust tools

[Daniel P. Berrangé]

On Fri, Nov 08, 2024 at 07:01:39PM +0100, Paolo Bonzini wrote:
> Code checks, as well as documentation generation, are not yet tied
> to "make check" because they need new version of the Rust toolchain
> (even nightly in the case of "rustfmt").  Run them in CI using the
> existing nightly-Rust container.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  .gitlab-ci.d/buildtest-template.yml                | 14 ++++++++++++++
>  .gitlab-ci.d/buildtest.yml                         | 14 ++++++++++++++
>  .../docker/dockerfiles/fedora-rust-nightly.docker  |  4 ++++
>  tests/lcitool/refresh                              |  4 ++++
>  4 files changed, 36 insertions(+)
> 
> diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml
> index 39da7698b09..612e968ff19 100644
> --- a/.gitlab-ci.d/buildtest-template.yml
> +++ b/.gitlab-ci.d/buildtest-template.yml
> @@ -79,6 +79,20 @@
>      - $MAKE NINJA=":" $MAKE_CHECK_ARGS
>      - section_end test
>  
> +.rust_test_job_template:
> +  extends: .base_job_template
> +  stage: test
> +  image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:$QEMU_CI_CONTAINER_TAG
> +  script:
> +    - source scripts/ci/gitlab-ci-section
> +    - section_start test "Running Rust code checks"
> +    - cd build
> +    - pyvenv/bin/meson devenv -w ../rust ${CARGO-cargo} fmt --check
> +    - make clippy
> +    - pyvenv/bin/meson devenv -w ../rust ${CARGO-cargo} doc --no-deps
> +    - pyvenv/bin/meson devenv -w ../rust ${CARGO-cargo} test --doc
> +    - section_end test
> +

I'd suggest that the static checks "fmt" and "doc" should be separated
from the dynamic (unit test) check in  "tests", and that the former
should be in a job defined in the static-checks.yml file.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [RFC PATCH 01/11] rust: qemu_api: do not disable lints outside bindgen-generated code

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> rust/qemu-api/src/lib.rs is disabling lints that cause problems
> with code generated by bindgen.

The commit title is misleading. Today the lint-disabling attributes are
outer ones which apply only to the bindings module. Those lints already
apply to the others in the qemu_api crate.

That said, moving the bindings mod and the related extensions (trait
impls, etc.) into a separate file still looks a good idea to me.

> Instead, include the bindgen
> code via include!(...) and move the #![allow()] directives
> into the bindings module.
>
> Add MESON_BUILD_ROOT to the devenv, so that it's easy for
> build.rs to find the include file.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build                   |  4 +++-
>  rust/qemu-api/.gitignore      |  2 +-
>  rust/qemu-api/build.rs        | 20 ++++++++++++++------
>  rust/qemu-api/meson.build     |  1 +
>  rust/qemu-api/src/bindings.rs | 29 +++++++++++++++++++++++++++++
>  rust/qemu-api/src/lib.rs      | 22 ----------------------
>  6 files changed, 48 insertions(+), 30 deletions(-)
>  create mode 100644 rust/qemu-api/src/bindings.rs
>
> diff --git a/meson.build b/meson.build
> index e0b880e4e13..a7342c6edbd 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3,6 +3,8 @@ project('qemu', ['c'], meson_version: '>=1.5.0',
>                            'b_staticpic=false', 'stdsplit=false', 'optimization=2', 'b_pie=true'],
>          version: files('VERSION'))
>
> +meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() })
> +
>  add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true)
>  add_test_setup('slow', exclude_suites: ['thorough'], env: ['G_TEST_SLOW=1', 'SPEED=slow'])
>  add_test_setup('thorough', env: ['G_TEST_SLOW=1', 'SPEED=thorough'])
> @@ -4089,7 +4091,7 @@ if have_rust
>    bindings_rs = rust.bindgen(
>      input: 'rust/wrapper.h',
>      dependencies: common_ss.all_dependencies(),
> -    output: 'bindings.rs',
> +    output: 'bindings.rs.inc',
>      include_directories: include_directories('.', 'include'),
>      bindgen_version: ['>=0.60.0'],
>      args: bindgen_args,
> diff --git a/rust/qemu-api/.gitignore b/rust/qemu-api/.gitignore
> index b9e7e004c86..2accb8745dc 100644
> --- a/rust/qemu-api/.gitignore
> +++ b/rust/qemu-api/.gitignore
> @@ -1,2 +1,2 @@
>  # Ignore generated bindings file overrides.
> -src/bindings.rs
> +/src/bindings.rs.inc
> diff --git a/rust/qemu-api/build.rs b/rust/qemu-api/build.rs
> index 20f8f718b90..e4eab718553 100644
> --- a/rust/qemu-api/build.rs
> +++ b/rust/qemu-api/build.rs
> @@ -2,18 +2,26 @@
>  // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>  // SPDX-License-Identifier: GPL-2.0-or-later
>
> -use std::path::Path;
> +use std::{env, path::Path};
>
>  use version_check as rustc;
>
>  fn main() {
> -    if !Path::new("src/bindings.rs").exists() {
> -        panic!(
> -            "No generated C bindings found! Either build them manually with bindgen or with meson \
> -             (`ninja bindings.rs`) and copy them to src/bindings.rs, or build through meson."
> -        );
> +    // Placing bindings.rs.inc in the source directory is supported
> +    // but not documented or encouraged.

I agree that storing generated stuff in the source directory should not
be encouraged.

Just want to mention that such changes can lead to trouble to
rust-analyzer. Today there are two ways to inform rust-analyzer of the
project structure:

  1. Use rust/Cargo.toml. In this case we'll hit an issue in
  rust-analyzer [1] that prevents it from including sources outside the
  crate directory. Thus, definitions in the bindgen-generated code
  cannot be found.

  2. Use the meson-generated rust-project.json. Due to the use of
  structured_sources(), that json file refers to the copied sources of
  qemu-api in the build directory. Rust-analyzer can find every symbol
  in the qemu-api crate but will jump to those copied files, making it a
  bit more annoying when developing the crate.

We can perhaps leave it as a separate topic for another series.

[1] https://github.com/rust-lang/rust-analyzer/issues/17040

--
Best Regards
Junjie Mao

> +    let path = env::var("MESON_BUILD_ROOT")
> +        .unwrap_or_else(|_| format!("{}/src", env!("CARGO_MANIFEST_DIR")));
> +
> +    let file = format!("{}/bindings.rs.inc", path);
> +    if !Path::new(&file).exists() {
> +        panic!(concat!(
> +            "No generated C bindings found! If you want to run `cargo`, start a subshell\n",
> +            "with `meson devenv`, or point MESON_BUILD_ROOT to the top of the build tree."
> +        ));
>      }
>
> +    println!("cargo:rustc-env=BINDINGS_RS_INC={}", file);
> +
>      // Check for available rustc features
>      if rustc::is_min_version("1.77.0").unwrap_or(false) {
>          println!("cargo:rustc-cfg=has_offset_of");
> diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
> index 6f637af7b1b..e3870e901e3 100644
> --- a/rust/qemu-api/meson.build
> +++ b/rust/qemu-api/meson.build
> @@ -9,6 +9,7 @@ _qemu_api_rs = static_library(
>    structured_sources(
>      [
>        'src/lib.rs',
> +      'src/bindings.rs',
>        'src/c_str.rs',
>        'src/definitions.rs',
>        'src/device_class.rs',
> diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
> new file mode 100644
> index 00000000000..1dac310594d
> --- /dev/null
> +++ b/rust/qemu-api/src/bindings.rs
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#![allow(
> +    dead_code,
> +    improper_ctypes_definitions,
> +    improper_ctypes,
> +    non_camel_case_types,
> +    non_snake_case,
> +    non_upper_case_globals,
> +    unsafe_op_in_unsafe_fn,
> +    clippy::missing_const_for_fn,
> +    clippy::too_many_arguments,
> +    clippy::approx_constant,
> +    clippy::use_self,
> +    clippy::useless_transmute,
> +    clippy::missing_safety_doc
> +)]
> +
> +#[cfg(MESON)]
> +include!("bindings.rs.inc");
> +
> +#[cfg(not(MESON))]
> +include!(env!("BINDINGS_RS_INC"));
> +
> +unsafe impl Send for Property {}
> +unsafe impl Sync for Property {}
> +unsafe impl Sync for TypeInfo {}
> +unsafe impl Sync for VMStateDescription {}
> +unsafe impl Sync for VMStateField {}
> +unsafe impl Sync for VMStateInfo {}
> diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
> index aa8d16ec94b..440aff3817d 100644
> --- a/rust/qemu-api/src/lib.rs
> +++ b/rust/qemu-api/src/lib.rs
> @@ -4,31 +4,9 @@
>
>  #![cfg_attr(not(MESON), doc = include_str!("../README.md"))]
>
> -#[allow(
> -    dead_code,
> -    improper_ctypes_definitions,
> -    improper_ctypes,
> -    non_camel_case_types,
> -    non_snake_case,
> -    non_upper_case_globals,
> -    unsafe_op_in_unsafe_fn,
> -    clippy::missing_const_for_fn,
> -    clippy::too_many_arguments,
> -    clippy::approx_constant,
> -    clippy::use_self,
> -    clippy::useless_transmute,
> -    clippy::missing_safety_doc,
> -)]
>  #[rustfmt::skip]
>  pub mod bindings;
>
> -unsafe impl Send for bindings::Property {}
> -unsafe impl Sync for bindings::Property {}
> -unsafe impl Sync for bindings::TypeInfo {}
> -unsafe impl Sync for bindings::VMStateDescription {}
> -unsafe impl Sync for bindings::VMStateField {}
> -unsafe impl Sync for bindings::VMStateInfo {}
> -
>  pub mod c_str;
>  pub mod definitions;
>  pub mod device_class;

Re: [RFC PATCH 02/11] rust: build: move rustc_args.py invocation to individual crates

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Only qemu-api needs access to the symbols in config-host.h.

This may no longer be the case when more complex, build-time
configurable devices are added in the future.

Moving rustc_args.py calls to each crate is still helpful because of the
changes in patches 3-6 in this series. So I think every crate under
rust/ needs a run_command(rustc_args, ...) for crate-specific arguments.

--
Best Regards
Junjie Mao

> Remove
> the temptation to use them by limiting the --cfg arguments to the
> qemu-api crate.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build               | 54 +++++++++++++++++----------------------
>  rust/qemu-api/meson.build |  4 +++
>  2 files changed, 28 insertions(+), 30 deletions(-)

Re: [RFC PATCH 05/11] rust: cargo: store desired warning levels in workspace Cargo.toml

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> An extra benefit of workspaces is that they allow to place lint level
> settings in a single Cargo.toml; the settings are then inherited by
> packages in the workspace.
>
> Correspondingly, teach rustc_build_args.py to get the unexpected_cfgs
> configuration from the workspace Cargo.toml.
>
> Note that it is still possible to allow or deny warnings per crate or
> module, via the #![] attribute syntax.  The rust/qemu-api/src/bindings.rs
> file is an example.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build                     |  8 +++++---
>  rust/Cargo.toml                 |  8 ++++++++
>  rust/hw/char/pl011/Cargo.toml   |  3 +++
>  rust/qemu-api-macros/Cargo.toml |  3 +++
>  rust/qemu-api/Cargo.toml        |  5 ++---
>  rust/qemu-api/meson.build       |  2 +-
>  scripts/rust/rustc_args.py      | 31 +++++++++++++++++++++++++++----
>  7 files changed, 49 insertions(+), 11 deletions(-)
>
[snip]
> diff --git a/rust/Cargo.toml b/rust/Cargo.toml
> index 0c94d5037da..0230b92a9fa 100644
> --- a/rust/Cargo.toml
> +++ b/rust/Cargo.toml
> @@ -5,3 +5,11 @@ members = [
>      "qemu-api",
>      "hw/char/pl011",
>  ]
> +
> +[workspace.lints.rust]
> +unexpected_cfgs = { level = "warn", check-cfg = [
> +    'cfg(MESON)', 'cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)',
> +    'cfg(has_offset_of)'] }
> +

Making a universal unexpected_cfgs apply to the whole workspace may lead
to a lengthy cfg list when more devices in Rust are added. As cargo does
not allow overriding workspace-defined lints once inherited, I think it
better to keep unexpected_cfgs crate-specific.

--
Best Regards
Junjie Mao

Re: [RFC PATCH 05/11] rust: cargo: store desired warning levels in workspace Cargo.toml

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 650 bytes --]

Il mar 12 nov 2024, 04:17 Junjie Mao <junjie.mao@hotmail.com> ha scritto:

> Making a universal unexpected_cfgs apply to the whole workspace may lead
> to a lengthy cfg list when more devices in Rust are added. As cargo does
> not allow overriding workspace-defined lints once inherited, I think it
> better to keep unexpected_cfgs crate-specific.
>

Is it possible? I thought you cannot override at a finer granularity once
you have a "workspace = true" line.

Based on the experience with C we shouldn't have many cfgs, but if it's
possible I would definitely make unexpected_cfgs specific to qemu-api.

Paolo

> --
> Best Regards
> Junjie Mao
>
>

[-- Attachment #2: Type: text/html, Size: 1313 bytes --]

Re: [RFC PATCH 01/11] rust: qemu_api: do not disable lints outside bindgen-generated code

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 4640 bytes --]

Il mar 12 nov 2024, 03:47 Junjie Mao <junjie.mao@hotmail.com> ha scritto:

> I agree that storing generated stuff in the source directory should not
> be encouraged.
>
> Just want to mention that such changes can lead to trouble to
> rust-analyzer. Today there are two ways to inform rust-analyzer of the
> project structure:
>
>   1. Use rust/Cargo.toml. In this case we'll hit an issue in
>   rust-analyzer [1] that prevents it from including sources outside the
>   crate directory. Thus, definitions in the bindgen-generated code
>   cannot be found.
>
>   2. Use the meson-generated rust-project.json. Due to the use of
>   structured_sources(), that json file refers to the copied sources of
>   qemu-api in the build directory. Rust-analyzer can find every symbol
>   in the qemu-api crate but will jump to those copied files, making it a
>   bit more annoying when developing the crate.
>

Would it help to move the bindgen-generated code to a completely separate
crate (e.g. qemu-api-sys), and avoid structured_sources for qemu-api? It
might even help build times.

Paolo

We can perhaps leave it as a separate topic for another series.
>
> [1] https://github.com/rust-lang/rust-analyzer/issues/17040
>
> --
> Best Regards
> Junjie Mao
>
> > +    let path = env::var("MESON_BUILD_ROOT")
> > +        .unwrap_or_else(|_| format!("{}/src",
> env!("CARGO_MANIFEST_DIR")));
> > +
> > +    let file = format!("{}/bindings.rs.inc", path);
> > +    if !Path::new(&file).exists() {
> > +        panic!(concat!(
> > +            "No generated C bindings found! If you want to run `cargo`,
> start a subshell\n",
> > +            "with `meson devenv`, or point MESON_BUILD_ROOT to the top
> of the build tree."
> > +        ));
> >      }
> >
> > +    println!("cargo:rustc-env=BINDINGS_RS_INC={}", file);
> > +
> >      // Check for available rustc features
> >      if rustc::is_min_version("1.77.0").unwrap_or(false) {
> >          println!("cargo:rustc-cfg=has_offset_of");
> > diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
> > index 6f637af7b1b..e3870e901e3 100644
> > --- a/rust/qemu-api/meson.build
> > +++ b/rust/qemu-api/meson.build
> > @@ -9,6 +9,7 @@ _qemu_api_rs = static_library(
> >    structured_sources(
> >      [
> >        'src/lib.rs',
> > +      'src/bindings.rs',
> >        'src/c_str.rs',
> >        'src/definitions.rs',
> >        'src/device_class.rs',
> > diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/
> bindings.rs
> > new file mode 100644
> > index 00000000000..1dac310594d
> > --- /dev/null
> > +++ b/rust/qemu-api/src/bindings.rs
> > @@ -0,0 +1,29 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +#![allow(
> > +    dead_code,
> > +    improper_ctypes_definitions,
> > +    improper_ctypes,
> > +    non_camel_case_types,
> > +    non_snake_case,
> > +    non_upper_case_globals,
> > +    unsafe_op_in_unsafe_fn,
> > +    clippy::missing_const_for_fn,
> > +    clippy::too_many_arguments,
> > +    clippy::approx_constant,
> > +    clippy::use_self,
> > +    clippy::useless_transmute,
> > +    clippy::missing_safety_doc
> > +)]
> > +
> > +#[cfg(MESON)]
> > +include!("bindings.rs.inc");
> > +
> > +#[cfg(not(MESON))]
> > +include!(env!("BINDINGS_RS_INC"));
> > +
> > +unsafe impl Send for Property {}
> > +unsafe impl Sync for Property {}
> > +unsafe impl Sync for TypeInfo {}
> > +unsafe impl Sync for VMStateDescription {}
> > +unsafe impl Sync for VMStateField {}
> > +unsafe impl Sync for VMStateInfo {}
> > diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
> > index aa8d16ec94b..440aff3817d 100644
> > --- a/rust/qemu-api/src/lib.rs
> > +++ b/rust/qemu-api/src/lib.rs
> > @@ -4,31 +4,9 @@
> >
> >  #![cfg_attr(not(MESON), doc = include_str!("../README.md"))]
> >
> > -#[allow(
> > -    dead_code,
> > -    improper_ctypes_definitions,
> > -    improper_ctypes,
> > -    non_camel_case_types,
> > -    non_snake_case,
> > -    non_upper_case_globals,
> > -    unsafe_op_in_unsafe_fn,
> > -    clippy::missing_const_for_fn,
> > -    clippy::too_many_arguments,
> > -    clippy::approx_constant,
> > -    clippy::use_self,
> > -    clippy::useless_transmute,
> > -    clippy::missing_safety_doc,
> > -)]
> >  #[rustfmt::skip]
> >  pub mod bindings;
> >
> > -unsafe impl Send for bindings::Property {}
> > -unsafe impl Sync for bindings::Property {}
> > -unsafe impl Sync for bindings::TypeInfo {}
> > -unsafe impl Sync for bindings::VMStateDescription {}
> > -unsafe impl Sync for bindings::VMStateField {}
> > -unsafe impl Sync for bindings::VMStateInfo {}
> > -
> >  pub mod c_str;
> >  pub mod definitions;
> >  pub mod device_class;
>
>

[-- Attachment #2: Type: text/html, Size: 7421 bytes --]

Re: [RFC PATCH 05/11] rust: cargo: store desired warning levels in workspace Cargo.toml

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il mar 12 nov 2024, 04:17 Junjie Mao <junjie.mao@hotmail.com> ha scritto:
>
>  Making a universal unexpected_cfgs apply to the whole workspace may lead
>  to a lengthy cfg list when more devices in Rust are added. As cargo does
>  not allow overriding workspace-defined lints once inherited, I think it
>  better to keep unexpected_cfgs crate-specific.
>
> Is it possible? I thought you cannot override at a finer granularity once you have a "workspace = true" line.

No, such overriding is not supported by cargo today. I'm thinking about
removing the workspace.lints.rust section, but ...

>
> Based on the experience with C we shouldn't have many cfgs, but if it's possible I would definitely make unexpected_cfgs specific to qemu-api.

... a quick grep finds 33 different CONFIG_* being used in C under
hw/. In that case one universal list of expected cfgs does not look like
a problem. Thanks for pointing this out.

--
Best Regards
Junjie Mao

>
> Paolo
>

Re: [RFC PATCH 01/11] rust: qemu_api: do not disable lints outside bindgen-generated code

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il mar 12 nov 2024, 03:47 Junjie Mao <junjie.mao@hotmail.com> ha scritto:
>
>  I agree that storing generated stuff in the source directory should not
>  be encouraged.
>
>  Just want to mention that such changes can lead to trouble to
>  rust-analyzer. Today there are two ways to inform rust-analyzer of the
>  project structure:
>
>    1. Use rust/Cargo.toml. In this case we'll hit an issue in
>    rust-analyzer [1] that prevents it from including sources outside the
>    crate directory. Thus, definitions in the bindgen-generated code
>    cannot be found.
>
>    2. Use the meson-generated rust-project.json. Due to the use of
>    structured_sources(), that json file refers to the copied sources of
>    qemu-api in the build directory. Rust-analyzer can find every symbol
>    in the qemu-api crate but will jump to those copied files, making it a
>    bit more annoying when developing the crate.
>
> Would it help to move the bindgen-generated code to a completely separate crate (e.g. qemu-api-sys), and avoid structured_sources for qemu-api? It might even help build times.

I just noticed that rust-analyzer is able to include files under
OUT_DIR. With the following changes, rust-analyzer under meson devenv
works nicely:

  1. Rust-analyzer refers to rust/qemu-api/src/* unless the definition
  is in bindings.inc.rs.

  2. No manual copy / symbolic link required, neither bindings.inc.rs
  nor rust-project.json.

The bindgen-generated file is renamed to bindings.inc.rs only because
rust-analyzer seems to refuse including a file without the .rs suffix.

That's at the cost of another file copy, though.

--- diff starts here ---

diff --git a/meson.build b/meson.build
index 1239f5c48c..8cea09ffe1 100644
--- a/meson.build
+++ b/meson.build
@@ -4,6 +4,7 @@ project('qemu', ['c'], meson_version: '>=1.5.0',
         version: files('VERSION'))

 meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() })
+meson.add_devenv({ 'CARGO_TARGET_DIR' : meson.project_build_root() / 'cargo' })

 add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true)
 add_test_setup('slow', exclude_suites: ['thorough'], env: ['G_TEST_SLOW=1', 'SPEED=slow'])
@@ -4083,7 +4084,7 @@ if have_rust
   bindings_rs = rust.bindgen(
     input: 'rust/wrapper.h',
     dependencies: common_ss.all_dependencies(),
-    output: 'bindings.rs.inc',
+    output: 'bindings.inc.rs',
     include_directories: include_directories('.', 'include'),
     bindgen_version: ['>=0.60.0'],
     args: bindgen_args,
diff --git a/rust/qemu-api/build.rs b/rust/qemu-api/build.rs
index d7b6d76828..1de86c721b 100644
--- a/rust/qemu-api/build.rs
+++ b/rust/qemu-api/build.rs
@@ -2,17 +2,17 @@
 // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
 // SPDX-License-Identifier: GPL-2.0-or-later

-use std::{env, path::Path};
+use std::{env, fs::copy, io::Result, path::Path};

 use version_check as rustc;

-fn main() {
+fn main() -> Result<()> {
     // Placing bindings.rs.inc in the source directory is supported
     // but not documented or encouraged.
     let path = env::var("MESON_BUILD_ROOT")
         .unwrap_or_else(|_| format!("{}/src", env!("CARGO_MANIFEST_DIR")));

-    let file = format!("{}/bindings.rs.inc", path);
+    let file = format!("{}/bindings.inc.rs", path);
     if !Path::new(&file).exists() {
         panic!(concat!(
             "\n",
@@ -24,7 +24,9 @@ fn main() {
         ));
     }

-    println!("cargo:rustc-env=BINDINGS_RS_INC={}", file);
+    let out_dir = env::var("OUT_DIR").unwrap();
+    let dest_path = format!("{}/bindings.inc.rs", out_dir);
+    copy(&file, Path::new(&dest_path))?;

     // Check for available rustc features
     if rustc::is_min_version("1.77.0").unwrap_or(false) {
@@ -32,4 +34,6 @@ fn main() {
     }

     println!("cargo:rerun-if-changed=build.rs");
+
+    Ok(())
 }
diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
index 972b1f1ee9..8a9b821bb9 100644
--- a/rust/qemu-api/src/bindings.rs
+++ b/rust/qemu-api/src/bindings.rs
@@ -16,10 +16,10 @@
 )]

 #[cfg(MESON)]
-include!("bindings.rs.inc");
+include!("bindings.inc.rs");

 #[cfg(not(MESON))]
-include!(env!("BINDINGS_RS_INC"));
+include!(concat!(env!("OUT_DIR"), "/bindings.inc.rs"));

 unsafe impl Send for Property {}
 unsafe impl Sync for Property {}

--
Best Regards
Junjie Mao

>
> Paolo
>
>  We can perhaps leave it as a separate topic for another series.
>
>  [1] https://github.com/rust-lang/rust-analyzer/issues/17040
>

Re: [RFC PATCH 01/11] rust: qemu_api: do not disable lints outside bindgen-generated code

[Paolo Bonzini]

On 11/12/24 11:10, Junjie Mao wrote:
> diff --git a/meson.build b/meson.build
> index 1239f5c48c..8cea09ffe1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -4,6 +4,7 @@ project('qemu', ['c'], meson_version: '>=1.5.0',
>           version: files('VERSION'))
> 
>   meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() })
> +meson.add_devenv({ 'CARGO_TARGET_DIR' : meson.project_build_root() / 'cargo' })

I think I'd rather avoid using CARGO_TARGET_DIR, in case someone forgets 
he/she is in the devenv.

>   add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true)
>   add_test_setup('slow', exclude_suites: ['thorough'], env: ['G_TEST_SLOW=1', 'SPEED=slow'])
> @@ -4083,7 +4084,7 @@ if have_rust
>     bindings_rs = rust.bindgen(
>       input: 'rust/wrapper.h',
>       dependencies: common_ss.all_dependencies(),
> -    output: 'bindings.rs.inc',
> +    output: 'bindings.inc.rs',

Needs a comment, but I guess it's okay.

> -    println!("cargo:rustc-env=BINDINGS_RS_INC={}", file);
> +    let out_dir = env::var("OUT_DIR").unwrap();
> +    let dest_path = format!("{}/bindings.inc.rs", out_dir);
> +    copy(&file, Path::new(&dest_path))?;

A symbolic link seems to work too.  Thanks for the tip!

Paolo

Re: [RFC PATCH 01/11] rust: qemu_api: do not disable lints outside bindgen-generated code

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 11/12/24 11:10, Junjie Mao wrote:
>> diff --git a/meson.build b/meson.build
>> index 1239f5c48c..8cea09ffe1 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -4,6 +4,7 @@ project('qemu', ['c'], meson_version: '>=1.5.0',
>>           version: files('VERSION'))
>>   meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() })
>> +meson.add_devenv({ 'CARGO_TARGET_DIR' : meson.project_build_root() / 'cargo' })
>
> I think I'd rather avoid using CARGO_TARGET_DIR, in case someone forgets he/she
> is in the devenv.
>

I should have dropped this line earlier. It was from my first attempt
where I wanted to put the generated bindings directly to cargo OUT_DIR
(so that file copy can be avoided). That didn't work because OUT_DIR
contains hashes that are hard to predict.

>>   add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true)
>>   add_test_setup('slow', exclude_suites: ['thorough'], env: ['G_TEST_SLOW=1', 'SPEED=slow'])
>> @@ -4083,7 +4084,7 @@ if have_rust
>>     bindings_rs = rust.bindgen(
>>       input: 'rust/wrapper.h',
>>       dependencies: common_ss.all_dependencies(),
>> -    output: 'bindings.rs.inc',
>> +    output: 'bindings.inc.rs',
>
> Needs a comment, but I guess it's okay.
>
>> -    println!("cargo:rustc-env=BINDINGS_RS_INC={}", file);
>> +    let out_dir = env::var("OUT_DIR").unwrap();
>> +    let dest_path = format!("{}/bindings.inc.rs", out_dir);
>> +    copy(&file, Path::new(&dest_path))?;
>
> A symbolic link seems to work too.  Thanks for the tip!

Linux-based hosts should be fine. Should we test if symlinks also work
on W32/W64 systems?

>
> Paolo

--
Best Regards
Junjie Mao

Re: [RFC PATCH 07/11] rust: fix a couple style issues from clippy

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> These are reported as clippy::semicolon_inside_block and clippy::as_ptr_cast_mut.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Junjie Mao <junjie.mao@hotmail.com>

One minor question below.

> ---
>  rust/hw/char/pl011/src/device.rs     | 6 ++++--
>  rust/hw/char/pl011/src/memory_ops.rs | 4 +++-
>  rust/qemu-api/tests/tests.rs         | 2 +-
>  3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
> index 2a85960b81f..7f40e7f71fa 100644
> --- a/rust/hw/char/pl011/src/device.rs
> +++ b/rust/hw/char/pl011/src/device.rs
> @@ -499,7 +499,9 @@ pub fn update(&self) {
>          let flags = self.int_level & self.int_enabled;
>          for (irq, i) in self.interrupts.iter().zip(IRQMASK) {
>              // SAFETY: self.interrupts have been initialized in init().
> -            unsafe { qemu_set_irq(*irq, i32::from(flags & i != 0)) };
> +            unsafe {
> +                qemu_set_irq(*irq, i32::from(flags & i != 0));
> +            }

clippy::semicolon_inside_block can be configured not to lint single-line
blocks [1]. I don't have any preference among different styles. Just
want to ask how others think.

[1] https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_inside_block

--
Best Regards
Junjie Mao

Re: [RFC PATCH 08/11] rust: build: establish a baseline of lints across all crates

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Many lints that default to allow can be helpful in detecting bugs or
> keeping the code style homogeneous.  Add them liberally, though perhaps
> not as liberally as in hw/char/pl011/src/lib.rs.  In particular, enabling
> entire groups can be problematic because of bitrot when new links are
> added in the future.
>
> For Clippy, this is actually a feature that is only present in Cargo
> 1.74.0 but, since we are not using Cargo to *build* QEMU, only developers
> will need a new-enough cargo and only to run tools such as clippy.
> The requirement does not apply to distros that are building QEMU.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/Cargo.toml               | 66 +++++++++++++++++++++++++++++++++++
>  rust/hw/char/pl011/src/lib.rs | 18 ++--------
>  rust/qemu-api/src/bindings.rs |  6 ++--
>  3 files changed, 71 insertions(+), 19 deletions(-)
>
> diff --git a/rust/Cargo.toml b/rust/Cargo.toml
> index 1ff8f5c2781..43cca33a8d8 100644
> --- a/rust/Cargo.toml
> +++ b/rust/Cargo.toml
> @@ -19,3 +19,69 @@ unknown_lints = "allow"
>
>  # Prohibit code that is forbidden in Rust 2024
>  unsafe_op_in_unsafe_fn = "deny"
> +
[snip]
> +
> +# nice to have, but cannot be enabled yet
> +#wildcard_imports = "deny"
> +
> +# these may have false positives
> +#option_if_let_else = "deny"
> +cognitive_complexity = "deny"

Just to confirm, CC <= 25 is to be enforced for all methods, right?

--
Best Regards
Junjie Mao

Re: [RFC PATCH 09/11] rust: build: add "make clippy", "make rustfmt"

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Abstract common invocations of "cargo", that do not require copying
> the generated bindgen file or setting up MESON_BUILD_ROOT.
>
> In the future these could also do completely without cargo and invoke
> the underlying programs directly.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Junjie Mao <junjie.mao@hotmail.com>

--
Best Regards
Junjie Mao

Re: [RFC PATCH 10/11] rust: fix doc test syntax

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Allow "cargo test --doc" to pass.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Junjie Mao <junjie.mao@hotmail.com>

--
Best Regards
Junjie Mao

Re: [RFC PATCH 08/11] rust: build: establish a baseline of lints across all crates

[Paolo Bonzini]

On 11/13/24 08:14, Junjie Mao wrote:
> 
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> Many lints that default to allow can be helpful in detecting bugs or
>> keeping the code style homogeneous.  Add them liberally, though perhaps
>> not as liberally as in hw/char/pl011/src/lib.rs.  In particular, enabling
>> entire groups can be problematic because of bitrot when new links are
>> added in the future.
>>
>> For Clippy, this is actually a feature that is only present in Cargo
>> 1.74.0 but, since we are not using Cargo to *build* QEMU, only developers
>> will need a new-enough cargo and only to run tools such as clippy.
>> The requirement does not apply to distros that are building QEMU.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   rust/Cargo.toml               | 66 +++++++++++++++++++++++++++++++++++
>>   rust/hw/char/pl011/src/lib.rs | 18 ++--------
>>   rust/qemu-api/src/bindings.rs |  6 ++--
>>   3 files changed, 71 insertions(+), 19 deletions(-)
>>
>> diff --git a/rust/Cargo.toml b/rust/Cargo.toml
>> index 1ff8f5c2781..43cca33a8d8 100644
>> --- a/rust/Cargo.toml
>> +++ b/rust/Cargo.toml
>> @@ -19,3 +19,69 @@ unknown_lints = "allow"
>>
>>   # Prohibit code that is forbidden in Rust 2024
>>   unsafe_op_in_unsafe_fn = "deny"
>> +
> [snip]
>> +
>> +# nice to have, but cannot be enabled yet
>> +#wildcard_imports = "deny"
>> +
>> +# these may have false positives
>> +#option_if_let_else = "deny"
>> +cognitive_complexity = "deny"
> 
> Just to confirm, CC <= 25 is to be enforced for all methods, right?

I wanted an opinion on that.  option_if_let_else has been more of a pain 
than a benefit, sometimes it suggests code that is worse or does not 
even compile.

So far I've never had any cognitive_complexity error show up, but pl011 
used it so I have kept it in Cargo.toml.  If we start having too many 
#[allow()] for cognitive_complexity we can remove it; for many of the 
others, instead, we might even change deny to forbid.

Paolo

Re: [RFC PATCH 08/11] rust: build: establish a baseline of lints across all crates

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 11/13/24 08:14, Junjie Mao wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>
>>> Many lints that default to allow can be helpful in detecting bugs or
>>> keeping the code style homogeneous.  Add them liberally, though perhaps
>>> not as liberally as in hw/char/pl011/src/lib.rs.  In particular, enabling
>>> entire groups can be problematic because of bitrot when new links are
>>> added in the future.
>>>
>>> For Clippy, this is actually a feature that is only present in Cargo
>>> 1.74.0 but, since we are not using Cargo to *build* QEMU, only developers
>>> will need a new-enough cargo and only to run tools such as clippy.
>>> The requirement does not apply to distros that are building QEMU.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>   rust/Cargo.toml               | 66 +++++++++++++++++++++++++++++++++++
>>>   rust/hw/char/pl011/src/lib.rs | 18 ++--------
>>>   rust/qemu-api/src/bindings.rs |  6 ++--
>>>   3 files changed, 71 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/rust/Cargo.toml b/rust/Cargo.toml
>>> index 1ff8f5c2781..43cca33a8d8 100644
>>> --- a/rust/Cargo.toml
>>> +++ b/rust/Cargo.toml
>>> @@ -19,3 +19,69 @@ unknown_lints = "allow"
>>>
>>>   # Prohibit code that is forbidden in Rust 2024
>>>   unsafe_op_in_unsafe_fn = "deny"
>>> +
>> [snip]
>>> +
>>> +# nice to have, but cannot be enabled yet
>>> +#wildcard_imports = "deny"
>>> +
>>> +# these may have false positives
>>> +#option_if_let_else = "deny"
>>> +cognitive_complexity = "deny"
>> Just to confirm, CC <= 25 is to be enforced for all methods, right?
>
> I wanted an opinion on that.  option_if_let_else has been more of a pain than a
> benefit, sometimes it suggests code that is worse or does not even compile.
>
> So far I've never had any cognitive_complexity error show up, but pl011 used it
> so I have kept it in Cargo.toml.  If we start having too many #[allow()] for
> cognitive_complexity we can remove it; for many of the others, instead, we might
> even change deny to forbid.

Agree. The most common case I have seen with a high CC is a long
switch/match statement, which should not be too many. For the time being
it should be a useful hint for complexity control.

Reviewed-by: Junjie Mao <junjie.mao@hotmail.com>

>
> Paolo

--
Best Regards
Junjie Mao

Re: [RFC PATCH 01/11] rust: qemu_api: do not disable lints outside bindgen-generated code

[Paolo Bonzini]

On Wed, Nov 13, 2024 at 7:52 AM Junjie Mao <junjie.mao@hotmail.com> wrote:
> > A symbolic link seems to work too.  Thanks for the tip!
>
> Linux-based hosts should be fine. Should we test if symlinks also work
> on W32/W64 systems?

We already use symlinks for scripts/symlink-install-tree.py, fortunately.

Paolo

Re: [RFC PATCH 00/11] rust: improved integration with cargo

[Alex Bennée]

Paolo Bonzini <pbonzini@redhat.com> writes:

> While we're not sure where we'll be going in the future, for now
> using cargo remains an important part of developing QEMU Rust code.
> This is because cargo is the easiest way to run clippy, rustfmt,
> rustdoc.  Cargo also allows working with doc tests, though there are
> pretty much none yet, and provides tools such as "cargo expand".
>
> This series aims at improving the integration with cargo and
> cargo-based tooling.
>
> First, while it is currently possible to run cargo on the rust/ directory,
> but it has the issue that the bindings.rs must be placed by hand in
> the build directory.  Therefore, this series starts by allowing
> cargo to "just work" when run in a "meson devenv" environment:
>
>     meson devenv -w ../rust cargo clippy --tests
>     meson devenv -w ../rust cargo fmt

Is this meant to be the rust source root, or the root of the rust
builddir:

$ meson devenv ../../rust

ERROR: Build data file './meson-private/build.dat' references functions or classes that don't exist. This probably means that it was generated with an old version of meson. Try running from the source directory meson setup . --wipe
🕙13:05:22 alex@draig:qemu.git/builds/rust  on  review/rust-cargo-rfc [$!?] [🔴 ERROR] 
$ meson devenv rust

ERROR: Build data file './meson-private/build.dat' references functions or classes that don't exist. This probably means that it was generated with an old version of meson. Try running from the source directory meson setup . --wipe
🕙13:05:53 alex@draig:qemu.git/builds/rust  on  review/rust-cargo-rfc [$!?] [🔴 ERROR] 

>
> If you are going to use cargo repeatedly, invoking just 'meson devenv'
> will put you in a shell where commands like 'cargo clippy' just work.
> For simplicity, I am also adding two targets 'make clippy' and 'make
> rustfmt'.
>
> Secondly, one problem with mixing Cargo and meson is having to redo the
> configuration of "lints" in both sides.  This series standardizes
> on using Cargo.toml to configure the build, and bringing the flags
> over to build.ninja with extensions to the existing rustc_args.py script.
> I admit that these additions to the script are pretty large and therefore
> I'm open to scrapping the idea.  I tried to organize the changes so that
> the changes are split over multiple patches.
>
> Finally, this series adds a CI job that runs rustfmt, clippy, and
> rustdoc, including running doctests.
>
> Please send comments!
>
> Paolo
>
> Paolo Bonzini (11):
>   rust: qemu_api: do not disable lints outside bindgen-generated code
>   rust: build: move rustc_args.py invocation to individual crates
>   rust: build: restrict --cfg generation to only required symbols
>   rust: build: generate warning flags from Cargo.toml
>   rust: cargo: store desired warning levels in workspace Cargo.toml
>   rust: build: move strict lints handling to rustc_args.py
>   rust: fix a couple style issues from clippy
>   rust: build: establish a baseline of lints across all crates
>   rust: build: add "make clippy", "make rustfmt"
>   rust: fix doc test syntax
>   rust: ci: add job that runs Rust tools
>
>  meson.build                                   |  56 +++---
>  .gitlab-ci.d/buildtest-template.yml           |  14 ++
>  .gitlab-ci.d/buildtest.yml                    |  14 ++
>  rust/Cargo.toml                               |  80 ++++++++
>  rust/hw/char/pl011/Cargo.toml                 |   3 +
>  rust/hw/char/pl011/src/device.rs              |   6 +-
>  rust/hw/char/pl011/src/lib.rs                 |  18 +-
>  rust/hw/char/pl011/src/memory_ops.rs          |   4 +-
>  rust/meson.build                              |  14 ++
>  rust/qemu-api-macros/Cargo.toml               |   3 +
>  rust/qemu-api/.gitignore                      |   2 +-
>  rust/qemu-api/Cargo.toml                      |   5 +-
>  rust/qemu-api/build.rs                        |  24 ++-
>  rust/qemu-api/meson.build                     |   5 +
>  rust/qemu-api/src/bindings.rs                 |  29 +++
>  rust/qemu-api/src/lib.rs                      |  22 ---
>  rust/qemu-api/src/zeroable.rs                 |   6 +-
>  rust/qemu-api/tests/tests.rs                  |   2 +-
>  scripts/rust/rustc_args.py                    | 178 ++++++++++++++++--
>  .../dockerfiles/fedora-rust-nightly.docker    |   4 +
>  tests/lcitool/refresh                         |   4 +
>  21 files changed, 391 insertions(+), 102 deletions(-)
>  create mode 100644 rust/qemu-api/src/bindings.rs

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC PATCH 00/11] rust: improved integration with cargo

[Paolo Bonzini]

On Thu, Nov 14, 2024 at 2:07 PM Alex Bennée <alex.bennee@linaro.org> wrote:
> > First, while it is currently possible to run cargo on the rust/ directory,
> > it has the issue that the bindings.rs must be placed by hand in
> > the build directory.  Therefore, this series starts by allowing
> > cargo to "just work" when run in a "meson devenv" environment:
> >
> >     meson devenv -w ../rust cargo clippy --tests
> >     meson devenv -w ../rust cargo fmt
>
> Is this meant to be the rust source root, or the root of the rust
> builddir:
>
> $ meson devenv ../../rust

rust/ in the source directory.  You also need to run "meson devenv"
from the root of the build directory.

In practice you can just use "make clippy" or similar.

> ERROR: Build data file './meson-private/build.dat' references functions or classes that don't exist. This probably means that it was generated with an old version of meson. Try running from the source directory meson setup . --wipe
> 🕙13:05:22 alex@draig:qemu.git/builds/rust  on  review/rust-cargo-rfc [$!?] [🔴 ERROR]
> $ meson devenv rust

Your meson-private/ directory is stale.  Any "make" or "ninja" invocation will
fix it.

Paolo

Re: [RFC PATCH 00/11] rust: improved integration with cargo

[Alex Bennée]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On Thu, Nov 14, 2024 at 2:07 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>> > First, while it is currently possible to run cargo on the rust/ directory,
>> > it has the issue that the bindings.rs must be placed by hand in
>> > the build directory.  Therefore, this series starts by allowing
>> > cargo to "just work" when run in a "meson devenv" environment:
>> >
>> >     meson devenv -w ../rust cargo clippy --tests
>> >     meson devenv -w ../rust cargo fmt
>>
>> Is this meant to be the rust source root, or the root of the rust
>> builddir:
>>
>> $ meson devenv ../../rust
>
> rust/ in the source directory.  You also need to run "meson devenv"
> from the root of the build directory.
>
> In practice you can just use "make clippy" or similar.

make clippy certainly works

>> ERROR: Build data file './meson-private/build.dat' references
>> functions or classes that don't exist. This probably means that it
>> was generated with an old version of meson. Try running from the
>> source directory meson setup . --wipe
>> 🕙13:05:22 alex@draig:qemu.git/builds/rust  on  review/rust-cargo-rfc [$!?] [🔴 ERROR]
>> $ meson devenv rust
>
> Your meson-private/ directory is stale.  Any "make" or "ninja" invocation will
> fix it.

✗  make -j30
[1/53] Generating tests/include/QAPI test (include) with a custom command
[2/21] Generating rust_arm_softmmu.rs with a custom command (wrapped by meson to capture output)
[3/21] Generating rust_aarch64_softmmu.rs with a custom command (wrapped by meson to capture output)
[4/21] Generating qemu-version.h with a custom command (wrapped by meson to capture output)
🕙15:18:58 alex@draig:qemu.git/builds/rust  on  review/rust-cargo-rfc [$!?] 
➜  meson devenv ../../rust

ERROR: Build data file './meson-private/build.dat' references functions or classes that don't exist. This probably means that it was generated with an old version of meson. Try running from the source directory meson setup . --wipe

I also tried a wipe and re-configure but the same thing.

➜  ls -la meson-private/
total 24768
drwxr-xr-x  4 alex alex     4096 Nov 14 15:20 ./
drwxr-xr-x 77 alex alex     4096 Nov 14 15:21 ../
-rw-r--r--  1 alex alex     7569 Nov 14 15:20 aarch64-softmmu-config-devices.mak.d
-rw-r--r--  1 alex alex     7084 Nov 14 15:20 arm-softmmu-config-devices.mak.d
-rw-r--r--  1 alex alex  1877658 Nov 14 15:20 build.dat
-rw-r--r--  1 alex alex    27208 Nov 14 15:20 cleantrees.dat
drwxr-xr-x  3 alex alex     4096 Nov 14 15:20 __CMake_compiler_info__/
drwxr-xr-x  3 alex alex     4096 Nov 14 15:20 cmake_libcbor/
-rw-r--r--  1 alex alex      162 Nov 14 15:20 cmd_line.txt
-rw-r--r--  1 alex alex   333651 Nov 14 15:20 coredata.dat
-rw-r--r--  1 alex alex    24920 Nov 14 15:20 install.dat
-rw-r--r--  1 alex alex 19049522 Nov 14 15:20 libsanity.a
-rw-r--r--  1 alex alex     1748 Nov 14 15:20 meson_benchmark_setup.dat
-rw-r--r--  1 alex alex        0 Nov 14 15:20 meson.lock
-rw-r--r--  1 alex alex   140166 Nov 14 15:20 meson_test_setup.dat
-rwxr-xr-x  1 alex alex  3826912 Nov 14 15:20 rusttest*
-rw-r--r--  1 alex alex       46 Nov 14 15:20 sanitycheckc.c
-rwxr-xr-x  1 alex alex    15840 Nov 14 15:20 sanitycheckc.exe*
-rw-r--r--  1 alex alex       30 Nov 14 15:20 sanity.rs
🕙15:21:27 alex@draig:qemu.git/builds/rust  on  review/rust-cargo-rfc [$!?] 
➜  meson devenv ../../rust

ERROR: Build data file './meson-private/build.dat' references functions or classes that don't exist. This probably means that it was generated with an old version of meson. Try running from the source directory meson setup . --wipe
🕙15:21:43 alex@draig:qemu.git/builds/rust  on  review/rust-cargo-rfc [$!?] [🔴 ERROR] 
✗  

>
> Paolo

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC PATCH 00/11] rust: improved integration with cargo

[Paolo Bonzini]

On 11/14/24 16:22, Alex Bennée wrote:
> ERROR: Build data file './meson-private/build.dat' references functions or classes that don't exist. This probably means that it was generated with an old version of meson. Try running from the source directory meson setup . --wipe
> 
> I also tried a wipe and re-configure but the same thing.

Ah, nevermind - you must have an older meson installation in /usr.  You 
have to do pyvenv/bin/meson to pick the right one.  I'll adjust the docs.

Paolo

Re: [RFC PATCH 00/11] rust: improved integration with cargo

[Alex Bennée]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 11/14/24 16:22, Alex Bennée wrote:
>> ERROR: Build data file './meson-private/build.dat' references
>> functions or classes that don't exist. This probably means that it
>> was generated with an old version of meson. Try running from the
>> source directory meson setup . --wipe
>> I also tried a wipe and re-configure but the same thing.
>
> Ah, nevermind - you must have an older meson installation in /usr.
> You have to do pyvenv/bin/meson to pick the right one.  I'll adjust
> the docs.

Hmm,

✗  ./pyvenv/bin/meson devenv ../../rust
Traceback (most recent call last):
  File "/home/alex/lsrc/qemu.git/builds/rust/pyvenv/lib/python3.11/site-packages/mesonbuild/mesonmain.py", line 188, in run
    return options.run_func(options)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alex/lsrc/qemu.git/builds/rust/pyvenv/lib/python3.11/site-packages/mesonbuild/mdevenv.py", line 228, in run
    return subprocess.call(args, close_fds=False,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 389, in call
    with Popen(*popenargs, **kwargs) as p:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 1024, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.11/subprocess.py", line 1901, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
PermissionError: [Errno 13] Permission denied: '../../rust'

ERROR: Unhandled python OSError. This is probably not a Meson bug, but an issue with your build environment.
🕙17:26:59 alex@draig:qemu.git/builds/rust  on  review/rust-cargo-rfc [$!?] [🔴 13] 
✗  ls -l ../../rust/
total 40
-rw-r--r-- 1 alex alex 3237 Nov 12 21:01 Cargo.lock
-rw-r--r-- 1 alex alex 2426 Nov 14 12:15 Cargo.toml
drwxr-xr-x 3 alex alex 4096 Nov 11 23:19 hw/
-rw-r--r-- 1 alex alex   18 Nov 11 23:19 Kconfig
-rw-r--r-- 1 alex alex  437 Nov 14 12:15 meson.build
drwxr-xr-x 4 alex alex 4096 Nov 14 12:15 qemu-api/
drwxr-xr-x 3 alex alex 4096 Nov 14 12:15 qemu-api-macros/
-rw-r--r-- 1 alex alex  191 Nov 11 23:19 rustfmt.toml
drwxr-xr-x 4 alex alex 4096 Nov 14 15:18 target/
-rw-r--r-- 1 alex alex 2262 Nov 12 21:01 wrapper.h

>
> Paolo

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC PATCH 00/11] rust: improved integration with cargo

[Paolo Bonzini]

On 11/14/24 18:27, Alex Bennée wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 11/14/24 16:22, Alex Bennée wrote:
>>> ERROR: Build data file './meson-private/build.dat' references
>>> functions or classes that don't exist. This probably means that it
>>> was generated with an old version of meson. Try running from the
>>> source directory meson setup . --wipe
>>> I also tried a wipe and re-configure but the same thing.
>>
>> Ah, nevermind - you must have an older meson installation in /usr.
>> You have to do pyvenv/bin/meson to pick the right one.  I'll adjust
>> the docs.
> 
> Hmm,
> 
> ✗  ./pyvenv/bin/meson devenv ../../rust
> PermissionError: [Errno 13] Permission denied: '../../rust'

You're confusing two things:

1) to start a shell

pyvenv/bin/meson devenv

2) to run clippy

pyvenv/bin/meson devenv -w ../../rust cargo clippy --tests

Note the -w.  Since the latter is typically covered by make, the common 
one will be the former.

Paolo

Re: [RFC PATCH 00/11] rust: improved integration with cargo

[Alex Bennée]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 11/14/24 18:27, Alex Bennée wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> On 11/14/24 16:22, Alex Bennée wrote:
>>>> ERROR: Build data file './meson-private/build.dat' references
>>>> functions or classes that don't exist. This probably means that it
>>>> was generated with an old version of meson. Try running from the
>>>> source directory meson setup . --wipe
>>>> I also tried a wipe and re-configure but the same thing.
>>>
>>> Ah, nevermind - you must have an older meson installation in /usr.
>>> You have to do pyvenv/bin/meson to pick the right one.  I'll adjust
>>> the docs.
>> Hmm,
>> ✗  ./pyvenv/bin/meson devenv ../../rust
>> PermissionError: [Errno 13] Permission denied: '../../rust'
>
> You're confusing two things:
>
> 1) to start a shell
>
> pyvenv/bin/meson devenv
>
> 2) to run clippy
>
> pyvenv/bin/meson devenv -w ../../rust cargo clippy --tests
>
> Note the -w.  Since the latter is typically covered by make, the
> common one will be the former.


Ahh right - I misunderstood, got it now.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro