[libvirt PATCH 246/351] meson: src: add check-augeas test

Pavel Hrdina posted 351 patches 5 years, 6 months ago
There is a newer version of this series
[libvirt PATCH 246/351] meson: src: add check-augeas test
Posted by Pavel Hrdina 5 years, 6 months ago
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 scripts/check-augeas.sh | 12 ++++++++++++
 scripts/meson.build     |  1 +
 src/Makefile.am         | 17 -----------------
 src/meson.build         | 18 +++++++++++++++++-
 4 files changed, 30 insertions(+), 18 deletions(-)
 create mode 100644 scripts/check-augeas.sh

diff --git a/scripts/check-augeas.sh b/scripts/check-augeas.sh
new file mode 100644
index 00000000000..68609d555a7
--- /dev/null
+++ b/scripts/check-augeas.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+AUGPARSE=$1
+srcdir=$2
+builddir=$3
+augeastest=$4
+
+set -vx
+
+for f in $augeastest; do
+    ${AUGPARSE} -I "$srcdir" -I "$builddir" $f
+done
diff --git a/scripts/meson.build b/scripts/meson.build
index 3038dfc8d21..05bf6ff7231 100644
--- a/scripts/meson.build
+++ b/scripts/meson.build
@@ -3,6 +3,7 @@ scripts = [
   'augeas-gentest.py',
   'check-aclperms.py',
   'check-aclrules.py',
+  'check-augeas.sh',
   'check-driverimpls.py',
   'check-drivername.py',
   'check-file-access.py',
diff --git a/src/Makefile.am b/src/Makefile.am
index d697114d7e8..88e44fab2b9 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -20,8 +20,6 @@
 # here. List them against the individual XXX_la_CFLAGS targets
 # that actually use them.
 
-augeas_DATA =
-augeastest_DATA =
 if WITH_DTRACE_PROBES
 tapset_DATA =
 endif WITH_DTRACE_PROBES
@@ -91,21 +89,6 @@ check-local: check-protocol \
 .PHONY: check-protocol $(PROTOCOL_STRUCTS:structs=struct)
 
 
-check-local: check-augeas
-
-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" || exit 1; \
-	    done; \
-	fi
-.PHONY: check-augeas
-
 if WITH_DTRACE_PROBES
 
 tapset_DATA += libvirt_functions.stp
diff --git a/src/meson.build b/src/meson.build
index 6747f11a3f3..9b4c26e32ea 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -646,6 +646,8 @@ foreach data : virt_test_aug_files
   )
 endforeach
 
+augeas_test_data = []
+
 foreach data : virt_daemon_confs
   daemon_conf = configuration_data()
   daemon_conf.set('runstatedir', runstatedir)
@@ -694,7 +696,7 @@ foreach data : virt_daemon_confs
   )
 
   test_aug_out = 'test_@0@.aug'.format(data['name'])
