This includes a few fixes to the Rust build system machinery, and
removes constructs that were added or stabilized after version 1.63.0:

- "let else" (by patching bilge-impl, patch 4; stable in 1.65.0)

- std::sync::OnceLock (patch 6; stable in 1.70.0)

- core::ffi (patch 7; stable in 1.64.0)

- c"" literals (patch 9; stable in 1.77.0)

- offset_of! (patch 10; stable in 1.77.0)

- MaybeUninit::zeroed() (patch 11; stable in 1.75.0 in const context)

It also replicates the configuration checks normally done by
proc-macro2's build.rs into our Meson-based build rules, so that
the crate can be made to work with an older version of rustc.

As a small bonus, patch 11 removes some uses of unsafe, so that patch
probably won't just be simply reverted even once we can assume version
1.75.0 of the language.  And as another small bonus this series introduces
the first use of Rust unit tests.

On top of this, the required version of bindgen is still too new
for Debian 12 and Ubuntu 22.04.  This is fixed by the last four patches.

This is an RFC for two reasons.  First, because it would be a valid
decision to delay enabling of Rust until at least some of these
features are available in all supported distros.  Another possibility
could be to accept Rust 1.64.0 but require installing a newer bindgen
(0.66.x for example) on those two distros with an older release.  Second,
because the series is missing the CI updates to actually ensure that
these minimum versions keep working.

The main purpose is to show the kind of hacks that would be needed
to support older toolchains.  The fixes (for example patches
1/2/3/6/8/11/13/14) can be easily extracted independent of the outcome
of the discussion, and/or while the CI updates are made.

Thanks,

Paolo

Paolo Bonzini (16):
  meson: import rust module into a global variable
  meson: remove repeated search for rust_root_crate.sh
  rust: pass rustc_args when building all crates
  rust: patch bilge-impl to allow compilation with 1.63.0
  rust: fix cfgs of proc-macro2 for 1.63.0
  rust: do not use OnceLock for properties
  rust: use std::os::raw instead of core::ffi
  rust: build tests for the qemu_api crate
  rust: introduce a c_str macro
  rust: introduce alternative implementation of offset_of!
  rust: do not use MaybeUninit::zeroed()
  rust: allow version 1.63.0 of rustc
  rust: do not use TYPE_CHARDEV unnecessarily
  rust: do not use --no-size_t-is-usize
  rust: do not use --generate-cstr
  rust: allow older version of bindgen

 meson.build                                   |  42 ++++---
 .gitattributes                                |   2 +
 rust/hw/char/pl011/src/device.rs              | 117 ++++++++++--------
 rust/hw/char/pl011/src/device_class.rs        |  13 +-
 rust/hw/char/pl011/src/lib.rs                 |   4 +-
 rust/hw/char/pl011/src/memory_ops.rs          |  21 ++--
 rust/qemu-api-macros/meson.build              |   2 +-
 rust/qemu-api/meson.build                     |  17 ++-
 rust/qemu-api/src/c_str.rs                    |  52 ++++++++
 rust/qemu-api/src/definitions.rs              |  10 +-
 rust/qemu-api/src/device_class.rs             |  49 +++++---
 rust/qemu-api/src/lib.rs                      |  14 ++-
 rust/qemu-api/src/offset_of.rs                | 106 ++++++++++++++++
 rust/qemu-api/src/tests.rs                    |  21 ++--
 rust/qemu-api/src/zeroable.rs                 |  75 +++++++++++
 subprojects/bilge-impl-0.2-rs.wrap            |   1 +
 .../packagefiles/bilge-impl-1.63.0.patch      |  45 +++++++
 .../packagefiles/proc-macro2-1-rs/meson.build |   4 +-
 18 files changed, 469 insertions(+), 126 deletions(-)
 create mode 100644 rust/qemu-api/src/c_str.rs
 create mode 100644 rust/qemu-api/src/offset_of.rs
 create mode 100644 rust/qemu-api/src/zeroable.rs
 create mode 100644 subprojects/packagefiles/bilge-impl-1.63.0.patch

-- 
2.46.2

Am 15.10.2024 um 15:17 hat Paolo Bonzini geschrieben:
> This includes a few fixes to the Rust build system machinery, and
> removes constructs that were added or stabilized after version 1.63.0:

Most of this series looks harmless in the sense that we need to write
some workaround code in a single place and can forget about it. So
that's good.

