Margaret Lewicka
2015-Feb-05 22:53 UTC
[Libguestfs] Patchable build problems on OS X 10.10
Hello, I'm attempting to create a Homebrew formula to get libguestfs to compile on Mac OS X. I've managed to achieve success with several monkey patches, but since Homebrew's policy is to contact maintainers about proper fixes in upstream, I would like to ask if there are any plans to fix these issues. I'm afraid I don't know C well enough to propose decent solutions myself. Here's a list of compilation issues for 1.28.6 on 10.10: * It's not possible to build Java bindings with Java 8, because javadoc fails on GuestFS.java with a hundred or so errors. * ruby/Makefile.in assumes a .so extension for the built file, while on Mac DLEXT appears to default to .bundle. * src/launch-libvirt.c uses SOCK_CLOEXEC and SOCK_NONBLOCK which don't exist on Darwin. * src/launch-libvirt.c needs to #include <sys/un.h> for sockaddr_un. The rest of the issues have been mentioned in https://www.redhat.com/archives/libguestfs/2013-October/msg00042.html, and I've reused RWMJ's patches. * fuse/guestunmount.c uses Linux-specific fusermount and '-v' option for fuser. * src/proto.c -- From Apple's xdr.h: "If your code invokes an xdrproc_t callback, it must be modified to pass a third parameter, which may simply be zero." * OS X does not provide open_memstream(). * src/guestfs-internal-frontend.h and gnulib/lib/error.c need monkey-patching to replace program_name with getprogname(). The patch I'm using right now to get libguestfs to successfully compile is here: https://gist.github.com/shulima/c194c5057e88dbd8f686 Configure flags are --disable-probes, --disable-daemon, --disable-appliance. -- Margaret
Richard W.M. Jones
2015-Feb-06 10:03 UTC
Re: [Libguestfs] Patchable build problems on OS X 10.10
On Thu, Feb 05, 2015 at 10:53:06PM +0000, Margaret Lewicka wrote:> Hello, > > I'm attempting to create a Homebrew formula to get libguestfs to > compile on Mac OS X. I've managed to achieve success with several > monkey patches, but since Homebrew's policy is to contact maintainers > about proper fixes in upstream, I would like to ask if there are any > plans to fix these issues. I'm afraid I don't know C well enough to > propose decent solutions myself.Thanks for looking at this. I am interested in getting libguestfs to work on Mac OS X again. I have included the patch below for review. As a general comment, it's easier for us if the patch is split into separate reviewable commits, and then sent to the mailing list using 'git send-email'.> diff --git fuse/guestunmount.c fuse/guestunmount.c > index c36c336..1ab7ff5 100644 > --- fuse/guestunmount.c > +++ fuse/guestunmount.c > @@ -257,7 +257,7 @@ do_fusermount (const char *mountpoint, char **error_rtn) > /* We have to parse error messages from fusermount, so ... */ > setenv ("LC_ALL", "C", 1); > > - execlp ("fusermount", "fusermount", "-u", mountpoint, NULL); > + execlp ("/sbin/umount", "umount", mountpoint, NULL);fusermount is needed on Linux, so this is an example of a patch which could be written instead as: #if !(defined __APPLE__ && defined __MACH__) execlp ("fusermount", "fusermount", "-u", mountpoint, NULL); #else execlp ("/sbin/umount", "umount", mountpoint, NULL); #endif> @@ -334,7 +334,14 @@ do_fuser (const char *mountpoint) > } > > if (pid == 0) { /* Child - run /sbin/fuser. */ > - execlp ("/sbin/fuser", "fuser", "-v", "-m", mountpoint, NULL); > + const char *cmd_prefix = "/bin/ps -p \"$(fuser -c "; > + const char *cmd_suffix = " 2>/dev/null)\" -o user,pid,comm 2>/dev/null"; > + char *cmd = malloc (strlen(cmd_prefix) + strlen(mountpoint) + strlen(cmd_suffix) + 1); > + if (cmd) { > + sprintf (cmd, "%s%s%s", cmd_prefix, mountpoint, cmd_suffix); > + execlp ("/bin/sh", "sh", "-c", cmd, NULL); > + free (cmd); > + } > _exit (EXIT_FAILURE); > } > > diff --git gnulib/lib/Makefile.in gnulib/lib/Makefile.in > index 426004d..6d1db79 100644 > --- gnulib/lib/Makefile.in > +++ gnulib/lib/Makefile.in > @@ -265,7 +265,7 @@ am_libgnu_la_OBJECTS = accept4.lo allocator.lo areadlink.lo \ > fd-hook.lo filenamecat-lgpl.lo filevercmp.lo full-read.lo \ > full-write.lo gettime.lo hash.lo hash-pjw.lo human.lo \ > i-ring.lo localcharset.lo glthread/lock.lo malloca.lo \ > - openat-die.lo openat-safer.lo pipe2.lo quotearg.lo \ > + open_memstream.lo openat-die.lo openat-safer.lo pipe2.lo quotearg.lo \ > read-file.lo safe-read.lo safe-write.lo save-cwd.lo sockets.lo \ > stat-time.lo strnlen1.lo sys_socket.lo tempname.lo \ > glthread/threadlib.lo timespec.lo unistd.lo dup-safer.lo \Changes in the gnulib/ directory come from gnulib, so need to be sent/suggested to them: https://www.gnu.org/software/gnulib/ [...]> diff --git ruby/Makefile.am ruby/Makefile.am > index f605188..d28d77b 100644 > --- ruby/Makefile.am > +++ ruby/Makefile.am > @@ -21,6 +21,8 @@ generator_built = \ > ext/guestfs/_guestfs.c \ > bindtests.rb > > +DLEXT := $(shell $(RUBY) -rrbconfig -e "puts RbConfig::CONFIG['DLEXT']") > + > EXTRA_DIST = \ > $(generator_built) \ > Rakefile.in \ > @@ -38,7 +40,7 @@ CLEANFILES = \ > ext/guestfs/*~ \ > ext/guestfs/extconf.h \ > ext/guestfs/_guestfs.o \ > - ext/guestfs/_guestfs.so \ > + ext/guestfs/_guestfs.$(DLEXT) \ > ext/guestfs/mkmf.log \ > ext/guestfs/Makefile > > @@ -59,7 +61,7 @@ install: > $(MKDIR_P) $(DESTDIR)$(RUBY_LIBDIR) > $(MKDIR_P) $(DESTDIR)$(RUBY_ARCHDIR) > $(INSTALL) -p -m 0644 $(srcdir)/lib/guestfs.rb $(DESTDIR)$(RUBY_LIBDIR) > - $(INSTALL) -p -m 0755 ext/guestfs/_guestfs.so $(DESTDIR)$(RUBY_ARCHDIR) > + $(INSTALL) -p -m 0755 ext/guestfs/_guestfs.$(DLEXT) $(DESTDIR)$(RUBY_ARCHDIR) > > TESTS = run-bindtests run-ruby-tests > > > diff --git ruby/Makefile.in ruby/Makefile.in > index 8718724..1693abb 100644 > --- ruby/Makefile.in > +++ ruby/Makefile.in > @@ -1428,6 +1428,7 @@ generator_built = \ > ext/guestfs/_guestfs.c \ > bindtests.rb > > +DLEXT := $(shell $(RUBY) -rrbconfig -e "puts RbConfig::CONFIG['DLEXT']") > EXTRA_DIST = \ > $(generator_built) \ > Rakefile.in \ > @@ -1445,7 +1446,7 @@ CLEANFILES = \ > ext/guestfs/*~ \ > ext/guestfs/extconf.h \ > ext/guestfs/_guestfs.o \ > - ext/guestfs/_guestfs.so \ > + ext/guestfs/_guestfs.$(DLEXT) \ > ext/guestfs/mkmf.log \ > ext/guestfs/Makefile > > @@ -1788,7 +1789,7 @@ $(top_builddir)/generator/generator: > @HAVE_RUBY_TRUE@ $(MKDIR_P) $(DESTDIR)$(RUBY_LIBDIR) > @HAVE_RUBY_TRUE@ $(MKDIR_P) $(DESTDIR)$(RUBY_ARCHDIR) > @HAVE_RUBY_TRUE@ $(INSTALL) -p -m 0644 $(srcdir)/lib/guestfs.rb $(DESTDIR)$(RUBY_LIBDIR) > -@HAVE_RUBY_TRUE@ $(INSTALL) -p -m 0755 ext/guestfs/_guestfs.so $(DESTDIR)$(RUBY_ARCHDIR) > +@HAVE_RUBY_TRUE@ $(INSTALL) -p -m 0755 ext/guestfs/_guestfs.$(DLEXT) $(DESTDIR)$(RUBY_ARCHDIR) > > # Tell versions [3.59,3.63) of GNU make to not export all variables. > # Otherwise a system limit (for SysV at least) may be exceeded. > diff --git ruby/Rakefile.in ruby/Rakefile.in > index 94a25fd..1dfc600 100644 > --- ruby/Rakefile.in > +++ ruby/Rakefile.in > @@ -40,9 +40,11 @@ end > PKG_NAME='@PACKAGE_NAME@' > PKG_VERSION='@PACKAGE_VERSION@' > > +DLEXT=RbConfig::CONFIG['DLEXT'] > + > EXT_CONF='@abs_builddir@/ext/guestfs/extconf.rb' > MAKEFILE='@builddir@/ext/guestfs/Makefile' > -GUESTFS_MODULE='@builddir@/ext/guestfs/_guestfs.so' > +GUESTFS_MODULE="@builddir@/ext/guestfs/_guestfs.#{DLEXT}" > GUESTFS_SRC='@srcdir@/ext/guestfs/_guestfs.c' > > CLEAN.include [ "@builddir@/ext/**/*.o", GUESTFS_MODULE,The DLEXT changes in the ruby directory all look good to me. I have pushed these upstream: https://github.com/libguestfs/libguestfs/commit/eaae0b614c59f799885ee940117c4fb507a1f2d3> diff --git src/guestfs-internal-frontend.h src/guestfs-internal-frontend.h > index ba3ddde..f9079c5 100644 > --- src/guestfs-internal-frontend.h > +++ src/guestfs-internal-frontend.h > @@ -167,7 +167,7 @@ extern GUESTFS_DLL_PUBLIC int guestfs___add_libvirt_dom (guestfs_h *g, virDomain > #if HAVE_DECL_PROGRAM_INVOCATION_SHORT_NAME == 1 > # define program_name program_invocation_short_name > #else > -# define program_name "libguestfs" > +# define program_name getprogname() > #endifLinux doesn't have getprogname. One way around this is to add getprogname to the list of functions in configure.ac AC_CHECK_FUNCS. That will cause a macro to be defined which you can use like this: #if HAVE_DECL_PROGRAM_INVOCATION_SHORT_NAME == 1 # define program_name program_invocation_short_name #elif HAVE_GETPROGNAME # define program_name getprogname() #else # define program_name "libguestfs" #endif> /* Close all file descriptors matching the condition. */ > diff --git src/launch-libvirt.c src/launch-libvirt.c > index aaa8501..c0adc80 100644 > --- src/launch-libvirt.c > +++ src/launch-libvirt.c > @@ -57,6 +57,18 @@ > #include "guestfs-internal-actions.h" > #include "guestfs_protocol.h" > > +/* Fixes for Mac OS X */ > +#if defined __APPLE__ && defined __MACH__ > +#include <sys/un.h> > +#endif > +#ifndef SOCK_CLOEXEC > +# define SOCK_CLOEXEC O_CLOEXEC > +#endif > +#ifndef SOCK_NONBLOCK > +# define SOCK_NONBLOCK O_NONBLOCK > +#endif > +/* End of fixes for Mac OS X */ > + > /* Check minimum required version of libvirt. The libvirt backend > * is new and not the default, so we can get away with forcing > * people who want to try it to have a reasonably new version ofI pushed this hunk upstream: https://github.com/libguestfs/libguestfs/commit/7ddf6bcbfdc66855b594afaaacdc4998177f2286> diff --git src/proto.c src/proto.c > index 8001c8c..53d1d6b 100644 > --- src/proto.c > +++ src/proto.c > @@ -252,7 +252,7 @@ guestfs___send (guestfs_h *g, int proc_nr, > * have no parameters. > */ > if (xdrp) { > - if (!(*xdrp) (&xdr, args)) { > + if (!(*xdrp) (&xdr, args, 0)) { > error (g, _("dispatch failed to marshal args")); > return -1; > } > @@ -681,7 +681,7 @@ guestfs___recv (guestfs_h *g, const char *fn, > return -1; > } > } else { > - if (xdrp && ret && !xdrp (&xdr, ret)) { > + if (xdrp && ret && !xdrp (&xdr, ret, 0)) { > error (g, "%s: failed to parse reply", fn); > xdr_destroy (&xdr); > return -1;This would break glibc's implementation of XDR. Not quite sure how to solve this except with #if defined ... #endif. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Richard W.M. Jones
2015-Feb-06 11:20 UTC
Re: [Libguestfs] Patchable build problems on OS X 10.10
[Let's keep the replies on the mailing list] On Fri, Feb 06, 2015 at 11:06:45AM +0000, Margaret Lewicka wrote:> https://gist.github.com/shulima/981973b7500b2adaac88Just a note that the the file which is patched here is actually generated, so the source file in git is `generator/java.ml'. In Fedora, we have something called java-1.8.0-openjdk-headless. Is that the same as Java[doc] 8? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Margaret Lewicka
2015-Feb-07 02:51 UTC
Re: [Libguestfs] Patchable build problems on OS X 10.10
On 6 February 2015 at 10:03, Richard W.M. Jones <rjones@redhat.com> wrote: [...]> Linux doesn't have getprogname. One way around this is to add > getprogname to the list of functions in configure.ac AC_CHECK_FUNCS. > That will cause a macro to be defined which you can use like this: > > #if HAVE_DECL_PROGRAM_INVOCATION_SHORT_NAME == 1 > # define program_name program_invocation_short_name > #elif HAVE_GETPROGNAME > # define program_name getprogname() > #else > # define program_name "libguestfs" > #endifUnfortunately that still doesn't help the issue with gnulib, since it expects char *program_name to be set on non-Linux platforms. -- M.
Pino Toscano
2015-Feb-09 09:56 UTC
Re: [Libguestfs] Patchable build problems on OS X 10.10
On Friday 06 February 2015 10:03:37 Richard W.M. Jones wrote:> On Thu, Feb 05, 2015 at 10:53:06PM +0000, Margaret Lewicka wrote: > > Hello, > > > > I'm attempting to create a Homebrew formula to get libguestfs to > > compile on Mac OS X. I've managed to achieve success with several > > monkey patches, but since Homebrew's policy is to contact maintainers > > about proper fixes in upstream, I would like to ask if there are any > > plans to fix these issues. I'm afraid I don't know C well enough to > > propose decent solutions myself. > > Thanks for looking at this. I am interested in getting libguestfs to > work on Mac OS X again.Thanks for your patches! In the past months I've done a number of commits to help making libguestfs (without guestfsd & appliance) build and work on FreeBSD.> I have included the patch below for review. As a general comment, > it's easier for us if the patch is split into separate reviewable > commits, and then sent to the mailing list using 'git send-email'. > > > diff --git fuse/guestunmount.c fuse/guestunmount.c > > index c36c336..1ab7ff5 100644 > > --- fuse/guestunmount.c > > +++ fuse/guestunmount.c > > @@ -257,7 +257,7 @@ do_fusermount (const char *mountpoint, char **error_rtn) > > /* We have to parse error messages from fusermount, so ... */ > > setenv ("LC_ALL", "C", 1); > > > > - execlp ("fusermount", "fusermount", "-u", mountpoint, NULL); > > + execlp ("/sbin/umount", "umount", mountpoint, NULL); > > fusermount is needed on Linux, so this is an example of a patch > which could be written instead as: > > #if !(defined __APPLE__ && defined __MACH__) > execlp ("fusermount", "fusermount", "-u", mountpoint, NULL); > #else > execlp ("/sbin/umount", "umount", mountpoint, NULL); > #endifOr rather use fusermount only on Linux and umount (with no path) elsewhere.> > diff --git src/launch-libvirt.c src/launch-libvirt.c > > index aaa8501..c0adc80 100644 > > --- src/launch-libvirt.c > > +++ src/launch-libvirt.c > > @@ -57,6 +57,18 @@ > > #include "guestfs-internal-actions.h" > > #include "guestfs_protocol.h" > > > > +/* Fixes for Mac OS X */ > > +#if defined __APPLE__ && defined __MACH__ > > +#include <sys/un.h> > > +#endif > > +#ifndef SOCK_CLOEXEC > > +# define SOCK_CLOEXEC O_CLOEXEC > > +#endif > > +#ifndef SOCK_NONBLOCK > > +# define SOCK_NONBLOCK O_NONBLOCK > > +#endif > > +/* End of fixes for Mac OS X */ > > + > > /* Check minimum required version of libvirt. The libvirt backend > > * is new and not the default, so we can get away with forcing > > * people who want to try it to have a reasonably new version ofThis IMHO is clearly wrong: the O_* constants are for open() & friends, not for socket & socket4. Theoretically, we could switch the socket() usages in launch-libvirt.c to socket4(), which can be replaced by gnulib if missing (we already use the "accept4" gnulib module). On the other hand, it seems that such gnulib emulation does not provide SOCK_NONBLOCK, so either a) fix that in gnulib b) use the "nonblocking" gnulib module, using set_nonblocking_flag() instead of SOCK_NONBLOCK Also, sys/un.h is POSIX [1], so it can be included unconditionally. [1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_un.h.html Thanks, -- Pino Toscano