Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-07-02 Thread Paolo Bonzini
On Tue, Jul 2, 2024 at 4:44 PM Manos Pitsidianakis
 wrote:
> >Normally you'd see either --enable-XXX or --with-XXX and their
> >corresponding --disable-XXX or --without-XXX.
>
> True. As the commit message says, `rust` is a reserved meson feature
> name, so the auto-generated scripts/meson-buildoptions.sh
> has the following args:
>
>   --enable-with-rust
>   --disable-with-rust
>
> I used the same in `configure` even though it's not autogenerated in
> order to keep the two synced. If there's a way to get around this I'd
> prefer it.

With the patch I posted, --with-rust/--without-rust is handled
entirely in configure, Meson gleans the result from the presence of
RUST_TARGET_TRIPLE in config-host.mak.

Paolo




Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-07-02 Thread Manos Pitsidianakis
Hi Daniel, I missed one of your questions and I just re-read the thread 
today,


On Mon, 24 Jun 2024 19:52, "Daniel P. Berrangé"  wrote:

On Wed, Jun 19, 2024 at 11:13:58PM +0300, Manos Pitsidianakis wrote:

Add options for Rust in meson_options.txt, meson.build, configure to
prepare for adding Rust code in the followup commits.

`rust` is a reserved meson name, so we have to use an alternative.
`with_rust` was chosen.

A cargo_wrapper.py script is added that is heavily based on the work of
Marc-André Lureau from 2021.

https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lur...@redhat.com/

Signed-off-by: Marc-André Lureau 
Signed-off-by: Manos Pitsidianakis 
---
 MAINTAINERS   |   5 +
 configure |  11 ++
 meson.build   |  11 ++
 meson_options.txt |   4 +
 scripts/cargo_wrapper.py  | 279 ++
 scripts/meson-buildoptions.sh |   6 +
 6 files changed, 316 insertions(+)
 create mode 100644 scripts/cargo_wrapper.py



diff --git a/configure b/configure
index 38ee257701..6894d7c2d1 100755
--- a/configure
+++ b/configure
@@ -302,6 +302,9 @@ else
   objcc="${objcc-${cross_prefix}clang}"
 fi

[..snip..]
+with_rust_target_triple=""
+
 ar="${AR-${cross_prefix}ar}"
 as="${AS-${cross_prefix}as}"
 ccas="${CCAS-$cc}"
@@ -760,6 +763,12 @@ for opt do
   ;;
   --gdb=*) gdb_bin="$optarg"
   ;;
+  --enable-with-rust) with_rust=enabled
+  ;;
+  --disable-with-rust) with_rust=disabled
+  ;;


--enable-with-XXX / --disable-with-XXX is pretty unsual naming.

Normally you'd see either --enable-XXX or --with-XXX and their
corresponding --disable-XXX or --without-XXX.


True. As the commit message says, `rust` is a reserved meson feature 
name, so the auto-generated scripts/meson-buildoptions.sh

has the following args:

 --enable-with-rust
 --disable-with-rust

I used the same in `configure` even though it's not autogenerated in 
order to keep the two synced. If there's a way to get around this I'd 
prefer it.





Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-06-26 Thread Paolo Bonzini

On 6/25/24 23:47, Manos Pitsidianakis wrote:

On Mon, 24 Jun 2024 20:14, Paolo Bonzini  wrote:

Yes, I agree. However, considering we haven't even checked the situation
with what language features are required by any idiomatic bindings vs the
1.63 version that we need to support for Debian, I think it's a bit
premature to make it mandatory.



FWIW, I just ran

`cargo msrv -- cargo check --target x86_64-unknown-linux-gnu`

And the result was `The MSRV is: 1.77.2`

and the most important issue was that the mem::offset_of! macro was 
unstable till then.


I looked for a way to avoid it and the most promising is 
https://play.rust-lang.org/?version=stable=debug=2018=10a22a9b8393abd7b541d8fc844bc0df


Basically, you replace

pub struct Foo {
foo: i32,
bar: i32
}

with

with_offsets! {
#[repr(C)]  // mandatory
pub struct Foo {
foo: i32,
bar: i32,
}
}

The macro walks the struct twice, once to actually declare it and once 
to determine the offsets using mem::size_of and mem::align_of.  The 
result is something like


#[repr(C)]  // mandatory
pub struct Foo {
foo: i32,
bar: i32,
}

pub struct FooOffsets {
foo: usize,
bar: usize,
}

impl Foo {
pub const offset_to: FooOffsets = FooOffsets {
foo: 0,
bar: 4,
}
}

(where 0 and 4 are actually the aforementioned computation based on 
size_of and align_of).


There are some limitations but the trick is really well done; the need 
for #[repr(C)] is not a problem for us (C<->Rust interoperability needs 
it anyway), and the implementation is fully "const".  And though it only 
works for structs that use "with_offsets!", and with just one level of 
fields, the implementation of offset_of is trivial:


