On 03/21/22 16:50, Richard W.M. Jones wrote:
On Mon, Mar 21, 2022 at 03:24:46PM +0000, Daniel P. Berrangé wrote:
> On Mon, Mar 21, 2022 at 04:16:18PM +0100, Laszlo Ersek wrote:
>> It turns out that all we need from gnulib @ 253f29d8b391 is xstrtoull(),
>> ignore_value(), and assure(), when building on Fedora 35 anyway.
>
> I guess the question is what OS distros is p2v targetting ?
>
> With very rare exceptions, Fedora has never really needed much of the
> platform portability stuff from gnulib.
>
> The primary benefit from gnulib comes if needing to target FreeBSD,
> macOS, Windows or somewhat old Linux like older RHEL versions, fixing
> their many flaws to make them operate more like modern Fedora would.
From our point of view virt-p2v always runs inside a Linux live ISO
environment.
I don't even know if *BSD has a concept of live CDs but if they do and
they want P2V then patches are welcome. We can still virtualize *BSD
machines even using the Linux ISO, although it would require virt-v2v
to support *BSD which it does not right now.
>> Constructing this patch must be the most arbitrary "programming"
I've ever
>> done. It started with capturing the output of "gnulib-tool" (invoked
>> through "autogen.sh" -> "bootstrap"), then trimming it as
much as
>> possible, guided by libguestfs commit 0f54df53d26e ("build: Remove
>> gnulib.", 2021-04-08), then filling in any new gaps.
>>
>> (The "manywarnings" functionality falls victim to this patch as well --
if
>> that change was good enough for libguestfs, then so should it be for
>> virt-p2v.)
>
> That need not be the case. It is pretty trivial to use "manywarnings"
> functionality in isolation from anything else related to gnulib, as
> it is nicely self contained. Just copy the manywarnings.m4 and
> warnings.m4 files out of gnulib.git into your local m4/ directory.
>
> We did this for libvirt and quite a few other virt userspace projects
> before we eventually switched to using meson.
Yup, manywarnings is a nice feature of gnulib.
I think this (Laszlo's) patch looks great as it is now, so ACK.
(With or without adding manywarnings.)
Thanks!
Pushed as commit 16c50a347801 -- I really wanted to put this behind me,
and I tested the build (post-patch) with "--enable-werror" (and checked
with V=1 that -Wall -Werror were indeed present on the gcc command
lines), and there were no problems.
Thanks,
Laszlo