Re: Makefile broken after removing included *.am file

[Ralf Wildenhues]

Hi Peter,

* Peter Johansson wrote on Sun, Nov 15, 2009 at 04:37:24AM CET:
> I included a file in my Makefile.am, but then I decided it was not
> useful anymore so I removed the include statement and deleted the
> file. That resulted in a broken Makefile, and running `make all'
> resulted in:
> 
> make: *** No rule to make target `aminclude.am', needed by
> `Makefile.in'.  Stop.
> 
> This is very similar to the "deleted header file problem" for *.m4
> files that was fixed in version 1.11 by adding a stub rule. I
> suppose this problem could be solved in the same fashion.

Thanks for the bug report and the test; that sounds like a good
suggestion to me.

I am currently testing this patch series (generated changes elided).
Notes:

- This made me find a bug in the current remaking rules: if we remove
some subdir Makefile.am and also let, say, aclocal.m4 be out of date,
then remaking won't fail, but IMHO it is more likely that failure would
be expected.  After all, if I change some Makefile from being automake-
generated to being not automake-generated, after removing Makefile.am
and writing Makefile.in, I can go the extra mile and issue
  ./config.status some/Makefile

manually, to avoid failure.  Ensuring that failure happens otherwise is
addressed in the second patch.

- The first patch merely adds some coverage that a generated Makefile.am
works (and continues to work with further changes).

- I was going to put %MAKEFILE-AM% in $(am__makefile_in_deps) as well,
however they are not symmetric in all respects: if automake is invoked
without a Makefile argument, then it ignores Makefile.in files without
a corresponding Makefile.am file.  Having an empty dependency like
  $(srcdir)/Makefile.am:

will not make this any worse, but it felt weird enough anyway; also,
for C code we also don't introduce such an empty rule for the primary
.c source file either.

The patch series needs (trivial) fixing of include.test at least.

Cheers,
Ralf


>From 0be2441c40c5ce68ec3b3bac32a852c3402be883 Mon Sep 17 00:00:00 2001
From: Ralf Wildenhues <address@hidden>
Date: Sun, 15 Nov 2009 13:49:21 +0100
Subject: [PATCH 1/3] Ensure remaking Makefile.in works with a generated 
Makefile.am.

* tests/remake8.test: New test.
* tests/Makefile.am: Update.
---
 tests/Makefile.am  |    1 +
 tests/Makefile.in  |    1 +
 tests/remake8.test |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 0 deletions(-)
 create mode 100755 tests/remake8.test

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2c2e89a..935b3d0 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -571,6 +571,7 @@ remake4.test \
 remake5.test \
 remake6.test \
 remake7.test \
+remake8.test \
 regex.test \
 req.test \
 reqd.test \
diff --git a/tests/remake8.test b/tests/remake8.test
new file mode 100755
index 0000000..b2c9573
--- /dev/null
+++ b/tests/remake8.test
@@ -0,0 +1,63 @@
+#! /bin/sh
+# Copyright (C) 2009  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/>.
+
+# Make sure remaking Makefile.in works with a generated Makefile.am.
+
+. ./defs || Exit 1
+set -e
+
+cat >> configure.in << 'END'
+AC_CONFIG_FILES([sub/Makefile])
+AC_OUTPUT
+END
+
+mkdir sub
+cat > Makefile.am <<'END'
+SUBDIRS = sub
+END
+
+cat > sub/Makefile.am.in <<'END'
+$(srcdir)/Makefile.am: $(srcdir)/Makefile.am.in generated.am
+       cat $(srcdir)/Makefile.am.in $(srcdir)/generated.am > $@
+
+# `make check' should fail without the generated content.
+check-local: foo
+## generated content follows.
+END
+
+cp sub/Makefile.am.in sub/Makefile.am
+
+cat > sub/generated.am <<'END'
+EXTRA_DIST = generated.am Makefile.am.in
+foo:
+       echo $@
+END
+
+$ACLOCAL
+$AUTOMAKE
+$AUTOCONF
+mkdir build
+cd build
+../configure
+$MAKE
+
+$sleep
+
+touch ../sub/Makefile.am.in
+$MAKE
+$MAKE distcheck
+
+Exit 0
-- 
1.6.5.1.31.gad12b


>From 1e543359949a364ef5ee97441e56b07735ad87ae Mon Sep 17 00:00:00 2001
From: Ralf Wildenhues <address@hidden>
Date: Sun, 15 Nov 2009 13:50:51 +0100
Subject: [PATCH 2/3] WIP Ensure removing a Makefile.am provokes an error.

