Re: Fix makedepend depmode to cope with VPATH builds

[Ralf Wildenhues]

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.
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.

>  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.

> 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.

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.

> > +$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.
+  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
+
+: