[libvirt] [PATCH 0/2] Fix 0.9.7 for mingw

I'm proposing two options to fix the build failure mentioned here: https://www.redhat.com/archives/libvir-list/2011-November/msg00309.html without having to cut a release of 0.9.8. Note that this email is presenting a diff of a diff - that is, patch 2/2 is a diff that creates two new files, whose contents are then diffs themselves; it is those resulting diffs that get used in two contexts - the gnulib local diffs when building libvirt.git, and the application of those diffs to the non-version-controlled libvirt-0.9.7/gnulib/lib files when backporting the changes for building a mingw rpm. Hopefully that doesn't confuse you too much. Option 1: Upstream applies both of these patches, then the fix for building the mingw32-libvirt RPM would be to take the resulting diff files in patch 2/2 and apply those directly to the gnulib/lib files present in the 0.9.7 tarball expansion (no .gnulib submodule updated, and no gnulib-tool run introduced into the spec file). Later, when upstream gnulib is fixed, we revert patch 2/2 at the same time we update our .gnulib submodule. Option 2 would be to apply patch 1/2, but then wait until the fixes in patch 2/2 are present in upstream gnulib, and just update .gnulib directly instead of bothering with local diffs. Personally, I prefer option 1. I've tested this by running ./autobuild.sh, which builds both Linux and cross-builds to mingw, validating that the diffs apply correctly so that mingw builds. Eric Blake (2): build: allow for local gnulib diffs build: fix mingw build of gnulib openpty autogen.sh | 3 +++ bootstrap.conf | 6 ++++-- cfg.mk | 1 + gnulib/local/lib/openpty.c.diff | 26 ++++++++++++++++++++++++++ gnulib/local/lib/pty.in.h.diff | 13 +++++++++++++ 5 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 gnulib/local/lib/openpty.c.diff create mode 100644 gnulib/local/lib/pty.in.h.diff -- 1.7.4.4

Commit f7bd00c12 pulled in a gnulib module that fails to compile on mingw. While it would be nice to pull in a newer version of .gnulib that fixes this, it is difficult to backport any .gnulib update to older releases. So, it makes sense to take advantage of gnulib-tool's ability to support local diffs, where we can apply specific diffs in our use of gnulib without waiting for upstream gnulib to pick up those changes, as well as avoiding a wholesale .gnulib update. The existence of local diffs will also make it easier to backport fixes against a tarball (as long as a tarball and libvirt.git share the same .gnulib commit, then the tarball can be patched by applying the same local diffs as a post-release libvirt.git commit, without having to rerun an entire gnulib-tool bootstrap). This patch introduces the framework for supporting local diffs, without actually introducing any. * bootstrap.conf (local_gl_dir): New variable. * autogen.sh (bootstrap_hash): Hash any local diffs, to force a re-bootstrap if just diffs change. * cfg.mk (_update_required): Likewise. --- autogen.sh | 3 +++ bootstrap.conf | 6 ++++-- cfg.mk | 1 + 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/autogen.sh b/autogen.sh index b64521e..f1591d8 100755 --- a/autogen.sh +++ b/autogen.sh @@ -41,10 +41,13 @@ fi # is required. The first is just the SHA1 that selects a gnulib snapshot. # The second ensures that whenever we change the set of gnulib modules used # by this package, we rerun bootstrap to pull in the matching set of files. +# The third ensures that whenever we change the set of local gnulib diffs, +# we rerun bootstrap to pull in those diffs. bootstrap_hash() { git submodule status | sed 's/^[ +-]//;s/ .*//' git hash-object bootstrap.conf + git ls-tree -d HEAD gnulib/local | awk '{print $3}' } # Ensure that whenever we pull in a gnulib update or otherwise change to a diff --git a/bootstrap.conf b/bootstrap.conf index 4557d2d..6498aba 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -160,9 +160,10 @@ fi # Tell gnulib to: # require LGPLv2+ +# apply any local diffs in gnulib/local/ dir # put *.m4 files in new gnulib/m4/ dir -# put *.[ch] files in new gnulib/lib/ dir. -# import gnulib tests in new gnulib/tests/ dir. +# put *.[ch] files in new gnulib/lib/ dir +# import gnulib tests in new gnulib/tests/ dir gnulib_name=libgnu m4_base=gnulib/m4 source_base=gnulib/lib @@ -172,6 +173,7 @@ gnulib_tool_option_extras="\ --with-tests\ --avoid=pt_chown\ " +local_gl_dir=gnulib/local # Convince bootstrap to use multiple m4 directories. : ${ACLOCAL=aclocal} diff --git a/cfg.mk b/cfg.mk index 463ce0c..574c7a4 100644 --- a/cfg.mk +++ b/cfg.mk @@ -627,6 +627,7 @@ ifeq (0,$(MAKELEVEL)) test -f po/Makevars || { echo 1; exit; }; \ actual=$$(git submodule status | $(_submodule_hash); \ git hash-object bootstrap.conf; \ + git ls-tree -d HEAD gnulib/local | awk '{print $$3}'; \ git diff .gnulib); \ stamp="$$($(_submodule_hash) $(_curr_status) 2>/dev/null)"; \ test "$$stamp" = "$$actual"; echo $$?) -- 1.7.4.4