If some subdir Makefile.am was removed, and also some configure
input file was out of date, then the rebuilding rules might not
cause an error due to the removed Makefile.am: the toplevel
refresh rule invokes automake without arguments, which ignores
nonexistent Makefile.am files.

* lib/am/configure.am [!TOPDIR_P] (%MAKEFILE-IN%): Only skip the
per-Makefile rebuild rule if %MAKEFILE-IN% is newer than all of
its local input files.
* tests/remake9.test: New test.
* tests/Makefile.am: Update.
---
 Makefile.in                    |    2 +-
 doc/Makefile.in                |    4 +-
 lib/Automake/Makefile.in       |    4 +-
 lib/Automake/tests/Makefile.in |    4 +-
 lib/Makefile.in                |    4 +-
 lib/am/Makefile.in             |    4 +-
 lib/am/configure.am            |    6 ++-
 m4/Makefile.in                 |    4 +-
 tests/Makefile.am              |    1 +
 tests/Makefile.in              |    5 ++-
 tests/remake9.test             |   80 ++++++++++++++++++++++++++++++++++++++++
 11 files changed, 108 insertions(+), 10 deletions(-)
 create mode 100755 tests/remake9.test

diff --git a/lib/am/configure.am b/lib/am/configure.am
index e9299d6..22c133a 100644
--- a/lib/am/configure.am
+++ b/lib/am/configure.am
@@ -41,13 +41,15 @@ endif %?TOPDIR_P%
            *$$dep*) \
 ?TOPDIR_P?           echo ' cd $(srcdir) && $(AUTOMAKE) %AUTOMAKE-OPTIONS%'; \
 ?TOPDIR_P?           $(am__cd) $(srcdir) && $(AUTOMAKE) %AUTOMAKE-OPTIONS% \
-?TOPDIR_P?             && exit 0; \
+?TOPDIR_P?             && exit 0; \
 ?!TOPDIR_P?          ( cd $(top_builddir) && $(MAKE) $(AM_MAKEFLAGS) 
am--refresh ) \
 ## If on the other hand, subdir/Makefile.in has been removed, then toplevel
 ## am--refresh will not be aware of any need to run.  We still invoke it
 ## due to $? listing all prerequisites.  Fix up for it by running the rebuild
 ## rule for this file only, below.
-?!TOPDIR_P?            && { if test -f $@; then exit 0; else break; fi; }; \
+?!TOPDIR_P?            && { if test -f $@ \
+?!TOPDIR_P?                 && test $@ = `ls -t $@ %MAKEFILE-AM% 
%MAKEFILE-IN-DEPS% | sed q`; \
+?!TOPDIR_P?                 then exit 0; else break; fi; }; \
              exit 1;; \
          esac; \
        done; \
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 935b3d0..c8b70d7 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -572,6 +572,7 @@ remake5.test \
 remake6.test \
 remake7.test \
 remake8.test \
+remake9.test \
 regex.test \
 req.test \
 reqd.test \
diff --git a/tests/remake9.test b/tests/remake9.test
new file mode 100755
index 0000000..39c9af1
--- /dev/null
+++ b/tests/remake9.test
@@ -0,0 +1,80 @@
+#! /bin/sh
+# Copyright (C) 2009  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/>.
+
+# Make sure removing Makefile.am provokes an error, regardless of whether
+# rebuilding is triggered for this Makefile.in only or for all of them.
+
+. ./defs || Exit 1
+set -e
+
+cat >> configure.in << 'END'
+AC_CONFIG_FILES([sub/Makefile])
+AC_OUTPUT
+END
+
+mkdir sub
+cat > Makefile.am <<'END'
+SUBDIRS = sub
+END
+
+: > sub/Makefile.am
+
+$ACLOCAL
+$AUTOMAKE
+$AUTOCONF
+mkdir build
+cd build
+../configure
+$MAKE
+
+$sleep
+
+rm ../sub/Makefile.am
+$MAKE && Exit 1
+
+: > ../sub/Makefile.am
+$MAKE
+
+$sleep
+
+rm ../sub/Makefile.am
+rm ../aclocal.m4
+$MAKE && Exit 1
+
+# We need another sleep here so that sub/Makefile.in will be newer
+# than sub/Makefile.
+$sleep
+cat > ../sub/Makefile.am <<'END'
+foo:
+       echo $@
+END
+$MAKE
+cd sub
+$MAKE foo
+cd ..
+
+$sleep
+rm ../aclocal.m4
+cat > ../sub/Makefile.am <<'END'
+bar:
+       echo $@
+END
+$MAKE
+cd sub
+$MAKE bar
+cd ..
+
+Exit 0
-- 
1.6.5.1.31.gad12b


