[libvirt] [PATCH] Conditionally include termios.h

The configure.ac file checks for termios.h, but its use is not protected by HAVE_TERMIOS_H. This breaks the build on mingw32 * src/util/util.c: Conditionally include termios.h --- src/util/util.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 586baee..b6b9712 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -43,7 +43,9 @@ #endif #include <string.h> #include <signal.h> -#include <termios.h> +#if HAVE_TERMIOS_H +# include <termios.h> +#endif #include "c-ctype.h" #ifdef HAVE_PATHS_H -- 1.7.2.3

On 10/13/2010 04:34 AM, Daniel P. Berrange wrote:
The configure.ac file checks for termios.h, but its use is not protected by HAVE_TERMIOS_H. This breaks the build on mingw32
* src/util/util.c: Conditionally include termios.h
NACK. Gnulib provides the termios header replacement, we just need to fix bootstrap.conf to use the gnulib module. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 10/13/2010 06:16 AM, Eric Blake wrote:
On 10/13/2010 04:34 AM, Daniel P. Berrange wrote:
The configure.ac file checks for termios.h, but its use is not protected by HAVE_TERMIOS_H. This breaks the build on mingw32
* src/util/util.c: Conditionally include termios.h
NACK. Gnulib provides the termios header replacement, we just need to fix bootstrap.conf to use the gnulib module.
And looking further, bootstrap.conf already includes 'termios'. What exactly was your failure? Besides, making things conditional should have caused a 'make syntax-check' failure, since we now try to enforce that all gnulib replacement headers be used unconditionally. Is there a bug in that syntax check rule? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Wed, Oct 13, 2010 at 06:16:03AM -0600, Eric Blake wrote:
On 10/13/2010 04:34 AM, Daniel P. Berrange wrote:
The configure.ac file checks for termios.h, but its use is not protected by HAVE_TERMIOS_H. This breaks the build on mingw32
* src/util/util.c: Conditionally include termios.h
NACK. Gnulib provides the termios header replacement, we just need to fix bootstrap.conf to use the gnulib module.
On mingw32 the gnulib replacement does not seem to work though, hence configure is undefining HAVE_TERMIOS_H in config.h. The gnulib replacement is doing an include_next <termios.h> but there is no next one to include. If the gnulib can be relied upon to always provide the header, then surely we don't need the termios.h header check in configure.ac ? Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

[adding bug-gnulib] On 10/13/2010 06:22 AM, Daniel P. Berrange wrote:
On Wed, Oct 13, 2010 at 06:16:03AM -0600, Eric Blake wrote:
On 10/13/2010 04:34 AM, Daniel P. Berrange wrote:
The configure.ac file checks for termios.h, but its use is not protected by HAVE_TERMIOS_H. This breaks the build on mingw32
* src/util/util.c: Conditionally include termios.h
NACK. Gnulib provides the termios header replacement, we just need to fix bootstrap.conf to use the gnulib module.
On mingw32 the gnulib replacement does not seem to work though, hence configure is undefining HAVE_TERMIOS_H in config.h.
The gnulib replacement is doing an include_next<termios.h> but there is no next one to include.
Then that's a bug in the gnulib replacement. I'll probably be able to fix that today.
If the gnulib can be relied upon to always provide the header, then surely we don't need the termios.h header check in configure.ac ?
Yep, I can do that cleanup in libvirt as well. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

* .gnulib: Update to latest, for termios fix. * configure.ac (AC_CHECK_HEADERS): Drop redundent check. Reported by Daniel P. Berrange. --- This fixes the problem in a more maintainable manner. Tested via ./autobuild.sh on a Fedora host with mingw cross-compiler. * .gnulib 2bb63bf...b6d1430 (32):
termios: fix compilation on mingw git-version-gen: don't require that .git/ be in the current dir test-select: avoid warn_unused_result warnings test-symlinkat: remove declaration of unused local test-inttostr: avoid shadowing warnings stdlib: Allow multiple gnulib generated replacements to coexist. fix a documentation typo futimens: work around Solaris 11 bug Indentation. test-futimens: avoid unwarranted test failure on Solaris 5.11 Indentation. spawn.in.h: make indentation consistent with parentheses Fix mismatched parens in previous commit rewrite int foo[2*X-1] to verify(X) or to int foo[X?1:-1] prefer (X ? 1 : -1) when converting from boolean (1,0) to int (1,-1) autoupdate time: enforce recent POSIX ruling that time_t is integral fdopendir: fix a bug on systems lacking openat and /proc support sys_select: Avoid warning due to undeclared memset() on OpenBSD 4.5. nanosleep: Make replacement POSIX compliant. bootstrap: add hook for altering gnulib.mk, for Bison bootstrap: reformat for readability docs: update cygwin progress autoupdate parse-datetime: avoid compilation failure on OpenBSD 4.7 docs: update cygwin progress docs: update parse-datetime history cygwin: use more robust version check string, sys_select: Avoid #including large headers unless necessary. memmem, strstr, strcasestr: fix bug with long periodic needle maint: fix order of ChangeLog entries parse-datetime: do some more renaming
.gnulib | 2 +- configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.gnulib b/.gnulib index 2bb63bf..b6d1430 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 2bb63bfb25474ea147ee9f1523c0337997359a4c +Subproject commit b6d1430494cdd252cd52eca6abf88b1a00f6c983 diff --git a/configure.ac b/configure.ac index bd92b65..b868e50 100644 --- a/configure.ac +++ b/configure.ac @@ -109,7 +109,7 @@ LIBS=$old_libs dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h \ - termios.h sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h]) + sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h]) AC_CHECK_LIB([intl],[gettext],[]) -- 1.7.2.3

