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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux