Libguestfs - Dec 2018 - Re: [nbdkit PATCH] maint: Adjust cleaning rules

'make distcheck' calls us out for leaving files behind after
'make distclean' that were not present in the tarball.  Either
these files are expensive enough that end users should not be
required to regenerate them (so they should be distributed),
or they should be cleaned when a user asks to get back to the
pristine tarball state.

Automake suggests this hierarchy of cleaning:
mostlyclean: .o and other obvious build artifacts
clean: everything that 'make' builds without rerunning configure
distclean: everything that 'configure' builds
maintainerclean: things that requires special tooling to rebuild

By that definition, things that are expensive to rebuild, but
which are built by make and not configure, belong in distclean
rather than maintainerclean (this includes the TLS temporary
files, as building them consumes host entropy); and things which
are not terribly expensive to rebuild can live in clean.  I
didn't find any of the files called out as being hard enough to
regenerate to make it worth including in the tarball, which
would be the only reason to keep such files in maintainerclean.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 docs/Makefile.am  |  2 +-
 tests/Makefile.am | 30 ++++++++++++++++++------------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index 1772c66..6b712e3 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -123,4 +123,4 @@ filter-links.pod: $(top_srcdir)/common-rules.mk
 	$(srcdir)/make-links.sh filter 1 $(filters) > $@-t
 	mv $@-t $@

-MAINTAINERCLEANFILES = plugin-links.pod filter-links.pod
+DISTCLEANFILES = plugin-links.pod filter-links.pod
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 4c7b59c..55db593 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -32,7 +32,8 @@

 include $(top_srcdir)/common-rules.mk

-MAINTAINERCLEANFILES +CLEANFILES +DISTCLEANFILES 
 EXTRA_DIST = \
 	make-pki.sh \
@@ -214,9 +215,10 @@ keys.psk: $(srcdir)/make-psk.sh
 	SRCDIR=$(srcdir) $(srcdir)/make-psk.sh

 # Keys are expensive to recreate so only delete them when we do
-# ‘make maintainer-clean’.
-MAINTAINERCLEANFILES += keys.psk
-maintainer-clean-local:
+# ‘make distclean’.
+DISTCLEANFILES += keys.psk
+distclean-local: distclean-local-tls
+distclean-local-tls:
 	rm -rf pki

 #----------------------------------------------------------------------
@@ -226,7 +228,7 @@ if HAVE_PLUGINS

 # Common data shared by multiple tests
 check_DATA += file-data
-MAINTAINERCLEANFILES += file-data
+CLEANFILES += file-data
 file-data:
 	rm -f $@ $@-t
 	for f in `$(SEQ) 1 512`; do echo -ne
'\x01\x02\x03\x04\x05\x06\x07\x08'; done > $@-t
@@ -286,7 +288,7 @@ endif HAVE_LIBGUESTFS
 # common disk image shared with several tests
 if HAVE_GUESTFISH
 check_DATA += disk
-MAINTAINERCLEANFILES += disk
+CLEANFILES += disk

 disk:
 	rm -f $@ test1.img
@@ -340,7 +342,7 @@ if HAVE_GUESTFISH

 LIBGUESTFS_TESTS += test-ext2
 check_DATA += ext2.img
-MAINTAINERCLEANFILES += ext2.img
+CLEANFILES += ext2.img

 ext2.img: disk
 	rm -f $@ $@-t
@@ -385,7 +387,7 @@ if HAVE_GUESTFISH

 LIBGUESTFS_TESTS += test-gzip
 check_DATA += disk.gz
-MAINTAINERCLEANFILES += disk.gz
+CLEANFILES += disk.gz

 test_gzip_SOURCES = test-gzip.c test.h
 test_gzip_CFLAGS = $(WARNINGS_CFLAGS) $(LIBGUESTFS_CFLAGS)
@@ -454,7 +456,7 @@ test_random_LDADD = libtest.la $(LIBGUESTFS_LIBS)

 # split files plugin test.
 check_DATA += split1 split2 split3
-MAINTAINERCLEANFILES += split1 split2 split3
+CLEANFILES += split1 split2 split3
 split1: file-data
 	rm -f $@ $@-t
 	dd if=$< of=$@-t bs=1 count=100
@@ -528,6 +530,10 @@ test-ocaml-plugin.so: test_ocaml_plugin.cmx
../plugins/ocaml/libnbdkitocaml.la .
 	  -output-obj -runtime-variant _pic -o $@ \
 	  NBDKit.cmx $< \
 	  -cclib -L../plugins/ocaml/.libs -cclib -lnbdkitocaml
+CLEANFILES += \
+	test_ocaml_plugin.cmx \
+	test_ocaml_plugin.cmi \
+	test-ocaml-plugin.so

 endif HAVE_OCAML

@@ -585,7 +591,7 @@ endif HAVE_RUBY
 # Shell (sh) plugin test.
 LIBGUESTFS_TESTS += test-shell
 check_DATA += test-shell.img
-MAINTAINERCLEANFILES += test-shell.img
+CLEANFILES += test-shell.img

 test_shell_SOURCES = test-lang-plugins.c test.h
 test_shell_CFLAGS = \
@@ -732,7 +738,7 @@ TESTS += test-nozero.sh

 # offset filter test.
 check_DATA += offset-data
-MAINTAINERCLEANFILES += offset-data
+CLEANFILES += offset-data
 LIBGUESTFS_TESTS += test-offset

 offset-data:
@@ -765,7 +771,7 @@ if HAVE_GUESTFISH

 LIBGUESTFS_TESTS += test-xz
 check_DATA += disk.xz
-MAINTAINERCLEANFILES += disk.xz
+CLEANFILES += disk.xz

 test_xz_SOURCES = test-xz.c test.h
 test_xz_CFLAGS = $(WARNINGS_CFLAGS) $(LIBGUESTFS_CFLAGS)
-- 
2.17.2

On Thu, Dec 13, 2018 at 08:55:41AM -0600, Eric Blake
wrote:
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 4c7b59c..55db593 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -32,7 +32,8 @@
> 
>  include $(top_srcdir)/common-rules.mk
> 
> -MAINTAINERCLEANFILES > +CLEANFILES 
Because common-rules.mk defines CLEANFILES, this should be giving a
warning.  And also because of that ...
> +CLEANFILES += \
> +	test_ocaml_plugin.cmx \
> +	test_ocaml_plugin.cmi \
> +	test-ocaml-plugin.so
... this should not be necessary because the CLEANFILES rule in
common-rules.mk cleans these files already.

If that is fixed, ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On 12/13/18 9:23 AM, Richard W.M. Jones wrote:
> On Thu, Dec 13, 2018 at 08:55:41AM -0600, Eric Blake wrote:
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index 4c7b59c..55db593 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -32,7 +32,8 @@
>>
>>   include $(top_srcdir)/common-rules.mk
>>
>> -MAINTAINERCLEANFILES >> +CLEANFILES > 
> Because common-rules.mk defines CLEANFILES, this should be giving a
> warning.  And also because of that ...
I didn't notice the warning - but agree that slamming the variable back 
to empty is not desired ;)
> 
>> +CLEANFILES += \
>> +	test_ocaml_plugin.cmx \
>> +	test_ocaml_plugin.cmi \
>> +	test-ocaml-plugin.so
> 
> ... this should not be necessary because the CLEANFILES rule in
> common-rules.mk cleans these files already.
> 
> If that is fixed, ACK.
Fixed and pushed.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org