subdirs: unify rules for "cleaning" and "normal" recursive targets

Before this change, the recursive invocation of cleaning targets in
the $(SUBDIRS) where done in inverse order, i.e., starting from the
last $(SUBDIRS) entry and proceeding towards the first.  According
to the code comments, this was done ...

    ... in an attempt to alleviate a problem that can happen when
    dependencies are enabled.  In this case, the .P file in one
    directory can depend on some automatically generated header
    in an earlier directory.  Since the dependencies are required
    before any target is examined, make bombs.

But this comment does not apply anymore to the current implementation
of automatic dependency tracking: the '.Po' and '.Plo' files does not
depend on any C header or source file, ever!

So it seems that the distinction between "normal" and "cleaning"
recursive targets is a stale leftover of an older implementation of
the automatic dependency tracking.  In fact, the Automake History
manual seems to confirm this suspect; the section "First Take on
Dependency Tracking" reads:

    Because each .P file was a dependency of Makefile, this meant
    that dependency tracking was done eagerly by make.  For instance,
    "make clean" would cause all the dependency files to be updated,
    and then immediately removed.  This eagerness also caused problems
    with some configurations; if a certain source file could not be
    compiled on a given architecture for some reason, dependency
    tracking would fail, aborting the entire build.

and the following section "Dependencies As Side Effects" reads:

    In this approach, the .P files were included using the -include
    command, which let us create these files lazily.  This avoided
    the "make clean" problem.

So the distinction between "normal" and "cleaning" recursive targets
has likely been obsolete since by then already.  We can thus remove
such distinction, thus reducing some complications and duplication in
our rules.  Doing so, the whole testsuite still passes (both with GCC
and Sun C 5.9), even the test 'c-demo.sh', which, among the other
things, exercise the setup described in the obsolete code comment
referenced above.

Finally, note that we still keep '$(RECURSIVE_CLEAN_TARGETS)' and
'$(RECURSIVE_TARGETS)' as two distinct variables, to ensure a better
backward-compatibility for any user-defined rules that happen to use
those variables.

* NEWS: Update.
* lib/am/subdirs.am ($(RECURSIVE_CLEAN_TARGETS), $(CLEAN_TARGETS)):
Merge their recipes.
* t/subdir-distclean.sh: New test, check that "./configure && make
&& make distclean" is actually a no-op, even when conditional SUBDIRS
are involved.
* t/list-of-tests.mk: Add it.

Signed-off-by: Stefano Lattarini <stefano.lattarini@gmail.com>

diff --git a/NEWS b/NEWS
index fd52dea3e07c847658d03cb9c3e58bf9e91fdb59..19d19636c2cf440dab7056e8f64285309effb333 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -60,6 +60,16 @@ New in 1.12.2:
     input file.  Such a warning will also be present in the next
     Autoconf version (2.70).
 
+* Cleaning rules:
+
+  - Recursive cleaning rules descends into the $(SUBDIRS) in the natural
+    order (as done by the other recursive rules), rather than in the
+    inverse order.  They used to do that in order to work a round a
+    limitation in an older implementation of the automatic dependency
+    tracking support, but that limitation had been lifted years ago
+    already, when the automatic dependency tracking based on side-effects
+    of compilation had been introduced.
+
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 New in 1.12.1:

diff --git a/lib/am/subdirs.am b/lib/am/subdirs.am
index 1d1295e1fb0328eb83df87765359d1aeb22432db..3fc28884b616146f9930775c24cd0906f7621ce1 100644 (file)
--- a/lib/am/subdirs.am
+++ b/lib/am/subdirs.am
@@ -32,7 +32,7 @@ AM_RECURSIVE_TARGETS += $(RECURSIVE_TARGETS:-recursive=) \
 #     (which will cause the Makefiles to be regenerated when you run 'make');
 # (2) otherwise, pass the desired values on the 'make' command line.
 
-$(RECURSIVE_TARGETS):
+$(RECURSIVE_TARGETS) $(RECURSIVE_CLEAN_TARGETS):
 ## Using $failcom allows "-k" to keep its natural meaning when running a
 ## recursive rule.
        @fail= failcom='exit 1'; \
@@ -44,7 +44,14 @@ $(RECURSIVE_TARGETS):
        done; \
        dot_seen=no; \
        target=`echo $@ | sed s/-recursive//`; \
-       list='$(SUBDIRS)'; for subdir in $$list; do \
+## For distclean and maintainer-clean we make sure to use the full
+## list of subdirectories.  We do this so that 'configure; make
+## distclean' really is a no-op, even if SUBDIRS is conditional.
+       case "$@" in \
+         distclean-* | maintainer-clean-*) list='$(DIST_SUBDIRS)' ;; \
+         *) list='$(SUBDIRS)' ;; \
+       esac; \
+       for subdir in $$list; do \
          echo "Making $$target in $$subdir"; \
          if test "$$subdir" = "."; then \
            dot_seen=yes; \
@@ -65,46 +72,3 @@ clean: clean-recursive
 distclean: distclean-recursive
 maintainer-clean: maintainer-clean-recursive
 
