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 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@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