macro_rules! offset_of {
($Container:ty, $field:ident) => {
<$Container>::offset_to.$field
};
}

Anyhow, this should _not_ be in the first version that is 
committed---which, as you remarked in the v2, should focus on working 
build system integration.  As long as we know it is doable, it can be 
left for later.


Paolo




Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-06-25 Thread Manos Pitsidianakis

On Mon, 24 Jun 2024 20:14, Paolo Bonzini  wrote:
Yes, I agree. However, considering we haven't even checked the 
situation

with what language features are required by any idiomatic bindings vs the
1.63 version that we need to support for Debian, I think it's a bit
premature to make it mandatory.



FWIW, I just ran

`cargo msrv -- cargo check --target x86_64-unknown-linux-gnu`

And the result was `The MSRV is: 1.77.2`

and the most important issue was that the mem::offset_of! macro was 
unstable till then.




Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-06-24 Thread Paolo Bonzini
Il lun 24 giu 2024, 18:52 Daniel P. Berrangé  ha
scritto:

> On Wed, Jun 19, 2024 at 11:13:58PM +0300, Manos Pitsidianakis wrote:
> > Add options for Rust in meson_options.txt, meson.build, configure to
> > prepare for adding Rust code in the followup commits.
> >
> > `rust` is a reserved meson name, so we have to use an alternative.
> > `with_rust` was chosen.
> >
> > A cargo_wrapper.py script is added that is heavily based on the work of
> > Marc-André Lureau from 2021.
> >
> >
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lur...@redhat.com/
> >
> > Signed-off-by: Marc-André Lureau 
> > Signed-off-by: Manos Pitsidianakis 
> > ---
> >  MAINTAINERS   |   5 +
> >  configure |  11 ++
> >  meson.build   |  11 ++
> >  meson_options.txt |   4 +
> >  scripts/cargo_wrapper.py  | 279 ++
> >  scripts/meson-buildoptions.sh |   6 +
> >  6 files changed, 316 insertions(+)
> >  create mode 100644 scripts/cargo_wrapper.py
>
> > diff --git a/configure b/configure
> > index 38ee257701..6894d7c2d1 100755
> > --- a/configure
> > +++ b/configure
> > @@ -302,6 +302,9 @@ else
> >objcc="${objcc-${cross_prefix}clang}"
> >  fi
> >
> > +with_rust="auto"
>
> On last week's call one of the things we discussed is the expectations
> for consumers of QEMU around the usage of Rust & its optionality (or
> not) long term.
>
> I'm of the view that to be valuable for QEMU, we need to consider
> Rust to become a mandatory feature. There's a question of how quickly
> we move, however, before declaring it mandatory. The longer we take
> to declare it mandatory, the longer we are limiting the value we
> can take from Rust.
>

Yes, I agree. However, considering we haven't even checked the situation
with what language features are required by any idiomatic bindings vs the
1.63 version that we need to support for Debian, I think it's a bit
premature to make it mandatory.

As soon as the PL011 device is a reasonable example of how to write good
Rust device models, however, we should switch to making it default-on, and
--without-rust can go away within one or two releases.

> +  --with-rust-target-triple=*) with_rust_target_triple="$optarg"
>
As with previous posting, IMHO, this should ideally not exist. We should
> honour the --cross-prefix= values, re-mapping to to Rust targets for all
> the combos we know about


More precisely it should be based not on the cross prefix, but on the same
OS and CPU values that are detected from cpp symbols and used for the meson
cross file. I have already made a rough list of differences between these
and the Rust target triples, but I haven't yet turned them to code.

It would however be the first patch I send after these are in a good shape
for inclusion.

The priority right now should be to investigate the build tree workspace
design that I sketched last week in my reply to Richard. Once that can be
considered a stable build system integration, further improvements can be
done in tree.

--with-rust-target-triple should only be needed
> as a workaround for where we might have missed a mapping.
>

Agreed.

Paolo


Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-06-24 Thread Daniel P . Berrangé
On Wed, Jun 19, 2024 at 11:13:58PM +0300, Manos Pitsidianakis wrote:
> Add options for Rust in meson_options.txt, meson.build, configure to
> prepare for adding Rust code in the followup commits.
> 
> `rust` is a reserved meson name, so we have to use an alternative.
> `with_rust` was chosen.
> 
> A cargo_wrapper.py script is added that is heavily based on the work of
> Marc-André Lureau from 2021.
> 
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lur...@redhat.com/
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Manos Pitsidianakis 
> ---
>  MAINTAINERS   |   5 +
>  configure |  11 ++
>  meson.build   |  11 ++
>  meson_options.txt |   4 +
>  scripts/cargo_wrapper.py  | 279 ++
>  scripts/meson-buildoptions.sh |   6 +
>  6 files changed, 316 insertions(+)
>  create mode 100644 scripts/cargo_wrapper.py

