Hi Ralf. Thanks for these fixes, I really think that "make -n" should really be dry-run if possible.
On Monday 01 November 2010, Ralf Wildenhues wrote: > I noticed more issues with automake-generated rules and `make -n': > > 1) The solutions documented in the `Multiple Outputs' node are not safe > for use with `make -n'. > > 2) Consequently, the lisp rules are broken, but also the Yacc, Vala, and > config.h rules in some cases. > > 3) The rules to update Makefile, but also those to update and > Makefile.in, are broken in some circumstances, too. > > The patch below fixes the documentation and addresses (2). The patch > doesn't add testsuite coverage for Vala because there are several > changes pending for this; I'm guessing the code will work differently > afterwards anyway. > > I'm not sure how useful it is to fix (3). It is not easy as a user to > get GNU make to not update any of the dependencies of the Makefile file, > thanks to its remaking feature (info make "Remaking Makefiles"). I'll > reply with a patch for the 'Makefile' rule, but in order to expose that > bug, you need to use something like this in a subdirectory of a package: > make -n Makefile AM_MAKEFLAGS="-n Makefile" > > I don't think users go to this extent just to have `make -n' work, and > they definitely won't get the above right on the first try; but then the > rebuild will already have kicked in, making the issue moot for the > second try. FWIW, I agree that (3) is a minor problem. > Before applying this (to maint, probably) I would appreciate if someone > could look over it to make sure the patch looks sane. Thanks. I didn't spot any obvious error in the "meat" of the patch. Just a couple of nits w.r.t. the test cases... > Fix and document rules to not touch the tree with `make -n'. > > * doc/automake.texi (Multiple Outputs): Document the problem of > modifications during dry-run execution, propose solution. > * NEWS: Update. > * automake.in (lang_vala_finish_target): Split recipe so the > stamp file is not removed with GNU `make -n'. > (lang_yacc_target_hook): Separate removal of parser output file > and header remaking. > * lib/am/lisp.am ($(am__ELCFILES)): Determine whether -n was > passed to make, take care not to remove any files in that case. > * lib/am/remake-hdr.am (%CONFIG_H%): Separate removal of > %STAMP% file from induced remaking of config header. > * tests/autohdrdry.test, tests/lispdry.test, tests/yaccdry.test: > New tests. > * tests/Makefile.am (TESTS): Update. > [MEGA-CUT] > diff --git a/tests/autohdrdry.test b/tests/autohdrdry.test > new file mode 100755 > index 0000000..1631005 > --- /dev/null > +++ b/tests/autohdrdry.test > @@ -0,0 +1,42 @@ > +#!/bin/sh > +# Copyright (C) 2010 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/>. > + > +# Removal recovery rules for AC_CONFIG_HEADERS should not remove files > +# with `make -n'. > + > +. ./defs It should be ". ./defs || Exit 1" I guess. > + > +set -e > + > +cat >>configure.in <<'EOF' > +AC_PROG_CC > +AC_CONFIG_HEADERS([config.h]) > +AC_OUTPUT > +EOF > + > +: >Makefile.am > + > +$ACLOCAL > +$AUTOCONF > +$AUTOHEADER > +$AUTOMAKE > + > +./configure > +$MAKE > + > +rm -f config.h > +$MAKE -n > +test -f stamp-h1 And also checking that "config.h" is not there, just to be sure? > diff --git a/tests/lispdry.test b/tests/lispdry.test > new file mode 100755 > index 0000000..9eea752 > --- /dev/null > +++ b/tests/lispdry.test > @@ -0,0 +1,62 @@ > +#! /bin/sh > +# Copyright (C) 2005, 2008, 2010 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 GNU Automake; see the file COPYING. If not, write to > + > +# Check that `make -n' works with the lisp_LISP recover rule. > + > +required='emacs non-root' > +. ./defs || Exit 1 > + > +set -e > + > +cat > Makefile.am << 'EOF' > +dist_lisp_LISP = am-one.el am-two.el am-three.el > +EOF > + > +cat >> configure.in << 'EOF' > +AM_PATH_LISPDIR > +AC_OUTPUT > +EOF > + > +echo "(require 'am-two)" > am-one.el > +echo "(require 'am-three) (provide 'am-two)" > am-two.el > +echo "(provide 'am-three)" > am-three.el > + > +$ACLOCAL > +$AUTOCONF > +$AUTOMAKE --add-missing > +./configure > + > +$MAKE > + > +test -f am-one.elc > +test -f am-two.elc > +test -f am-three.elc > +test -f elc-stamp > + > +rm -f am-*.elc elc-stamp > + > +chmod a-w . > + > +$MAKE -n > + > +test ! -f am-one.elc > +test ! -f am-two.elc > +test ! -f am-three.elc > +test ! -f elc-stamp > + > +chmod u+w . Why this? Isn't lack of write permissions permissions taken care of by the cleanup trap already? > diff --git a/tests/yaccdry.test b/tests/yaccdry.test > new file mode 100755 > index 0000000..aab43a1 > --- /dev/null > +++ b/tests/yaccdry.test > @@ -0,0 +1,58 @@ > +#! /bin/sh > +# Copyright (C) 2010 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/>. > + > +# Removal recovery rules for headers should not remove files with `make -n'. > + > +. ./defs || Exit 1 > + > +set -e > + > +cat >> configure.in << 'END' > +AC_PROG_CC > +AC_PROG_YACC > +AC_OUTPUT > +END > + > +cat > Makefile.am << 'END' > +AM_YFLAGS = -d > +bin_PROGRAMS = foo > +foo_SOURCES = foo.c parse.y > +END > + > +cat > foo.c << 'END' > +int main () { return 0; } > +END > + > +cat > parse.y << 'END' > +%{ > +int yylex () {return 0;} > +void yyerror (char *s) {} > +%} > +%% > +foobar : 'f' 'o' 'o' 'b' 'a' 'r' {}; > +END > + > +$ACLOCAL > +$AUTOMAKE --add-missing > +$AUTOCONF > +./configure > +$MAKE > + > +rm -f parse.h > +$MAKE -n parse.h > +test -f parse.c And also checking that "parse.h" is not there, just to be sure? > + > +: > Thanks, Stefano