On 10/14/2010 09:25 AM, Eric Blake wrote: <snip>
diff --git a/.gnulib b/.gnulib index 2bb63bf..b6d1430 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 2bb63bfb25474ea147ee9f1523c0337997359a4c +Subproject commit b6d1430494cdd252cd52eca6abf88b1a00f6c983
Hmmmm, this looked a bit weird, so tried the patch: $ git am ../fix_mingw_build.txt Applying: build: fix mingw build warning: unable to rmdir .gnulib: Directory not empty $ Is ".gnulib" really the right target for changing?

On Thu, Oct 14, 2010 at 03:31:27PM +1100, Justin Clift wrote:
On 10/14/2010 09:25 AM, Eric Blake wrote: <snip>
diff --git a/.gnulib b/.gnulib index 2bb63bf..b6d1430 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 2bb63bfb25474ea147ee9f1523c0337997359a4c +Subproject commit b6d1430494cdd252cd52eca6abf88b1a00f6c983
Hmmmm, this looked a bit weird, so tried the patch:
$ git am ../fix_mingw_build.txt Applying: build: fix mingw build warning: unable to rmdir .gnulib: Directory not empty $
Is ".gnulib" really the right target for changing?
Yes, this is one of those bits of GIT black-magic. .gnulib isn't a file, its a checkout of an entire 3rd party GIT repository, at the specified changeset has. Not sure why git am isn't happy about appliny it though. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On 10/14/2010 07:47 PM, Daniel P. Berrange wrote: <snip>
Yes, this is one of those bits of GIT black-magic. .gnulib isn't a file, its a checkout of an entire 3rd party GIT repository, at the specified changeset has. Not sure why git am isn't happy about appliny it though.
Yeah. Applying it worked after nuking that dir, then rechecking out the empty .gnulib dir (git checkout -f -- .gnulib). Anyway, sounds this this patch should be ACK. Tested here and it works/compiles on OSX too.

On Wed, Oct 13, 2010 at 04:25:45PM -0600, Eric Blake wrote:
* .gnulib: Update to latest, for termios fix. * configure.ac (AC_CHECK_HEADERS): Drop redundent check. Reported by Daniel P. Berrange. ---
This fixes the problem in a more maintainable manner.
Tested via ./autobuild.sh on a Fedora host with mingw cross-compiler.
* .gnulib 2bb63bf...b6d1430 (32):
termios: fix compilation on mingw git-version-gen: don't require that .git/ be in the current dir test-select: avoid warn_unused_result warnings test-symlinkat: remove declaration of unused local test-inttostr: avoid shadowing warnings stdlib: Allow multiple gnulib generated replacements to coexist. fix a documentation typo futimens: work around Solaris 11 bug Indentation. test-futimens: avoid unwarranted test failure on Solaris 5.11 Indentation. spawn.in.h: make indentation consistent with parentheses Fix mismatched parens in previous commit rewrite int foo[2*X-1] to verify(X) or to int foo[X?1:-1] prefer (X ? 1 : -1) when converting from boolean (1,0) to int (1,-1) autoupdate time: enforce recent POSIX ruling that time_t is integral fdopendir: fix a bug on systems lacking openat and /proc support sys_select: Avoid warning due to undeclared memset() on OpenBSD 4.5. nanosleep: Make replacement POSIX compliant. bootstrap: add hook for altering gnulib.mk, for Bison bootstrap: reformat for readability docs: update cygwin progress autoupdate parse-datetime: avoid compilation failure on OpenBSD 4.7 docs: update cygwin progress docs: update parse-datetime history cygwin: use more robust version check string, sys_select: Avoid #including large headers unless necessary. memmem, strstr, strcasestr: fix bug with long periodic needle maint: fix order of ChangeLog entries parse-datetime: do some more renaming
.gnulib | 2 +- configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/.gnulib b/.gnulib index 2bb63bf..b6d1430 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 2bb63bfb25474ea147ee9f1523c0337997359a4c +Subproject commit b6d1430494cdd252cd52eca6abf88b1a00f6c983 diff --git a/configure.ac b/configure.ac index bd92b65..b868e50 100644 --- a/configure.ac +++ b/configure.ac @@ -109,7 +109,7 @@ LIBS=$old_libs
dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h \ - termios.h sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h]) + sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h])
AC_CHECK_LIB([intl],[gettext],[])
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 10/14/2010 03:34 AM, Daniel Veillard wrote:
On Wed, Oct 13, 2010 at 04:25:45PM -0600, Eric Blake wrote:
* .gnulib: Update to latest, for termios fix. * configure.ac (AC_CHECK_HEADERS): Drop redundent check. Reported by Daniel P. Berrange. ---
This fixes the problem in a more maintainable manner.
ACK,
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange
-
Daniel Veillard
-
Eric Blake
-
Justin Clift