Re: [RFC PATCH v6 0/5] rust-pl011-rfc-v6

[Manos Pitsidianakis]

On Thu, 08 Aug 2024 09:10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 8/4/24 23:04, Manos Pitsidianakis wrote:
> > Changes
> > =======
> >
> > - Setting MSRV to 1.77.0:
> >    * cstr crate MSRV is 1.64, which is more recent than Debian bookworm
> >      (1.63.0) <https://github.com/upsuper/cstr/blob/master/Cargo.toml>
> >
> >    * pl011's dependencies (mostly proc-macro2) don't support 1.63.0
>
> proc-macro2 is listed as supporting 1.56.0, and in general I don't see 
> particularly high MSRVs for any of your dependencies.

The issue was with transitive deps, proc-macro-error crates etc stopped 
compiling when lowering the version, which means we'd have to patch the 
dependency's dependency to see if that'd work; otherwise, yes!

> cstr needs to use version 0.2.10 in order to work with Rust 1.63.0.
>
> As discussed on IRC, there are obvious advantages and disadvantages to 
> using meson.  The main disadvantage is the extra work when bumping the 
> version of the dependencies or when adding a new one.  The advantage is 
> more uniformity and less moving parts.  Overall, I think it's doable to 
> use it.  Dependencies will mostly be added in the early days of QEMU, 
> and won't be updated too often due to our MSRV constraints.
>
> The automatic Cargo.toml support in Meson is promising, but it doesn't 
> work right now when cross compiling build-time dependencies (which have 
> to use "native: true" or Meson rightly warns about mixing build-machine 
> and host-machine binaries).  So right now we'd have to write meson.build 
> by hand for those.
>
> My suggestion is however to name our manually-managed subprojects with 
> the same convention that is used by "method = cargo" in Meson 1.5.0+, 
> i.e. name-APIVER-rs:
>
>         arbitrary-int-1-rs.wrap
>         bilge-0.2-rs.wrap
>         bilge-impl-0.2-rs.wrap
>         either-1-rs.wrap
>         itertools-0.11-rs.wrap
>         proc-macro2-1-rs.wrap
>         proc-macro-error-1-rs.wrap
>         proc-macro-error-attr-1-rs.wrap
>         quote-1-rs.wrap
>         syn-2-rs.wrap
>         unicode-ident-1-rs.wrap
>
> and to access dependencies using meson.override_dependency() and 
> dependency(), instead of get_variable().  This at least reduces future 
> churn.

Yes that makes sense!

> As to the individual patches:
>
> - for patch 1, roughly the same changes I had made for cargo can be done 
> for rustc, so that the cross file contains the right --target option. 
> I'll reply to the individual patch.

That'd be great since I'm not familiar with how the cross file works.

> - for patch 2, the only issue is that you are specifying 
> --no-include-path-detection and that breaks for me on Fedora.  I have 
> not finished testing but it seems that it's enough to remove that line.

I had added that when trying to debug bindgen failing to find headers 
when dependencies were added (e.g. linux io_uring) or when compiling on 
macos, let's test again to see if it's indeed unnecessary!

> - for patches 4 and 5, I have minimal comments on the meson.build.  For 
> patch 5, however, I have already done the above renaming as part of 
> getting cross compilation to work.  We can synchronize on IRC on the 
> best way of getting the changes to you.

Sounds good to me.

Thanks,
Manos