[Pkg-libvirt-maintainers] Bug#946594: [Libguestfs] Fwd: libguestfs incorrectly detects host CPU architecture

[Pino Toscano]

On Thursday, 12 December 2019 13:30:41 CET Richard W.M. Jones wrote:
> On Thu, Dec 12, 2019 at 11:01:02AM +0100, Vincent Danjean wrote:
> > [resend to the good (cloned) bug, sorry for the message in the original bug,
> > it was a mistake]
> > 
> >   Hi,
> > 
> > On Thu, 5 Dec 2019 11:12:56 +0000 "Richard W.M. Jones" <rjones at redhat.com> wrote:
> > > I believe this is a new bug and nothing much to do with:
> > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=775761
> > 
> >   So I just cloned the bug into #946594 in order to manage the bug reported
> > by Pierre Neyron.
> > 
> > > However about this new bug, what is supposed to happen is that
> > > common/mlstdutils/guestfs_config.ml is generated by ./configure with
> > > the correct @host_cpu@ substituted.  If that's not happening then it's
> > > a build issue on Debian of some kind.
> > 
> >   I closely look at the build system with him. It occurs that :
> > - the Debian package use an 'out-of-source' build (ie the build is
> >   *not* done into the source directories)
> > - Debian applies some patches to archive this
> > - but the current rules about guestfs_config.ml especially target
> >   the source directory (whereas ./configure generates it in the
> >   build directory, as for all other configure generated files)
> > 
> > Easy test :
> > - remove common/mlstdutils/guestfs_config.ml in the source directory
> > - build the Debian package (dpkg-buildpackage -us -uc)
> >   => the build fails with:
> > ocamlfind ocamlopt -g -annot -safe-string -warn-error CDEFLMPSUVYZX+52-3 -ccopt '-g -O2 -fdebug-prefix-map=/home/polaris/danjean/libguestfs/libguestfs=. -fstack-protector-strong -Wformat -Werror=format-security -fno-strict-overflow -Wno-strict-overflow' -package str,unix -I . -c ../../../../common/mlstdutils/std_utils.ml -o std_utils.cmx
> > File "_none_", line 1:
> > Warning 58: no cmx file was found in path for module Guestfs_config, and its interface was not compiled with -opaque
> > 
> > => the guestfs_config.ml generated in the build directory is not
> > taken into account.
> > => the Debian package is currently using the provided (x86-64)
> > guestfs_config.ml in the source directory and ignore the generated
> > one (in the build directory) on all architectures
> > => the Debian package is correctly built only on x86-64...
> > 
> >   The root of the problem is in subdir-rules.mk at the root of the
> > package. I found a fix the seems to work:
> > =============
> > --- subdir-rules.mk	2019-12-11 15:45:01.274572831 +0100
> > +++ subdir-rules.mk.fixed	2019-12-11 15:44:23.738419530 +0100
> > @@ -79,12 +79,12 @@
> >  guestfs_am_v_jar_ = $(guestfs_am_v_jar_ at AM_DEFAULT_V@)
> >  guestfs_am_v_jar_0 = @echo "  JAR     " $@;
> > 
> > -%.cmi: $(srcdir)/%.mli
> > +%.cmi: %.mli
> >  	$(guestfs_am_v_ocamlcmi)$(OCAMLFIND) ocamlc $(OCAMLFLAGS) $(OCAMLPACKAGES) -c $< -o $@
> > -%.cmo: $(srcdir)/%.ml
> > +%.cmo: %.ml
> >  	$(guestfs_am_v_ocamlc)$(OCAMLFIND) ocamlc $(OCAMLFLAGS) $(OCAMLPACKAGES) -c $< -o $@
> >  if HAVE_OCAMLOPT
> > -%.cmx: $(srcdir)/%.ml
> > +%.cmx: %.ml
> >  	$(guestfs_am_v_ocamlopt)$(OCAMLFIND) ocamlopt $(OCAMLFLAGS) $(OCAMLPACKAGES) -c $< -o $@
> >  endif
> > =============
> > 
> >   The idea is to let "make" to check itself in the build directory and then
> > in the source directory by not forcing a path and using the VPATH feature
> > (as for all internal automake rules)
> 
> Pino, Hillu, what do you think?

I tested it in a couple of cases, and seems to work.

However, I think the bigger problem here is that we ship in the release
tarballs files that are generated by configure. Without them, we would
have caught this issue much earlier, as simply removing
common/mlstdutils/guestfs_config.ml shows (as already pointed out by
Vincent Danjean).

Looking at the latest unstable release (1.41.8), it contains the
following files generated by configure:

common/mlstdutils/guestfs_config.ml (2)
daemon/daemon_config.ml (2)
python/t/tests_helper.py (1)
rust/Cargo.toml (3)
tests/daemon/captive-daemon.pm (1)

Regarding (1): I just sent a patch in a series [1].
Regarding (2): I'm testing a fix, it will take a bit more tough.
Regarding (3): the Rust binding (or cargo) does not seem to support
building outside the source directory, so for now I will add a rejection
check like we do in the PHP binding.

[1] https://www.redhat.com/archives/libguestfs/2019-December/msg00053.html

> >   As upstream does not seem to support out-of-source build, this problem should
> > not appear for it. So the fix can go into a debian patch (even if I think that
> > my patch is a no-op for a 'in-source' build)

It is somehow tricky to support in our case, however we take fixes for
it. A couple of fixes are included in the small patch series I sent [1].
Most of the library will build fine now, and the issues left (at least
according to what I see) are mostly in bindings: PHP, Perl, Rust.

[1] https://www.redhat.com/archives/libguestfs/2019-December/msg00053.html

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://alioth-lists.debian.net/pipermail/pkg-libvirt-maintainers/attachments/20191212/f3f7ce4a/attachment.sig>