Re: [PATCH v4 0/4] cutils: Introduce bundle mechanism

[Paolo Bonzini <pbonzini@redhat.com>]

On 6/14/22 23:07, Akihiko Odaki wrote:
> Developers often run QEMU without installing. The bundle mechanism
> allows to look up files which should be present in installation even in
> such a situation.
> 
> It is a general mechanism and can find any files located relative
> to the installation tree. The build tree must have a new directory,
> qemu-bundle, to represent what files the installation tree would
> have for reference by the executables.
> 
> v4:
> * Add Daniel P. Berrangé to CC. Hopefully this helps merging his patch:
>    https://mail.gnu.org/archive/html/qemu-devel/2022-06/msg02276.html
> * Rebased to the latest QEMU.
> 
> v3:
> * Note that the bundle mechanism is for any files located relative to the
>    installation tree including but not limited to datadir. (Peter Maydell)
> * Fix "bridge" typo (Philippe Mathieu-Daudé)
> 
> v2: Rebased to the latest QEMU.

I like the idea, but I have a couple issues with the implementation:

- at the meson level, there is some repetition of mkdir and ln 
run_commands.  Perhaps you could just fill in a dictionary, and then do 
something like

   created_paths = {}
   foreach source, dest: var
     path = fs.parent(qemu_bundledir / dest)
     created_paths += {path: true}
   endforeach
   run_command('mkdir', '-p', created_paths.keys())
   foreach source, dest: var
     run_command('ln', '-sf', meson.project_source_root() / source,
                 qemu_bundledir / dest)
   endforeach

at the end of the toplevel meson.build.

- at the code level, it seems to me that this could reuse a lot of the 
logic of get_relocated_path().  In particular, I would include $prefix 
in the qemu_bundledir, so that the files in the bundle directory would 
look like qemu-bundle/usr/share/qemu/bios.bin: just like an install that 
uses DESTDIR.  Then, if an uninstalled QEMU somehow returns 
$exec_path/qemu-bundle/$prefix/$bindir for qemu_get_exec_dir() instead 
of $exec_path, then get_relocated_path() will automatically return the 
correct paths from qemu-bundle/.

Thanks,

Paolo