[PATCH 08/13] rust: build integration test for the qemu_api crate

Paolo Bonzini posted 13 patches 4 days, 8 hours ago
There is a newer version of this series
[PATCH 08/13] rust: build integration test for the qemu_api crate
Posted by Paolo Bonzini 4 days, 8 hours ago
Adjust the integration test to compile with a subset of QEMU object
files, and make it actually create an object of the class it defines.

Follow the Rust filesystem conventions, where tests go in tests/ if
they use the library in the same way any other code would.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                  | 10 ++++-
 rust/qemu-api/meson.build    | 20 +++++++--
 rust/qemu-api/src/tests.rs   | 49 ----------------------
 rust/qemu-api/tests/tests.rs | 78 ++++++++++++++++++++++++++++++++++++
 4 files changed, 104 insertions(+), 53 deletions(-)
 delete mode 100644 rust/qemu-api/src/tests.rs
 create mode 100644 rust/qemu-api/tests/tests.rs

diff --git a/meson.build b/meson.build
index 48f82fd4ba6..e50a7edf6ea 100644
--- a/meson.build
+++ b/meson.build
@@ -3324,7 +3324,15 @@ if have_rust and have_system
     capture : true,
     check: true).stdout().strip().split()
   rustc_args += ['-D', 'unsafe_op_in_unsafe_fn']
-  add_project_arguments(rustc_args, native: false, language: 'rust')
+
+  # 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
 
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 42ea815fa5a..d24e0c0725e 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -14,11 +14,25 @@ _qemu_api_rs = static_library(
     '--cfg', 'MESON',
     # '--cfg', 'feature="allocator"',
   ],
-  dependencies: [
-    qemu_api_macros,
-  ],
 )
 
 qemu_api = declare_dependency(
   link_with: _qemu_api_rs,
+  dependencies: qemu_api_macros,
 )
