1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 00/41] Split the libvirtd daemon into per-driver daemons
  4. View patch

[libvirt] [PATCH 04/41] build: use a common rule for checking augeas test data files

Daniel P. Berrangé posted 41 patches 6 years, 6 months ago
There is a newer version of this series
[libvirt] [PATCH 04/41] build: use a common rule for checking augeas test data files
Posted by Daniel P. Berrangé 6 years, 6 months ago 
Instead of each subdir containing its own custom rule for checking the
augeas tests, use common rule for all.

The new rule searches both src + build dirs for include files, since
some augeas files will be auto-generated very shortly.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/Makefile.am             | 14 +++++++++-----
 src/bhyve/Makefile.inc.am   | 11 -----------
 src/libxl/Makefile.inc.am   | 11 -----------
 src/locking/Makefile.inc.am | 34 ++--------------------------------
 src/logging/Makefile.inc.am |  9 ---------
 src/lxc/Makefile.inc.am     |  8 --------
 src/qemu/Makefile.inc.am    |  8 --------
 src/remote/Makefile.inc.am  |  8 --------
 8 files changed, 11 insertions(+), 92 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 58f0c792ed..cf6f920576 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -74,7 +74,6 @@ GENERATED_SYM_FILES =
 augeas_DATA =
 augeastest_DATA =
 conf_DATA =
-AUGEAS_DIRS =
 if WITH_DTRACE_PROBES
 tapset_DATA =
 endif WITH_DTRACE_PROBES
@@ -396,13 +395,18 @@ GENERATED_SYM_FILES += \
 
 
 
-
-
-
+AUG_TEST_NAMES = $(subst /,-, $(augeastest_DATA))
 
 check-local: check-augeas
 
-check-augeas: $(AUGEAS_DIRS:%=check-augeas-%)
+check-augeas: $(AUG_TEST_NAMES:%=check-augeas-%)
+
+check-augeas-%: $(augeas_DATA) $(augeastest_DATA)
+	$(AM_V_GEN)export FILE=`echo $* | sed -e 's/.*-//'`; \
+	export DIR=`echo $* | sed -e 's/-.*//'`; \
+	if test -x '$(AUGPARSE)'; then \
+	    '$(AUGPARSE)' -I $(srcdir)/$$DIR -I $(builddir)/$$DIR $$DIR/$$FILE; \
+	fi
 
 AUG_GENTEST = $(PERL) $(top_srcdir)/build-aux/augeas-gentest.pl
 
diff --git a/src/bhyve/Makefile.inc.am b/src/bhyve/Makefile.inc.am
index 0aef5e17c7..8b662e9775 100644
--- a/src/bhyve/Makefile.inc.am
+++ b/src/bhyve/Makefile.inc.am
@@ -51,17 +51,10 @@ conf_DATA += bhyve/bhyve.conf
 augeas_DATA += bhyve/libvirtd_bhyve.aug
 augeastest_DATA += bhyve/test_libvirtd_bhyve.aug
 