-## We run all 'clean' targets in reverse order.  Why?  It's an attempt
-## to alleviate a problem that can happen when dependencies are
-## enabled.  In this case, the .P file in one directory can depend on
-## some automatically generated header in an earlier directory.  Since
-## the dependencies are required before any target is examined, make
-## bombs.
-$(RECURSIVE_CLEAN_TARGETS):
-## Using $failcom allows "-k" to keep its natural meaning when running a
-## recursive rule.
-       @fail= failcom='exit 1'; \
-       for f in x $$MAKEFLAGS; do \
-         case $$f in \
-           *=* | --[!k]*);; \
-           *k*) failcom='fail=yes';; \
-         esac; \
-       done; \
-       dot_seen=no; \
-## For distclean and maintainer-clean we make sure to use the full
-## list of subdirectories.  We do this so that 'configure; make
-## distclean' really is a no-op, even if SUBDIRS is conditional.  For
-## other clean targets this doesn't matter.
-       case "$@" in \
-         distclean-* | maintainer-clean-*) list='$(DIST_SUBDIRS)' ;; \
-         *) list='$(SUBDIRS)' ;; \
-       esac; \
-       rev=''; for subdir in $$list; do \
-         if test "$$subdir" = "."; then :; else \
-           rev="$$subdir $$rev"; \
-         fi; \
-       done; \
-## Always do '.' last.
-       rev="$$rev ."; \
-       target=`echo $@ | sed s/-recursive//`; \
-       for subdir in $$rev; do \
-         echo "Making $$target in $$subdir"; \
-         if test "$$subdir" = "."; then \
-           local_target="$$target-am"; \
-         else \
-           local_target="$$target"; \
-         fi; \
-         ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS) $$local_target) \
-         || eval $$failcom; \
-       done && test -z "$$fail"

diff --git a/t/list-of-tests.mk b/t/list-of-tests.mk
index 24172cf621b9fbb7b314405b07362ff9a1e119d4..ebec34d63c9c2acf605ebeaa8333e370c0b3261a 100644 (file)
--- a/t/list-of-tests.mk
+++ b/t/list-of-tests.mk
@@ -1027,6 +1027,7 @@ t/subdir7.sh \
 t/subdir8.sh \
 t/subdir9.sh \
 t/subdir10.sh \
+t/subdir-distclean.sh \
 t/subdirbuiltsources.sh \
 t/subcond.sh \
 t/subcond2.sh \

diff --git a/t/subdir-distclean.sh b/t/subdir-distclean.sh
new file mode 100755 (executable)
index 0000000..df89420
--- /dev/null
+++ b/t/subdir-distclean.sh
@@ -0,0 +1,103 @@
+#! /bin/sh
+# Copyright (C) 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that "./configure && make && make distclean" is actually a
+# no-op, even when conditional SUBDIRS are involved.
+
+. ./defs || Exit 1
+
+cat >> configure.ac << 'END'
+AC_CONFIG_FILES([sub1/Makefile sub2/Makefile sub1/subsub/Makefile])
+AM_CONDITIONAL([COND], [false])
+AC_SUBST([extra_subdirs], [''])
+AC_OUTPUT
+END
+
+mkdir sub1 sub2 sub1/subsub
+
+cat > Makefile.am << 'END'
+SUBDIRS = sub1
+if COND
+SUBDIRS += sub2
+endif
+END
+
+cat > sub1/Makefile.am << 'END'
+all-local:
+       : > run
+CLEANFILES = run
+SUBDIRS = @extra_subdirs@
+DIST_SUBDIRS = subsub
+END
+
+cat > sub2/Makefile.am << 'END'
+all-local:
+       @echo "Should not run in `pwd`!"
+       exit 1
+DISTCLEANFILES = oops
+END
+cp sub2/Makefile.am sub1/subsub/Makefile.am
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE -c --add-missing
+
+./configure
+
+test -f sub1/Makefile
+test -f sub2/Makefile
+test -f sub1/subsub/Makefile
+
+$MAKE
+test -f sub1/run
+touch sub2/oops sub1/subsub/oops
+
+$MAKE distclean
+test ! -f sub1/run
+test ! -f sub2/oops
+test ! -f sub1/subsub/oops
+test ! -f sub1/Makefile
+test ! -f sub2/Makefile
+test ! -f sub1/subsub/Makefile
+
+mkdir build
+cd build
+
+../configure
+
+$MAKE
+
+test -f sub1/Makefile
+test -f sub2/Makefile
+test -f sub1/subsub/Makefile
+
+test -f sub1/run
+touch sub2/oops sub1/subsub/oops
+
+$MAKE -j4 maintainer-clean
+test ! -f sub1/run
+test ! -f sub2/oops
+test ! -f sub1/subsub/oops
+test ! -f sub1/Makefile
+test ! -f sub2/Makefile
+test ! -f sub1/subsub/Makefile
+
+cd ..
+
+./configure
+$MAKE distclean
+
+: