Re: [PATCH 3/5] meson: subprojects: replace submodules with wrap files

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

On Sat, May 27, 2023 at 11:28:49AM +0200, Paolo Bonzini wrote:
> Compared to submodules, .wrap files have several advantages:
> 
> * option parsing and downloading is delegated to meson
> 
> * the commit is stored in a text file instead of a magic entry in the
>   git tree object
> 
> * we could stop shipping external dependencies that are only used as a
>   fallback, but not break compilation on platforms that lack them.
>   For example it may make sense to download dtc at build time, controlled
>   by --enable-download, even when building from a tarball.  Right now,
>   this patch does the opposite: make-release treats dtc like libvfio-user
>   (which is not stable API and therefore hasn't found its way into any
>   distros) and keycodemap (which is a copylib, for better or worse).
> 
> dependency() can fall back to a wrap automatically.  However, this
> is only possible for libraries that come with a .pc file, and this
> is not very common for libfdt even though the upstream project in
> principle provides it; it also removes the control that we provide with
> --enable-fdt={system,internal}.  Therefore, the logic to pick system
> vs. internal libfdt is left untouched.
> 
> --enable-fdt=git is removed; it was already a synonym for
> --enable-fdt=internal.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  .gitlab-ci.d/buildtest-template.yml |  1 +
>  .gitmodules                         |  9 ----
>  configure                           | 75 ++++++++++++-----------------
>  meson.build                         | 15 +-----
>  scripts/archive-source.sh           | 11 ++++-
>  scripts/make-release                |  5 ++
>  subprojects/.gitignore              |  3 ++
>  subprojects/dtc                     |  1 -
>  subprojects/dtc.wrap                |  4 ++
>  subprojects/keycodemapdb            |  1 -
>  subprojects/keycodemapdb.wrap       |  4 ++
>  subprojects/libvfio-user            |  1 -
>  subprojects/libvfio-user.wrap       |  4 ++
>  13 files changed, 61 insertions(+), 73 deletions(-)
>  delete mode 160000 subprojects/dtc
>  create mode 100644 subprojects/dtc.wrap
>  delete mode 160000 subprojects/keycodemapdb
>  create mode 100644 subprojects/keycodemapdb.wrap
>  delete mode 160000 subprojects/libvfio-user
>  create mode 100644 subprojects/libvfio-user.wrap
> 
> diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml
> index c9f2e737c09a..3c997d7265b2 100644
> --- a/.gitlab-ci.d/buildtest-template.yml
> +++ b/.gitlab-ci.d/buildtest-template.yml
> @@ -44,6 +44,7 @@
>    script:
>      - scripts/git-submodule.sh update
>          $(sed -n '/GIT_SUBMODULES=/ s/.*=// p' build/config-host.mak)
> +    - meson subprojects download $(cd build/subprojects && echo *)
>      - cd build
>      - find . -type f -exec touch {} +
>      # Avoid recompiling by hiding ninja with NINJA=":"
> diff --git a/.gitmodules b/.gitmodules
> index f8b2ddf3877c..a934c85e4e40 100644
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -13,9 +13,6 @@
>  [submodule "roms/qemu-palcode"]
>  	path = roms/qemu-palcode
>  	url = https://gitlab.com/qemu-project/qemu-palcode.git
> -[submodule "subprojects/dtc"]
> -	path = subprojects/dtc
> -	url = https://gitlab.com/qemu-project/dtc.git
>  [submodule "roms/u-boot"]
>  	path = roms/u-boot
>  	url = https://gitlab.com/qemu-project/u-boot.git
> @@ -25,9 +22,6 @@
>  [submodule "roms/QemuMacDrivers"]
>  	path = roms/QemuMacDrivers
>  	url = https://gitlab.com/qemu-project/QemuMacDrivers.git
> -[submodule "subprojects/keycodemapdb"]
> -	path = subprojects/keycodemapdb
> -	url = https://gitlab.com/qemu-project/keycodemapdb.git
>  [submodule "roms/seabios-hppa"]
>  	path = roms/seabios-hppa
>  	url = https://gitlab.com/qemu-project/seabios-hppa.git
> @@ -55,6 +49,3 @@
>  [submodule "tests/lcitool/libvirt-ci"]
>  	path = tests/lcitool/libvirt-ci
>  	url = https://gitlab.com/libvirt/libvirt-ci.git
> -[submodule "subprojects/libvfio-user"]
> -	path = subprojects/libvfio-user
> -	url = https://gitlab.com/qemu-project/libvfio-user.git
> diff --git a/configure b/configure
> index 2f908660e4f1..1d1b8736c0eb 100755
> --- a/configure
> +++ b/configure
> @@ -253,7 +253,7 @@ else
>      git_submodules_action="ignore"
>  fi
>  
> -git_submodules="subprojects/keycodemapdb"
> +git_submodules=""
>  git="git"
>  debug_tcg="no"
>  docs="auto"
> @@ -269,7 +269,6 @@ python=
>  download="enabled"
>  bindir="bin"
>  skip_meson=no
> -vfio_user_server="disabled"
>  use_containers="yes"
>  gdb_bin=$(command -v "gdb-multiarch" || command -v "gdb")
>  gdb_arches=""
> @@ -281,16 +280,13 @@ unset target_list_exclude
>  
>  # The following Meson options are handled manually (still they
>  # are included in the automatically generated help message)
> -
> -# 1. Track which submodules are needed
> -fdt="auto"
> -
> -# 2. Automatically enable/disable other options
> +# because they automatically enable/disable other options
>  tcg="auto"
>  cfi="false"
>  
> -# 3. Need to check for -static-pie before Meson runs.  Also,
> -# Meson has PIE as a boolean rather than enabled/disabled/auto.
> +# Meson has PIE as a boolean rather than enabled/disabled/auto,
> +# and we also need to check for -static-pie before Meson runs
> +# which requires knowing whether --static is enabled.
>  pie=""
>  static="no"
>  
> @@ -743,14 +739,6 @@ for opt do
>    ;;
>    --disable-cfi) cfi="false"
>    ;;
> -  --disable-fdt) fdt="disabled"
> -  ;;
> -  --enable-fdt) fdt="enabled"
> -  ;;
> -  --enable-fdt=git) fdt="internal"
> -  ;;
> -  --enable-fdt=*) fdt="$optarg"
> -  ;;
>    --with-git=*) git="$optarg"
>    ;;
>    --with-git-submodules=*)
> @@ -774,10 +762,6 @@ for opt do
>    ;;
>    --gdb=*) gdb_bin="$optarg"
>    ;;
> -  --enable-vfio-user-server) vfio_user_server="enabled"
> -  ;;
> -  --disable-vfio-user-server) vfio_user_server="disabled"
> -  ;;
>    # everything else has the same name in configure and meson
>    --*) meson_option_parse "$opt" "$optarg"
>    ;;
> @@ -789,6 +773,30 @@ if test "$plugins" = "yes" -a "$tcg" = "disabled"; then
>      error_exit "Can't enable plugins on non-TCG builds"
>  fi
>  
> +if ! test -f "$source_path/subprojects/keycodemapdb/README" \
> +    && test -f "$download" = disabled
> +then
> +    echo
> +    echo "ERROR: missing subprojects"
> +    echo
> +    if test -e "$source_path/.git"; then
> +        echo "--disable-download specified but subprojects were not"
> +        echo "checked out.  Please download subprojects."

Building from tarball will be ok as we bundle everything affected. This
errror will hit anyone building from git with --disable-download though.

How should developers/users download the subprojects ? Can we tell them
what command is needed ?


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