On Tue, Nov 08, 2011 at 05:37:19PM -0700, Eric Blake wrote:
Commit f7bd00c12 pulled in a gnulib module that fails to compile on mingw. While it would be nice to pull in a newer version of .gnulib that fixes this, it is difficult to backport any .gnulib update to older releases. So, it makes sense to take advantage of gnulib-tool's ability to support local diffs, where we can apply specific diffs in our use of gnulib without waiting for upstream gnulib to pick up those changes, as well as avoiding a wholesale .gnulib update. The existence of local diffs will also make it easier to backport fixes against a tarball (as long as a tarball and libvirt.git share the same .gnulib commit, then the tarball can be patched by applying the same local diffs as a post-release libvirt.git commit, without having to rerun an entire gnulib-tool bootstrap).
This patch introduces the framework for supporting local diffs, without actually introducing any.
* bootstrap.conf (local_gl_dir): New variable. * autogen.sh (bootstrap_hash): Hash any local diffs, to force a re-bootstrap if just diffs change. * cfg.mk (_update_required): Likewise. --- autogen.sh | 3 +++ bootstrap.conf | 6 ++++-- cfg.mk | 1 + 3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/autogen.sh b/autogen.sh index b64521e..f1591d8 100755 --- a/autogen.sh +++ b/autogen.sh @@ -41,10 +41,13 @@ fi # is required. The first is just the SHA1 that selects a gnulib snapshot. # The second ensures that whenever we change the set of gnulib modules used # by this package, we rerun bootstrap to pull in the matching set of files. +# The third ensures that whenever we change the set of local gnulib diffs, +# we rerun bootstrap to pull in those diffs. bootstrap_hash() { git submodule status | sed 's/^[ +-]//;s/ .*//' git hash-object bootstrap.conf + git ls-tree -d HEAD gnulib/local | awk '{print $3}' }
# Ensure that whenever we pull in a gnulib update or otherwise change to a diff --git a/bootstrap.conf b/bootstrap.conf index 4557d2d..6498aba 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -160,9 +160,10 @@ fi
# Tell gnulib to: # require LGPLv2+ +# apply any local diffs in gnulib/local/ dir # put *.m4 files in new gnulib/m4/ dir -# put *.[ch] files in new gnulib/lib/ dir. -# import gnulib tests in new gnulib/tests/ dir. +# put *.[ch] files in new gnulib/lib/ dir +# import gnulib tests in new gnulib/tests/ dir gnulib_name=libgnu m4_base=gnulib/m4 source_base=gnulib/lib @@ -172,6 +173,7 @@ gnulib_tool_option_extras="\ --with-tests\ --avoid=pt_chown\ " +local_gl_dir=gnulib/local
# Convince bootstrap to use multiple m4 directories. : ${ACLOCAL=aclocal} diff --git a/cfg.mk b/cfg.mk index 463ce0c..574c7a4 100644 --- a/cfg.mk +++ b/cfg.mk @@ -627,6 +627,7 @@ ifeq (0,$(MAKELEVEL)) test -f po/Makevars || { echo 1; exit; }; \ actual=$$(git submodule status | $(_submodule_hash); \ git hash-object bootstrap.conf; \ + git ls-tree -d HEAD gnulib/local | awk '{print $$3}'; \ git diff .gnulib); \ stamp="$$($(_submodule_hash) $(_curr_status) 2>/dev/null)"; \ test "$$stamp" = "$$actual"; echo $$?)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 11/09/2011 06:47 AM, Daniel P. Berrange wrote:
On Tue, Nov 08, 2011 at 05:37:19PM -0700, Eric Blake wrote:
Commit f7bd00c12 pulled in a gnulib module that fails to compile on mingw. While it would be nice to pull in a newer version of .gnulib that fixes this, it is difficult to backport any .gnulib update to older releases. So, it makes sense to take advantage of gnulib-tool's ability to support local diffs, where we can apply specific diffs in our use of gnulib without waiting for upstream gnulib to pick up those changes, as well as avoiding a wholesale .gnulib update. The existence of local diffs will also make it easier to backport fixes against a tarball (as long as a tarball and libvirt.git share the same .gnulib commit, then the tarball can be patched by applying the same local diffs as a post-release libvirt.git commit, without having to rerun an entire gnulib-tool bootstrap).
This patch introduces the framework for supporting local diffs, without actually introducing any.
ACK
Now pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Commit f7bd00c12 pulled in a gnulib module that fails to compile on mingw. Work around it while waiting for an upstream gnulib fix. * gnulib/local/lib/pty.in.h (openpty): Provide forward declarations of opaque structs not present on mingw. * gnulib/local/lib/openpty.c (openpty): Provide stub for mingw. --- gnulib/local/lib/openpty.c.diff | 26 ++++++++++++++++++++++++++ gnulib/local/lib/pty.in.h.diff | 13 +++++++++++++ 2 files changed, 39 insertions(+), 0 deletions(-) create mode 100644 gnulib/local/lib/openpty.c.diff create mode 100644 gnulib/local/lib/pty.in.h.diff diff --git a/gnulib/local/lib/openpty.c.diff b/gnulib/local/lib/openpty.c.diff new file mode 100644 index 0000000..f17e566 --- /dev/null +++ b/gnulib/local/lib/openpty.c.diff @@ -0,0 +1,26 @@ +diff --git c/lib/openpty.c i/lib/openpty.c +index c398db5..d61d5ba 100644 +--- c/lib/openpty.c ++++ i/lib/openpty.c +@@ -32,6 +32,21 @@ rpl_openpty (int *amaster, int *aslave, char *name, + (struct winsize *) winp); + } + ++#elif (defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__ /* mingw */ ++ ++# include <errno.h> ++ ++int ++openpty (int *amaster _GL_UNUSED, int *aslave _GL_UNUSED, ++ char *name _GL_UNUSED, ++ struct termios const *termp _GL_UNUSED, ++ struct winsize const *winp _GL_UNUSED) ++{ ++ /* Mingw lacks pseudo-terminals altogether. */ ++ errno = ENOSYS; ++ return -1; ++} ++ + #else /* AIX 5.1, HP-UX 11, IRIX 6.5, Solaris 10, mingw */ + + # include <fcntl.h> diff --git a/gnulib/local/lib/pty.in.h.diff b/gnulib/local/lib/pty.in.h.diff new file mode 100644 index 0000000..9470700 --- /dev/null +++ b/gnulib/local/lib/pty.in.h.diff @@ -0,0 +1,13 @@ +diff --git c/lib/pty.in.h i/lib/pty.in.h +index aff989c..00eecc6 100644 +--- c/lib/pty.in.h ++++ i/lib/pty.in.h +@@ -92,6 +92,8 @@ _GL_WARN_ON_USE (forkpty, "forkpty is not declared consistently - " + /* Create pseudo tty master slave pair and set terminal attributes + according to TERMP and WINP. Return handles for both ends in + *AMASTER and *ASLAVE, and return the name of the slave end in NAME. */ ++struct termios; ++struct winsize; + # if @REPLACE_OPENPTY@ + # if !(defined __cplusplus && defined GNULIB_NAMESPACE) + # undef openpty -- 1.7.4.4