> diff --git a/configure b/configure
> index 38ee257701..6894d7c2d1 100755
> --- a/configure
> +++ b/configure
> @@ -302,6 +302,9 @@ else
>objcc="${objcc-${cross_prefix}clang}"
>  fi
>  
> +with_rust="auto"

On last week's call one of the things we discussed is the expectations
for consumers of QEMU around the usage of Rust & its optionality (or
not) long term.

If we consider Rust to be an optional feature, then this largely
precludes re-writing existing C code in Rust, as we would be
forced to either remove existing features from users, or maintain
parallel impls in both C & Rust. Neither of these are desirable
situations.

I'm of the view that to be valuable for QEMU, we need to consider
Rust to become a mandatory feature. There's a question of how quickly
we move, however, before declaring it mandatory. The longer we take
to declare it mandatory, the longer we are limiting the value we
can take from Rust.

If we want to make Rust mandatory, then we should set the user
expectations to reflect this from the start.


IOW, with_rust="enabled" should be the default, and require an
explicit  --disable-rust to opt-out on a very time limited
basis.  Any use of --disable-rust ought to print a warning at
the end of configure, and solicit feedback:

WARNING: Building without Rust is deprecated. QEMU intends
WARNING: to make Rust a mandatory build dependency in the
WARNING: 10.0.0 release.
WARNING:
WARNING: If you have concerns about this requirement
WARNING: please contact qemu-devel@nongnu.org

I illustrated '10.0.0' on the assumption that we add Rust
support in 9.1.0, and thus have 2 releases where it is
optional to align with our deprecation policy. Even though
we don't usually apply our deprecation policy to build
dependencies, this is a significant unique situation so
IMHO worth doing.


> +with_rust_target_triple=""
> +
>  ar="${AR-${cross_prefix}ar}"
>  as="${AS-${cross_prefix}as}"
>  ccas="${CCAS-$cc}"
> @@ -760,6 +763,12 @@ for opt do
>;;
>--gdb=*) gdb_bin="$optarg"
>;;
> +  --enable-with-rust) with_rust=enabled
> +  ;;
> +  --disable-with-rust) with_rust=disabled
> +  ;;

--enable-with-XXX / --disable-with-XXX is pretty unsual naming.

Normally you'd see either --enable-XXX or --with-XXX and their
corresponding --disable-XXX or --without-XXX.




> +  --with-rust-target-triple=*) with_rust_target_triple="$optarg"

As with previous posting, IMHO, this should ideally not exist. We should
honour the --cross-prefix= values, re-mapping to to Rust targets for all
the combos we know about. --with-rust-target-triple should only be needed
as a workaround for where we might have missed a mapping.


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




Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-06-24 Thread Paolo Bonzini
Il lun 24 giu 2024, 10:36 Zhao Liu  ha scritto:

> [snip]
>
> > diff --git a/meson.build b/meson.build
> > index c5360fbd299..ad7dbc0d641 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -290,6 +290,11 @@ foreach lang : all_languages
> >endif
> >  endforeach
> > +cargo = not_found
> > +if 'RUST_TARGET_TRIPLE' in config_host
> > +  cargo = find_program('cargo', required: true)
> > +endif
> > +
>
> As with the original Manos version, it looks like there's no need to
> check cargo here? Since patch 2 checks cargo and others in
> rust/meson.build.
>
> Otherwise, cargo was checked twice.
>

Yes, I would check it here though because it's used in the summary, already
in this patch.

Paolo


> >  # default flags for all hosts
> >  # We use -fwrapv to tell the compiler that we require a C dialect where
> >  # left shift of signed integers is well defined and has the expected
> > @@ -4239,6 +4244,10 @@ if 'objc' in all_languages
> >  else
> >summary_info += {'Objective-C compiler': false}
> >  endif
> > +summary_info += {'Rust support':  cargo.found()}
> > +if cargo.found() and config_host['RUST_TARGET_TRIPLE']) !=
> config_host['RUST_HOST_TRIPLE']
> > +  summary_info += {'Rust target': config_host['RUST_TARGET_TRIPLE']}
> > +endif
> >  option_cflags = (get_option('debug') ? ['-g'] : [])
> >  if get_option('optimization') != 'plain'
> >option_cflags += ['-O' + get_option('optimization')]
> >
> >
>
>


Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-06-24 Thread Zhao Liu
[snip]

> diff --git a/meson.build b/meson.build
> index c5360fbd299..ad7dbc0d641 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -290,6 +290,11 @@ foreach lang : all_languages
>endif
>  endforeach
> +cargo = not_found
> +if 'RUST_TARGET_TRIPLE' in config_host
> +  cargo = find_program('cargo', required: true)
> +endif
> +

As with the original Manos version, it looks like there's no need to
check cargo here? Since patch 2 checks cargo and others in rust/meson.build.

Otherwise, cargo was checked twice.

