Cleanup Makefile.am CLEANFILES definitions
Commit Message
Since b2f225d6bff8 ("Consolidate and add files to clean target variables")
autoreconf (automake) produces these warnings:
debuginfod/Makefile.am:130: warning: CLEANFILES multiply defined in condition TRUE ...
config/eu.am:138: ... 'CLEANFILES' previously defined here
debuginfod/Makefile.am:32: 'config/eu.am' included from here
libasm/Makefile.am:91: warning: CLEANFILES multiply defined in condition TRUE ...
config/eu.am:138: ... 'CLEANFILES' previously defined here
libasm/Makefile.am:30: 'config/eu.am' included from here
libcpu/Makefile.am:105: warning: CLEANFILES multiply defined in condition TRUE ...
config/eu.am:138: ... 'CLEANFILES' previously defined here
libcpu/Makefile.am:30: 'config/eu.am' included from here
libdw/Makefile.am:156: warning: CLEANFILES multiply defined in condition TRUE ...
config/eu.am:138: ... 'CLEANFILES' previously defined here
libdw/Makefile.am:30: 'config/eu.am' included from here
libelf/Makefile.am:142: warning: CLEANFILES multiply defined in condition TRUE ...
config/eu.am:138: ... 'CLEANFILES' previously defined here
libelf/Makefile.am:30: 'config/eu.am' included from here
src/Makefile.am:47: warning: CLEANFILES multiply defined in condition TRUE ...
config/eu.am:138: ... 'CLEANFILES' previously defined here
src/Makefile.am:19: 'config/eu.am' included from here
tests/Makefile.am:891: warning: CLEANFILES multiply defined in condition TRUE ...
config/eu.am:138: ... 'CLEANFILES' previously defined here
tests/Makefile.am:19: 'config/eu.am' included from here
This is because config/eu.am defines a default CLEANFILES. So those
Makefile.am files should add to CLEANFILES with += instead of
redefining CLEANFILES with =.
* debuginfod/Makefile.am (CLEANFILES): Use +=.
* libasm/Makefile.am (CLEANFILES): Likewise.
* libcpu/Makefile.am (CLEANFILES): Likewise.
* libdw/Makefile.am (CLEANFILES): Likewise.
* libelf/Makefile.am (CLEANFILES): Likewise.
* src/Makefile.am (CLEANFILES): Likewise.
* tests/Makefile.am (CLEANFILES): Likewise.
Signed-off-by: Mark Wielaard <mark@klomp.org>
---
debuginfod/Makefile.am | 2 +-
libasm/Makefile.am | 2 +-
libcpu/Makefile.am | 2 +-
libdw/Makefile.am | 2 +-
libelf/Makefile.am | 2 +-
src/Makefile.am | 2 +-
tests/Makefile.am | 2 +-
7 files changed, 7 insertions(+), 7 deletions(-)
Comments
On Mon, Nov 18, 2024 at 1:12 PM Mark Wielaard <mark@klomp.org> wrote:
>
> Since b2f225d6bff8 ("Consolidate and add files to clean target variables")
> autoreconf (automake) produces these warnings:
>
> debuginfod/Makefile.am:130: warning: CLEANFILES multiply defined in condition TRUE ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> debuginfod/Makefile.am:32: 'config/eu.am' included from here
> libasm/Makefile.am:91: warning: CLEANFILES multiply defined in condition TRUE ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> libasm/Makefile.am:30: 'config/eu.am' included from here
> libcpu/Makefile.am:105: warning: CLEANFILES multiply defined in condition TRUE ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> libcpu/Makefile.am:30: 'config/eu.am' included from here
> libdw/Makefile.am:156: warning: CLEANFILES multiply defined in condition TRUE ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> libdw/Makefile.am:30: 'config/eu.am' included from here
> libelf/Makefile.am:142: warning: CLEANFILES multiply defined in condition TRUE ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> libelf/Makefile.am:30: 'config/eu.am' included from here
> src/Makefile.am:47: warning: CLEANFILES multiply defined in condition TRUE ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> src/Makefile.am:19: 'config/eu.am' included from here
> tests/Makefile.am:891: warning: CLEANFILES multiply defined in condition TRUE ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> tests/Makefile.am:19: 'config/eu.am' included from here
>
> This is because config/eu.am defines a default CLEANFILES. So those
> Makefile.am files should add to CLEANFILES with += instead of
> redefining CLEANFILES with =.
>
> * debuginfod/Makefile.am (CLEANFILES): Use +=.
> * libasm/Makefile.am (CLEANFILES): Likewise.
> * libcpu/Makefile.am (CLEANFILES): Likewise.
> * libdw/Makefile.am (CLEANFILES): Likewise.
> * libelf/Makefile.am (CLEANFILES): Likewise.
> * src/Makefile.am (CLEANFILES): Likewise.
> * tests/Makefile.am (CLEANFILES): Likewise.
Thanks Mark, this patch fixes the warnings for me.
Aaron
Hi Mark,
> Since b2f225d6bff8 ("Consolidate and add files to clean target variables")
> autoreconf (automake) produces these warnings:
oops, sorry...
> This is because config/eu.am defines a default CLEANFILES. So those
> Makefile.am files should add to CLEANFILES with += instead of
> redefining CLEANFILES with =.
May I suggest to instead simply use
"DISTCLEANFILES" for what is in eu.am?
It's not being used anywhere else currently...
If that's no good, I would then like to see a '+='
in all the "*CLEANFILES" definitions in each level for each file
so it's not confusing or looking like a possible mistake.
--
MCP
Hi Michael,
On Tue, Nov 19, 2024 at 04:18:17AM +0000, Michael Pratt wrote:
>
> > This is because config/eu.am defines a default CLEANFILES. So those
> > Makefile.am files should add to CLEANFILES with += instead of
> > redefining CLEANFILES with =.
>
> May I suggest to instead simply use
> "DISTCLEANFILES" for what is in eu.am?
> It's not being used anywhere else currently...
But aren't DISTCLEANFILES only remove for make distclean? The *.gcno
*.gcda could be added to MOSTLYCLEANFILES since you would normally
want to regenerate them. I am not sure that would resolve your issue
though.
> If that's no good, I would then like to see a '+='
> in all the "*CLEANFILES" definitions in each level for each file
> so it's not confusing or looking like a possible mistake.
You mean even for MAINTAINERCLEANFILES and MOSTLYCLEANFILES? and even
for those (config/Makefile.am ./Makefile.am doc/Makefile.am) that
don't include eu.am (and also don't use CLEANFILES, except for the
top-level one)?
The "rule" now seems to be:
- When you use CLEANFILES you should now always use +=.
= MAINTAINERCLEANFILES and MOSTLYCLEANFILES uses =.
Is that "rule" really that confusing?
Cheers,
Mark
Hi Mark,
On 11/21/24 7:44 PM, Mark Wielaard <mark@klomp.org> wrote:
> Hi Michael,
>
> On Tue, Nov 19, 2024 at 04:18:17AM +0000, Michael Pratt wrote:
> >
> > > This is because config/eu.am defines a default CLEANFILES. So those
> > > Makefile.am files should add to CLEANFILES with += instead of
> > > redefining CLEANFILES with =.
> >
> > May I suggest to instead simply use
> > "DISTCLEANFILES" for what is in eu.am?
> > It's not being used anywhere else currently...
>
> But aren't DISTCLEANFILES only remove for make distclean? The *.gcno
> *.gcda could be added to MOSTLYCLEANFILES since you would normally
> want to regenerate them. I am not sure that would resolve your issue
> though.
You would be a better judge of when the files
should be removed, I'm not as familiar with their purpose.
I was imagining that only maintainers
or only those who develop like the maintainers would bother with these files,
so making it closer to the "maintainer-clean" target would be good enough.
Since the DISTCLEANFILES variable is currently unused
it would be one way to resolve the issue,
although provided that it's not added elsewhere later... so maybe not the best step.
> > If that's no good, I would then like to see a '+='
> > in all the "*CLEANFILES" definitions in each level for each file
> > so it's not confusing or looking like a possible mistake.
>
> You mean even for MAINTAINERCLEANFILES and MOSTLYCLEANFILES? and even
> for those (config/Makefile.am ./Makefile.am doc/Makefile.am) that
> don't include eu.am (and also don't use CLEANFILES, except for the
> top-level one)?
The Makefile.am files that don't include eu.am
also don't have "CLEANFILES" variables...
except for the top-level one...
Ironically, the CLEANFILES definitions
in both eu.am and the top-level Makefile.am
seem to be for the same purpose which is coverage files...
Further simplification can also be a solution to this,
so I'm going to send a small patch set for you all to consider.
It's likely a better idea than the 3 ideas presented here so far.
> The "rule" now seems to be:
> - When you use CLEANFILES you should now always use +=.
> = MAINTAINERCLEANFILES and MOSTLYCLEANFILES uses =.
>
> Is that "rule" really that confusing?
It's not written anywhere that this is the case
so someone seeing this for the first time
would have to do some digging to see why it is there
provided that they realize there is a good reason for it.
I think several people including myself
see the subdirectories as independent
and would consider the list of files to clean
being independent to logically follow from that.
I don't really see it as a rule,
rather just an inconsistency.
It just happens to be the way that the first person
to write these cleaning steps happen to do so
when there's really many ways to write it.
And again, I'm sorry for not noticing this earlier.
For whatever reason I didn't see the pattern,
and the '+=' looked unnecessary at first glance,
also since some other instances of the '+=' I saw
were because of other lines in the same file in other cases.
For me, a rule would be to write the rules
the most simple way whenever possible,
leading to simpler and smaller automake usage.
--
MCP
@@ -127,7 +127,7 @@ endif
EXTRA_DIST = libdebuginfod.map
MOSTLYCLEANFILES = $(am_libdebuginfod_a_OBJECTS) $(am_libdebuginfod_pic_a_OBJECTS) $(LIBDEBUGINFOD_SONAME)
-CLEANFILES = libdebuginfod.so
+CLEANFILES += libdebuginfod.so
# automake std-options override: arrange to pass LD_LIBRARY_PATH
installcheck-binPROGRAMS: $(bin_PROGRAMS)
@@ -88,4 +88,4 @@ noinst_HEADERS = libasmP.h symbolhash.h
EXTRA_DIST = libasm.map
MOSTLYCLEANFILES = $(am_libasm_a_OBJECTS) $(am_libasm_pic_a_OBJECTS) libasm.so.$(VERSION)
-CLEANFILES = libasm.so
+CLEANFILES += libasm.so
@@ -102,5 +102,5 @@ bpf_disasm_CFLAGS = -Wno-format-nonliteral
EXTRA_DIST = defs/i386
MOSTLYCLEANFILES = $(am_libcpu_a_OBJECTS) $(am_libcpu_pic_a_OBJECTS) $(i386_gendis_OBJECTS)
-CLEANFILES = $(foreach P,i386 x86_64,$P_defs $P.mnemonics)
+CLEANFILES += $(foreach P,i386 x86_64,$P_defs $P.mnemonics)
MAINTAINERCLEANFILES = $(foreach P,i386 x86_64, $P_defs $P_dis.h $P_parse.h)
@@ -153,5 +153,5 @@ noinst_HEADERS = libdwP.h memory-access.h dwarf_abbrev_hash.h \
EXTRA_DIST = libdw.map
MOSTLYCLEANFILES = $(am_libdw_a_OBJECTS) $(am_libdw_pic_a_OBJECTS) libdw.so.$(VERSION)
-CLEANFILES = libdw.so
+CLEANFILES += libdw.so
MAINTAINERCLEANFILES = $(srcdir)/known-dwarf.h
@@ -139,4 +139,4 @@ uninstall: uninstall-am
EXTRA_DIST = libelf.map
MOSTLYCLEANFILES = $(am_libelf_a_OBJECTS) $(am_libelf_pic_a_OBJECTS) libelf.so.$(VERSION)
-CLEANFILES = libelf.so
+CLEANFILES += libelf.so
@@ -44,7 +44,7 @@ bin_SCRIPTS = make-debug-archive
EXTRA_DIST = arlib.h debugpred.h make-debug-archive.in
MOSTLYCLEANFILES = *.gconv
-CLEANFILES = $(bin_SCRIPTS)
+CLEANFILES += $(bin_SCRIPTS)
if BUILD_STATIC
libasm = ../libasm/libasm.a
@@ -888,7 +888,7 @@ system_elf_gelf_test_LDADD = $(libelf)
# A lock file used to make sure only one test dumps core at a time
MOSTLYCLEANFILES = core-dump-backtrace.lock
-CLEANFILES = $(BUILT_SOURCES)
+CLEANFILES += $(BUILT_SOURCES)
if GCOV
check: check-am coverage