Re: [RFC PATCH v1 1/6] build-sys: Add rust feature option

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Jun 11, 2024 at 02:25:39PM -0400, Stefan Hajnoczi wrote:
> On Tue, 11 Jun 2024 at 13:54, Manos Pitsidianakis
> <manos.pitsidianakis@linaro.org> wrote:
> >
> > On Tue, 11 Jun 2024 at 17:05, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > >
> > > On Mon, Jun 10, 2024 at 09:22:36PM +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.
> > > >
> > > > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > > > ---
> > > > The cargo wrapper script hardcodes some rust target triples. This is
> > > > just temporary.
> > > > ---
> > > >  .gitignore               |   2 +
> > > >  configure                |  12 +++
> > > >  meson.build              |  11 ++
> > > >  meson_options.txt        |   4 +
> > > >  scripts/cargo_wrapper.py | 211 +++++++++++++++++++++++++++++++++++++++
> > > >  5 files changed, 240 insertions(+)
> > > >  create mode 100644 scripts/cargo_wrapper.py

> > > > diff --git a/configure b/configure
> > > > index 38ee257701..c195630771 100755
> > > > --- a/configure
> > > > +++ b/configure

snip

> > > > +  test "$with_rust_target_triple" != "" && meson_option_add "-Dwith_rust_target_triple=$with_rust_target_triple"

So the --rust-target-triple is only needed when cross compiling,
but this is not the way we normally handle passing cross compiler
info to meson. Instead we create a meson cross compiler options
file containing the target info.

eg for ./configure --cross-prefix=x86_64-w64-mingw32-

we end up creating:

$ cat build/config-meson.cross 
# Automatically generated by configure - do not modify
[properties]
[built-in options]
c_args = []
cpp_args = []
objc_args = []
c_link_args = []
cpp_link_args = []
# environment defaults, can still be overridden on 
# the command line
werror = true
[project options]

[binaries]
c = ['x86_64-w64-mingw32-gcc','-m64']
cpp = ['x86_64-w64-mingw32-g++','-m64']
objc = ['x86_64-w64-mingw32-clang','-m64']
ar = ['x86_64-w64-mingw32-ar']
dlltool = ['x86_64-w64-mingw32-dlltool']
nm = ['x86_64-w64-mingw32-nm']
pkgconfig = ['x86_64-w64-mingw32-pkg-config']
pkg-config = ['x86_64-w64-mingw32-pkg-config']
ranlib = ['x86_64-w64-mingw32-ranlib']
strip = ['x86_64-w64-mingw32-strip']
widl = ['x86_64-w64-mingw32-widl']
windres = ['x86_64-w64-mingw32-windres']
windmc = ['x86_64-w64-mingw32-windmc']
[host_machine]
system = 'windows'
cpu_family = 'x86_64'
cpu = 'x86_64'
endian = 'little'


Should we not be passing the rust compiler target through
this meson options file by setting something like this

  rust = ['rustc', '--target', '$target_target_triple']


Also I don't think we should be requiring --rust-target-triple
to be passed by the user. For all the combinations we know &
test, we should have configure "do the right thing" and set a
suitable rust target triple based on the --cross-prefix argument
that is given, so there is no extra burden on users cross
compiling. Users should then only use --rust-target-triple
if our default logic is wrong for some reason.

> > > >    run_meson() {
> > > >      NINJA=$ninja $meson setup "$@" "$PWD" "$source_path"
> > > >    }
> > > > diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py
> > > > new file mode 100644
> > > > index 0000000000..d338effdaa
> > > > --- /dev/null
> > > > +++ b/scripts/cargo_wrapper.py
> > > > @@ -0,0 +1,211 @@
> > > > +#!/usr/bin/env python3
> > > > +# Copyright (c) 2020 Red Hat, Inc.
> > > > +# Copyright (c) 2023 Linaro Ltd.
> > > > +#
> > > > +# Authors:
> > > > +#  Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > > > +#  Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > +#
> > > > +# This work is licensed under the terms of the GNU GPL, version 2 or
> > > > +# later.  See the COPYING file in the top-level directory.
> > > > +
> > > > +import argparse
> > > > +import configparser
> > > > +import distutils.file_util
> > > > +import json
> > > > +import logging
> > > > +import os
> > > > +import os.path
> > > > +import re
> > > > +import subprocess
> > > > +import sys
> > > > +import pathlib
> > > > +import shutil
> > > > +import tomllib
> > > > +
> > > > +from pathlib import Path
> > > > +from typing import Any, Dict, List, Tuple
> > > > +
> > > > +RUST_TARGET_TRIPLES = (
> > > > +    "aarch64-unknown-linux-gnu",
> > > > +    "x86_64-unknown-linux-gnu",
> > > > +    "x86_64-apple-darwin",
> > > > +    "aarch64-apple-darwin",
> > > > +)
> > >
> > > Is this hardcoded to avoid calling `rustc --print target-list`?
> > >
> > > Or is this the support matrix? In that case it would be interesting to
> > > figure out the target triples for all host OSes and CPUs that QEMU is
> > > supported on.
> >
> > Yes, it's what I tested it on (the x86-64-apple-darwin part through rosetta).
> >
> > Do you think running -print target-list would be a better choice here?
> > This is only for providing the valid choices for the target triplet
> > CLI argument in argparse.
> 
> How about not restricting choices? If the user specifies an invalid
> choice then the compiler will fail with an error message. That seems
> okay and avoids the issue altogether.

Yes, we should not artifically limit the choices of target at all, as
we don't do that for existing cross compiler targets.

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