>  # default flags for all hosts
>  # We use -fwrapv to tell the compiler that we require a C dialect where
>  # left shift of signed integers is well defined and has the expected
> @@ -4239,6 +4244,10 @@ if 'objc' in all_languages
>  else
>summary_info += {'Objective-C compiler': false}
>  endif
> +summary_info += {'Rust support':  cargo.found()}
> +if cargo.found() and config_host['RUST_TARGET_TRIPLE']) != 
> config_host['RUST_HOST_TRIPLE']
> +  summary_info += {'Rust target': config_host['RUST_TARGET_TRIPLE']}
> +endif
>  option_cflags = (get_option('debug') ? ['-g'] : [])
>  if get_option('optimization') != 'plain'
>option_cflags += ['-O' + get_option('optimization')]
> 
> 



Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-06-20 Thread Paolo Bonzini
Il gio 20 giu 2024, 20:13 Manos Pitsidianakis <
manos.pitsidiana...@linaro.org> ha scritto:

> On Thu, 20 Jun 2024 16:21, Paolo Bonzini  wrote:
> >On 6/19/24 22:13, Manos Pitsidianakis wrote:
> >> Add options for Rust in meson_options.txt, meson.build, configure to
> >> prepare for adding Rust code in the followup commits.
> >>
> >> `rust` is a reserved meson name, so we have to use an alternative.
> >> `with_rust` was chosen.
> >>
> >> A cargo_wrapper.py script is added that is heavily based on the work of
> >> Marc-André Lureau from 2021.
> >>
> >>
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lur...@redhat.com/
> >>
> >> Signed-off-by: Marc-André Lureau 
> >> Signed-off-by: Manos Pitsidianakis 
> >
> >The cargo_wrapper.py script is not used yet, so it should be
> >delayed until it's used.
>
> That's true, I just wanted to make review easier by splitting it out.
> Can we squash them later or do you think I should I do it for the next
> series version?
>

I guess it depends on what the next version looks like. If you start
working on the workspace/build tree/Kconfig parts, it might not have a lot
of cargo_wrapper.py code surviving.

Feel free to take this patch and add

Signed-off-by: Paolo Bonzini 

Paolo

