Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Oct 08, 2019 at 02:35:20PM +0200, Thomas Huth wrote:
> On 08/10/2019 14.18, Aleksandar Markovic wrote:
> > 
> > 
> > On Tuesday, October 8, 2019, Thomas Huth <thuth@redhat.com
> > <mailto:thuth@redhat.com>> wrote:
> > 
> >     When running "make distclean" we currently leave a lot of generated
> >     files in the build directory. These should be completely removed.
> >     Some of the generated files are removed in the "clean" target (which
> >     is a prerequisite for the "distclean" target), since binary files
> >     should be removed in this step already.
> > 
> >     Signed-off-by: Thomas Huth <thuth@redhat.com <mailto:thuth@redhat.com>>
> >     ---
> >      v2:
> >      - Remove tests/qemu-iotests/common.env in "distclean", not in "clean"
> >      - Improved patch description
> > 
> >      Makefile               |  6 +++---
> >      tests/Makefile.include | 12 +++++++++++-
> >      2 files changed, 14 insertions(+), 4 deletions(-)
> > 
> >     diff --git a/Makefile b/Makefile
> >     index 30f0abfb42..767b1ffb25 100644
> >     --- a/Makefile
> >     +++ b/Makefile
> >     @@ -696,14 +696,14 @@ clean: recurse-clean
> >                     -exec rm {} +
> >             rm -f $(edk2-decompressed)
> >             rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) TAGS
> >     cscope.* *.pod *~ */*~
> >     -       rm -f fsdev/*.pod scsi/*.pod
> >     +       rm -f fsdev/*.pod scsi/*.pod docs/*.pod docs/*/*.pod
> >     docs/*/.buildinfo
> >             rm -f qemu-img-cmds.h
> >             rm -f ui/shader/*-vert.h ui/shader/*-frag.h
> >             @# May not be present in generated-files-y
> >             rm -f trace/generated-tracers-dtrace.dtrace*
> >             rm -f trace/generated-tracers-dtrace.h*
> >             rm -f $(foreach f,$(generated-files-y),$(f) $(f)-timestamp)
> >     -       rm -f qapi-gen-timestamp
> >     +       rm -f qapi-gen-timestamp vhost-user-input
> >             rm -rf qga/qapi-generated
> >             rm -f config-all-devices.mak
> > 
> >     @@ -724,7 +724,7 @@ distclean: clean
> >             rm -f tests/tcg/config-*.mak
> >             rm -f config-all-devices.mak config-all-disas.mak config.status
> >             rm -f $(SUBDIR_DEVICES_MAK)
> >     -       rm -f po/*.mo tests/qemu-iotests/common.env
> >     +       rm -f po/*.mo
> >             rm -f roms/seabios/config.mak roms/vgabios/config.mak
> >             rm -f qemu-doc.info <http://qemu-doc.info> qemu-doc.aux
> >     qemu-doc.cp qemu-doc.cps
> >             rm -f qemu-doc.fn qemu-doc.fns qemu-doc.info
> >     <http://qemu-doc.info> qemu-doc.ky <http://qemu-doc.ky> qemu-doc.kys
> >     diff --git a/tests/Makefile.include b/tests/Makefile.include
> >     index 3543451ed3..694f193fb6 100644
> >     --- a/tests/Makefile.include
> >     +++ b/tests/Makefile.include
> >     @@ -1176,11 +1176,21 @@ check: check-block check-qapi-schema
> >     check-unit check-softfloat check-qtest chec
> >      check-clean:
> >             rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
> >             rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST),
> >     $(check-qtest-$(target)-y)) $(check-qtest-generic-y))
> >     -       rm -f tests/test-qapi-gen-timestamp
> >             rm -rf $(TESTS_VENV_DIR) $(TESTS_RESULTS_DIR)
> >     +       rm -f tests/test-qapi-gen-timestamp tests/qht-bench$(EXESUF) \
> >     +               tests/fp/fp-test tests/fp/*.out
> >     tests/qapi-schema/*.test.*
> > 
> >      clean: check-clean
> > 
> >     +check-distclean:
> >     +       rm -f tests/qemu-iotests/common.env tests/qemu-iotests/check.*
> >     +       rm -f tests/test-qapi-types*.c tests/test-qapi-visit*.c \
> >     +               tests/test-qapi-commands*.c tests/test-qapi-events*.c \
> >     +               tests/test-qapi-emit-events.[ch]
> >     tests/test-qapi-introspect.c \
> >     +               tests/include/test-qapi-*.c
> >     +
> >     +distclean: check-distclean
> >     +
> > 
> > 
> > Sorry for bad email format, I have some difficultues with mail accounts
> > and  mail clients.
> > 
> > Thomas,
> > 
> > If I remember well, QAPI-related c files are generated while doing
> > 'make'. If that is true, these files should be deleted by 'make clean',
> > shouldn't they?
> 
> I think that's a philosophical question. Should "make clean" delete all
> files that have been generated by a Makefile (i.e. not by "configure"),
> or rather mainly the binary files?
> 
> If you have a look at our main Makefile in the topmost directory, you
> can see that we remove lots of files that are obviously generated by
> "make" in the "distclean" target, so it seems we have no clear rule what
> should be deleted in each step...

Given that we're aiming to convert to meson, how much effort do we really
want to put into fixing this inconsistency ?

I'd argue the best way to fix "clean" and "distclean" is to delete them
entirely. Instead enforce that builddir != srcdir when running configure,
so cleaning up simply means deleting the builddir sub-tree entirely.

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