automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3] Work around a bug in Solaris make's file-inclusion mechan


From: Ralf Wildenhues
Subject: Re: [PATCH v3] Work around a bug in Solaris make's file-inclusion mechanism.
Date: Sun, 26 Sep 2010 19:41:21 +0200
User-agent: Mutt/1.5.20 (2010-08-04)

Hello Stefano,

* Stefano Lattarini wrote on Thu, Sep 23, 2010 at 12:18:07AM CEST:
> Subject: [PATCH] Work around a bug in file-inclusion mechanism of Solaris 
> make.
> 
> * automake.in (handle_single_transform): In the name of the
> dependency file: collapse multiple slash characters into a single
> one.
> * tests/subobj11a.test: New test.
> * tests/subobj11b.test: Likewise.
> * tests/subobj11c.test: Likewise.
> * tests/depcomp8a.test: Likewise.
> * tests/depcomp8b.test: Likewise.
> * tests/Makefile.am (TESTS): Updated.
> * NEWS: Updated.
> Report by Stefano Lattarini, quick fix by Ralf Wildenhues, final
> patch and tests by Stefano Lattarini.


> --- a/NEWS
> +++ b/NEWS
> @@ -48,6 +48,10 @@ Bugs fixed in 1.11.0a:
>    - The order of Yacc and Lex flags is fixed to be consistent with other
>      languages: $(AM_YFLAGS) comes before $(YFLAGS), and $(AM_LFLAGS) before
>      $(LFLAGS), so that the user variables override the developer variables.
> +
> +  - The code for automatic dependency tracking works around a Solaris
> +    make bug triggered by sources containg repeated slashes when the

containing

> +    `subdir-objects' was used.
>  

> --- a/automake.in
> +++ b/automake.in
> @@ -2108,14 +2108,24 @@ sub handle_single_transform ($$$$$%)
>  
>       # Transform .o or $o file into .P file (for automatic
>       # dependency code).
> -     if ($lang && $lang->autodep ne 'no')
> -     {
> -         my $depfile = $object;
> -         $depfile =~ s/\.([^.]*)$/.P$1/;
> -         $depfile =~ s/\$\(OBJEXT\)$/o/;
> -         $dep_files{dirname ($depfile) . '/$(DEPDIR)/'
> -                      . basename ($depfile)} = 1;
> -     }
> +        # Properly flatten multiple adjacent slashes, as Solaris 10 make
> +        # might fail over them in an include statement.
> +        # Leading double slashes may be special, as per Posix, so deal
> +        # with them carefully.
> +        if ($lang && $lang->autodep ne 'no')
> +        {
> +            my $depfile = $object;
> +            $depfile =~ s/\.([^.]*)$/.P$1/;
> +            $depfile =~ s/\$\(OBJEXT\)$/o/;

> +            my $maybe_extra_leading_slash = '';
> +            $maybe_extra_leading_slash = '/' if $depfile =~ m,^//[^/],;
> +            $depfile =~ s,/+,/,g;
> +            my $basename = basename ($depfile);
> +            # This might make $dirname empty, but we account for that below.
> +            (my $dirname = dirname ($depfile)) =~ s/\/*$//;
> +            $dirname = $maybe_extra_leading_slash . $dirname;
> +            $dep_files{$dirname . '/$(DEPDIR)/' . $basename} = 1;

I guess I don't get why this code is so complex.  The comment seems
wrong: the result of dirname is never empty, nor should it end in a
slash.  The  s,/+,/,g  regex matches more often than necessary, causing
unneeded copies; this can be fixed by adding another /.

Why not something like this instead of the above eight lines (untested)?

               my ($depname = dirname ($depfile) . '/$(DEPDIR)/'
                              . basename ($depfile)) =~ s{([^/])//+}{$1/}g;
               $dep_files{$depname} = 1;

or maybe adding this as second command
               $depname =~ s{/./}{/}g;

if you want to avoid unneeded in-string './' instances?

I'm not actually sure whether we want to avoid leading './' instances.
GNU make will always start searching for include files in the current
directory, but I haven't checked other makes yet.  Hmm, but since we
don't explicitly add leading './', I guess doing without ought to be ok.

> +        }
>      }

> --- /dev/null
> +++ b/tests/depcomp8a.test

> +# Test for regressions in computation of names of .Po files for
> +# automatic dependency tracking.

What was the regression, actually?

> +# Keep this in sync with sister test `depcomp8b.test', which checks the
> +# same thing for libtool objects.
> +
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat >> configure.in << 'END'
> +AC_PROG_CC
> +AC_OUTPUT
> +END
> +
> +cat > Makefile.am << 'END'
> +bin_PROGRAMS = zardoz
> +zardoz_SOURCES = foo.c sub/bar.c
> +END
> +
> +mkdir sub
> +cat > foo.c << 'END'
> +int main(void)
> +{
> +  extern int bar;
> +  return bar;
> +}
> +END
> +cat > sub/bar.c << 'END'
> +extern int bar = 0;
> +END
> +
> +$ACLOCAL
> +$AUTOMAKE -a
> +grep include Makefile.in # for debugging
> +grep 'include.*\./\$(DEPDIR)/foo\.P' Makefile.in
> +grep 'include.*\./\$(DEPDIR)/bar\.P' Makefile.in
> +grep 'include.*/\./\$(DEPDIR)' Makefile.in && Exit 1
> +
> +$AUTOCONF
> +./configure
> +$MAKE
> +./zardoz
> +$MAKE distcheck
> +
> +# Try again with subdir-objects option.
> +
> +echo AM_PROG_CC_C_O >> configure.in
> +echo AUTOMAKE_OPTIONS = subdir-objects >> Makefile.am
> +
> +$ACLOCAL
> +$AUTOMAKE -a
> +grep include Makefile.in # for debugging
> +grep 'include.*\./\$(DEPDIR)/foo\.P' Makefile.in
> +grep 'include.*[^a-zA-Z0-9_/]sub/\$(DEPDIR)/bar\.P' Makefile.in
> +$EGREP 'include.*/(\.|sub)/\$(DEPDIR)' Makefile.in && Exit 1