> - "let else" (by patching bilge-impl, patch 4; stable in 1.65.0)

This one affects all of the code we'll write and the replacement is a
bit unwieldy. It might be the most annoying one from the list.

> - std::sync::OnceLock (patch 6; stable in 1.70.0)
> 
> - core::ffi (patch 7; stable in 1.64.0)
> 
> - c"" literals (patch 9; stable in 1.77.0)

This one will be fairly widespread, too, but only a minor inconvenience.

> - offset_of! (patch 10; stable in 1.77.0)

Requiring structs to be wrapped in with_offsets! has potential to become
quite annoying, too, but it seems we already have a solution for this
with the proc macro.

> - MaybeUninit::zeroed() (patch 11; stable in 1.75.0 in const context)
> 
> It also replicates the configuration checks normally done by
> proc-macro2's build.rs into our Meson-based build rules, so that
> the crate can be made to work with an older version of rustc.
> 
> As a small bonus, patch 11 removes some uses of unsafe, so that patch
> probably won't just be simply reverted even once we can assume version
> 1.75.0 of the language.  And as another small bonus this series introduces
> the first use of Rust unit tests.
> 
> On top of this, the required version of bindgen is still too new
> for Debian 12 and Ubuntu 22.04.  This is fixed by the last four patches.
> 
> This is an RFC for two reasons.  First, because it would be a valid
> decision to delay enabling of Rust until at least some of these
> features are available in all supported distros.  Another possibility
> could be to accept Rust 1.64.0 but require installing a newer bindgen
> (0.66.x for example) on those two distros with an older release.  Second,
> because the series is missing the CI updates to actually ensure that
> these minimum versions keep working.
> 
> The main purpose is to show the kind of hacks that would be needed
> to support older toolchains.  The fixes (for example patches
> 1/2/3/6/8/11/13/14) can be easily extracted independent of the outcome
> of the discussion, and/or while the CI updates are made.

It would probably make sense to just go ahead and apply the fixes. They
seem a lot more obvious than the question which toolchains we want to
support.

Kevin

On Tue, Oct 15, 2024 at 03:17:18PM +0200, Paolo Bonzini wrote:
> On top of this, the required version of bindgen is still too new
> for Debian 12 and Ubuntu 22.04.  This is fixed by the last four patches.
> 
> This is an RFC for two reasons.  First, because it would be a valid
> decision to delay enabling of Rust until at least some of these
> features are available in all supported distros.

Lets say we maximise our back compatibility today, and have to
carry some sub-optimal code patterns.

1, 2, 3, 4 years down the lines, we can gradually eliminate
those undesired code patterns / workarounds, as older distros
naturally age-out of our matrix.  After 4 years our entire
matrix will have cycled, so we're not needing to carry this
debt for very long (4 years is not long in the context of a
project like QEMU which has been going several decades)

IOW, we're deciding between

 * creating a bit of rust technical debt in the immediate
   term, in order to enable rust by default sooner

vs

 * avoiding Rust technical debt, but delaying ability to
   enable rust by default.

We could consider all C code to be technical debt though,
and if we don't have Rust by default we'll continue  adding
yet more C code. IOW, option is just moving the debt from
Rust back to C, which is arguably worse on balance.

Personally I tend towards quicker adoption of Rust, despite
the need for short term workarounds, as they'll disappear
again reasonably quickly.

>                                                  Another possibility
> could be to accept Rust 1.64.0 but require installing a newer bindgen
> (0.66.x for example) on those two distros with an older release.

How difficult is it to get newer 'bindgen' installed on these
platforms ? The audience here is not so much distros trying to
package new QEMU, as that's ony relevant for new distro, but
rather it is end usrs/contributors building QEMU for themslves.

Can it be done automagically in the same way we "do the right thing"
with the 3rd party crates we depend on, or is bindgen special in
some way that makes it more inconvenient for users ?

>                                                                   Second,
> because the series is missing the CI updates to actually ensure that
> these minimum versions keep working.

On the last point, see

  https://lists.nongnu.org/archive/html/qemu-devel/2024-10/msg02688.html

with that series, it should be just a matter of adding '--enable-rust'
in a few key jobs.


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

