'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
Richard W.M. Jones
2018-Dec-13 15:23 UTC
Re: [Libguestfs] [nbdkit PATCH] maint: Adjust cleaning rules
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 > +CLEANFILESBecause 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
Eric Blake
2018-Dec-13 16:11 UTC
Re: [Libguestfs] [nbdkit PATCH] maint: Adjust cleaning rules
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
Reasonably Related Threads
- [nbdkit PATCH 2/1] RFC: tests: Run tests that don't require libguestfs
- [PATCH nbdkit v2 2/4] Rename nbdkit-reflection-plugin to nbdkit-info-plugin.
- Re: [nbdkit PATCH] maint: Adjust cleaning rules
- [PATCH nbdkit 1/4] Add reflection plugin.
- [nbdkit PATCH] ocaml: Make build VPATH aware