Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency

[Paolo Bonzini]

On Thu, Jul 4, 2024 at 2:16 PM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> Add mechanism to generate rust hw targets that depend on a custom
> bindgen target for rust bindings to C.
>
> This way bindings will be created before the rust crate is compiled.
>
> The bindings will end up in BUILDDIR/{target}-generated.rs and have the same 
> name
> as a target:
>
> ninja aarch64-softmmu-generated.rs

Is there anything target-dependent in these files? You can generate it
just once I think.


> +  if with_rust and target_type == 'system'
> +       # FIXME: meson outputs the following warnings, which should be 
> resolved
> +       # before merging:
> +       # > WARNING: Project specifies a minimum meson_version '>=0.63.0' but
> +       # > uses features which were added in newer versions:
> +       # > * 0.64.0: {'fs.copyfile'}

you can use configure_file() instead.

> +       # > * 1.0.0: {'dependencies arg in rust.bindgen', 'module rust as 
> stable module'}

You can use

if meson.version().version_compare('>=1.0.0')
  ...
else
  error('Rust support requires Meson version 1.0.0')
endif

> +      if target in rust_targets
> +        rust_hw = ss.source_set()
> +        foreach t: rust_targets[target]
> +          rust_device_cargo_build = custom_target(t['name'],
> +                                       output: t['output'],
> +                                       depends: [bindings_rs],
> +                                       build_always_stale: true,
> +                                       command: t['command'])

Also "console: true".

> +          rust_dep = declare_dependency(link_args: [
> +                                          '-Wl,--whole-archive',
> +                                          t['output'],
> +                                          '-Wl,--no-whole-archive'
> +                                          ],

This is not portable, but I think you can use just "link_whole:
rust_device_cargo_build".

> +msrv = {
> +  'rustc': '1.77.2',
> +  'cargo': '1.77.2',

I think rustc and cargo are always matching, so no need to check both of them?

> +foreach rust_hw_target, rust_hws: rust_hw_target_list
> +  foreach rust_hw_dev: rust_hws

Finding the available devices should be done using Kconfig symbols,
probably by passing the path to the config-devices.mak file to
build.rs via an environment variable.

> +#include "qemu/osdep.h"
> +#include "qemu/module.h"
> +#include "qemu-io.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/sysbus.h"
> +#include "exec/memory.h"
> +#include "chardev/char-fe.h"
> +#include "hw/clock.h"
> +#include "hw/qdev-clock.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/qdev-properties-system.h"
> +#include "hw/irq.h"
> +#include "qapi/error.h"
> +#include "migration/vmstate.h"
> +#include "chardev/char-serial.h"

I think all of these should be target-independent?

> diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py
> index d2c7265461..e7d9238c16 100644
> --- a/scripts/cargo_wrapper.py
> +++ b/scripts/cargo_wrapper.py
> @@ -111,6 +111,8 @@ def get_cargo_rustc(args: argparse.Namespace) -> 
> tuple[Dict[str, Any], List[str]
>
>      env = os.environ
>      env["CARGO_ENCODED_RUSTFLAGS"] = cfg
> +    env["MESON_BUILD_DIR"] = str(target_dir)
> +    env["MESON_BUILD_ROOT"] = str(args.meson_build_root)
>
>      return (env, cargo_cmd)
>
> @@ -231,19 +233,11 @@ def main() -> None:
>          default=[],
>      )
>      parser.add_argument(
> -        "--meson-build-dir",
> -        metavar="BUILD_DIR",
> -        help="meson.current_build_dir()",
> +        "--meson-build-root",
> +        metavar="BUILD_ROOT",
> +        help="meson.project_build_root(): the root build directory. Example: 
> '/path/to/qemu/build'",
>          type=Path,
> -        dest="meson_build_dir",
> -        required=True,
> -    )
> -    parser.add_argument(
> -        "--meson-source-dir",
> -        metavar="SOURCE_DIR",
> -        help="meson.current_source_dir()",
> -        type=Path,
> -        dest="meson_build_dir",
> +        dest="meson_build_root",
>          required=True,
>      )
>      parser.add_argument(

Please squash in the previous patch.

Paolo