[PATCH] subdirs: unify rules for "cleaning" and "normal" recursive targets

[Stefano Lattarini]

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, reading in section "First Take
on Dependency Tracking":

    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 then, in the following section "Dependencies As Side Effects":

  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.  Not however that we still keep '$(RECURSIVE_TARGETS)' and
'$(RECURSIVE_CLEAN_TARGETS)' as two distinct variables, to ensure a
better backward-compatibility for any user-defined rules that happen
to use those variables.

* 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 <address@hidden>
---
 lib/am/subdirs.am     |   54 +++++---------------------
 t/list-of-tests.mk    |    1 +
 t/subdir-distclean.sh |  103 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 45 deletions(-)
 create mode 100755 t/subdir-distclean.sh

diff --git a/lib/am/subdirs.am b/lib/am/subdirs.am
index 1d1295e..3fc2888 100644
--- 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 a79f8cb..5418716 100644
--- a/t/list-of-tests.mk
+++ b/t/list-of-tests.mk
@@ -1008,6 +1008,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
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
+
+:
-- 
1.7.9.5