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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