On 10/18/24 15:31, Daniel P. Berrangé wrote:
> On Tue, Oct 15, 2024 at 03:17:18PM +0200, Paolo Bonzini wrote:
>> On top of this, the required version of bindgen is still too new
>> for Debian 12 and Ubuntu 22.04.  This is fixed by the last four patches.
>>
>> This is an RFC for two reasons.  First, because it would be a valid
>> decision to delay enabling of Rust until at least some of these
>> features are available in all supported distros.
> 
> Lets say we maximise our back compatibility today, and have to
> carry some sub-optimal code patterns.
> 
> 1, 2, 3, 4 years down the lines, we can gradually eliminate
> those undesired code patterns / workarounds, as older distros
> naturally age-out of our matrix.  After 4 years our entire
> matrix will have cycled, so we're not needing to carry this
> debt for very long (4 years is not long in the context of a
> project like QEMU which has been going several decades)

I agree, for what it's worth.

> Personally I tend towards quicker adoption of Rust, despite
> the need for short term workarounds, as they'll disappear
> again reasonably quickly.

Yes, especially since (as Kevin pointed out) most of the workarounds are 
okay in terms of maintainability.  If the worst is "if let", and it only 
occurs in a dependency, we're in a good place overall.

>>                                                   Another possibility
>> could be to accept Rust 1.64.0 but require installing a newer bindgen
>> (0.66.x for example) on those two distros with an older release.
> 
> How difficult is it to get newer 'bindgen' installed on these
> platforms ? The audience here is not so much distros trying to
> package new QEMU, as that's ony relevant for new distro, but
> rather it is end usrs/contributors building QEMU for themslves.

Very simple - "cargo install bindgen-cli", as already seen in the 
fedora-rust-nightly container's Dockerfile (note: building QEMU does 
_not_ need cargo).  In fact we could in fact do it via libvirt-ci, and 
it's quite possible that MacOS or some BSDs will need it.

Personally I'd be okay with allowing Debian 12 but not Ubuntu 22.04, for 
various reasons:

- Ubuntu 22.04 has a new rustc and an old bindgen---so it's really just 
laziness.

- any workarounds for Debian 12 would last shorter, and anyway

- Debian 12 has the really important feature (--allowlist-file), whereas 
the lack of --generate-cstr is only annoying.

> Can it be done automagically in the same way we "do the right thing"
> with the 3rd party crates we depend on, or is bindgen special in
> some way that makes it more inconvenient for users ?

bindgen is special in that it has a metric ton of indirect dependencies, 
which we'd all have to write a meson.build for (by hand). :/

> On the last point, see
> 
>    https://lists.nongnu.org/archive/html/qemu-devel/2024-10/msg02688.html
> 
> with that series, it should be just a matter of adding '--enable-rust'
> in a few key jobs.

Indeed, thanks.

Paolo

On Fri, 18 Oct 2024 at 16:43, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 10/18/24 15:31, Daniel P. Berrangé wrote:
> > On Tue, Oct 15, 2024 at 03:17:18PM +0200, Paolo Bonzini wrote:
> >>                                                   Another possibility
> >> could be to accept Rust 1.64.0 but require installing a newer bindgen
> >> (0.66.x for example) on those two distros with an older release.
> >
> > How difficult is it to get newer 'bindgen' installed on these
> > platforms ? The audience here is not so much distros trying to
> > package new QEMU, as that's ony relevant for new distro, but
> > rather it is end usrs/contributors building QEMU for themslves.
>
> Very simple - "cargo install bindgen-cli", as already seen in the
> fedora-rust-nightly container's Dockerfile (note: building QEMU does
> _not_ need cargo).  In fact we could in fact do it via libvirt-ci, and
> it's quite possible that MacOS or some BSDs will need it.

Why doesn't 'rustup update' do this automatically? My Ubuntu 22.04
system I'm using 'rustup' to provide the rust toolchain,
and 'rustup update' updates rustc, cargo, clippy, etc, so
why isn't it also providing and updating bindgen?
('bindgen' for me is ~/.cargo/bin/bindgen, so not the system one.)
My expectation here was that "rustup update" would keep
the whole toolchain up-to-date...

thanks
-- PMM

On 10/21/24 16:15, Peter Maydell wrote:
>> Very simple - "cargo install bindgen-cli", as already seen in the
>> fedora-rust-nightly container's Dockerfile (note: building QEMU does
>> _not_ need cargo).  In fact we could in fact do it via libvirt-ci, and
>> it's quite possible that MacOS or some BSDs will need it.
> 
> Why doesn't 'rustup update' do this automatically? My Ubuntu 22.04
> system I'm using 'rustup' to provide the rust toolchain,
> and 'rustup update' updates rustc, cargo, clippy, etc, so
> why isn't it also providing and updating bindgen?
> ('bindgen' for me is ~/.cargo/bin/bindgen, so not the system one.)
> My expectation here was that "rustup update" would keep
> the whole toolchain up-to-date...