>For the detection of the toolchain, I'd rather do everything in
> >configure since that's where the cross file is built.  Something like:
> >
> >diff --git a/configure b/configure
> >index 8b6a2f16ceb..6412a1021c3 100755
> >--- a/configure
> >+++ b/configure
> >@@ -173,6 +173,8 @@ fi
> >
> >  # default parameters
> >  container_engine="auto"
> >+rust_target_triple=""
> >+with_rust="no"
> >  cpu=""
> >  cross_compile="no"
> >  cross_prefix=""
> >@@ -201,6 +202,8 @@ for opt do
> >--cross-prefix=*) cross_prefix="$optarg"
> >  cross_compile="yes"
> >;;
> >+  --cargo=*) CARGO="$optarg"
> >+  ;;
> >--cc=*) CC="$optarg"
> >;;
> >--cxx=*) CXX="$optarg"
> >@@ -317,6 +322,8 @@ windmc="${WINDMC-${cross_prefix}windmc}"
> >  pkg_config="${PKG_CONFIG-${cross_prefix}pkg-config}"
> >  sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
> >
> >+cargo="${CARGO-cargo}"
> >+
> >  check_define() {
> >  cat > $TMPC < >  #if !defined($1)
> >@@ -628,6 +635,8 @@ for opt do
> >;;
> >--cross-prefix=*)
> >;;
> >+  --cargo=*)
> >+  ;;
> >--cc=*)
> >;;
> >--host-cc=*) host_cc="$optarg"
> >@@ -755,8 +764,14 @@ for opt do
> >;;
> >--container-engine=*) container_engine="$optarg"
> >;;
> >+  --rust-target-triple=*) rust_target_triple="$optarg"
> >+  ;;
> >--gdb=*) gdb_bin="$optarg"
> >;;
> >+  --with-rust) with_rust=yes
> >+  ;;
> >+  --without-rust) with_rust=no
> >+  ;;
> ># everything else has the same name in configure and meson
> >--*) meson_option_parse "$opt" "$optarg"
> >;;
> >@@ -854,6 +869,7 @@ $(echo Available targets: $default_target_list | \
> >  Advanced options (experts only):
> >-Dmesonoptname=val   passthrough option to meson unmodified
> >--cross-prefix=PREFIXuse PREFIX for compile tools, PREFIX can be
> blank [$cross_prefix]
> >+  --cargo=CARGOuse Cargo binary CARGO [$cargo]
> >--cc=CC  use C compiler CC [$cc]
> >--host-cc=CC when cross compiling, use C compiler CC for
> code run
> > at build time [$host_cc]
> >@@ -869,11 +885,13 @@ Advanced options (experts only):
> >--python=PYTHON  use specified python [$python]
> >--ninja=NINJAuse specified ninja [$ninja]
> >--static enable static build [$static]
> >-  --without-default-features default all --enable-* options to "disabled"
> >-  --without-default-devices  do not include any device that is not
> needed to
> >+  --rust-target-triple=TRIPLE  target for Rust cross compilation
> >+  --without-default-features   default all --enable-* options to
> "disabled"
> >+  --without-default-devicesdo not include any device that is not
> needed to
> > start the emulator (only use if you are
> including
> > desired devices in configs/devices/)
> >--with-devices-ARCH=NAME override default configs/devices
> >+  --with-rust  enable experimental Rust code
> >--enable-debug   enable common debug build options
> >--cpu=CPUBuild for host CPU [$cpu]
> >--disable-containers don't use containers for cross-building
> >@@ -1138,6 +1159,20 @@ EOF
> >fi
> >  fi
> >
> >+##
> >+# detect rust triples
> >+
> >+if test "$with_rust" = yes; then
> >+  $CARGO -vV > "${TMPDIR1}/${TMPB}.out"
> >+  if test $? != 0; then
> >+error_exit "could not execute cargo binary \"$CARGO\""
> >+  fi
> >+  rust_host_triple=$(sed -n 's/^host: //p' "${TMPDIR1}/${TMPB}.out")
> >+  if test "$rust_target_triple" = ""; then
> >+rust_target_triple=$rust_host_triple
> >+  fi
> >+fi
> >+
> >  

Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-06-20 Thread Manos Pitsidianakis

On Thu, 20 Jun 2024 16:41, Alex Bennée  wrote:

+summary_info += {'Rust support':  with_rust}
+if with_rust and get_option('with_rust_target_triple') != ''
+  summary_info += {'Rust target': get_option('with_rust_target_triple')}
+endif



I wonder if we should display the auto-probed triple here as well, not
just when its been overridden?


I agree, once we straighten out host target / cross target detection 
logic the target summary info print should be unconditional.




Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-06-20 Thread Manos Pitsidianakis

On Thu, 20 Jun 2024 16:21, Paolo Bonzini  wrote:

On 6/19/24 22:13, Manos Pitsidianakis wrote:

Add options for Rust in meson_options.txt, meson.build, configure to
prepare for adding Rust code in the followup commits.

`rust` is a reserved meson name, so we have to use an alternative.
`with_rust` was chosen.

A cargo_wrapper.py script is added that is heavily based on the work of
Marc-André Lureau from 2021.

https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lur...@redhat.com/

Signed-off-by: Marc-André Lureau 
Signed-off-by: Manos Pitsidianakis 


The cargo_wrapper.py script is not used yet, so it should be
delayed until it's used.


That's true, I just wanted to make review easier by splitting it out. 
Can we squash them later or do you think I should I do it for the next 
series version?






For the detection of the toolchain, I'd rather do everything in
configure since that's where the cross file is built.  Something like:

diff --git a/configure b/configure
index 8b6a2f16ceb..6412a1021c3 100755
--- a/configure
+++ b/configure
@@ -173,6 +173,8 @@ fi
 
 # default parameters

 container_engine="auto"
+rust_target_triple=""
+with_rust="no"
 cpu=""
 cross_compile="no"
 cross_prefix=""
@@ -201,6 +202,8 @@ for opt do
   --cross-prefix=*) cross_prefix="$optarg"
 cross_compile="yes"
   ;;
+  --cargo=*) CARGO="$optarg"
+  ;;
   --cc=*) CC="$optarg"
   ;;
   --cxx=*) CXX="$optarg"
@@ -317,6 +322,8 @@ windmc="${WINDMC-${cross_prefix}windmc}"
 pkg_config="${PKG_CONFIG-${cross_prefix}pkg-config}"
 sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
 
+cargo="${CARGO-cargo}"

