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

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

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