-  custom_target(
+  augeas_test_file = custom_target(
     test_aug_out,
     input: [ conf_out, test_aug_tmp ],
     output: test_aug_out,
@@ -703,6 +705,7 @@ foreach data : virt_daemon_confs
     install: true,
     install_dir: virt_test_aug_dir,
   )
+  augeas_test_data += augeas_test_file
 endforeach
 
 
@@ -857,3 +860,16 @@ test(
   args: [ check_aclrules_prog.path(), files('remote/remote_protocol.x'), stateful_driver_source_files ],
   env: runutf8,
 )
+
+if augparse_prog.found()
+  test(
+    'check-augeas',
+    check_augeas_prog,
+    args: [
+      augparse_prog.path(),
+      meson.current_source_dir(),
+      meson.current_build_dir(),
+      augeas_test_data,
+    ],
+  )
+endif
-- 
2.26.2

Re: [libvirt PATCH 246/351] meson: src: add check-augeas test
Posted by Peter Krempa 5 years, 6 months ago
On Thu, Jul 16, 2020 at 11:58:02 +0200, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  scripts/check-augeas.sh | 12 ++++++++++++
>  scripts/meson.build     |  1 +
>  src/Makefile.am         | 17 -----------------
>  src/meson.build         | 18 +++++++++++++++++-
>  4 files changed, 30 insertions(+), 18 deletions(-)
>  create mode 100644 scripts/check-augeas.sh

[...]

> @@ -857,3 +860,16 @@ test(
>    args: [ check_aclrules_prog.path(), files('remote/remote_protocol.x'), stateful_driver_source_files ],
>    env: runutf8,
>  )
> +
> +if augparse_prog.found()

Can't we use foreach on augeas_test_data and invoke the test
individually for each file rather than adding a script which does the
same?

> +  test(
> +    'check-augeas',
> +    check_augeas_prog,
> +    args: [
> +      augparse_prog.path(),
> +      meson.current_source_dir(),
> +      meson.current_build_dir(),
> +      augeas_test_data,
> +    ],
> +  )
> +endif
> -- 
> 2.26.2
> 

Re: [libvirt PATCH 246/351] meson: src: add check-augeas test
Posted by Pavel Hrdina 5 years, 6 months ago
On Tue, Jul 28, 2020 at 09:51:40AM +0200, Peter Krempa wrote:
> On Thu, Jul 16, 2020 at 11:58:02 +0200, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> >  scripts/check-augeas.sh | 12 ++++++++++++
> >  scripts/meson.build     |  1 +
> >  src/Makefile.am         | 17 -----------------
> >  src/meson.build         | 18 +++++++++++++++++-
> >  4 files changed, 30 insertions(+), 18 deletions(-)
> >  create mode 100644 scripts/check-augeas.sh
> 
> [...]
> 
> > @@ -857,3 +860,16 @@ test(
> >    args: [ check_aclrules_prog.path(), files('remote/remote_protocol.x'), stateful_driver_source_files ],
> >    env: runutf8,
> >  )
> > +
> > +if augparse_prog.found()
> 
> Can't we use foreach on augeas_test_data and invoke the test
> individually for each file rather than adding a script which does the
> same?

Sure we can, I wanted to stay as close as to autotools where
check-augeas is a single target. Using foreach would result into having
separate target for each file. In general it is most likely better but
I was trying to avoid any changes like this with the rewrite and do them
as a followup patches to cleanup the build system.

Pavel

> > +  test(
> > +    'check-augeas',
> > +    check_augeas_prog,
> > +    args: [
> > +      augparse_prog.path(),
> > +      meson.current_source_dir(),
> > +      meson.current_build_dir(),
> > +      augeas_test_data,
> > +    ],
> > +  )
> > +endif
> > -- 
> > 2.26.2
> > 
> 
Re: [libvirt PATCH 246/351] meson: src: add check-augeas test
Posted by Peter Krempa 5 years, 6 months ago
On Tue, Jul 28, 2020 at 10:09:11 +0200, Pavel Hrdina wrote:
> On Tue, Jul 28, 2020 at 09:51:40AM +0200, Peter Krempa wrote:
> > On Thu, Jul 16, 2020 at 11:58:02 +0200, Pavel Hrdina wrote:
> > > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > > ---
> > >  scripts/check-augeas.sh | 12 ++++++++++++
> > >  scripts/meson.build     |  1 +
> > >  src/Makefile.am         | 17 -----------------
> > >  src/meson.build         | 18 +++++++++++++++++-
> > >  4 files changed, 30 insertions(+), 18 deletions(-)
> > >  create mode 100644 scripts/check-augeas.sh
> > 
> > [...]
> > 
> > > @@ -857,3 +860,16 @@ test(
> > >    args: [ check_aclrules_prog.path(), files('remote/remote_protocol.x'), stateful_driver_source_files ],
> > >    env: runutf8,
> > >  )
> > > +
> > > +if augparse_prog.found()
> > 
> > Can't we use foreach on augeas_test_data and invoke the test
> > individually for each file rather than adding a script which does the
> > same?
> 
> Sure we can, I wanted to stay as close as to autotools where
> check-augeas is a single target. Using foreach would result into having
> separate target for each file. In general it is most likely better but
> I was trying to avoid any changes like this with the rewrite and do them
> as a followup patches to cleanup the build system.

In autotools the loop is in the automake file (yes, still shell),
while here you put it into a separate script file. I'd argue that
putting the loop into the meson file (regardless of how many test
targets it creates) is more equivalent to the original source than this
way.

Given that you are now changing all the shell helpers to python I'd
rather see this done in pure meson, than having a python file looper
intermediate helper.

Re: [libvirt PATCH 246/351] meson: src: add check-augeas test
Posted by Pavel Hrdina 5 years, 6 months ago
On Tue, Jul 28, 2020 at 10:22:27AM +0200, Peter Krempa wrote:
> On Tue, Jul 28, 2020 at 10:09:11 +0200, Pavel Hrdina wrote:
> > On Tue, Jul 28, 2020 at 09:51:40AM +0200, Peter Krempa wrote:
> > > On Thu, Jul 16, 2020 at 11:58:02 +0200, Pavel Hrdina wrote:
> > > > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > > > ---
> > > >  scripts/check-augeas.sh | 12 ++++++++++++
> > > >  scripts/meson.build     |  1 +
> > > >  src/Makefile.am         | 17 -----------------
> > > >  src/meson.build         | 18 +++++++++++++++++-
> > > >  4 files changed, 30 insertions(+), 18 deletions(-)
> > > >  create mode 100644 scripts/check-augeas.sh
> > > 
> > > [...]
> > > 
> > > > @@ -857,3 +860,16 @@ test(
> > > >    args: [ check_aclrules_prog.path(), files('remote/remote_protocol.x'), stateful_driver_source_files ],
> > > >    env: runutf8,
> > > >  )
> > > > +
> > > > +if augparse_prog.found()
> > > 
> > > Can't we use foreach on augeas_test_data and invoke the test
> > > individually for each file rather than adding a script which does the
> > > same?
> > 
> > Sure we can, I wanted to stay as close as to autotools where
> > check-augeas is a single target. Using foreach would result into having
> > separate target for each file. In general it is most likely better but
> > I was trying to avoid any changes like this with the rewrite and do them
> > as a followup patches to cleanup the build system.
> 
> In autotools the loop is in the automake file (yes, still shell),
> while here you put it into a separate script file. I'd argue that
> putting the loop into the meson file (regardless of how many test
> targets it creates) is more equivalent to the original source than this
> way.
> 
> Given that you are now changing all the shell helpers to python I'd
> rather see this done in pure meson, than having a python file looper
> intermediate helper.

The change to python is already done. But I don't care so I can redo it.

Pavel