On 2022/06/15 21:04, Daniel P. Berrangé wrote:
On Wed, Jun 15, 2022 at 01:42:58PM +0200, Paolo Bonzini wrote:
On 6/15/22 12:52, Daniel P. Berrangé wrote:
+ case QEMU_FILE_TYPE_HELPER:
+ rel_install_dir = "";
+ rel_build_dir = "";
+ default_install_dir = default_helper_dir;
+ break;
+
You're replacing ad hoc rules in Akihiko's meson.build with an ad hoc enum +
the corresponding "case"s here in qemu_find_file(). There is duplication
anyway, in this case between Meson and QEMU (plus QEMU needs to know about
two filesystem layouts).
IMHO this is simpler to deal with than the meson additions, and also
avoids the confusion of having files appearing in two places in the
build dir.
Thanks to Paolo's suggestion to unify the common code to build the
bundle tree, the required code for each bundled file is just a statement
now (something like: bundles += { destination: source }) in the v6.
Doing everything in Meson also allows to reuse the knowledge of the
build tree Meson already has. I do no longer think my patch series are
complicated more than yours. It even has less lines now.
There is still a room for improvements though. Particularly, the
installing code and bundle-tree code are still duplicate. For example,
pc-bios/meson.build now has the following code:
install_data(blobs, install_dir: qemu_datadir)
foreach f : blobs
bundles += { qemu_datadir / f: meson.current_source_dir() / f }
endforeach
It would be nice if it can be written like:
foreach f : blobs
bundle_data(qemu_datadir / f, f)
endforeach
Unfortunately Meson does not allow this.
Another problem is that the top-level meson.build is somewhat clutter.
In my opinion, it is a persistent problem of the build system but I
don't have a solution.
Anyway, I think my patch series is as close to the ideal as Meson
currently allows.
The confusion caused by the files appearing in two places in the
build tree should be minimal. qemu-bundle is implemented entirely with
symbolic links. If you know what is a symbolic link, you also know it is
an alias and the files appear in different places, and I expect everyone
hacking QEMU knows symbolic link.
If we really want to have the build dir look just like the install
dir though, why write custom meson commands per file type at all,
instead add a rule that always invokes
DESTDIR=$(BUILDDIR)/vroot ninja install
to populate a dir that's guaranteed identical to the install layout
Unfortunately Meson cannot define a rule which will be always invoked as
far as I know.
Regards,
Akihiko Odaki
Regards,
Daniel