Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Jun 11, 2024 at 01:51:13PM +0100, Alex Bennée wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Tue, Jun 11, 2024 at 01:58:10PM +0300, Manos Pitsidianakis wrote:
> >> On Tue, 11 Jun 2024 13:57, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
> >> > On Mon, Jun 10, 2024 at 11:29:36PM +0300, Manos Pitsidianakis wrote:
> >> > > On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
> >> > > > Hello Manos,
> >> > > > > On 6/10/24 11:22, Manos Pitsidianakis wrote:
> >> > > > > Hello everyone,
> >> > > > > > > This is an early draft of my work on implementing a very
> >> > > simple device,
> >> > > > > in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
> >> > > > > and is used in hw/arm/virt.c).
> >> > > > > > > The device is functional, with copied logic from the C code
> >> > > but with
> >> > > > > effort not to make a direct C to Rust translation. In other words, do
> >> > > > > not write Rust as a C developer would.
> >> > > > > > > That goal is not complete but a best-effort case. To give a
> >> > > specific
> >> > > > > example, register values are typed but interrupt bit flags are not (but
> >> > > > > could be). I will leave such minutiae for later iterations.
> >> > 
> >> > snip
> >> > 
> >> > > > Maybe it could be better if build.rs file was *not* needed for new
> >> > > > devices/folders, and could be abstracted as a detail of the python
> >> > > > wrapper script instead of something that should be committed.
> >> > > 
> >> > > 
> >> > > That'd mean you cannot work on the rust files with a LanguageServer, you
> >> > > cannot run cargo build or cargo check or cargo clippy, etc. That's why I
> >> > > left the alternative choice of including a manually generated bindings file
> >> > > (generated.rs.inc)
> >> > 
> >> > I would not expect QEMU developers to be running 'cargo <anything>'
> >> > directly at all.
> >> > 
> >> > QEMU's build system is 'meson' + 'ninja' with a 'configure' + 'make'
> >> > convenience facade.
> >> > 
> >> > Any use of 'cargo' would be an internal impl detail of meson rules
> >> > for building rust code, and developers should still exclusively work
> >> > with 'make' or 'ninja' to run builds & tests.
> >> 
> >> No, that's not true. If I wrote the pl011 device with this workflow I'd just
> >> waste time using meson. Part of the development is making sure the library
> >> type checks, compiles, using cargo to run style formatting, to check for
> >> lints, perhaps run tests. Doing this only through meson is an unnecessary
> >> complication.
> >
> > I don't see why it should waste time, when we ultimately end up calling
> > the same underlying tools. We need to have a consistent experiance for
> > developers working on QEMU, not have to use different tools for different
> > parts of QEMU depending on whether a piece of code happens to be rust
> > or C.
> 
> For example if I wanted to run rust-based unit tests (which I think
> potentially offer an easier solution than qtest) I would expect that to
> be done from the normal make/ninja targets.

Meson provides a nice "suite" concept to facilitate selection of a
subset of tests.

eg, to limit to running just 'rust' unit tests, I might expect we
should have

  meson test --suite rustunit

and have this invoked by 'make check-rustunit' 

Similar can be done for clippy, or other types of rust tests

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 :|