| Message ID | rust-pl011-rfc-v6.git.manos.pitsidianakis@linaro.org |
|---|---|
| Headers | show |
| Series | rust-pl011-rfc-v6 | expand |
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. 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. 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. - 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. - 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. Paolo
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
On Thu, Aug 8, 2024 at 9:58 AM Manos Pitsidianakis <manos.pitsidianakis@linaro.org> wrote: > > 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! Ah, I see now - you have to set the right cfg for proc-macro2 to compile with 1.63.0. Normally (including with "method = cargo") they are detected by build.rs: if rustc < 66 { println!("cargo:rustc-cfg=no_source_text"); } if rustc < 79 { println!("cargo:rustc-cfg=no_literal_byte_character"); println!("cargo:rustc-cfg=no_literal_c_string"); } (Meson's support for build.rs is very limited, but it does handle some simple cases and parses rustc-cfg from the output). And bilge-impl uses let...else; we can patch it locally via the .wrap file's "diff_files" entry. Not great, but it's good that we can do it and that we have an example for similar issues in the future. I updated my rust-for-manos branch with these discoveries. Of course it doesn't compile with 1.63.0 but it does at least configure successfully with ../configure --rustc=$(rustup +1.63.0 which rustc) --enable-rust and build the subprojects' rlibs. > >The automatic Cargo.toml support in Meson is promising [...] > >My suggestion is 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: > > Yes that makes sense! Good! > >- 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! Ok, crossing fingers. Paolo
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 - Dropped CI/lcitool patches. - Dropped vendored dependencies in favor of meson subprojects. - Added a qom ObjectImpl trait and declaration macros - Added # SAFETY comments. - Changed configure flag to --{enable,disable}-rust Manos Pitsidianakis (5): build-sys: Add rust feature option rust: add bindgen step as a meson dependency .gitattributes: add Rust diff and merge attributes rust: add crate to expose bindings and interfaces rust: add PL011 device model MAINTAINERS | 20 + configure | 2 + meson.build | 77 ++- rust/wrapper.h | 39 ++ .gitattributes | 3 + Kconfig | 1 + Kconfig.host | 3 + hw/arm/Kconfig | 33 +- meson_options.txt | 3 + rust/.gitignore | 3 + rust/Kconfig | 1 + rust/hw/Kconfig | 2 + rust/hw/char/Kconfig | 3 + rust/hw/char/meson.build | 1 + rust/hw/char/pl011/.gitignore | 2 + rust/hw/char/pl011/Cargo.lock | 125 ++++ rust/hw/char/pl011/Cargo.toml | 26 + rust/hw/char/pl011/README.md | 31 + rust/hw/char/pl011/meson.build | 28 + rust/hw/char/pl011/rustfmt.toml | 1 + rust/hw/char/pl011/src/definitions.rs | 26 + rust/hw/char/pl011/src/device.rs | 586 ++++++++++++++++++ rust/hw/char/pl011/src/device_class.rs | 58 ++ rust/hw/char/pl011/src/lib.rs | 584 +++++++++++++++++ rust/hw/char/pl011/src/memory_ops.rs | 56 ++ rust/hw/meson.build | 1 + rust/meson.build | 15 + rust/qemu-api/.gitignore | 2 + rust/qemu-api/Cargo.lock | 7 + rust/qemu-api/Cargo.toml | 23 + rust/qemu-api/README.md | 17 + rust/qemu-api/build.rs | 13 + rust/qemu-api/meson.build | 19 + rust/qemu-api/rustfmt.toml | 1 + rust/qemu-api/src/bindings.rs | 7 + rust/qemu-api/src/definitions.rs | 108 ++++ rust/qemu-api/src/device_class.rs | 128 ++++ rust/qemu-api/src/lib.rs | 100 +++ rust/qemu-api/src/tests.rs | 48 ++ rust/rustfmt.toml | 7 + scripts/meson-buildoptions.sh | 3 + scripts/rustc_args.py | 84 +++ subprojects/.gitignore | 11 + subprojects/arbitrary-int.wrap | 9 + subprojects/bilge-impl.wrap | 10 + subprojects/bilge.wrap | 10 + subprojects/either.wrap | 10 + subprojects/itertools.wrap | 10 + .../packagefiles/arbitrary-int/meson.build | 19 + .../packagefiles/bilge-impl/meson.build | 36 ++ subprojects/packagefiles/bilge/meson.build | 25 + subprojects/packagefiles/either/meson.build | 21 + .../packagefiles/itertools/meson.build | 25 + .../proc-macro-error-attr/meson.build | 27 + .../packagefiles/proc-macro-error/meson.build | 32 + .../packagefiles/proc-macro2/meson.build | 26 + subprojects/packagefiles/quote/meson.build | 25 + subprojects/packagefiles/syn/meson.build | 33 + .../packagefiles/unicode-ident/meson.build | 19 + subprojects/proc-macro-error-attr.wrap | 10 + subprojects/proc-macro-error.wrap | 11 + subprojects/proc-macro2.wrap | 10 + subprojects/quote.wrap | 10 + subprojects/syn.wrap | 11 + subprojects/unicode-ident.wrap | 10 + 65 files changed, 2695 insertions(+), 12 deletions(-) create mode 100644 rust/wrapper.h create mode 100644 rust/.gitignore create mode 100644 rust/Kconfig create mode 100644 rust/hw/Kconfig create mode 100644 rust/hw/char/Kconfig create mode 100644 rust/hw/char/meson.build create mode 100644 rust/hw/char/pl011/.gitignore create mode 100644 rust/hw/char/pl011/Cargo.lock create mode 100644 rust/hw/char/pl011/Cargo.toml create mode 100644 rust/hw/char/pl011/README.md create mode 100644 rust/hw/char/pl011/meson.build create mode 120000 rust/hw/char/pl011/rustfmt.toml create mode 100644 rust/hw/char/pl011/src/definitions.rs create mode 100644 rust/hw/char/pl011/src/device.rs create mode 100644 rust/hw/char/pl011/src/device_class.rs create mode 100644 rust/hw/char/pl011/src/lib.rs create mode 100644 rust/hw/char/pl011/src/memory_ops.rs create mode 100644 rust/hw/meson.build create mode 100644 rust/meson.build create mode 100644 rust/qemu-api/.gitignore create mode 100644 rust/qemu-api/Cargo.lock create mode 100644 rust/qemu-api/Cargo.toml create mode 100644 rust/qemu-api/README.md create mode 100644 rust/qemu-api/build.rs create mode 100644 rust/qemu-api/meson.build create mode 120000 rust/qemu-api/rustfmt.toml create mode 100644 rust/qemu-api/src/bindings.rs create mode 100644 rust/qemu-api/src/definitions.rs create mode 100644 rust/qemu-api/src/device_class.rs create mode 100644 rust/qemu-api/src/lib.rs create mode 100644 rust/qemu-api/src/tests.rs create mode 100644 rust/rustfmt.toml create mode 100644 scripts/rustc_args.py create mode 100644 subprojects/arbitrary-int.wrap create mode 100644 subprojects/bilge-impl.wrap create mode 100644 subprojects/bilge.wrap create mode 100644 subprojects/either.wrap create mode 100644 subprojects/itertools.wrap create mode 100644 subprojects/packagefiles/arbitrary-int/meson.build create mode 100644 subprojects/packagefiles/bilge-impl/meson.build create mode 100644 subprojects/packagefiles/bilge/meson.build create mode 100644 subprojects/packagefiles/either/meson.build create mode 100644 subprojects/packagefiles/itertools/meson.build create mode 100644 subprojects/packagefiles/proc-macro-error-attr/meson.build create mode 100644 subprojects/packagefiles/proc-macro-error/meson.build create mode 100644 subprojects/packagefiles/proc-macro2/meson.build create mode 100644 subprojects/packagefiles/quote/meson.build create mode 100644 subprojects/packagefiles/syn/meson.build create mode 100644 subprojects/packagefiles/unicode-ident/meson.build create mode 100644 subprojects/proc-macro-error-attr.wrap create mode 100644 subprojects/proc-macro-error.wrap create mode 100644 subprojects/proc-macro2.wrap create mode 100644 subprojects/quote.wrap create mode 100644 subprojects/syn.wrap create mode 100644 subprojects/unicode-ident.wrap base-commit: f9851d2ffef59b3a7f39513469263ab3b019480f