+
 check_define() {
 cat > $TMPC < 
+##

+# detect rust triples
+
+if test "$with_rust" = yes; then
+  $CARGO -vV > "${TMPDIR1}/${TMPB}.out"
+  if test $? != 0; then
+error_exit "could not execute cargo binary \"$CARGO\""
+  fi
+  rust_host_triple=$(sed -n 's/^host: //p' "${TMPDIR1}/${TMPB}.out")
+  if test "$rust_target_triple" = ""; then
+rust_target_triple=$rust_host_triple
+  fi
+fi
+
 ##
 # functions to probe cross compilers
 
@@ -1604,6 +1639,10 @@ if test "$container" != no; then

 echo "RUNC=$runc" >> $config_host_mak
 fi
 echo "SUBDIRS=$subdirs" >> $config_host_mak
+if test "$with_rust" = yes; then
+  echo "RUST_HOST_TRIPLE=$rust_host_triple" >> $config_host_mak
+  echo "RUST_TARGET_TRIPLE=$rust_target_triple" >> $config_host_mak
+fi
 echo "PYTHON=$python" >> $config_host_mak
 echo "MKVENV_ENSUREGROUP=$mkvenv ensuregroup $mkvenv_online_flag" >> 
$config_host_mak
 echo "GENISOIMAGE=$genisoimage" >> $config_host_mak
@@ -1731,6 +1770,13 @@ if test "$skip_meson" = no; then
   echo "c = [$(meson_quote $cc $CPU_CFLAGS)]" >> $cross
   test -n "$cxx" && echo "cpp = [$(meson_quote $cxx $CPU_CFLAGS)]" >> $cross
   test -n "$objcc" && echo "objc = [$(meson_quote $objcc $CPU_CFLAGS)]" >> 
$cross
+  if test "$with_rust" = yes; then
+if test "$rust_host_triple" != "$rust_target_triple"; then
+  echo "cargo = [$(meson_quote $cargo --target "$rust_target_triple")]" >> 
$cross
+else
+  echo "cargo = [$(meson_quote $cargo)]" >> $cross
+fi
+  fi



Hm that looks better indeed, thanks!



   echo "ar = [$(meson_quote $ar)]" >> $cross
   echo "dlltool = [$(meson_quote $dlltool)]" >> $cross
   echo "nm = [$(meson_quote $nm)]" >> $cross
diff --git a/meson.build b/meson.build
index c5360fbd299..ad7dbc0d641 100644
--- a/meson.build
+++ b/meson.build
@@ -290,6 +290,11 @@ foreach lang : all_languages
   endif
 endforeach
 
+cargo = not_found

+if 'RUST_TARGET_TRIPLE' in config_host
+  cargo = find_program('cargo', required: true)
+endif
+
 # default flags for all hosts
 # We use -fwrapv to tell the compiler that we require a C dialect where
 # left shift of signed integers is well defined and has the expected
@@ -4239,6 +4244,10 @@ if 'objc' in all_languages
 else
   summary_info += {'Objective-C compiler': false}
 endif
+summary_info += {'Rust support':  cargo.found()}
+if cargo.found() and config_host['RUST_TARGET_TRIPLE']) != 
config_host['RUST_HOST_TRIPLE']
+  summary_info += {'Rust target': config_host['RUST_TARGET_TRIPLE']}
+endif
 option_cflags = (get_option('debug') ? ['-g'] : [])
 if get_option('optimization') != 'plain'
   option_cflags += ['-O' + get_option('optimization')]






Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-06-20 Thread Alex Bennée
Manos Pitsidianakis  writes:

> Add options for Rust in meson_options.txt, meson.build, configure to
> prepare for adding Rust code in the followup commits.
>
> `rust` is a reserved meson name, so we have to use an alternative.
> `with_rust` was chosen.
>
> A cargo_wrapper.py script is added that is heavily based on the work of
> Marc-André Lureau from 2021.
>
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lur...@redhat.com/
>
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Manos Pitsidianakis 

>  
> +with_rust="auto"
> +with_rust_target_triple=""
> +
>  ar="${AR-${cross_prefix}ar}"
>  as="${AS-${cross_prefix}as}"
>  ccas="${CCAS-$cc}"
> @@ -760,6 +763,12 @@ for opt do
>;;
>--gdb=*) gdb_bin="$optarg"
>;;
> +  --enable-with-rust) with_rust=enabled
> +  ;;
> +  --disable-with-rust) with_rust=disabled
> +  ;;
> +  --with-rust-target-triple=*) with_rust_target_triple="$optarg"
> +  ;;
># everything else has the same name in configure and meson
>--*) meson_option_parse "$opt" "$optarg"
>;;
> @@ -1796,6 +1805,8 @@ if test "$skip_meson" = no; then
>test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add 
> "-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
>test "$plugins" = yes && meson_option_add "-Dplugins=true"
>test "$tcg" != enabled && meson_option_add "-Dtcg=$tcg"
> +  test "$with_rust" != enabled && meson_option_add "-Dwith_rust=$with_rust"
> +  test "$with_rust_target_triple" != "" && meson_option_add 
> "-Dwith_rust_target_triple=$with_rust_target_triple"
>run_meson() {
>  NINJA=$ninja $meson setup "$@" "$PWD" "$source_path"
>}


> +summary_info += {'Rust support':  with_rust}
> +if with_rust and get_option('with_rust_target_triple') != ''
> +  summary_info += {'Rust target': get_option('with_rust_target_triple')}
> +endif


I wonder if we should display the auto-probed triple here as well, not
just when its been overridden?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro



Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-06-20 Thread Paolo Bonzini

On 6/19/24 22:13, Manos Pitsidianakis wrote:

Add options for Rust in meson_options.txt, meson.build, configure to
prepare for adding Rust code in the followup commits.

`rust` is a reserved meson name, so we have to use an alternative.
`with_rust` was chosen.

A cargo_wrapper.py script is added that is heavily based on the work of
Marc-André Lureau from 2021.

https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lur...@redhat.com/

Signed-off-by: Marc-André Lureau 
Signed-off-by: Manos Pitsidianakis 


The cargo_wrapper.py script is not used yet, so it should be
delayed until it's used.

For the detection of the toolchain, I'd rather do everything in
configure since that's where the cross file is built.  Something like:

