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

Thomas Huth posted 1 patch 4 years, 6 months ago
Test FreeBSD passed
Test docker-mingw@fedora passed
Test asan passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191008082815.8267-1-thuth@redhat.com
Makefile               |  6 +++---
tests/Makefile.include | 12 +++++++++++-
2 files changed, 14 insertions(+), 4 deletions(-)
[PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Thomas Huth 4 years, 6 months ago
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>
---
 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 qemu-doc.aux qemu-doc.cp qemu-doc.cps
 	rm -f qemu-doc.fn qemu-doc.fns qemu-doc.info 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
+
 # Build the help program automatically
 
 all: $(QEMU_IOTESTS_HELPERS-y)
-- 
2.18.1


[PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Aleksandar Markovic 4 years, 6 months ago
On Tuesday, October 8, 2019, Thomas Huth <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>
> ---
>  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 qemu-doc.aux qemu-doc.cp qemu-doc.cps
>         rm -f qemu-doc.fn qemu-doc.fns qemu-doc.info 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?

Thanks,
Aleksandar



>  # Build the help program automatically
>
>  all: $(QEMU_IOTESTS_HELPERS-y)
> --
> 2.18.1
>
>
>
Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Thomas Huth 4 years, 6 months ago
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...

 Thomas

Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Peter Maydell 4 years, 6 months ago
On Tue, 8 Oct 2019 at 13:37, Thomas Huth <thuth@redhat.com> wrote:
>
> On 08/10/2019 14.18, Aleksandar Markovic wrote:
> > 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?

https://www.gnu.org/software/automake/manual/html_node/Clean.html

The autoconf manual suggests some heuristics:
 - If make built it, and it is commonly something that one would want
   to rebuild (for instance, a .o file), then mostlyclean should delete it.
 - Otherwise, if make built it, then clean should delete it.
 - If configure built it, then distclean should delete it.
 - If the maintainer built it (for instance, a .info file), then
   maintainer-clean should delete it. However maintainer-clean should
   not delete anything that needs to exist in order to run
'./configure && make'.

Not all of that applies for QEMU, but it seems like a reasonable
set of ideas.

thanks
-- PMM

Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Aleksandar Markovic 3 years, 10 months ago
On Tue, Oct 8, 2019 at 2:41 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 8 Oct 2019 at 13:37, Thomas Huth <thuth@redhat.com> wrote:
> >
> > On 08/10/2019 14.18, Aleksandar Markovic wrote:
> > > 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?
>
> https://www.gnu.org/software/automake/manual/html_node/Clean.html
>
> The autoconf manual suggests some heuristics:
>  - If make built it, and it is commonly something that one would want
>    to rebuild (for instance, a .o file), then mostlyclean should delete it.
>  - Otherwise, if make built it, then clean should delete it.
>  - If configure built it, then distclean should delete it.
>  - If the maintainer built it (for instance, a .info file), then
>    maintainer-clean should delete it. However maintainer-clean should
>    not delete anything that needs to exist in order to run
> './configure && make'.
>

Thomas, can we reincarnate this patch? It needs only fairly simple
corrections, as said by Peter above. It would be nice to have at least
one release of QEMU with clean clean and distclean.

Yours,
Aleksandar
> Not all of that applies for QEMU, but it seems like a reasonable
> set of ideas.
>
> thanks
> -- PMM

Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Thomas Huth 3 years, 10 months ago
On 13/06/2020 18.28, Aleksandar Markovic wrote:
> On Tue, Oct 8, 2019 at 2:41 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Tue, 8 Oct 2019 at 13:37, Thomas Huth <thuth@redhat.com> wrote:
>>>
>>> On 08/10/2019 14.18, Aleksandar Markovic wrote:
>>>> 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?
>>
>> https://www.gnu.org/software/automake/manual/html_node/Clean.html
>>
>> The autoconf manual suggests some heuristics:
>>  - If make built it, and it is commonly something that one would want
>>    to rebuild (for instance, a .o file), then mostlyclean should delete it.
>>  - Otherwise, if make built it, then clean should delete it.
>>  - If configure built it, then distclean should delete it.
>>  - If the maintainer built it (for instance, a .info file), then
>>    maintainer-clean should delete it. However maintainer-clean should
>>    not delete anything that needs to exist in order to run
>> './configure && make'.
>>
> 
> Thomas, can we reincarnate this patch? It needs only fairly simple
> corrections, as said by Peter above. It would be nice to have at least
> one release of QEMU with clean clean and distclean.

That depends ... Marc-André, Paolo, what's the status of the meson patch
series? Do you think it will be merged for the next release already? If
so, it does not make much sense to respin my distclean patch. Otherwise
I could do another iteration...

 Thomas


Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Paolo Bonzini 3 years, 10 months ago
On 15/06/20 07:17, Thomas Huth wrote:
> That depends ... Marc-André, Paolo, what's the status of the meson patch
> series? Do you think it will be merged for the next release already? If
> so, it does not make much sense to respin my distclean patch. Otherwise
> I could do another iteration...

I prefer to aim for 5.2, making it the very first series merged there
and forcing everyone else to rebase.

Paolo


Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Thomas Huth 3 years, 10 months ago
On 15/06/2020 09.13, Paolo Bonzini wrote:
> On 15/06/20 07:17, Thomas Huth wrote:
>> That depends ... Marc-André, Paolo, what's the status of the meson patch
>> series? Do you think it will be merged for the next release already? If
>> so, it does not make much sense to respin my distclean patch. Otherwise
>> I could do another iteration...
> 
> I prefer to aim for 5.2, making it the very first series merged there
> and forcing everyone else to rebase.

Ok, so I think it's likely really not worth the effort to respin this
rather cosmetic patch just for one single release.

Aleksandar, I won't work on this patch anymore, but if it is still
important for you, feel free to take the patch and send an updated version.

 Thomas


Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Markus Armbruster 3 years, 10 months ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 15/06/20 07:17, Thomas Huth wrote:
>> That depends ... Marc-André, Paolo, what's the status of the meson patch
>> series? Do you think it will be merged for the next release already? If
>> so, it does not make much sense to respin my distclean patch. Otherwise
>> I could do another iteration...
>
> I prefer to aim for 5.2, making it the very first series merged there
> and forcing everyone else to rebase.

Makes sense to me.

I wish I had the time for supporting this effort with more review, but
who am I kidding.


Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Paolo Bonzini 4 years, 6 months ago
On 08/10/19 14:41, Peter Maydell wrote:
> The autoconf manual suggests some heuristics:
>  - If make built it, and it is commonly something that one would want
>    to rebuild (for instance, a .o file), then mostlyclean should delete it.
>  - Otherwise, if make built it, then clean should delete it.
>  - If configure built it, then distclean should delete it.
>  - If the maintainer built it (for instance, a .info file), then
>    maintainer-clean should delete it. However maintainer-clean should
>    not delete anything that needs to exist in order to run
> './configure && make'.
> 
> Not all of that applies for QEMU, but it seems like a reasonable
> set of ideas.

Right, considering we don't have mostlyclean, it boils down to "if make
built it, then clean should delete it; if configure built it, then
distclean should delete it".

Paolo

Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Daniel P. Berrangé 4 years, 6 months ago
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 :|

Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Markus Armbruster 4 years, 6 months ago
Daniel P. Berrangé <berrange@redhat.com> writes:

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

I don't wish to belittle anybody's efforts at improving make targets
clean and distclean, but like Daniel, I've long given up on them[*].  My
"make distclean" is "rm -rf BUILD-DIR", and my "make clean" is "nuke-bld
BUILD-DIR", where nuke-bld is the script below.

#!/bin/sh -e

if [ $# -gt 1 ]
then
    echo "Usage: $0 [bld-dir]" >&2
    exit 1
fi

[ $# -eq 1 ] && cd "$1"

if [ ! -e config.status ]
then
    echo "$0: not a build tree" >&2
    exit 1
fi

rm -f .config.status
mv config.status .config.status
rm -r *
mv .config.status config.status
./config.status


[*] Along with .gitignore.

Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Thomas Huth 4 years, 6 months ago
On 08/10/2019 14.41, Daniel P. Berrangé wrote:
> 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 don't think that we'll convert to meson within the next release, I
rather assume that this will take quite a bit of additional time, so
fixing this now seems reasonable to me.

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

I could agree with "distclean", but as long as nobody cares to get such
a "builddir != srcdir"  merged, we should not remove it.
And I don't agree for removing the "clean" target. Sometimes you want to
recompile without destroying your "config.status", and for these cases
it's quite useful.

 Thomas

Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Aleksandar Markovic 4 years, 6 months ago
On Tuesday, October 8, 2019, Thomas Huth <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>
> ---
>  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(-)
>
> Hello, Thomas,

Do you intend to send a new version of this patch?

It looks to me this is a very nice clean up that definitely should go into
4.2, no?

Aleksandat



> 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 qemu-doc.aux qemu-doc.cp qemu-doc.cps
>         rm -f qemu-doc.fn qemu-doc.fns qemu-doc.info 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
> +
>  # Build the help program automatically
>
>  all: $(QEMU_IOTESTS_HELPERS-y)
> --
> 2.18.1
>
>
>
Re: [PATCH v2] Makefile: Remove generated files when doing 'distclean' (and 'clean')
Posted by Thomas Huth 4 years, 6 months ago
----- Original Message -----
> From: "Aleksandar Markovic" <aleksandar.m.mail@gmail.com>
> Sent: Tuesday, October 22, 2019 10:35:51 PM
> 
> On Tuesday, October 8, 2019, Thomas Huth <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>
> > ---
> >  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(-)
> >
> > Hello, Thomas,
> 
> Do you intend to send a new version of this patch?
> 
> It looks to me this is a very nice clean up that definitely should go into
> 4.2, no?

I'm still planning to send a v3 of this patch, but currently I don't have time to work on this during the next two weeks. So this will likely miss the deadline for 4.2. Feel free to pick up my patch and send a fixed version if you think that this should still go into 4.2. Otherwise, I'll take care of it in 5.0.

 Thomas