+
+# Rust executable do not support objects, so add an intermediate step.
+rust_qemu_api_objs = static_library(
+    'rust_qemu_api_objs',
+    objects: [libqom.extract_all_objects(recursive: false),
+              libhwcore.extract_all_objects(recursive: false)])
+
+rust.test('rust-qemu-api-integration',
+     static_library(
+         'rust_qemu_api_integration',
+         'tests/tests.rs',
+         override_options: ['rust_std=2021', 'build.rust_std=2021'],
+         link_whole: [rust_qemu_api_objs, libqemuutil]),
+
+     dependencies: [qemu_api, qemu_api_macros],
+     suite: ['unit', 'rust'])
diff --git a/rust/qemu-api/src/tests.rs b/rust/qemu-api/src/tests.rs
deleted file mode 100644
index df54edbd4e2..00000000000
--- a/rust/qemu-api/src/tests.rs
+++ /dev/null
@@ -1,49 +0,0 @@
-// Copyright 2024, Linaro Limited
-// Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
-// SPDX-License-Identifier: GPL-2.0-or-later
-
-use crate::{
-    bindings::*, declare_properties, define_property, device_class_init, vm_state_description,
-};
-
-#[test]
-fn test_device_decl_macros() {
-    // Test that macros can compile.
-    vm_state_description! {
-        VMSTATE,
-        name: c"name",
-        unmigratable: true,
-    }
-
-    #[repr(C)]
-    pub struct DummyState {
-        pub char_backend: CharBackend,
-        pub migrate_clock: bool,
-    }
-
-    declare_properties! {
-        DUMMY_PROPERTIES,
-            define_property!(
-                c"chardev",
-                DummyState,
-                char_backend,
-                unsafe { &qdev_prop_chr },
-                CharBackend
-            ),
-            define_property!(
-                c"migrate-clk",
-                DummyState,
-                migrate_clock,
-                unsafe { &qdev_prop_bool },
-                bool
-            ),
-    }
-
-    device_class_init! {
-        dummy_class_init,
-        props => DUMMY_PROPERTIES,
-        realize_fn => None,
-        reset_fn => None,
-        vmsd => VMSTATE,
-    }
-}
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
new file mode 100644
index 00000000000..57bab62772d
--- /dev/null
+++ b/rust/qemu-api/tests/tests.rs
@@ -0,0 +1,78 @@
+// Copyright 2024, Linaro Limited
+// Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+use core::ffi::CStr;
+
+use qemu_api::{
+    bindings::*, declare_properties, define_property,
+    definitions::Class,
+    definitions::ObjectImpl,
+    device_class_init, vm_state_description,
+};
+
+#[test]
+fn test_device_decl_macros() {
+    // Test that macros can compile.
+    vm_state_description! {
+        VMSTATE,
+        name: c"name",
+        unmigratable: true,
+    }
+
+    #[repr(C)]
+    #[derive(qemu_api_macros::Object)]
+    pub struct DummyState {
+        pub _parent: DeviceState,
+        pub migrate_clock: bool,
+    }
+
+    #[repr(C)]
+    pub struct DummyClass {
+        pub _parent: DeviceClass,
+    }
+
+    declare_properties! {
+        DUMMY_PROPERTIES,
+            define_property!(
+                c"migrate-clk",
+                DummyState,
+                migrate_clock,
+                unsafe { &qdev_prop_bool },
+                bool
+            ),
+    }
+
+    device_class_init! {
+        dummy_class_init,
+        props => DUMMY_PROPERTIES,
+        realize_fn => None,
+        legacy_reset_fn => None,
+        vmsd => VMSTATE,
+    }
+
+    impl ObjectImpl for DummyState {
+        type Class = DummyClass;
+        const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self };
+        const TYPE_NAME: &'static CStr = c"dummy";
+        const PARENT_TYPE_NAME: Option<&'static CStr> = Some(TYPE_DEVICE);
+        const ABSTRACT: bool = false;
+        const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
+        const INSTANCE_POST_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
+        const INSTANCE_FINALIZE: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
+    }
+
+    impl Class for DummyClass {
+        const CLASS_INIT: Option<
+            unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut core::ffi::c_void),
+            > = Some(dummy_class_init);
+        const CLASS_BASE_INIT: Option<
+            unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut core::ffi::c_void),
+            > = None;
+    }
+
+    unsafe {
+        module_call_init(module_init_type::MODULE_INIT_QOM);
+        object_unref(object_new(DummyState::TYPE_NAME.as_ptr()) as *mut _);
+    }
+}
-- 
2.46.2
Re: [PATCH 08/13] rust: build integration test for the qemu_api crate
Posted by Junjie Mao 1 day, 12 hours ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> Adjust the integration test to compile with a subset of QEMU object
> files, and make it actually create an object of the class it defines.
>
> Follow the Rust filesystem conventions, where tests go in tests/ if
> they use the library in the same way any other code would.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build                  | 10 ++++-
>  rust/qemu-api/meson.build    | 20 +++++++--
>  rust/qemu-api/src/tests.rs   | 49 ----------------------
>  rust/qemu-api/tests/tests.rs | 78 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 104 insertions(+), 53 deletions(-)
>  delete mode 100644 rust/qemu-api/src/tests.rs
>  create mode 100644 rust/qemu-api/tests/tests.rs
<snip>
> diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
> index 42ea815fa5a..d24e0c0725e 100644
> --- a/rust/qemu-api/meson.build
> +++ b/rust/qemu-api/meson.build
> @@ -14,11 +14,25 @@ _qemu_api_rs = static_library(
>      '--cfg', 'MESON',
>      # '--cfg', 'feature="allocator"',
>    ],
> -  dependencies: [
> -    qemu_api_macros,
> -  ],
>  )
>
>  qemu_api = declare_dependency(
>    link_with: _qemu_api_rs,
> +  dependencies: qemu_api_macros,
>  )
> +
> +# Rust executable do not support objects, so add an intermediate step.
> +rust_qemu_api_objs = static_library(
> +    'rust_qemu_api_objs',
> +    objects: [libqom.extract_all_objects(recursive: false),
> +              libhwcore.extract_all_objects(recursive: false)])
> +
> +rust.test('rust-qemu-api-integration',
> +     static_library(
> +         'rust_qemu_api_integration',
> +         'tests/tests.rs',
> +         override_options: ['rust_std=2021', 'build.rust_std=2021'],
> +         link_whole: [rust_qemu_api_objs, libqemuutil]),
> +
> +     dependencies: [qemu_api, qemu_api_macros],
> +     suite: ['unit', 'rust'])