>From 4f334c35ca62b41f66c797f59aef9bf4c46da51d Mon Sep 17 00:00:00 2001
From: Ralf Wildenhues <address@hidden>
Date: Sun, 15 Nov 2009 13:59:37 +0100
Subject: [PATCH 3/3] Avoid "deleted header file problem" for Makefile.am 
fragments.

* automake.in (handle_configure): Define am__makefile_in_deps
variable, distribute it here, rather than ...
(read_am_file): ... its contents here.
* lib/am/configure.am (%MAKEFILE-IN%): Replace
%MAKEFILE-IN-DEPS% with $(am__makefile_in_deps).
[?MAKEFILE-IN-DEPS?] ($(am__makefile_in_deps)): New target,
without dependencies.
* tests/remake10.test: New test.
* tests/Makefile.am: Update.
* THANKS, NEWS: Update.
Report by Peter Johansson.
---
 Makefile.in                    |   10 ++--
 automake.in                    |   25 ++++-----
 doc/Makefile.in                |   13 +++--
 lib/Automake/Makefile.in       |    9 ++--
 lib/Automake/tests/Makefile.in |    8 ++-
 lib/Makefile.in                |   15 +++---
 lib/am/Makefile.in             |    9 ++--
 lib/am/configure.am            |    7 ++-
 m4/Makefile.in                 |    9 ++--
 tests/Makefile.am              |    1 +
 tests/Makefile.in              |   13 +++--
 tests/remake10.test            |  112 ++++++++++++++++++++++++++++++++++++++++
 12 files changed, 178 insertions(+), 53 deletions(-)
 create mode 100755 tests/remake10.test

diff --git a/automake.in b/automake.in
index e7f2cac..d0d2671 100755
--- a/automake.in
+++ b/automake.in
@@ -4219,6 +4219,10 @@ sub handle_configure ($$$@)
   my $automake_options = '--' . (global_option 'cygnus' ? 'cygnus' : 
$strictness_name)
                         . (global_option 'no-dependencies' ? ' --ignore-deps' 
: '');
 