On Tue, Nov 08, 2011 at 05:37:20PM -0700, Eric Blake wrote:
Commit f7bd00c12 pulled in a gnulib module that fails to compile on mingw. Work around it while waiting for an upstream gnulib fix.
* gnulib/local/lib/pty.in.h (openpty): Provide forward declarations of opaque structs not present on mingw. * gnulib/local/lib/openpty.c (openpty): Provide stub for mingw. --- gnulib/local/lib/openpty.c.diff | 26 ++++++++++++++++++++++++++ gnulib/local/lib/pty.in.h.diff | 13 +++++++++++++ 2 files changed, 39 insertions(+), 0 deletions(-) create mode 100644 gnulib/local/lib/openpty.c.diff create mode 100644 gnulib/local/lib/pty.in.h.diff
diff --git a/gnulib/local/lib/openpty.c.diff b/gnulib/local/lib/openpty.c.diff new file mode 100644 index 0000000..f17e566 --- /dev/null +++ b/gnulib/local/lib/openpty.c.diff @@ -0,0 +1,26 @@ +diff --git c/lib/openpty.c i/lib/openpty.c +index c398db5..d61d5ba 100644 +--- c/lib/openpty.c ++++ i/lib/openpty.c +@@ -32,6 +32,21 @@ rpl_openpty (int *amaster, int *aslave, char *name, + (struct winsize *) winp); + } + ++#elif (defined _WIN32 || defined __WIN32__) && !defined __CYGWIN__ /* mingw */ ++ ++# include <errno.h> ++ ++int ++openpty (int *amaster _GL_UNUSED, int *aslave _GL_UNUSED, ++ char *name _GL_UNUSED, ++ struct termios const *termp _GL_UNUSED, ++ struct winsize const *winp _GL_UNUSED) ++{ ++ /* Mingw lacks pseudo-terminals altogether. */ ++ errno = ENOSYS; ++ return -1; ++} ++ + #else /* AIX 5.1, HP-UX 11, IRIX 6.5, Solaris 10, mingw */ + + # include <fcntl.h> diff --git a/gnulib/local/lib/pty.in.h.diff b/gnulib/local/lib/pty.in.h.diff new file mode 100644 index 0000000..9470700 --- /dev/null +++ b/gnulib/local/lib/pty.in.h.diff @@ -0,0 +1,13 @@ +diff --git c/lib/pty.in.h i/lib/pty.in.h +index aff989c..00eecc6 100644 +--- c/lib/pty.in.h ++++ i/lib/pty.in.h +@@ -92,6 +92,8 @@ _GL_WARN_ON_USE (forkpty, "forkpty is not declared consistently - " + /* Create pseudo tty master slave pair and set terminal attributes + according to TERMP and WINP. Return handles for both ends in + *AMASTER and *ASLAVE, and return the name of the slave end in NAME. */ ++struct termios; ++struct winsize; + # if @REPLACE_OPENPTY@ + # if !(defined __cplusplus && defined GNULIB_NAMESPACE) + # undef openpty -- 1.7.4.4
ACK, these fix the mingw32 build for me Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 11/09/2011 06:46 AM, Daniel P. Berrange wrote:
On Tue, Nov 08, 2011 at 05:37:20PM -0700, Eric Blake wrote:
Commit f7bd00c12 pulled in a gnulib module that fails to compile on mingw. Work around it while waiting for an upstream gnulib fix.
* gnulib/local/lib/pty.in.h (openpty): Provide forward declarations of opaque structs not present on mingw. * gnulib/local/lib/openpty.c (openpty): Provide stub for mingw.
ACK, these fix the mingw32 build for me
Pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Tue, Nov 08, 2011 at 05:37:18PM -0700, Eric Blake wrote:
I'm proposing two options to fix the build failure mentioned here: https://www.redhat.com/archives/libvir-list/2011-November/msg00309.html without having to cut a release of 0.9.8.
BTW, slightly related, so see a non-fatal compile warning from gnulib on mingw32 too which i presume wants fixing upstream: CC sigaction.lo CC sigprocmask.lo sigprocmask.c: In function '_gl_raise_SIGPIPE': sigprocmask.c:349:1: warning: control reaches end of non-void function CC sleep.lo CC snprintf.lo Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 11/09/2011 06:48 AM, Daniel P. Berrange wrote:
On Tue, Nov 08, 2011 at 05:37:18PM -0700, Eric Blake wrote:
I'm proposing two options to fix the build failure mentioned here: https://www.redhat.com/archives/libvir-list/2011-November/msg00309.html without having to cut a release of 0.9.8.
BTW, slightly related, so see a non-fatal compile warning from gnulib on mingw32 too which i presume wants fixing upstream:
CC sigaction.lo CC sigprocmask.lo sigprocmask.c: In function '_gl_raise_SIGPIPE': sigprocmask.c:349:1: warning: control reaches end of non-void function
Thanks for pointing that out, and yes I'll fix it upstream in gnulib. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange
-
Eric Blake