[libvirt] [PATCH 0/2] host-validate: Don't build on Windows
The recent fixes to virt-host-validate have caused the mingw build to fail[1]. Instead of working around Windows' quirks in the tool, just stop building it altogether - it's not like it made any sense to have it available on that OS anyway. Cheers. [1] https://ci.centos.org/view/libvirt-project/job/libvirt-mingw/1496/ Andrea Bolognani (2): tools: Reorganize conditional bits configure: Make virt-host-validate optional configure.ac | 29 ++++++++++++++++++----------- m4/virt-host-validate.m4 | 40 ++++++++++++++++++++++++++++++++++++++++ tools/Makefile.am | 35 ++++++++++++++++++----------------- 3 files changed, 76 insertions(+), 28 deletions(-) create mode 100644 m4/virt-host-validate.m4 -- 2.5.5
Instead of having separate handling for programs and man pages, deal with both in the same place. --- tools/Makefile.am | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/tools/Makefile.am b/tools/Makefile.am index 6005b8b..930e6c3 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -66,34 +66,28 @@ conf_DATA = bin_SCRIPTS = virt-xml-validate virt-pki-validate bin_PROGRAMS = virsh virt-host-validate virt-admin libexec_SCRIPTS = libvirt-guests.sh +dist_man1_MANS = \ + virt-host-validate.1 \ + virt-pki-validate.1 \ + virt-xml-validate.1 \ + virsh.1 \ + virt-admin.1 if WITH_SANLOCK sbin_SCRIPTS = virt-sanlock-cleanup +dist_man8_MANS = virt-sanlock-cleanup.8 DISTCLEANFILES += virt-sanlock-cleanup endif WITH_SANLOCK if WITH_LOGIN_SHELL conf_DATA += virt-login-shell.conf bin_PROGRAMS += virt-login-shell -else ! WITH_LOGIN_SHELL -EXTRA_DIST += virt-login-shell.conf -endif ! WITH_LOGIN_SHELL - - -dist_man1_MANS = \ - virt-host-validate.1 \ - virt-pki-validate.1 \ - virt-xml-validate.1 \ - virsh.1 \ - virt-admin.1 -if WITH_LOGIN_SHELL dist_man1_MANS += virt-login-shell.1 else ! WITH_LOGIN_SHELL -EXTRA_DIST += virt-login-shell.1 +EXTRA_DIST += \ + virt-login-shell.conf \ + virt-login-shell.1 endif ! WITH_LOGIN_SHELL -if WITH_SANLOCK -dist_man8_MANS = virt-sanlock-cleanup.8 -endif WITH_SANLOCK virt-xml-validate: virt-xml-validate.in Makefile $(AM_V_GEN)sed -e 's|[@]schemadir@|$(pkgdatadir)/schemas|g' \ -- 2.5.5
virt-host-validate, just like virt-login-shell, doesn't make sense on Windows, so we should avoid building it. Make the tool optional and build it by default on all platforms except Windows, erroring out if the user attempts to build it anyway. --- configure.ac | 29 ++++++++++++++++++----------- m4/virt-host-validate.m4 | 40 ++++++++++++++++++++++++++++++++++++++++ tools/Makefile.am | 9 ++++++++- 3 files changed, 66 insertions(+), 12 deletions(-) create mode 100644 m4/virt-host-validate.m4 diff --git a/configure.ac b/configure.ac index 1eb19ee..b1500f6 100644 --- a/configure.ac +++ b/configure.ac @@ -1098,6 +1098,12 @@ dnl LIBVIRT_CHECK_LOGIN_SHELL +dnl +dnl Check for virt-host-validate +dnl + +LIBVIRT_CHECK_HOST_VALIDATE + AM_CONDITIONAL([WITH_SETUID_RPC_CLIENT], [test "$with_lxc$with_login_shell" != "nono"]) dnl @@ -2925,17 +2931,18 @@ AC_MSG_NOTICE([ Alloc OOM: $enable_oom]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Miscellaneous]) AC_MSG_NOTICE([]) -AC_MSG_NOTICE([ Debug: $enable_debug]) -AC_MSG_NOTICE([ Use -Werror: $set_werror]) -AC_MSG_NOTICE([ Warning Flags: $WARN_CFLAGS]) -AC_MSG_NOTICE([ DTrace: $with_dtrace]) -AC_MSG_NOTICE([ numad: $with_numad]) -AC_MSG_NOTICE([ XML Catalog: $XML_CATALOG_FILE]) -AC_MSG_NOTICE([ Init script: $with_init_script]) -AC_MSG_NOTICE([Char device locks: $with_chrdev_lock_files]) -AC_MSG_NOTICE([ Default Editor: $DEFAULT_EDITOR]) -AC_MSG_NOTICE([ Loader/NVRAM: $with_loader_nvram]) -AC_MSG_NOTICE([ virt-login-shell: $with_login_shell]) +AC_MSG_NOTICE([ Debug: $enable_debug]) +AC_MSG_NOTICE([ Use -Werror: $set_werror]) +AC_MSG_NOTICE([ Warning Flags: $WARN_CFLAGS]) +AC_MSG_NOTICE([ DTrace: $with_dtrace]) +AC_MSG_NOTICE([ numad: $with_numad]) +AC_MSG_NOTICE([ XML Catalog: $XML_CATALOG_FILE]) +AC_MSG_NOTICE([ Init script: $with_init_script]) +AC_MSG_NOTICE([ Char device locks: $with_chrdev_lock_files]) +AC_MSG_NOTICE([ Default Editor: $DEFAULT_EDITOR]) +AC_MSG_NOTICE([ Loader/NVRAM: $with_loader_nvram]) +AC_MSG_NOTICE([ virt-login-shell: $with_login_shell]) +AC_MSG_NOTICE([virt-host-validate: $with_host_validate]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Developer Tools]) AC_MSG_NOTICE([]) diff --git a/m4/virt-host-validate.m4 b/m4/virt-host-validate.m4 new file mode 100644 index 0000000..a56fe41 --- /dev/null +++ b/m4/virt-host-validate.m4 @@ -0,0 +1,40 @@ +dnl Copyright (C) 2015 SUSE LINUX Products GmbH, Nuernberg, Germany. +dnl Copyright (C) 2016 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. + +AC_DEFUN([LIBVIRT_CHECK_HOST_VALIDATE], [ + AC_ARG_WITH([host_validate], + [AS_HELP_STRING([--with-host-validate], + [build virt-host-validate @<:@default=check@:>@])]) + m4_divert_text([DEFAULTS], [with_host_validate=check]) + + if test "x$with_host_validate" != "xno"; then + if test "x$with_win" = "xyes"; then + if test "x$with_host_validate" = "xyes"; then + AC_MSG_ERROR([virt-host-validate is not supported on Windows]) + else + with_host_validate=no; + fi + else + with_host_validate=yes; + fi + fi + + if test "x$with_host_validate" = "xyes" ; then + AC_DEFINE_UNQUOTED([WITH_HOST_VALIDATE], 1, [whether virt-host-validate is built]) + fi + AM_CONDITIONAL([WITH_HOST_VALIDATE], [test "x$with_host_validate" = "xyes"]) +]) diff --git a/tools/Makefile.am b/tools/Makefile.am index 930e6c3..16a1aed 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -64,7 +64,7 @@ confdir = $(sysconfdir)/libvirt conf_DATA = bin_SCRIPTS = virt-xml-validate virt-pki-validate -bin_PROGRAMS = virsh virt-host-validate virt-admin +bin_PROGRAMS = virsh virt-admin libexec_SCRIPTS = libvirt-guests.sh dist_man1_MANS = \ virt-host-validate.1 \ @@ -89,6 +89,13 @@ EXTRA_DIST += \ virt-login-shell.1 endif ! WITH_LOGIN_SHELL +if WITH_HOST_VALIDATE +bin_PROGRAMS += virt-host-validate +dist_man1_MANS += virt-host-validate.1 +else ! WITH_HOST_VALIDATE +EXTRA_DIST += virt-host-validate.1 +endif ! WITH_HOST_VALIDATE + virt-xml-validate: virt-xml-validate.in Makefile $(AM_V_GEN)sed -e 's|[@]schemadir@|$(pkgdatadir)/schemas|g' \ -e 's|[@]VERSION@|$(VERSION)|g' \ -- 2.5.5
On Fri, Apr 08, 2016 at 06:04:51PM +0200, Andrea Bolognani wrote:
virt-host-validate, just like virt-login-shell, doesn't make sense on Windows, so we should avoid building it.
Make the tool optional and build it by default on all platforms except Windows, erroring out if the user attempts to build it anyway. --- configure.ac | 29 ++++++++++++++++++----------- m4/virt-host-validate.m4 | 40 ++++++++++++++++++++++++++++++++++++++++ tools/Makefile.am | 9 ++++++++- 3 files changed, 66 insertions(+), 12 deletions(-) create mode 100644 m4/virt-host-validate.m4
[...]
diff --git a/tools/Makefile.am b/tools/Makefile.am index 930e6c3..16a1aed 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -64,7 +64,7 @@ confdir = $(sysconfdir)/libvirt conf_DATA =
bin_SCRIPTS = virt-xml-validate virt-pki-validate -bin_PROGRAMS = virsh virt-host-validate virt-admin +bin_PROGRAMS = virsh virt-admin libexec_SCRIPTS = libvirt-guests.sh dist_man1_MANS = \ virt-host-validate.1 \ @@ -89,6 +89,13 @@ EXTRA_DIST += \ virt-login-shell.1 endif ! WITH_LOGIN_SHELL
+if WITH_HOST_VALIDATE +bin_PROGRAMS += virt-host-validate +dist_man1_MANS += virt-host-validate.1 +else ! WITH_HOST_VALIDATE +EXTRA_DIST += virt-host-validate.1 +endif ! WITH_HOST_VALIDATE +
I haven't looked that much into it, but I don't think we should put the generated man pages into the dist, but rather the .pod files and that are there by default, right? Could just removing the: EXTRA_DIST += virt-host-validate.1 be enough to fix what Cole saw happen?
On Mon, 2016-04-11 at 10:48 +0200, Martin Kletzander wrote:
I haven't looked that much into it, but I don't think we should put the generated man pages into the dist, but rather the .pod files and that are there by default, right? Could just removing the: EXTRA_DIST += virt-host-validate.1 be enough to fix what Cole saw happen?
We want to ship the generated man pages so that users don't need to have pod2man(1) installed on their system. Anyway, the problem is just that a hunk got lost while shuffling commits around. After squashing it in distcheck works again for me. Cheers. diff --git a/tools/Makefile.am b/tools/Makefile.am index ab37d70..560a9a5 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -67,7 +67,6 @@ bin_SCRIPTS = virt-xml-validate virt-pki-validate bin_PROGRAMS = virsh virt-admin libexec_SCRIPTS = libvirt-guests.sh dist_man1_MANS = \ - virt-host-validate.1 \ virt-pki-validate.1 \ virt-xml-validate.1 \ virsh.1 \ -- Andrea Bolognani Software Engineer - Virtualization Team
On Mon, Apr 11, 2016 at 11:29:51AM +0200, Andrea Bolognani wrote:
On Mon, 2016-04-11 at 10:48 +0200, Martin Kletzander wrote:
I haven't looked that much into it, but I don't think we should put the generated man pages into the dist, but rather the .pod files and that are there by default, right? Could just removing the: EXTRA_DIST += virt-host-validate.1 be enough to fix what Cole saw happen?
We want to ship the generated man pages so that users don't need to have pod2man(1) installed on their system.
Anyway, the problem is just that a hunk got lost while shuffling commits around. After squashing it in distcheck works again for me.
Cheers.
diff --git a/tools/Makefile.am b/tools/Makefile.am index ab37d70..560a9a5 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -67,7 +67,6 @@ bin_SCRIPTS = virt-xml-validate virt-pki-validate bin_PROGRAMS = virsh virt-admin libexec_SCRIPTS = libvirt-guests.sh dist_man1_MANS = \ - virt-host-validate.1 \ virt-pki-validate.1 \ virt-xml-validate.1 \ virsh.1 \ --
Yes, or that ^^ if we don't require pod2man =) That's fine with me, but I don't have mingw build working, so I'll leave it to others to properly ACK it.
Andrea Bolognani Software Engineer - Virtualization Team
On 04/08/2016 12:04 PM, Andrea Bolognani wrote:
The recent fixes to virt-host-validate have caused the mingw build to fail[1].
Instead of working around Windows' quirks in the tool, just stop building it altogether - it's not like it made any sense to have it available on that OS anyway.
Cheers.
[1] https://ci.centos.org/view/libvirt-project/job/libvirt-mingw/1496/
Andrea Bolognani (2): tools: Reorganize conditional bits configure: Make virt-host-validate optional
configure.ac | 29 ++++++++++++++++++----------- m4/virt-host-validate.m4 | 40 ++++++++++++++++++++++++++++++++++++++++ tools/Makefile.am | 35 ++++++++++++++++++----------------- 3 files changed, 76 insertions(+), 28 deletions(-) create mode 100644 m4/virt-host-validate.m4
I applied them both, tried 'make distcheck', and hit: /usr/bin/install -c -m 644 ../../../tools/virt-host-validate.1 ../../../tools/virt-pki-validate.1 ../../../tools/virt-xml-validate.1 ../../../tools/virsh.1 ../../../tools/virt-admin.1 ../../../tools/virt-login-shell.1 ../../../tools/virt-host-validate.1 '/home/crobinso/src/libvirt/libvirt-1.3.4/_inst/share/man/man1' /usr/bin/install: will not overwrite just-created '/home/crobinso/src/libvirt/libvirt-1.3.4/_inst/share/man/man1/virt-host-validate.1' with '../../../tools/virt-host-validate.1' Makefile:2863: recipe for target 'install-man1' failed make[4]: *** [install-man1] Error 1 make[4]: *** Waiting for unfinished jobs.... Makefiles suck :) - Cole
On Fri, 2016-04-08 at 14:03 -0400, Cole Robinson wrote:
On 04/08/2016 12:04 PM, Andrea Bolognani wrote:
The recent fixes to virt-host-validate have caused the mingw build to fail[1].
Instead of working around Windows' quirks in the tool, just stop building it altogether - it's not like it made any sense to have it available on that OS anyway.
Cheers.
[1] https://ci.centos.org/view/libvirt-project/job/libvirt-mingw/1496/
Andrea Bolognani (2): tools: Reorganize conditional bits configure: Make virt-host-validate optional
configure.ac | 29 ++++++++++++++++++----------- m4/virt-host-validate.m4 | 40 ++++++++++++++++++++++++++++++++++++++++ tools/Makefile.am | 35 ++++++++++++++++++----------------- 3 files changed, 76 insertions(+), 28 deletions(-) create mode 100644 m4/virt-host-validate.m4
I applied them both, tried 'make distcheck', and hit:
/usr/bin/install -c -m 644 ../../../tools/virt-host-validate.1 ../../../tools/virt-pki-validate.1 ../../../tools/virt-xml-validate.1 ../../../tools/virsh.1 ../../../tools/virt-admin.1 ../../../tools/virt-login-shell.1 ../../../tools/virt-host-validate.1 '/home/crobinso/src/libvirt/libvirt-1.3.4/_inst/share/man/man1' /usr/bin/install: will not overwrite just-created '/home/crobinso/src/libvirt/libvirt-1.3.4/_inst/share/man/man1/virt-host-validate.1' with '../../../tools/virt-host-validate.1' Makefile:2863: recipe for target 'install-man1' failed make[4]: *** [install-man1] Error 1 make[4]: *** Waiting for unfinished jobs....
Makefiles suck :)
I just realized Martin didn't include you when replying. Would you mind trying again after squashing in the hunk from[1]? That should fix the failure you're seeing. Cheers. [1] https://www.redhat.com/archives/libvir-list/2016-April/msg00485.html -- Andrea Bolognani Software Engineer - Virtualization Team
On 04/12/2016 01:07 PM, Andrea Bolognani wrote:
On Fri, 2016-04-08 at 14:03 -0400, Cole Robinson wrote:
On 04/08/2016 12:04 PM, Andrea Bolognani wrote:
The recent fixes to virt-host-validate have caused the mingw build to fail[1].
Instead of working around Windows' quirks in the tool, just stop building it altogether - it's not like it made any sense to have it available on that OS anyway.
Cheers.
[1] https://ci.centos.org/view/libvirt-project/job/libvirt-mingw/1496/
Andrea Bolognani (2): tools: Reorganize conditional bits configure: Make virt-host-validate optional
configure.ac | 29 ++++++++++++++++++----------- m4/virt-host-validate.m4 | 40 ++++++++++++++++++++++++++++++++++++++++ tools/Makefile.am | 35 ++++++++++++++++++----------------- 3 files changed, 76 insertions(+), 28 deletions(-) create mode 100644 m4/virt-host-validate.m4
I applied them both, tried 'make distcheck', and hit:
/usr/bin/install -c -m 644 ../../../tools/virt-host-validate.1 ../../../tools/virt-pki-validate.1 ../../../tools/virt-xml-validate.1 ../../../tools/virsh.1 ../../../tools/virt-admin.1 ../../../tools/virt-login-shell.1 ../../../tools/virt-host-validate.1 '/home/crobinso/src/libvirt/libvirt-1.3.4/_inst/share/man/man1' /usr/bin/install: will not overwrite just-created '/home/crobinso/src/libvirt/libvirt-1.3.4/_inst/share/man/man1/virt-host-validate.1' with '../../../tools/virt-host-validate.1' Makefile:2863: recipe for target 'install-man1' failed make[4]: *** [install-man1] Error 1 make[4]: *** Waiting for unfinished jobs....
Makefiles suck :)
I just realized Martin didn't include you when replying.
Would you mind trying again after squashing in the hunk from[1]? That should fix the failure you're seeing.
Yup, works for my test case. ACK Thanks, Cole
On Tue, 2016-04-12 at 18:13 -0400, Cole Robinson wrote:
Would you mind trying again after squashing in the hunk from[1]? That should fix the failure you're seeing.
Yup, works for my test case. ACK
Pushed, thanks :) -- Andrea Bolognani Software Engineer - Virtualization Team
participants (3)
-
Andrea Bolognani -
Cole Robinson -
Martin Kletzander