I met the following error when trying to build the test:

    [4/8] Compiling Rust source ../rust/qemu-api/tests/tests.rs
    FAILED: rust/qemu-api/librust_qemu_api_integration.rlib
    rustc -C linker=cc -C link-arg=-m64 --color=auto -C debug-assertions=yes -C overflow-checks=no --crate-type rlib -D warnings --edition=2021 -C opt-level=2 -g --cfg CONFIG_ACCEPT4 --cfg CONFIG_AF_VSOCK --cfg CONFIG_ASAN_IFACE_FIBER --cfg CONFIG_ATOMIC64 --cfg CONFIG_ATTR --cfg CONFIG_AUDIO_OSS --cfg CONFIG_AUDIO_PA --cfg CONFIG_AUDIO_SDL --cfg CONFIG_AVX2_OPT --cfg CONFIG_AVX512BW_OPT --cfg CONFIG_BDRV_RO_WHITELIST --cfg CONFIG_BDRV_RW_WHITELIST --cfg CONFIG_BLKZONED --cfg CONFIG_CLOCK_ADJTIME --cfg CONFIG_CLOSE_RANGE --cfg CONFIG_CMPXCHG128 --cfg CONFIG_COROUTINE_POOL --cfg CONFIG_CPUID_H --cfg CONFIG_CURSES --cfg CONFIG_DBUS_DISPLAY --cfg CONFIG_DUP3 --cfg CONFIG_EPOLL --cfg CONFIG_EPOLL_CREATE1 --cfg CONFIG_EVENTFD --cfg CONFIG_FALLOCATE --cfg CONFIG_FALLOCATE_PUNCH_HOLE --cfg CONFIG_FALLOCATE_ZERO_RANGE --cfg CONFIG_FDATASYNC --cfg CONFIG_FDT --cfg CONFIG_FIEMAP --cfg CONFIG_FSFREEZE --cfg CONFIG_FSTRIM --cfg CONFIG_GBM --cfg CONFIG_GETAUXVAL --cfg CONFIG_GETCPU --cfg CONFIG_GETRANDOM --cfg CONFIG_GETTID --cfg CONFIG_GIO --cfg CONFIG_GNUTLS --cfg CONFIG_GNUTLS_CRYPTO --cfg CONFIG_HAVE_RUST --cfg CONFIG_HEXAGON_IDEF_PARSER --cfg CONFIG_INOTIFY --cfg CONFIG_INOTIFY1 --cfg CONFIG_INT128 --cfg CONFIG_INT128_TYPE --cfg CONFIG_IOVEC --cfg CONFIG_L2TPV3 --cfg CONFIG_LIBDW --cfg CONFIG_LIBUDEV --cfg CONFIG_LINUX --cfg CONFIG_LINUX_MAGIC_H --cfg CONFIG_MADVISE --cfg CONFIG_MALLOC_TRIM --cfg CONFIG_MEMALIGN --cfg CONFIG_MEMFD --cfg CONFIG_OPENGL --cfg CONFIG_OPEN_BY_HANDLE --cfg CONFIG_PIXMAN --cfg CONFIG_PLUGIN --cfg CONFIG_PNG --cfg CONFIG_POSIX --cfg CONFIG_POSIX_FALLOCATE --cfg CONFIG_POSIX_MADVISE --cfg CONFIG_POSIX_MEMALIGN --cfg CONFIG_PPOLL --cfg CONFIG_PRCTL_PR_SET_TIMERSLACK --cfg CONFIG_PREADV --cfg CONFIG_PTHREAD_AFFINITY_NP --cfg CONFIG_PTHREAD_SETNAME_NP_W_TID --cfg CONFIG_QOM_CAST_DEBUG --cfg CONFIG_RELOCATABLE --cfg CONFIG_REPLICATION --cfg CONFIG_RTNETLINK --cfg CONFIG_SDL --cfg CONFIG_SECRET_KEYRING --cfg CONFIG_SELINUX --cfg CONFIG_SENDFILE --cfg CONFIG_SETNS --cfg CONFIG_SIGNALFD --cfg CONFIG_SLIRP --cfg CONFIG_SPLICE --cfg CONFIG_STATX --cfg CONFIG_STATX_MNT_ID --cfg CONFIG_SYNCFS --cfg CONFIG_SYNC_FILE_RANGE --cfg CONFIG_SYSMACROS --cfg CONFIG_TASN1 --cfg CONFIG_TCG --cfg CONFIG_TIMERFD --cfg CONFIG_TPM --cfg CONFIG_TRACE_LOG --cfg CONFIG_VALGRIND_H --cfg CONFIG_VALLOC --cfg CONFIG_VDUSE_BLK_EXPORT --cfg CONFIG_VHOST --cfg CONFIG_VHOST_CRYPTO --cfg CONFIG_VHOST_KERNEL --cfg CONFIG_VHOST_NET --cfg CONFIG_VHOST_NET_USER --cfg CONFIG_VHOST_NET_VDPA --cfg CONFIG_VHOST_USER --cfg CONFIG_VHOST_USER_BLK_SERVER --cfg CONFIG_VHOST_VDPA --cfg CONFIG_VIRTFS --cfg CONFIG_VNC --cfg CONFIG_VNC_JPEG --cfg CONFIG_XKBCOMMON --cfg HAVE_BLK_ZONE_REP_CAPACITY --cfg HAVE_BTRFS_H --cfg HAVE_COPY_FILE_RANGE --cfg HAVE_DRM_H --cfg HAVE_FSXATTR --cfg HAVE_GETIFADDRS --cfg HAVE_GLIB_WITH_ALIGNED_ALLOC --cfg HAVE_GLIB_WITH_SLICE_ALLOCATOR --cfg HAVE_HOST_BLOCK_DEVICE --cfg HAVE_IPPROTO_MPTCP --cfg HAVE_MLOCKALL --cfg HAVE_OPENAT2_H --cfg HAVE_OPENPTY --cfg HAVE_PTY_H --cfg HAVE_STRCHRNUL --cfg HAVE_STRUCT_STAT_ST_ATIM --cfg HAVE_SYSTEM_FUNCTION --cfg HAVE_UTMPX -D unsafe_op_in_unsafe_fn -C default-linker-libraries --crate-name rust_qemu_api_integration --emit dep-info=rust/qemu-api/rust_qemu_api_integration.d --emit link=rust/qemu-api/librust_qemu_api_integration.rlib --out-dir rust/qemu-api/librust_qemu_api_integration.rlib.p -C metadata=81e2432@@rust_qemu_api_integration@sta -lstatic:+verbatim=librust_qemu_api_objs.a -lstatic:+verbatim=libqemuutil.a -lstatic:-bundle,+verbatim=libvhost-user-glib.a -lstatic:-bundle,+verbatim=libvhost-user.a -ldylib:+verbatim=libgio-2.0.so -ldylib:+verbatim=libgobject-2.0.so -ldylib:+verbatim=libglib-2.0.so -ldylib:+verbatim=libgmodule-2.0.so -Clink-arg=-pthread -ldylib:+verbatim=libgnutls.so -Clink-arg=-pthread -Clink-arg=-pthread -Clink-arg=-pthread -Lrust/qemu-api -L. -Lsubprojects/libvhost-user -L/usr/lib/x86_64-linux-gnu ../rust/qemu-api/tests/tests.rs
    error[E0433]: failed to resolve: use of undeclared crate or module `qemu_api`
     --> ../rust/qemu-api/tests/tests.rs:7:5
      |
    7 | use qemu_api::{
      |     ^^^^^^^^ use of undeclared crate or module `qemu_api`

    error[E0432]: unresolved import `qemu_api`
     --> ../rust/qemu-api/tests/tests.rs:7:5
      |
    7 | use qemu_api::{
      |     ^^^^^^^^ use of undeclared crate or module `qemu_api`

    error: unused import: `core::ffi::CStr`
     --> ../rust/qemu-api/tests/tests.rs:5:5
      |
    5 | use core::ffi::CStr;
      |     ^^^^^^^^^^^^^^^
      |
      = note: `-D unused-imports` implied by `-D warnings`
      = help: to override `-D warnings` add `#[allow(unused_imports)]`

    error: aborting due to 3 previous errors

    Some errors have detailed explanations: E0432, E0433.
    For more information about an error, try `rustc --explain E0432`.

This is probably because rust.test() is designed to build unit tests
(i.e., tests within the same crate under test), but tests/*.rs are
integration tests which are standalone crates to be linked with the
crate under test.

Rust.test() builds unit tests by "copying the sources and arguments
passed to the original target and adding the --test argument to the
compilation" [1], but that doesn't seem to apply to integration tests.

[1] https://mesonbuild.com/Rust-module.html

I can build and run the test with the following instead:

test('rust-qemu-api-integration',
    executable(
        'rust_qemu_api_integration',
        'tests/tests.rs',
        override_options: ['rust_std=2021', 'build.rust_std=2021'],
        rust_args: [
            '--test',
        ],
        install: false,
        dependencies: [qemu_api, qemu_api_macros],
        link_whole: [rust_qemu_api_objs, libqemuutil]),
    args: [
        '--test',
        '--format', 'pretty',
    ],
    protocol: 'rust',
    suite: ['unit', 'rust'])

--
Best Regards
Junjie Mao
Re: [PATCH 08/13] rust: build integration test for the qemu_api crate
Posted by Paolo Bonzini 1 day, 11 hours ago
On Mon, Oct 21, 2024 at 1:35 PM Junjie Mao <junjie.mao@hotmail.com> wrote:
>
>
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
> > Adjust the integration test to compile with a subset of QEMU object
> > files, and make it actually create an object of the class it defines.
> >
> > Follow the Rust filesystem conventions, where tests go in tests/ if
> > they use the library in the same way any other code would.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  meson.build                  | 10 ++++-
> >  rust/qemu-api/meson.build    | 20 +++++++--
> >  rust/qemu-api/src/tests.rs   | 49 ----------------------
> >  rust/qemu-api/tests/tests.rs | 78 ++++++++++++++++++++++++++++++++++++
> >  4 files changed, 104 insertions(+), 53 deletions(-)
> >  delete mode 100644 rust/qemu-api/src/tests.rs
> >  create mode 100644 rust/qemu-api/tests/tests.rs
> <snip>
> > diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
> > index 42ea815fa5a..d24e0c0725e 100644
> > --- a/rust/qemu-api/meson.build
> > +++ b/rust/qemu-api/meson.build
> > @@ -14,11 +14,25 @@ _qemu_api_rs = static_library(
> >      '--cfg', 'MESON',
> >      # '--cfg', 'feature="allocator"',
> >    ],
> > -  dependencies: [
> > -    qemu_api_macros,
> > -  ],
> >  )
> >
> >  qemu_api = declare_dependency(
> >    link_with: _qemu_api_rs,
> > +  dependencies: qemu_api_macros,
> >  )
> > +
> > +# Rust executable do not support objects, so add an intermediate step.
> > +rust_qemu_api_objs = static_library(
> > +    'rust_qemu_api_objs',
> > +    objects: [libqom.extract_all_objects(recursive: false),
> > +              libhwcore.extract_all_objects(recursive: false)])
> > +
> > +rust.test('rust-qemu-api-integration',
> > +     static_library(
> > +         'rust_qemu_api_integration',
> > +         'tests/tests.rs',
> > +         override_options: ['rust_std=2021', 'build.rust_std=2021'],
> > +         link_whole: [rust_qemu_api_objs, libqemuutil]),
> > +
> > +     dependencies: [qemu_api, qemu_api_macros],
> > +     suite: ['unit', 'rust'])
>
> I met the following error when trying to build the test:

It works for me, but I'll switch to your meson.build code just to be safe.

Paolo

> test('rust-qemu-api-integration',
>     executable(
>         'rust_qemu_api_integration',
>         'tests/tests.rs',
>         override_options: ['rust_std=2021', 'build.rust_std=2021'],
>         rust_args: [
>             '--test',
>         ],
>         install: false,
>         dependencies: [qemu_api, qemu_api_macros],
>         link_whole: [rust_qemu_api_objs, libqemuutil]),
>     args: [
>         '--test',
>         '--format', 'pretty',
>     ],
>     protocol: 'rust',
>     suite: ['unit', 'rust'])
>
> --
> Best Regards
> Junjie Mao
>
Re: [PATCH 08/13] rust: build integration test for the qemu_api crate
Posted by Junjie Mao 1 day, 11 hours ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> On Mon, Oct 21, 2024 at 1:35 PM Junjie Mao <junjie.mao@hotmail.com> wrote:
>>
>>
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>
>> > Adjust the integration test to compile with a subset of QEMU object
>> > files, and make it actually create an object of the class it defines.
>> >
>> > Follow the Rust filesystem conventions, where tests go in tests/ if
>> > they use the library in the same way any other code would.
>> >
>> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> > ---
>> >  meson.build                  | 10 ++++-
>> >  rust/qemu-api/meson.build    | 20 +++++++--
>> >  rust/qemu-api/src/tests.rs   | 49 ----------------------
>> >  rust/qemu-api/tests/tests.rs | 78 ++++++++++++++++++++++++++++++++++++
>> >  4 files changed, 104 insertions(+), 53 deletions(-)
>> >  delete mode 100644 rust/qemu-api/src/tests.rs
>> >  create mode 100644 rust/qemu-api/tests/tests.rs
>> <snip>
>> > diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
>> > index 42ea815fa5a..d24e0c0725e 100644
>> > --- a/rust/qemu-api/meson.build
>> > +++ b/rust/qemu-api/meson.build
>> > @@ -14,11 +14,25 @@ _qemu_api_rs = static_library(
>> >      '--cfg', 'MESON',
>> >      # '--cfg', 'feature="allocator"',
>> >    ],
>> > -  dependencies: [
>> > -    qemu_api_macros,
>> > -  ],
>> >  )
>> >
>> >  qemu_api = declare_dependency(
>> >    link_with: _qemu_api_rs,
>> > +  dependencies: qemu_api_macros,
>> >  )
>> > +
>> > +# Rust executable do not support objects, so add an intermediate step.
>> > +rust_qemu_api_objs = static_library(
>> > +    'rust_qemu_api_objs',
>> > +    objects: [libqom.extract_all_objects(recursive: false),
>> > +              libhwcore.extract_all_objects(recursive: false)])
>> > +
>> > +rust.test('rust-qemu-api-integration',
>> > +     static_library(
>> > +         'rust_qemu_api_integration',
>> > +         'tests/tests.rs',
>> > +         override_options: ['rust_std=2021', 'build.rust_std=2021'],
>> > +         link_whole: [rust_qemu_api_objs, libqemuutil]),
>> > +
>> > +     dependencies: [qemu_api, qemu_api_macros],
>> > +     suite: ['unit', 'rust'])
>>
>> I met the following error when trying to build the test:
>
> It works for me, but I'll switch to your meson.build code just to be safe.

That's odd. What's the version of Rust and meson you have used in your
test? On my side they're 1.82.0 and 1.5.1.

Rust.test() is still preferrable to me for its brevity, as long as it
works.

--
Best Regards
Junjie Mao
Re: [PATCH 08/13] rust: build integration test for the qemu_api crate
Posted by Paolo Bonzini 1 day, 10 hours ago
Il lun 21 ott 2024, 13:55 Junjie Mao <junjie.mao@hotmail.com> ha scritto:

>
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
> > On Mon, Oct 21, 2024 at 1:35 PM Junjie Mao <junjie.mao@hotmail.com>
> wrote:
> >>
> >>
> >> Paolo Bonzini <pbonzini@redhat.com> writes:
> >>
> >> > Adjust the integration test to compile with a subset of QEMU object
> >> > files, and make it actually create an object of the class it defines.
> >> >
> >> > Follow the Rust filesystem conventions, where tests go in tests/ if
> >> > they use the library in the same way any other code would.
> >> >
> >> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> > ---
> >> >  meson.build                  | 10 ++++-
> >> >  rust/qemu-api/meson.build    | 20 +++++++--
> >> >  rust/qemu-api/src/tests.rs   | 49 ----------------------
> >> >  rust/qemu-api/tests/tests.rs | 78
> ++++++++++++++++++++++++++++++++++++
> >> >  4 files changed, 104 insertions(+), 53 deletions(-)
> >> >  delete mode 100644 rust/qemu-api/src/tests.rs
> >> >  create mode 100644 rust/qemu-api/tests/tests.rs
> >> <snip>
> >> > diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
> >> > index 42ea815fa5a..d24e0c0725e 100644
> >> > --- a/rust/qemu-api/meson.build
> >> > +++ b/rust/qemu-api/meson.build
> >> > @@ -14,11 +14,25 @@ _qemu_api_rs = static_library(
> >> >      '--cfg', 'MESON',
> >> >      # '--cfg', 'feature="allocator"',
> >> >    ],
> >> > -  dependencies: [
> >> > -    qemu_api_macros,
> >> > -  ],
> >> >  )
> >> >
> >> >  qemu_api = declare_dependency(
> >> >    link_with: _qemu_api_rs,
> >> > +  dependencies: qemu_api_macros,
> >> >  )
> >> > +
> >> > +# Rust executable do not support objects, so add an intermediate
> step.
> >> > +rust_qemu_api_objs = static_library(
> >> > +    'rust_qemu_api_objs',
> >> > +    objects: [libqom.extract_all_objects(recursive: false),
> >> > +              libhwcore.extract_all_objects(recursive: false)])
> >> > +
> >> > +rust.test('rust-qemu-api-integration',
> >> > +     static_library(
> >> > +         'rust_qemu_api_integration',
> >> > +         'tests/tests.rs',
> >> > +         override_options: ['rust_std=2021', 'build.rust_std=2021'],
> >> > +         link_whole: [rust_qemu_api_objs, libqemuutil]),
> >> > +
> >> > +     dependencies: [qemu_api, qemu_api_macros],
> >> > +     suite: ['unit', 'rust'])
> >>
> >> I met the following error when trying to build the test:
> >
> > It works for me, but I'll switch to your meson.build code just to be
> safe.
>
> That's odd. What's the version of Rust and meson you have used in your
> test? On my side they're 1.82.0 and 1.5.1.
>

Nightly and 1.5.1, but I also tested with 1.63.0.

Anyhow the extra static_library() is not too nice either; so using test()
and executable() is fine by me.

Paolo

Rust.test() is still preferrable to me for its brevity, as long as it
> works.
>
> --
> Best Regards
> Junjie Mao
>
>
Re: [PATCH 08/13] rust: build integration test for the qemu_api crate
Posted by Junjie Mao 1 day, 13 hours ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> Adjust the integration test to compile with a subset of QEMU object
> files, and make it actually create an object of the class it defines.
>
> Follow the Rust filesystem conventions, where tests go in tests/ if
> they use the library in the same way any other code would.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

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

A few minor comments on cosmetic below.

> ---
>  meson.build                  | 10 ++++-
>  rust/qemu-api/meson.build    | 20 +++++++--
>  rust/qemu-api/src/tests.rs   | 49 ----------------------
>  rust/qemu-api/tests/tests.rs | 78 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 104 insertions(+), 53 deletions(-)
>  delete mode 100644 rust/qemu-api/src/tests.rs
>  create mode 100644 rust/qemu-api/tests/tests.rs
>
<snip>
> diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
> new file mode 100644
> index 00000000000..57bab62772d
> --- /dev/null
> +++ b/rust/qemu-api/tests/tests.rs
> @@ -0,0 +1,78 @@
> +// Copyright 2024, Linaro Limited
> +// Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +use core::ffi::CStr;
> +
> +use qemu_api::{
> +    bindings::*, declare_properties, define_property,
> +    definitions::Class,
> +    definitions::ObjectImpl,
> +    device_class_init, vm_state_description,

Cargo fmt (with the current rust/rustfmt.toml) formats those lines in a
different way, and ...

> +};
> +
<snip>
> +    impl ObjectImpl for DummyState {
> +        type Class = DummyClass;
> +        const TYPE_INFO: qemu_api::bindings::TypeInfo = qemu_api::type_info! { Self };
> +        const TYPE_NAME: &'static CStr = c"dummy";
> +        const PARENT_TYPE_NAME: Option<&'static CStr> = Some(TYPE_DEVICE);
> +        const ABSTRACT: bool = false;
> +        const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
> +        const INSTANCE_POST_INIT: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
> +        const INSTANCE_FINALIZE: Option<unsafe extern "C" fn(obj: *mut Object)> = None;
> +    }
> +
> +    impl Class for DummyClass {
> +        const CLASS_INIT: Option<
> +            unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut core::ffi::c_void),
> +            > = Some(dummy_class_init);

... ditto. Shall we tweak the formats here or adjust the rustfmt
settings later?

--
Best Regards
Junjie Mao
Re: [PATCH 08/13] rust: build integration test for the qemu_api crate
Posted by Paolo Bonzini 1 day, 12 hours ago
Il lun 21 ott 2024, 12:34 Junjie Mao <junjie.mao@hotmail.com> ha scritto:

>
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
> > Adjust the integration test to compile with a subset of QEMU object
> > files, and make it actually create an object of the class it defines.
> >
> > Follow the Rust filesystem conventions, where tests go in tests/ if
> > they use the library in the same way any other code would.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Reviewed-by: Junjie Mao <junjie.mao@hotmail.com>
>
> A few minor comments on cosmetic below.
>
> > ---
> >  meson.build                  | 10 ++++-
> >  rust/qemu-api/meson.build    | 20 +++++++--
> >  rust/qemu-api/src/tests.rs   | 49 ----------------------
> >  rust/qemu-api/tests/tests.rs | 78 ++++++++++++++++++++++++++++++++++++
> >  4 files changed, 104 insertions(+), 53 deletions(-)
> >  delete mode 100644 rust/qemu-api/src/tests.rs
> >  create mode 100644 rust/qemu-api/tests/tests.rs
> >
> <snip>
> > diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
> > new file mode 100644
> > index 00000000000..57bab62772d
> > --- /dev/null
> > +++ b/rust/qemu-api/tests/tests.rs
> > @@ -0,0 +1,78 @@
> > +// Copyright 2024, Linaro Limited
> > +// Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +use core::ffi::CStr;
> > +
> > +use qemu_api::{
> > +    bindings::*, declare_properties, define_property,
> > +    definitions::Class,
> > +    definitions::ObjectImpl,
> > +    device_class_init, vm_state_description,
>
> Cargo fmt (with the current rust/rustfmt.toml) formats those lines in a
> different way, and ...
>

I will tweak this.

Paolo

> +};
> > +
> <snip>
> > +    impl ObjectImpl for DummyState {
> > +        type Class = DummyClass;
> > +        const TYPE_INFO: qemu_api::bindings::TypeInfo =
> qemu_api::type_info! { Self };
> > +        const TYPE_NAME: &'static CStr = c"dummy";
> > +        const PARENT_TYPE_NAME: Option<&'static CStr> =
> Some(TYPE_DEVICE);
> > +        const ABSTRACT: bool = false;
> > +        const INSTANCE_INIT: Option<unsafe extern "C" fn(obj: *mut
> Object)> = None;
> > +        const INSTANCE_POST_INIT: Option<unsafe extern "C" fn(obj: *mut
> Object)> = None;
> > +        const INSTANCE_FINALIZE: Option<unsafe extern "C" fn(obj: *mut
> Object)> = None;
> > +    }
> > +
> > +    impl Class for DummyClass {
> > +        const CLASS_INIT: Option<
> > +            unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut
> core::ffi::c_void),
> > +            > = Some(dummy_class_init);
>
> ... ditto. Shall we tweak the formats here or adjust the rustfmt
> settings later?
>
> --
> Best Regards
> Junjie Mao
>
>