-AUGEAS_DIRS += bhyve
-
 bhyve/test_libvirtd_bhyve.aug: bhyve/test_libvirtd_bhyve.aug.in \
 		$(srcdir)/bhyve/bhyve.conf $(AUG_GENTEST)
 	$(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/bhyve/bhyve.conf $< > $@
 
-check-augeas-bhyve: bhyve/test_libvirtd_bhyve.aug
-	$(AM_V_GEN)if test -x '$(AUGPARSE)'; then \
-	    '$(AUGPARSE)' -I $(srcdir)/bhyve bhyve/test_libvirtd_bhyve.aug; \
-	fi
-
 endif WITH_BHYVE
 
 EXTRA_DIST += \
@@ -69,7 +62,3 @@ EXTRA_DIST += \
 	bhyve/libvirtd_bhyve.aug \
 	bhyve/test_libvirtd_bhyve.aug.in \
 	$(NULL)
-
-.PHONY: \
-	check-augeas-bhyve \
-	$(NULL)
diff --git a/src/libxl/Makefile.inc.am b/src/libxl/Makefile.inc.am
index e73f34db8e..467c2720b2 100644
--- a/src/libxl/Makefile.inc.am
+++ b/src/libxl/Makefile.inc.am
@@ -69,17 +69,10 @@ conf_DATA += libxl/libxl.conf
 augeas_DATA += libxl/libvirtd_libxl.aug
 augeastest_DATA += libxl/test_libvirtd_libxl.aug
 
-AUGEAS_DIRS += libxl
-
 libxl/test_libvirtd_libxl.aug: libxl/test_libvirtd_libxl.aug.in \
 		$(srcdir)/libxl/libxl.conf $(AUG_GENTEST)
 	$(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/libxl/libxl.conf $< > $@
 
-check-augeas-libxl: libxl/test_libvirtd_libxl.aug
-	$(AM_V_GEN)if test -x '$(AUGPARSE)'; then \
-	    '$(AUGPARSE)' -I $(srcdir)/libxl libxl/test_libvirtd_libxl.aug; \
-	fi
-
 INSTALL_DATA_DIRS += libxl
 
 install-data-libxl:
@@ -94,10 +87,6 @@ uninstall-data-libxl:
 
 endif WITH_LIBXL
 
-.PHONY: \
-	check-augeas-libxl \
-	$(NULL)
-
 EXTRA_DIST += \
 	libxl/libxl.conf \
 	libxl/libvirtd_libxl.aug \
diff --git a/src/locking/Makefile.inc.am b/src/locking/Makefile.inc.am
index 0f284faf25..24d83fdd80 100644
--- a/src/locking/Makefile.inc.am
+++ b/src/locking/Makefile.inc.am
@@ -227,16 +227,8 @@ locking/test_libvirt_sanlock.aug: locking/test_libvirt_sanlock.aug.in \
 		locking/qemu-sanlock.conf $(AUG_GENTEST)
 	$(AM_V_GEN)$(AUG_GENTEST) locking/qemu-sanlock.conf $< > $@
 
-check-augeas-sanlock: locking/test_libvirt_sanlock.aug
-	$(AM_V_GEN)if test -x '$(AUGPARSE)'; then \
-	    '$(AUGPARSE)' -I $(srcdir)/locking locking/test_libvirt_sanlock.aug; \
-	fi
-else ! WITH_QEMU
-check-augeas-sanlock:
-endif ! WITH_QEMU
-else ! WITH_SANLOCK
-check-augeas-sanlock:
-endif ! WITH_SANLOCK
+endif WITH_QEMU
+endif WITH_SANLOCK
 
 if WITH_QEMU
 locking/test_libvirt_lockd.aug: locking/test_libvirt_lockd.aug.in \
@@ -248,33 +240,11 @@ locking/test_virtlockd.aug: locking/test_virtlockd.aug.in \
 		locking/virtlockd.conf $(AUG_GENTEST)
 	$(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/locking/virtlockd.conf $< > $@
 
-if WITH_QEMU
-check-augeas-lockd: locking/test_libvirt_lockd.aug
-	$(AM_V_GEN)if test -x '$(AUGPARSE)'; then \
-	    '$(AUGPARSE)' -I $(srcdir)/locking locking/test_libvirt_lockd.aug; \
-	fi
-else ! WITH_QEMU
-check-augeas-lockd:
-endif ! WITH_QEMU
-
-check-augeas-virtlockd: locking/test_virtlockd.aug
-	$(AM_V_GEN)if test -x '$(AUGPARSE)'; then \
-	    '$(AUGPARSE)' -I $(srcdir)/locking locking/test_virtlockd.aug; \
-	fi
-
-AUGEAS_DIRS += locking
-
-check-augeas-locking: check-augeas-virtlockd check-augeas-lockd check-augeas-sanlock
-
 endif WITH_LIBVIRTD
 
 .PHONY: \
 	install-data-locking \
 	uninstall-data-locking \
-	check-augeas-locking \
-	check-augeas-virtlockd \
-	check-augeas-lockd \
-	check-augeas-sanlock \
 	$(NULL)
 
 locking/%-lockd.conf: $(srcdir)/locking/lockd.conf
diff --git a/src/logging/Makefile.inc.am b/src/logging/Makefile.inc.am
index 58a139ec2f..f0c49330f5 100644
--- a/src/logging/Makefile.inc.am
+++ b/src/logging/Makefile.inc.am
@@ -101,17 +101,8 @@ logging/test_virtlogd.aug: logging/test_virtlogd.aug.in \
 		logging/virtlogd.conf $(AUG_GENTEST)
 	$(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/logging/virtlogd.conf $< > $@
 
-AUGEAS_DIRS += logging
-
-check-augeas-logging: logging/test_virtlogd.aug
-	$(AM_V_GEN)if test -x '$(AUGPARSE)'; then \
-	    '$(AUGPARSE)' -I $(srcdir)/logging logging/test_virtlogd.aug; \
-	fi
-
 endif WITH_LIBVIRTD
 
-.PHONY: check-augeas-logging
-
 logging/log_daemon_dispatch_stubs.h: $(LOG_PROTOCOL) \
 		$(srcdir)/rpc/gendispatch.pl Makefile.am
 	$(AM_V_GEN)perl -w $(srcdir)/rpc/gendispatch.pl --mode=server \
diff --git a/src/lxc/Makefile.inc.am b/src/lxc/Makefile.inc.am
index f011d90e95..0c241fc5c1 100644
--- a/src/lxc/Makefile.inc.am
+++ b/src/lxc/Makefile.inc.am
@@ -158,17 +158,10 @@ conf_DATA += lxc/lxc.conf
 augeas_DATA += lxc/libvirtd_lxc.aug
 augeastest_DATA += lxc/test_libvirtd_lxc.aug
 
-AUGEAS_DIRS += lxc
-
 lxc/test_libvirtd_lxc.aug: lxc/test_libvirtd_lxc.aug.in \
 		$(srcdir)/lxc/lxc.conf $(AUG_GENTEST)
 	$(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/lxc/lxc.conf $< > $@
 
-check-augeas-lxc: lxc/test_libvirtd_lxc.aug
-	$(AM_V_GEN)if test -x '$(AUGPARSE)'; then \
-	    '$(AUGPARSE)' -I $(srcdir)/lxc lxc/test_libvirtd_lxc.aug; \
-	fi
-
 INSTALL_DATA_DIRS += lxc
 
 install-data-lxc:
@@ -196,7 +189,6 @@ lxc/lxc_controller_dispatch.h: $(srcdir)/rpc/gendispatch.pl \
 	  $(srcdir)/lxc/lxc_controller_dispatch.h
 
 .PHONY: \
-	check-agueas-lxc \
 	install-data-lxc \
 	uninstall-data-lxc \
 	$(NULL)
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index 750b8a5c85..12236a9e7b 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -117,17 +117,10 @@ conf_DATA += qemu/qemu.conf
 augeas_DATA += qemu/libvirtd_qemu.aug
 augeastest_DATA += qemu/test_libvirtd_qemu.aug
 
-AUGEAS_DIRS += qemu
-
 qemu/test_libvirtd_qemu.aug: qemu/test_libvirtd_qemu.aug.in \
 		$(srcdir)/qemu/qemu.conf $(AUG_GENTEST)
 	$(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/qemu/qemu.conf $< > $@
 
-check-augeas-qemu: qemu/test_libvirtd_qemu.aug
-	$(AM_V_GEN)if test -x '$(AUGPARSE)'; then \
-	    '$(AUGPARSE)' -I $(srcdir)/qemu qemu/test_libvirtd_qemu.aug; \
-	fi
-
 INSTALL_DATA_DIRS += qemu
 
 install-data-qemu:
@@ -151,7 +144,6 @@ uninstall-data-qemu:
 endif WITH_QEMU
 
 .PHONY: \
-	check-augeas-qemu \
 	install-data-qemu \
 	uninstall-data-qemu \
 	$(NULL)
diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am
index 18519b129d..0400dabad9 100644
--- a/src/remote/Makefile.inc.am
+++ b/src/remote/Makefile.inc.am
@@ -184,17 +184,10 @@ install-data-remote:
 uninstall-data-remote:
 	rmdir "$(DESTDIR)$(localstatedir)/log/libvirt" ||:
 
-AUGEAS_DIRS += remote
-
 remote/test_libvirtd.aug: remote/test_libvirtd.aug.in \
 		remote/libvirtd.conf $(AUG_GENTEST)
 	$(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/remote/libvirtd.conf $< > $@
 
-check-augeas-remote: remote/test_libvirtd.aug
-	$(AM_V_GEN)if test -x '$(AUGPARSE)'; then \
-	    '$(AUGPARSE)' -I $(srcdir)/remote remote/test_libvirtd.aug; \
-	fi
-
 if WITH_SYSCTL
 # Use $(prefix)/lib rather than $(libdir), since man sysctl.d insists on
 # /usr/lib/sysctl.d/ even when libdir is /usr/lib64
@@ -241,7 +234,6 @@ endif WITH_LIBVIRTD
 .PHONY: \
 	install-data-remote \
 	uninstall-data-remote \
-	check-augeas-remote \
 	$(NULL)
 
 # This is needed for clients too, so can't wrap in
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/41] build: use a common rule for checking augeas test data files
Posted by Andrea Bolognani 6 years, 6 months ago 
On Tue, 2019-07-23 at 17:02 +0100, Daniel P. Berrangé wrote:
[...]
> +AUG_TEST_NAMES = $(subst /,-, $(augeastest_DATA))
>  
>  check-local: check-augeas
>  
> -check-augeas: $(AUGEAS_DIRS:%=check-augeas-%)
> +check-augeas: $(AUG_TEST_NAMES:%=check-augeas-%)
> +
> +check-augeas-%: $(augeas_DATA) $(augeastest_DATA)
> +	$(AM_V_GEN)export FILE=`echo $* | sed -e 's/.*-//'`; \
> +	export DIR=`echo $* | sed -e 's/-.*//'`; \
> +	if test -x '$(AUGPARSE)'; then \
> +	    '$(AUGPARSE)' -I $(srcdir)/$$DIR -I $(builddir)/$$DIR $$DIR/$$FILE; \
> +	fi

How about we skip the double conversion steps and just do

  check-augeas: $(augeas_DATA) $(augeastest_DATA)
      $(AM_V_GEN) \
      if test -x "$(AUGPARSE)"; then \
          for f in $(augeastest_DATA); do \
              DIR=$$(dirname "$$f"); \
              FILE=$$(basename "$$f"); \
              "$(AUGPARSE)" \
                  -I "$(srcdir)/$$DIR" -I "$(builddir)/$$DIR" \
                  "$$DIR/$$FILE"; \
          done; \
      fi
  .PHONY: check-augeas

instead? As an added bonus, this version avoids doing any work if
augparse is not available and is correctly marked as PHONY, which
the rules you're replacing also were.

The rest of the changes look good.

[...]
>  bhyve/test_libvirtd_bhyve.aug: bhyve/test_libvirtd_bhyve.aug.in \
>  		$(srcdir)/bhyve/bhyve.conf $(AUG_GENTEST)
>  	$(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/bhyve/bhyve.conf $< > $@

Later on it would be nice to remove duplication for all these rules
as well... I don't think you do it in your series. But it's perfectly
fine not to do it right now, I just though I'd point it out :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/41] build: use a common rule for checking augeas test data files
Posted by Daniel P. Berrangé 6 years, 6 months ago 
On Fri, Jul 26, 2019 at 11:18:03AM +0200, Andrea Bolognani wrote:
> On Tue, 2019-07-23 at 17:02 +0100, Daniel P. Berrangé wrote:
> [...]
> > +AUG_TEST_NAMES = $(subst /,-, $(augeastest_DATA))
> >  
> >  check-local: check-augeas
> >  
> > -check-augeas: $(AUGEAS_DIRS:%=check-augeas-%)
> > +check-augeas: $(AUG_TEST_NAMES:%=check-augeas-%)
> > +
> > +check-augeas-%: $(augeas_DATA) $(augeastest_DATA)
> > +	$(AM_V_GEN)export FILE=`echo $* | sed -e 's/.*-//'`; \
> > +	export DIR=`echo $* | sed -e 's/-.*//'`; \
> > +	if test -x '$(AUGPARSE)'; then \
> > +	    '$(AUGPARSE)' -I $(srcdir)/$$DIR -I $(builddir)/$$DIR $$DIR/$$FILE; \
> > +	fi
> 
> How about we skip the double conversion steps and just do
> 
>   check-augeas: $(augeas_DATA) $(augeastest_DATA)
>       $(AM_V_GEN) \
>       if test -x "$(AUGPARSE)"; then \
>           for f in $(augeastest_DATA); do \
>               DIR=$$(dirname "$$f"); \
>               FILE=$$(basename "$$f"); \
>               "$(AUGPARSE)" \
>                   -I "$(srcdir)/$$DIR" -I "$(builddir)/$$DIR" \
>                   "$$DIR/$$FILE"; \
>           done; \
>       fi
>   .PHONY: check-augeas
> 
> instead? As an added bonus, this version avoids doing any work if
> augparse is not available and is correctly marked as PHONY, which
> the rules you're replacing also were.

This doesn't show any output for the files - I wanted to see the
make output for each file being checked, as its a useful confirmation
that we're actually processing the files we expect to have.

> 
> The rest of the changes look good.
> 
> [...]
> >  bhyve/test_libvirtd_bhyve.aug: bhyve/test_libvirtd_bhyve.aug.in \
> >  		$(srcdir)/bhyve/bhyve.conf $(AUG_GENTEST)
> >  	$(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/bhyve/bhyve.conf $< > $@
> 
> Later on it would be nice to remove duplication for all these rules
> as well... I don't think you do it in your series. But it's perfectly
> fine not to do it right now, I just though I'd point it out :)
> 
> -- 
> Andrea Bolognani / Red Hat / Virtualization
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/41] build: use a common rule for checking augeas test data files
Posted by Andrea Bolognani 6 years, 6 months ago 
On Fri, 2019-07-26 at 10:23 +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 26, 2019 at 11:18:03AM +0200, Andrea Bolognani wrote:
> > On Tue, 2019-07-23 at 17:02 +0100, Daniel P. Berrangé wrote:
> > [...]
> > > +AUG_TEST_NAMES = $(subst /,-, $(augeastest_DATA))
> > >  
> > >  check-local: check-augeas
> > >  
> > > -check-augeas: $(AUGEAS_DIRS:%=check-augeas-%)
> > > +check-augeas: $(AUG_TEST_NAMES:%=check-augeas-%)
> > > +
> > > +check-augeas-%: $(augeas_DATA) $(augeastest_DATA)
> > > +	$(AM_V_GEN)export FILE=`echo $* | sed -e 's/.*-//'`; \
> > > +	export DIR=`echo $* | sed -e 's/-.*//'`; \
> > > +	if test -x '$(AUGPARSE)'; then \
> > > +	    '$(AUGPARSE)' -I $(srcdir)/$$DIR -I $(builddir)/$$DIR $$DIR/$$FILE; \
> > > +	fi
> > 
> > How about we skip the double conversion steps and just do
> > 
> >   check-augeas: $(augeas_DATA) $(augeastest_DATA)
> >       $(AM_V_GEN) \
> >       if test -x "$(AUGPARSE)"; then \
> >           for f in $(augeastest_DATA); do \
> >               DIR=$$(dirname "$$f"); \
> >               FILE=$$(basename "$$f"); \
> >               "$(AUGPARSE)" \
> >                   -I "$(srcdir)/$$DIR" -I "$(builddir)/$$DIR" \
> >                   "$$DIR/$$FILE"; \
> >           done; \
> >       fi
> >   .PHONY: check-augeas
> > 
> > instead? As an added bonus, this version avoids doing any work if
> > augparse is not available and is correctly marked as PHONY, which
> > the rules you're replacing also were.
> 
> This doesn't show any output for the files - I wanted to see the
> make output for each file being checked, as its a useful confirmation
> that we're actually processing the files we expect to have.

That's only a couple small tweaks away:

  check-augeas: $(augeas_DATA) $(augeastest_DATA)
    @if test -x "$(AUGPARSE)"; then \
      for f in $(augeastest_DATA); do \
        DIR=$$(dirname "$$f"); \
        FILE=$$(basename "$$f"); \
        echo "AUGPARSE $$f"; \
        "$(AUGPARSE)" \
          -I "$(srcdir)/$$DIR" -I "$(builddir)/$$DIR" \
          "$$DIR/$$FILE"; \
      done; \
    fi
  .PHONY: check-augeas

This version even results in a more accurate output, as we're not
really generating the files but rather validating them.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.