+  define_pretty_variable ('am__makefile_in_deps', TRUE, INTERNAL,
+                         @include_stack);
+  push_dist_common ('$(am__makefile_in_deps)');
+
   $output_rules .= file_contents
     ('configure',
      new Automake::Location,
@@ -6854,28 +6858,21 @@ sub read_am_file ($$)
        {
            my $path = $1;
 
+           # Any included file will be distributed:
+           # @include_stack will be added to DIST_COMMON as is.
+           # The $(top_srcdir) resp. $(srcdir) prefixes are needed,
+           # otherwise OSF make will implicitly copy the included
+           # file in the build tree during `make distdir' to satisfy
+           # the dependency.
+           # (subdircond2.test and subdircond3.test will fail.)
            if ($path =~ s/^\$\(top_srcdir\)\///)
              {
                push (@include_stack, "\$\(top_srcdir\)/$path");
-               # Distribute any included file.
-
-               # Always use the $(top_srcdir) prefix in DIST_COMMON,
-               # otherwise OSF make will implicitly copy the included
-               # file in the build tree during `make distdir' to satisfy
-               # the dependency.
-               # (subdircond2.test and subdircond3.test will fail.)
-               push_dist_common ("\$\(top_srcdir\)/$path");
              }
            else
              {
                $path =~ s/\$\(srcdir\)\///;
                push (@include_stack, "\$\(srcdir\)/$path");
-               # Always use the $(srcdir) prefix in DIST_COMMON,
-               # otherwise OSF make will implicitly copy the included
-               # file in the build tree during `make distdir' to satisfy
-               # the dependency.
-               # (subdircond2.test and subdircond3.test will fail.)
-               push_dist_common ("\$\(srcdir\)/$path");
                $path = $relative_dir . "/" . $path if $relative_dir ne '.';
              }
            $where->push_context ("`$path' included from here");
diff --git a/lib/am/configure.am b/lib/am/configure.am
index 22c133a..b993cc0 100644
--- a/lib/am/configure.am
+++ b/lib/am/configure.am
@@ -31,7 +31,7 @@ endif %?TOPDIR_P%
 ## --------------------- ##
 
 ## This rule remakes the Makefile.in.
-%MAKEFILE-IN%: %MAINTAINER-MODE% %MAKEFILE-AM% %MAKEFILE-IN-DEPS% 
$(am__configure_deps)
+%MAKEFILE-IN%: %MAINTAINER-MODE% %MAKEFILE-AM% $(am__makefile_in_deps) 
$(am__configure_deps)
 ## If configure.ac or one of configure's dependencies has changed, all
 ## Makefile.in are to be updated; it is then more efficient to run
 ## automake on all the Makefiles at once.  It also allow Automake to be
@@ -48,7 +48,7 @@ endif %?TOPDIR_P%
 ## due to $? listing all prerequisites.  Fix up for it by running the rebuild
 ## rule for this file only, below.
 ?!TOPDIR_P?            && { if test -f $@ \
-?!TOPDIR_P?                 && test $@ = `ls -t $@ %MAKEFILE-AM% 
%MAKEFILE-IN-DEPS% | sed q`; \
+?!TOPDIR_P?                 && test $@ = `ls -t $@ %MAKEFILE-AM% 
$(am__makefile_in_deps) | sed q`; \
 ?!TOPDIR_P?                 then exit 0; else break; fi; }; \
              exit 1;; \
          esac; \
@@ -58,6 +58,9 @@ endif %?TOPDIR_P%
        $(am__cd) $(top_srcdir) && \
          $(AUTOMAKE) %AUTOMAKE-OPTIONS% %MAKEFILE-AM-SOURCES%
 
+## Avoid the "deleted header file" problem for included fragments.
+?MAKEFILE-IN-DEPS?$(am__makefile_in_deps):
+
 ## Ensure that GNU make doesn't remove Makefile if ./config.status (below)
 ## is interrupted.  Otherwise, the user would need to know to rerun
 ## ./config.status to recreate the lost Makefile.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index c8b70d7..9977006 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -573,6 +573,7 @@ remake6.test \
 remake7.test \
 remake8.test \
 remake9.test \
+remake10.test \
 regex.test \
 req.test \
 reqd.test \
diff --git a/tests/remake10.test b/tests/remake10.test
new file mode 100755
index 0000000..234ec99
--- /dev/null
+++ b/tests/remake10.test
@@ -0,0 +1,112 @@
+#! /bin/sh
+# Copyright (C) 2009  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/>.
+
+# Make sure remaking Makefile.in works with deleted fragments that
+# are not used any more.  Report by Peter Johansson.
+
+. ./defs || Exit 1
+set -e
+
+cat >> configure.in << 'END'
+AM_MAINTAINER_MODE
+AC_CONFIG_FILES([sub/Makefile])
+AC_OUTPUT
+END
+
+mkdir sub
+cat > Makefile.am <<'END'
+SUBDIRS = sub
+include $(top_srcdir)/frag.am
+END
+
+cat > sub/Makefile.am <<'END'
+include $(srcdir)/frog.am
+END
+
+: >frag.am
+: >sub/frog.am
+
+$ACLOCAL
+$AUTOMAKE
+$AUTOCONF
+mkdir build
+cd build
+../configure --enable-maintainer-mode
+$MAKE
+
+$sleep
+
+# Fail for broken input files.
+
+echo "if FOO" > ../frag.am
+$MAKE && Exit 1
+
+: > ../frag.am
+$MAKE
+
+echo "if FOO" > ../sub/frog.am
+$MAKE && Exit 1
+
+: > ../sub/frog.am
+$MAKE
+
+$sleep
+
+# Fail for missing input files.
+rm ../sub/frog.am
+$MAKE && Exit 1
+
+# Do not fail with missing input files that are not needed any more.
+: > ../sub/Makefile.am
+$MAKE
+
+rm ../frag.am
+cat > ../Makefile.am <<'END'
+SUBDIRS = sub
+END
+$MAKE
+
+$MAKE dist
+
+$sleep
+
+# Without maintainer-mode, missing fragments should not cause failure
+# until `make dist'.
+
+cat > ../Makefile.am <<'END'
+SUBDIRS = sub
+include $(top_srcdir)/frag.am
+END
+
+cat > ../sub/Makefile.am <<'END'
+include $(srcdir)/frog.am
+END
+: > ../frag.am
+: > ../sub/frog.am
+
+$MAKE
+
+../configure --disable-maintainer-mode
+$MAKE
+rm ../sub/frog.am
+$MAKE
+$MAKE dist && Exit 1
+: > ../sub/frog.am
+rm ../frag.am
+$MAKE
+$MAKE dist && Exit 1
+
+Exit 0
-- 
1.6.5.1.31.gad12b