diff --git a/configure b/configure
index 8b6a2f16ceb..6412a1021c3 100755
--- a/configure
+++ b/configure
@@ -173,6 +173,8 @@ fi
 
 # default parameters

 container_engine="auto"
+rust_target_triple=""
+with_rust="no"
 cpu=""
 cross_compile="no"
 cross_prefix=""
@@ -201,6 +202,8 @@ for opt do
   --cross-prefix=*) cross_prefix="$optarg"
 cross_compile="yes"
   ;;
+  --cargo=*) CARGO="$optarg"
+  ;;
   --cc=*) CC="$optarg"
   ;;
   --cxx=*) CXX="$optarg"
@@ -317,6 +322,8 @@ windmc="${WINDMC-${cross_prefix}windmc}"
 pkg_config="${PKG_CONFIG-${cross_prefix}pkg-config}"
 sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
 
+cargo="${CARGO-cargo}"

+
 check_define() {
 cat > $TMPC < 
+##

+# detect rust triples
+
+if test "$with_rust" = yes; then
+  $CARGO -vV > "${TMPDIR1}/${TMPB}.out"
+  if test $? != 0; then
+error_exit "could not execute cargo binary \"$CARGO\""
+  fi
+  rust_host_triple=$(sed -n 's/^host: //p' "${TMPDIR1}/${TMPB}.out")
+  if test "$rust_target_triple" = ""; then
+rust_target_triple=$rust_host_triple
+  fi
+fi
+
 ##
 # functions to probe cross compilers
 
@@ -1604,6 +1639,10 @@ if test "$container" != no; then

 echo "RUNC=$runc" >> $config_host_mak
 fi
 echo "SUBDIRS=$subdirs" >> $config_host_mak
+if test "$with_rust" = yes; then
+  echo "RUST_HOST_TRIPLE=$rust_host_triple" >> $config_host_mak
+  echo "RUST_TARGET_TRIPLE=$rust_target_triple" >> $config_host_mak
+fi
 echo "PYTHON=$python" >> $config_host_mak
 echo "MKVENV_ENSUREGROUP=$mkvenv ensuregroup $mkvenv_online_flag" >> 
$config_host_mak
 echo "GENISOIMAGE=$genisoimage" >> $config_host_mak
@@ -1731,6 +1770,13 @@ if test "$skip_meson" = no; then
   echo "c = [$(meson_quote $cc $CPU_CFLAGS)]" >> $cross
   test -n "$cxx" && echo "cpp = [$(meson_quote $cxx $CPU_CFLAGS)]" >> $cross
   test -n "$objcc" && echo "objc = [$(meson_quote $objcc $CPU_CFLAGS)]" >> 
$cross
+  if test "$with_rust" = yes; then
+if test "$rust_host_triple" != "$rust_target_triple"; then
+  echo "cargo = [$(meson_quote $cargo --target "$rust_target_triple")]" >> 
$cross
+else
+  echo "cargo = [$(meson_quote $cargo)]" >> $cross
+fi
+  fi
   echo "ar = [$(meson_quote $ar)]" >> $cross
   echo "dlltool = [$(meson_quote $dlltool)]" >> $cross
   echo "nm = [$(meson_quote $nm)]" >> $cross
diff --git a/meson.build b/meson.build
index c5360fbd299..ad7dbc0d641 100644
--- a/meson.build
+++ b/meson.build
@@ -290,6 +290,11 @@ foreach lang : all_languages
   endif
 endforeach
 
+cargo = not_found

+if 'RUST_TARGET_TRIPLE' in config_host
+  cargo = find_program('cargo', required: true)
+endif
+
 # default flags for all hosts
 # We use -fwrapv to tell the compiler that we require a C dialect where
 # left shift of signed integers is well defined and has the expected
@@ -4239,6 +4244,10 @@ if 'objc' in all_languages
 else
   summary_info += {'Objective-C compiler': false}
 endif
+summary_info += {'Rust support':  cargo.found()}
+if cargo.found() and config_host['RUST_TARGET_TRIPLE']) != 
config_host['RUST_HOST_TRIPLE']
+  summary_info += {'Rust target': config_host['RUST_TARGET_TRIPLE']}
+endif
 option_cflags = (get_option('debug') ? ['-g'] : [])
 if get_option('optimization') != 'plain'
   option_cflags += ['-O' + get_option('optimization')]





[RFC PATCH v3 1/5] build-sys: Add rust feature option

2024-06-19 Thread Manos Pitsidianakis
Add options for Rust in meson_options.txt, meson.build, configure to
prepare for adding Rust code in the followup commits.

`rust` is a reserved meson name, so we have to use an alternative.
`with_rust` was chosen.

A cargo_wrapper.py script is added that is heavily based on the work of
Marc-André Lureau from 2021.

https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lur...@redhat.com/