This regex is not right.  (DEPDIR) should be \(DEPDIR\).
I don't quite understand why you want to forbid this regex, it doesn't
seem to have anything to do with the Solaris make bug?

Both of these comments apply to the other tests as well, I guess.

> +$AUTOCONF
> +./configure
> +$MAKE
> +./zardoz
> +$MAKE distcheck
> +
> +:

> --- /dev/null
> +++ b/tests/depcomp8b.test
> @@ -0,0 +1,73 @@

> +# Test for regressions in computation of names of .Plo files for
> +# automatic dependency tracking.
> +# Keep this in sync with sister test `depcomp8a.test', which checks the
> +# same thing for non-libtool objects.
> +
> +required='libtool libtoolize'

libtool is not required for this, only libtoolize.

> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat >> configure.in << 'END'
> +AC_PROG_CC
> +AC_PROG_LIBTOOL
> +AC_OUTPUT
> +END
> +
> +cat > Makefile.am << 'END'
> +lib_LTLIBRARIES = libzardoz.la
> +libzardoz_la_SOURCES = foo.c sub/bar.c
> +END
> +
> +mkdir sub
> +echo 'extern int foo = 0;' > foo.c
> +echo 'extern int bar = 0;' > sub/bar.c
> +
> +libtoolize
> +
> +$ACLOCAL
> +$AUTOMAKE -a
> +grep include Makefile.in # for debugging
> +grep 'include.*\./\$(DEPDIR)/foo\.P' Makefile.in
> +grep 'include.*\./\$(DEPDIR)/bar\.P' Makefile.in
> +grep 'include.*/\./\$(DEPDIR)' Makefile.in && Exit 1
> +
> +$AUTOCONF
> +./configure
> +$MAKE
> +$MAKE distcheck
> +
> +# Try again with subdir-objects option.
> +
> +echo AM_PROG_CC_C_O >> configure.in
> +echo AUTOMAKE_OPTIONS = subdir-objects >> Makefile.am
> +
> +$ACLOCAL
> +$AUTOMAKE -a
> +grep include Makefile.in # for debugging
> +grep 'include.*\./\$(DEPDIR)/foo\.P' Makefile.in
> +grep 'include.*[^a-zA-Z0-9_/]sub/\$(DEPDIR)/bar\.P' Makefile.in
> +$EGREP 'include.*/(\.|sub)/\$(DEPDIR)' Makefile.in && Exit 1
> +
> +$AUTOCONF
> +./configure
> +$MAKE
> +$MAKE distcheck
> +
> +:
> diff --git a/tests/subobj11a.test b/tests/subobj11a.test
> new file mode 100755
> index 0000000..752d492
> --- /dev/null
> +++ b/tests/subobj11a.test

> +# Test that automake works around a bug of Solaris Make. The bug is the
> +# following.  If we have a Makefile containg a file inclusion like this:
> +#   include .//foo.mk
> +# Solaris make fails with a message like:
> +#   make: ... can't find `/foo.mk': No such file or directory
> +#   make: fatal error ... read of include file `/foo.mk' failed
> +# (even if the file `foo.mk' exists). The error disappear by collapsing
> +# the repeated slash `/' characters into a single one.
> +#
> +# See also sister "grepping" test `subobj11b.test', and related test
> +# `subobj11c.test'.
> +
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat >> configure.in << 'END'
> +AC_PROG_CC
> +AM_PROG_CC_C_O
> +AC_OUTPUT
> +END
> +
> +cat > Makefile.am << 'END'
> +AUTOMAKE_OPTIONS = subdir-objects
> +bin_PROGRAMS = foo
> +## the `.//' is meant
> +foo_SOURCES = .//src/foo.c
> +END
> +
> +mkdir src
> +
> +cat > src/foo.c << 'END'
> +int main(void)
> +{
> +  return 0;
> +}
> +END
> +
> +$ACLOCAL
> +$AUTOCONF
> +$AUTOMAKE -a
> +
> +./configure --enable-dependency-tracking
> +
> +if test -d src/.deps; then
> +  depdir=src/.deps
> +elif test -d src/_deps; then
> +  depdir=src/_deps

