Paolo Bonzini <pbonz...@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 <pbonz...@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.pitsidiana...@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;
