On Friday 08 April 2011, Ralf Wildenhues wrote: > Hi Stefano, > > * Stefano Lattarini wrote on Fri, Apr 08, 2011 at 01:05:53PM CEST: > > Hello Ralf, sorry for the delay. > > I don't think you need to apologize for any delays on your behalf, > for at least a few months. ;-) > > > On Wednesday 06 April 2011, Ralf Wildenhues wrote: > > > Ugh, a looong-standing depmode=makedepend bug that silently breaks > > > rebuilds in VPATH trees. It has now hit me in the wild. :-( > > > > > Just out of curiosity: with which compiler? > > IBM XL on BlueGene. > > > > --- a/lib/depcomp > > > +++ b/lib/depcomp > > > > @@ -557,7 +557,9 @@ makedepend) > > > touch "$tmpdepfile" > > > ${MAKEDEPEND-makedepend} -o"$obj_suffix" -f"$tmpdepfile" "$@" > > > rm -f "$depfile" > > > - cat < "$tmpdepfile" > "$depfile" > > > + # makedepend may prepend the VPATH from the source file name to the > > > object. > > > + sed_object_re=`echo $object | sed 's/[].[^$\\*|]/\\\\&/g'` > > > > > A couple of questions/observations about the sed command: > > 1. I wasn't sure whether `.', `$', `^' and `*' are to be escaped when used > > in > > a bracket expression; however, the POSIX standard at: > > <http://pubs.opengroup.org/onlinepubs/007908799/xbd/re.html> > > says they shouldn't be. Also, I couldn't find any reference in the > > Autoconf manual pointing to a sed implementation that doesn't respect > > this part of the standard. > > Ah, cool. Then I recon we don't need this escaping step at all. > Why not? The sed command below: sed "s|^.*\($sed_object_re *:\)|\1|" "$tmpdepfile" > "$depfile" doesn't use any bracket, does it?
> The only character I was worried about was $, as it can occur in > object names now and then (ok, mostly java, and that doesn't use > depcomp). Any others in the list are pretty much excluded anyway, > because they will evoke shell expansion errors when used in a > makefile unquoted. Except for '.', but I'm not afraid of matching > a bit too broadly here, the final 'o' will take care that we don't > match any header files. > OK, this makes sense (even if I'd err on the safe side w.r.t. the `.' character), but I don't see how it's related to my observation above. What am I missing? > > 2. I guess the doubled `\\' in the sed command is there to account for the > > extra stripping performed by the backtick `` command substitution, > > right? > > But is that truly portable, considering that the doubled `\\' is also > > into a single-quotes string? Or would it be better to play safe and use > > an indirection like: > > sed_quoting_cmd='s/[].[^$\\*|]/\\&/g' > > sed_object_re=`echo $object | sed -e "$sed_quoting_cmd"` > > WDYT? > > This is all moot given above. > > > > + sed "s|^.*\($sed_object_re *:\)|\1|" "$tmpdepfile" > "$depfile" > > > sed '1,2d' "$tmpdepfile" | tr ' ' ' > > > ' | \ > > > ## Some versions of the HPUX 10.20 sed can't process this invocation > > > > --- /dev/null > > > +++ b/tests/depcomp9.test > > > > +cd build > > > +../configure am_cv_CC_dependencies_compiler_type=makedepend > > > +$MAKE || Exit 77 > > > > > Why skip the test here? > > Well, makedepend mode might not actually work for some reason. > It's pretty contrived, granted, but let's say the makedepend script > is so broken that _AM_DEPENDENCIES wouldn't have chosen it because > it would fail one of the tests in m4/depend.m4. Then I don't really > want to see a test failure here. > OK. > > Wouldn't be better to just let the test fail if $MAKE fails the first > > time? > > See above. > > > (Which means the first $MAKE > > call and the subsequent "$MAKE clean" are just to be removed as > > redundant). > > Ah, no. This sequence is very important. Sorry for not being clear > enough. The error I'm chasing does not happen upon the first make, > only the second one: the first time, the .Po files contain only the > dummy text. During the first build, makedepend fills them, but without > the depcomp fix, it would create dependencies like this: > > ../../src/foo.o: ../../src/foo.c ../../src/foo.h > > Then, the second time, make sees that foo.o is a prerequisite of foo, > sees a '.c.o' inference rule, and the above. GNU make then infers that > 'foo.o' should actually be '../../src/foo.o', so the command to compile > foo.c gets to be > source=../../src/foo.c' object='../../src/foo.o' ... depcomp $(CC) ... > > instead of the correct > source=../../src/foo.c' object='foo.o' ... depcomp $(CC) ... > > and that messes up the depcomp script work, causing the error messages > quoted in my patch. > Ah, I hadn't thought in detail about this. Sorry for the sloppiness. > I'm adding a comment now: > > # Do not error out with the first make, as the forced 'makedepend' > # depmode might not actually work, but we have overridden the > # _AM_DEPENDENCIES tests. The actual error only happens the second time > # the objects are built, because 'makedepend' has silently messed up the > # .Po files the first time. > > Hope that makes it clearer. > It does for me. Thanks. > > > +$MAKE clean > > > + > > > +$MAKE >out 2>&1 || { cat out; Exit 1; } > > > +cat out > > > +$FGREP 'src/.deps' out && Exit 1 > > > + > > This is probably paranoid but ... what if $(DEPDIR) is != `.deps'? > > (e.g., it is `_deps'). > > Thanks, fixed now. > > Below's what I'm pushing to maint. > > Thanks for the review, > Ralf > > Fix makedepend depmode for VPATH builds. > > * lib/depcomp [makedepend]: Remove any VPATH prefix from the > object file name, so a rebuild doesn't attempt to update the > .Po files in the source tree. > * tests/depcomp9.test: New test. > * tests/Makefile.am (TESTS): Update. > * NEWS: Update. > > diff --git a/NEWS b/NEWS > index 3132b16..6bd67f6 100644 > --- a/NEWS > +++ b/NEWS > @@ -56,6 +56,8 @@ Bugs fixed in 1.11.0a: > > - The parallel-tests driver now does not produce erroneous results > with Tru64/OSF 5.1 sh upon unreadable log files any more. > + > + - The makedepend depmode now works better with VPATH builds. > > New in 1.11: > > diff --git a/lib/depcomp b/lib/depcomp > index df8eea7..ce9419c 100755 > --- a/lib/depcomp > +++ b/lib/depcomp > @@ -1,10 +1,10 @@ > #! /bin/sh > # depcomp - compile a program generating dependencies as side-effects > > -scriptversion=2009-04-28.21; # UTC > +scriptversion=2011-04-08-18; # UTC > > -# Copyright (C) 1999, 2000, 2003, 2004, 2005, 2006, 2007, 2009 Free > -# Software Foundation, Inc. > +# Copyright (C) 1999, 2000, 2003, 2004, 2005, 2006, 2007, 2009, 2011, > +# 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 > @@ -503,7 +503,8 @@ makedepend) > touch "$tmpdepfile" > ${MAKEDEPEND-makedepend} -o"$obj_suffix" -f"$tmpdepfile" "$@" > rm -f "$depfile" > - cat < "$tmpdepfile" > "$depfile" > + # makedepend may prepend the VPATH from the source file name to the object. > I'd add a comment here telling why there's no need to escape $object for the sed commmand just below. > + sed "s|^.*\($object *:\)|\1|" "$tmpdepfile" > "$depfile" > sed '1,2d' "$tmpdepfile" | tr ' ' ' > ' | \ > ## Some versions of the HPUX 10.20 sed can't process this invocation > diff --git a/tests/Makefile.am b/tests/Makefile.am > index d4d9474..c894864 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -277,6 +277,7 @@ depcomp6.test \ > depcomp7.test \ > depcomp8a.test \ > depcomp8b.test \ > +depcomp9.test \ > depdist.test \ > depend.test \ > depend2.test \ > diff --git a/tests/depcomp9.test b/tests/depcomp9.test > new file mode 100755 > index 0000000..d137fad > --- /dev/null > +++ b/tests/depcomp9.test > @@ -0,0 +1,90 @@ > +#! /bin/sh > +# Copyright (C) 2011 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/>. > + > +# makedepend should work in VPATH mode. > + > +# Here's the bug: makedepend will prefix VPATH to the object file name, > +# thus the second make will invoke depcomp with object='../../src/foo.o', > +# causing errors such as: > +# touch: cannot touch `../../src/.deps/foo.TPo': No such file or directory > +# makedepend: error: cannot open "../../src/.deps/foo.TPo" > +# ../../depcomp: line 560: ../../src/.deps/foo.TPo: No such file or directory > + > +# We include subfoo only to be sure that we don't remove too much > +# from the object file name. > + > +required='makedepend' > +. ./defs || Exit 1 > + > +mkdir src src/sub build > + > +cat >> configure.in << 'END' > +AC_PROG_CC > +AM_PROG_CC_C_O > +AC_CONFIG_FILES([src/Makefile]) > +AC_OUTPUT > +END > + > +cat > Makefile.am << 'END' > +SUBDIRS = src > +END > + > +cat > src/Makefile.am << 'END' > +AUTOMAKE_OPTIONS = subdir-objects > +bin_PROGRAMS = foo > +foo_SOURCES = foo.c foo.h sub/subfoo.c > +END > + > +cat > src/foo.h <<EOF > +extern int subfoo (void); > +EOF > + > +cat >src/foo.c <<EOF > +#include "foo.h" > +int main (void) > +{ > + return subfoo (); > +} > +EOF > + > +cat >src/sub/subfoo.c <<EOF > +#include "foo.h" > +int subfoo (void) > +{ > + return 0; > +} > +EOF > + > +$ACLOCAL > +$AUTOCONF > +$AUTOMAKE -a > + > +cd build > +../configure am_cv_CC_dependencies_compiler_type=makedepend > + > +# Do not error out with the first make, as the forced 'makedepend' > +# depmode might not actually work, but we have overridden the > +# _AM_DEPENDENCIES tests. The actual error only happens the second time > +# the objects are built, because 'makedepend' has silently messed up the > +# .Po files the first time. > +$MAKE || Exit 77 > +$MAKE clean > + > +$MAKE >out 2>&1 || { cat out; Exit 1; } > +cat out > +grep 'src/[._]deps' out && Exit 1 > + > +: >