Signed-off-by: Marc-André Lureau 
Signed-off-by: Manos Pitsidianakis 
---
 MAINTAINERS   |   5 +
 configure |  11 ++
 meson.build   |  11 ++
 meson_options.txt |   4 +
 scripts/cargo_wrapper.py  | 279 ++
 scripts/meson-buildoptions.sh |   6 +
 6 files changed, 316 insertions(+)
 create mode 100644 scripts/cargo_wrapper.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 1b79767d61..431010ddbf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4223,6 +4223,11 @@ F: docs/sphinx/
 F: docs/_templates/
 F: docs/devel/docs.rst
 
+Rust build system integration
+M: Manos Pitsidianakis 
+S: Maintained
+F: scripts/cargo_wrapper.py
+
 Miscellaneous
 -
 Performance Tools and Tests
diff --git a/configure b/configure
index 38ee257701..6894d7c2d1 100755
--- a/configure
+++ b/configure
@@ -302,6 +302,9 @@ else
   objcc="${objcc-${cross_prefix}clang}"
 fi
 
+with_rust="auto"
+with_rust_target_triple=""
+
 ar="${AR-${cross_prefix}ar}"
 as="${AS-${cross_prefix}as}"
 ccas="${CCAS-$cc}"
@@ -760,6 +763,12 @@ for opt do
   ;;
   --gdb=*) gdb_bin="$optarg"
   ;;
+  --enable-with-rust) with_rust=enabled
+  ;;
+  --disable-with-rust) with_rust=disabled
+  ;;
+  --with-rust-target-triple=*) with_rust_target_triple="$optarg"
+  ;;
   # everything else has the same name in configure and meson
   --*) meson_option_parse "$opt" "$optarg"
   ;;
@@ -1796,6 +1805,8 @@ if test "$skip_meson" = no; then
   test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add 
"-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
   test "$plugins" = yes && meson_option_add "-Dplugins=true"
   test "$tcg" != enabled && meson_option_add "-Dtcg=$tcg"
+  test "$with_rust" != enabled && meson_option_add "-Dwith_rust=$with_rust"
+  test "$with_rust_target_triple" != "" && meson_option_add 
"-Dwith_rust_target_triple=$with_rust_target_triple"
   run_meson() {
 NINJA=$ninja $meson setup "$@" "$PWD" "$source_path"
   }
diff --git a/meson.build b/meson.build
index a9de71d450..3533889852 100644
--- a/meson.build
+++ b/meson.build
@@ -290,6 +290,12 @@ foreach lang : all_languages
   endif
 endforeach
 
+cargo = not_found
+if get_option('with_rust').allowed()
+  cargo = find_program('cargo', required: get_option('with_rust'))
+endif
+with_rust = cargo.found()
+
 # default flags for all hosts
 # We use -fwrapv to tell the compiler that we require a C dialect where
 # left shift of signed integers is well defined and has the expected
@@ -2066,6 +2072,7 @@ endif
 
 config_host_data = configuration_data()
 
+config_host_data.set('CONFIG_WITH_RUST', with_rust)
 audio_drivers_selected = []
 if have_system
   audio_drivers_available = {
@@ -4190,6 +4197,10 @@ if 'objc' in all_languages
 else
   summary_info += {'Objective-C compiler': false}
 endif
+summary_info += {'Rust support':  with_rust}
+if with_rust and get_option('with_rust_target_triple') != ''
+  summary_info += {'Rust target': get_option('with_rust_target_triple')}
+endif
 option_cflags = (get_option('debug') ? ['-g'] : [])
 if get_option('optimization') != 'plain'
   option_cflags += ['-O' + get_option('optimization')]
diff --git a/meson_options.txt b/meson_options.txt
index 4c1583eb40..223491b731 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -366,3 +366,7 @@ option('qemu_ga_version', type: 'string', value: '',
 
 option('hexagon_idef_parser', type : 'boolean', value : true,
description: 'use idef-parser to automatically generate TCG code for 
the Hexagon frontend')
+option('with_rust', type: 'feature', value: 'auto',
+   description: 'Enable Rust support')
+option('with_rust_target_triple', type : 'string', value: '',
+   description: 'Rust target triple')
diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py
new file mode 100644
index 00..927336f80e
--- /dev/null
+++ b/scripts/cargo_wrapper.py
@@ -0,0 +1,279 @@
+#!/usr/bin/env python3
+
+"""Wrap cargo builds for meson integration
+
+This program builds Rust library crates and makes sure:
+ - They receive the correct --cfg compile flags from the QEMU build that calls
+   it.
+ - They receive the generated Rust bindings path so that they can copy it
+   inside their output subdirectories.
+ - Cargo puts all its build artifacts in the appropriate meson build directory.
+ - The produced static libraries are copied to the path the caller (meson)
+   defines.
+
+Copyright (c) 2020 Red Hat, Inc.
+Copyright (c) 2024 Linaro Ltd.
+
+Authors:
+ Marc-André Lureau 
+ Manos Pitsidianakis 
+
+This program is free software; you can