How about
  depdir=`sed -n 's/^DEPDIR = //p' Makefile`
  if test -z "$depdir"; then

> +else
> +  echo "$me: depdir not found in src/" >&2
> +  Exit 1
> +fi
> +
> +test -f $depdir/foo.Po
> +
> +echo 'quux:; echo "address@hidden@z" >$@' >> $depdir/foo.Po
> +
> +$MAKE quux
> +$FGREP "address@hidden@z" quux
> +
> +$MAKE
> +
> +$MAKE DISTCHECK_CONFIGURE_FLAGS='--enable-dependency-tracking' distcheck
> +$MAKE DISTCHECK_CONFIGURE_FLAGS='--disable-dependency-tracking' distcheck
> +
> +:

> --- /dev/null
> +++ b/tests/subobj11b.test

> +# Test that automake works around a bug of Solaris Make. The bug is the
> +# following.  If we have a Makefile containg a file inclusion like this:
> +#   include .//foo.mk
> +# Solaris make fails with a message like:
> +#   make: ... can't find `/foo.mk': No such file or directory
> +#   make: fatal error ... read of include file `/foo.mk' failed
> +# (even if the file `foo.mk' exists). The error disappear by collapsing
> +# the repeated slash `/' characters into a single one.
> +#
> +# See also "semantic" sister test `subobj11a.test', and related test
> +# `subobj11c.test'.
> +
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat >> configure.in << 'END'
> +AC_PROG_CC
> +AM_PROG_CC_C_O
> +END
> +
> +cat > Makefile.am << 'END'
> +AUTOMAKE_OPTIONS = subdir-objects
> +bin_PROGRAMS = foo
> +## The `zardoz' sources should activate a code paths in Automake that
> +## cannot be sensibly tested by sister test `subobj11a.test'.  The other
> +## sources provide some sort of stress testing.
> +foo_SOURCES = \
> +  //server/zardoz0.c \
> +  //server//zardoz1.c \
> +  //server/path/to/zardoz2.c \
> +  //server/another//path///to////zardoz3.c \
> +  /foobar0.c \
> +  ///foobar1.c \
> +  ////foobar2.c \
> +  /sub///foobar3.c \
> +  ///sub/foobar4.c \
> +  .//foobar5.c \
> +  .//sub/foobar6.c \
> +  ./sub//foobar7.c \
> +  .//sub//foobar8.c \
> +  sub/sub//sub///sub////foobar9.c
> +END
> +
> +$ACLOCAL
> +$AUTOMAKE -a
> +
> +# Be lax in the regexp, to account for automake conditionals, the
> +# use of @am__include@, and similar stuff.
> +grep 'include.*//.*foobar' Makefile.in && Exit 1
> +
> +# These checks depend on automake internals, but presently this is
> +# the only way to test the code path we are interested in.
> +# Please update these checks when (and if) the relevant automake
> +# internals are changed.
> +for x in zardoz0 zardoz1 path/to/zardoz2 another/path/to/zardoz3; do
> +  case $x in
> +   */*) d=`echo $x | sed 's,[^/]*$,,'`; b=`echo $x | sed 's,^.*/,,'`;;
> +     *) d=''; b=$x;;
> +  esac
> +  # Be a little lax in the regexp, to account for automake conditionals,
> +  # quoting, and similar stuff.
> +  grep "^[^/]*am__include[^/]*//server/$d\\\$(DEPDIR)/$b\\.[^/]*$" 
> Makefile.in
> +done
> +
> +# Sanity checks.
> +for i in 0 1 2 3 4 5 6 7 8 9; do
> +  grep "am__include.*/foobar$i\\." Makefile.in
> +done
> +
> +:

> --- /dev/null
> +++ b/tests/subobj11c.test

> +# Automatic dependency tracking with subdir-objects option active:
> +# check for a patologic case of slash-collapsing in the name of

pathologic

> +# included makefile fragments (containing dependency info).
> +# See also related tests `subobj11a.test' and `subobj11b.test'
> +
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat >> configure.in << 'END'
> +AC_PROG_CC
> +AM_PROG_CC_C_O
> +END
> +
> +cat > Makefile.am << 'END'
> +AUTOMAKE_OPTIONS = subdir-objects
> +bin_PROGRAMS = foo
> +foo_SOURCES = //zardoz.c
> +END
> +
> +$ACLOCAL
> +$AUTOMAKE -a
> +
> +#
> +# This check depends on automake internals, but presently this is
> +# the only way to test the code path we are interested in.
> +# Please update these checks when (and if) the relevant automake
> +# internals are changed.
> +#
> +# Be a little lax in the regexp, to account for automake conditionals,
> +# quoting, and similar stuff.
> +#
> +# FIXME: Are we sure this is the most sensible output in our situation?
> +#
> +grep '^[^/]*am__include[^/]*//\$(DEPDIR)/zardoz\.[^/]*$' Makefile.in
> +
> +:

Both subobj11b and subobj11c would better be unit tests for the to-be
factored-out helper function normalize_file_name.  Oh well.

Cheers,
Ralf



reply via email to

[Prev in Thread] Current Thread [Next in Thread]