I'd agree with you, but bindgen is not part of the toolchain. :/  In the 
Cargo way of doing things, bindgen is specified in build-dependencies 
and it's rebuilt together with the rest of the library that uses it.

This is as wasteful as it sounds; I guess it makes it easier for bindgen 
authors to "move fast and break things"?  Even in the 3-years span of 
bindgen versions supported by QEMU we have the case where 
--size_t-is-usize is needed on older versions and was removed only a 
handful of versions after it became the default.

Hopefully, this becomes less problematic as Linux forces the tools to 
mature (my hope is that, in general, Linux will force the Rust team to 
give more consideration to non-Cargo mixed-language projects).

Paolo

Am 18.10.2024 um 17:43 hat Paolo Bonzini geschrieben:
> On 10/18/24 15:31, Daniel P. Berrangé wrote:
> > On Tue, Oct 15, 2024 at 03:17:18PM +0200, Paolo Bonzini wrote:
> > > On top of this, the required version of bindgen is still too new
> > > for Debian 12 and Ubuntu 22.04.  This is fixed by the last four patches.
> > > 
> > > This is an RFC for two reasons.  First, because it would be a valid
> > > decision to delay enabling of Rust until at least some of these
> > > features are available in all supported distros.
> > 
> > Lets say we maximise our back compatibility today, and have to
> > carry some sub-optimal code patterns.
> > 
> > 1, 2, 3, 4 years down the lines, we can gradually eliminate
> > those undesired code patterns / workarounds, as older distros
> > naturally age-out of our matrix.  After 4 years our entire
> > matrix will have cycled, so we're not needing to carry this
> > debt for very long (4 years is not long in the context of a
> > project like QEMU which has been going several decades)
> 
> I agree, for what it's worth.
> 
> > Personally I tend towards quicker adoption of Rust, despite
> > the need for short term workarounds, as they'll disappear
> > again reasonably quickly.
> 
> Yes, especially since (as Kevin pointed out) most of the workarounds are
> okay in terms of maintainability.  If the worst is "if let", and it only
> occurs in a dependency, we're in a good place overall.

s/if let/let else/

"only occurs in a dependency" is probably not the right argument while
we haven't really started writing our own Rust code. If it were
available, we would probably use it in new code. But even without that,
the conclusion is the same, of course: This doesn't prevent implementing
anything and is far from being a show stopper.

I'm in favour of anything that lets up keep the phase of duplication as
short as possible if it doesn't severly limit what we can do. And I
don't see anything in this series that would do that.

> > >                                                   Another possibility
> > > could be to accept Rust 1.64.0 but require installing a newer bindgen
> > > (0.66.x for example) on those two distros with an older release.
> > 
> > How difficult is it to get newer 'bindgen' installed on these
> > platforms ? The audience here is not so much distros trying to
> > package new QEMU, as that's ony relevant for new distro, but
> > rather it is end usrs/contributors building QEMU for themslves.
> 
> Very simple - "cargo install bindgen-cli", as already seen in the
> fedora-rust-nightly container's Dockerfile (note: building QEMU does _not_
> need cargo).  In fact we could in fact do it via libvirt-ci, and it's quite
> possible that MacOS or some BSDs will need it.
> 
> Personally I'd be okay with allowing Debian 12 but not Ubuntu 22.04, for
> various reasons:
> 
> - Ubuntu 22.04 has a new rustc and an old bindgen---so it's really just
> laziness.
> 
> - any workarounds for Debian 12 would last shorter, and anyway
> 
> - Debian 12 has the really important feature (--allowlist-file), whereas the
> lack of --generate-cstr is only annoying.
> 
> > Can it be done automagically in the same way we "do the right thing"
> > with the 3rd party crates we depend on, or is bindgen special in
> > some way that makes it more inconvenient for users ?
> 
> bindgen is special in that it has a metric ton of indirect dependencies,
> which we'd all have to write a meson.build for (by hand). :/

If configure errors out with a message "Please run 'cargo install
bindgen-cli'" and that is really enough to make it build, I think that
should be enough to be counted as supporting the distro.

Kevin