automake
[Top][All Lists]
Advanced

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

Re: bug: depcomp misplaced


From: Derek R. Price
Subject: Re: bug: depcomp misplaced
Date: Wed, 06 Dec 2000 10:05:48 -0500

Okay, I fixed this as well as all the special casing of '.' that the code is
littered with FIXME comments about.  I've included ChangeLog entries in the 
patch
as well as a new test case, but here's a bit more detail:

1)  In the @require_file_paths case (since it only subbed $relative_dir for '.',
all that was required was to pass in $relative_dir instead of '.' for the three
locations that mattered).  This is what actually fixed the depcomp bug since
@require_file_internal now only looks in $relative_dir when specifically &
literally requested.

2)  In the case of $config_aux_dir, I removed all the switches on '.' and '' and
replaced three with a switch on a new variable 
$config_aux_dir_set_in_configure_in
(yeah, I know that's a little long, but it sure is easy to interpret) which is 
set
when AC_CONFIG_AUX_DIR is noticed initially since it seemed a little silly to
check _every_ use of $config_aux_dir for '.' and '' for only three places which
wanted to look in a local directory rather than use AC_CONFIG_AUX_DIR's actual
value.

3)  Added a test case which confirms that depcomp is added correctly when two
subdirs contain C source.

4)  Based on the assumption that depcomp is a new feature which has never been 
out
of alpha, depcomp is now restricted to the same locations as the other build
scripts (install-sh, missing, & mkinstalldirs).  Allowing it to also be in any
directory containing a C source file would be trivial, but I didn't think it was
important.

5)  tweaked two test cases to look for $(top_srcdir)/depcomp rather than
csubdir/depcomp.

This is good stuff.  I'm pretty sure it should be checked in, but please,
questions, comments?

Derek

--
Derek Price                      CVS Solutions Architect ( http://CVSHome.org )
mailto:address@hidden     OpenAvenue ( http://OpenAvenue.com )
--
Round up the usual suspects.

                - Claude Rains as Captain Louis Renault, _Casablanca_

"Derek R. Price" wrote:

> Unfortunately, not quite.  This causes depcomp to be incorrectly pushed
> onto DIST_COMMON in the sudir/Makefile.in when maybe_push_required_file
> gets called by require_file_internal.  Basically,
> maybe_push_required_file expects $relative_dir to be set right and pushes
> its file onto DIST_COMMON if $relative_dir/$file exists.  Unfortunately
> $relative_dir gets set back to normal after require_config_file returns
> and then DIST_COMMON gets written out into the wrong Makefile.in.  Or,
> rather, the correct Makefile.in but containing the wrong 'depcomp'
> reference.  I'm too tired and hungry to try and decipher any more right
> now.  Anyone have any better ideas?
>
> I've included my patch containing what I have so far since it fixes some
> tests which should be fixed in case anyone wants to take a look before
> I wake up from my nap and get something to eat.  Or in case I find
> something better to do since I do have that workaround and other things
> I could be working on.  pr87.test and subdir4.test are still failing
> because of this.  Sorry about the rewritten toplevel Makefile.in, but
> I figured I might as well include it since it gets rewritten on the first
> make after a fresh checkout right now anyhow.  Oh, and I removed a hack
> from automake.in that became unnecessary after $relative_dir was
> redefined for require_config_file.  Or what should still be an
> unnecessary hack after this problem gets fixed.
>
> Derek
>
> --
> Derek Price                      CVS Solutions Architect ( http://CVSHome.org 
> )
> mailto:address@hidden     OpenAvenue ( http://OpenAvenue.com )
> --
> 82. Hold a hard drive to your ear -- listen to the C.
>
> Raja R Harinath wrote:
>
> > "Derek R. Price" <address@hidden> writes:
> > > The problem appears to start on line 3054 of automake.in (in the
> > > handle_dependencies function):
> > >
> > >     # Set location of depcomp.
> > >     local ($prefix) = ($config_aux_dir_specified ? $config_aux_dir
> > >             : '$(top_srcdir)');
> > >
> > >     &define_variable ('depcomp', "\$(SHELL) $prefix/depcomp");
> > >
> > >     &require_config_file ($FOREIGN, 'depcomp');
> >
> > I remember that playing around with $relative_dir worked at some time.
> > I don't have CVS automake checked out right now, but can you try
> > replacing the last line above
> >
> >    &require_config_file ($FOREIGN, 'depcomp');
> >
> > with the following hack
> >
> >    local ($save_dir) = ($relative_dir);
> >    $relative_dir = '.';
> >    &require_config_file ($FOREIGN, 'depcomp');
> >    $relative_dir = $save_dir;
> >
> > - Hari
> > --
> > Raja R Harinath ------------------------------ address@hidden
> > "When all else fails, read the instructions."      -- Cahn's Axiom
> > "Our policy is, when in doubt, do the right thing."   -- Roy L Ash
? automake/tests/depcomp.test
Index: automake/automake.in
===================================================================
RCS file: /cvs/automake/automake/automake.in,v
retrieving revision 1.803
diff -u -r1.803 automake.in
--- automake.in 2000/11/26 01:39:48     1.803
+++ automake.in 2000/12/05 21:34:57
@@ -156,6 +156,7 @@
 # can be set by AC_CONFIG_AUX_DIR.
 @config_aux_path = ('.', '..', '../..');
 $config_aux_dir = '';
+$config_aux_dir_set_in_configure_in = 0;
 
 # Whether AC_PROG_MAKE_SET has been seen in configure.in.
 $seen_make_set = 0;
@@ -2265,14 +2266,14 @@
                                          'mdate-sh');
 
            local ($conf_pat, $conf_dir);
-           if ($config_aux_dir eq '.' || $config_aux_dir eq '')
+           if ($config_aux_dir_set_in_configure_in)
            {
-               $conf_dir = '$(srcdir)/';
+               $conf_dir = $config_aux_dir;
+               $conf_dir .= '/' unless $conf_dir =~ /\/$/;
            }
            else
            {
-               $conf_dir = $config_aux_dir;
-               $conf_dir .= '/' unless $conf_dir =~ /\/$/;
+               $conf_dir = '$(srcdir)/';
            }
            ($conf_pat = $conf_dir) =~ s/(\W)/\\$1/g;
            $output_rules .=
@@ -2353,7 +2354,7 @@
        &define_variable ('TEXINFO_TEX', $texinfo_tex);
 
     }
-    elsif ($config_aux_dir ne '.' && $config_aux_dir ne '')
+    elsif ($config_aux_dir_set_in_configure_in)
     {
        $texinfo_tex = $config_aux_dir . '/texinfo.tex';
        &define_variable ('TEXINFO_TEX', $texinfo_tex);
@@ -3039,24 +3040,9 @@
        # be empty.
        if (&saw_sources_p (0) && keys %dep_files)
        {
-           local ($config_aux_dir_specified) = ($config_aux_dir ne '.'
-                                                && $config_aux_dir ne '');
-
-           # Set $require_file_found{'depcomp'} if the depcomp file exists,
-           # before calling require_config_file on `depcomp'.  This makes
-           # require_file_internal skip its buggy existence test that would
-           # make automake fail (with `required file `lib/depcomp' not found')
-           # when AC_CONFIG_AUX_DIR is not set.  See tests/subdir4.test.
-           local ($depcomp_dir) = ($config_aux_dir_specified ? $config_aux_dir
-                                   : '.');
-           $require_file_found{'depcomp'} = 1 if -f "$depcomp_dir/depcomp";
-
            # Set location of depcomp.
-           local ($prefix) = ($config_aux_dir_specified ? $config_aux_dir
-                              : '$(top_srcdir)');
+           &define_variable ('depcomp', "\$(SHELL) $config_aux_dir/depcomp");
 
-           &define_variable ('depcomp', "\$(SHELL) $prefix/depcomp");
-
            &require_config_file ($FOREIGN, 'depcomp');
 
            local ($iter);
@@ -3472,16 +3458,7 @@
     }
 
     # Set location of mkinstalldirs.
-    if ($config_aux_dir ne '.' && $config_aux_dir ne '')
-    {
-       &define_variable ('mkinstalldirs', ('$(SHELL) ' . $config_aux_dir
-                                           . '/mkinstalldirs'));
-    }
-    else
-    {
-       &define_variable ('mkinstalldirs',
-                         '$(SHELL) $(top_srcdir)/mkinstalldirs');
-    }
+    &define_variable ('mkinstalldirs', ('$(SHELL) ' . $config_aux_dir . 
'/mkinstalldirs'));
 
     &am_line_error ('CONFIG_HEADER',
                    "\`CONFIG_HEADER' is an anachronism; now determined from 
\`configure.in'")
@@ -4248,14 +4225,7 @@
     &am_error ("\`python_PYTHON' defined but \`AM_CHECK_PYTHON' not in 
\`configure.in'")
        if ! $seen_pythondir && &variable_defined ('python_PYTHON');
 
-    if ($config_aux_dir eq '.' || $config_aux_dir eq '')
-    {
-       &define_variable ('py_compile', '$(top_srcdir)/py-compile');
-    }
-    else
-    {
-       &define_variable ('py_compile', $config_aux_dir . '/py-compile');
-    }
+    &define_variable ('py_compile', $config_aux_dir . '/py-compile');
 }
 
 # Handle Java.
@@ -4508,7 +4478,8 @@
 
        if (/$AC_CONFIG_AUX_DIR_PATTERN/o)
        {
-           @config_aux_path = $1;
+           @config_aux_path = $config_aux_dir = $1;
+           $config_aux_dir_set_in_configure_in = 1;
        }
 
        # Check for ansi2knr.
@@ -5059,7 +5030,7 @@
     # allow parallel builds to work correctly.  FIXME: for now, no
     # line number.
     &require_config_file ($FOREIGN, 'ylwrap');
-    if ($config_aux_dir ne '.' && $config_aux_dir ne '')
+    if ($config_aux_dir_set_in_configure_in)
     {
        &define_variable ('YLWRAP', $config_aux_dir . "/ylwrap");
     }
@@ -7443,8 +7414,7 @@
 {
     local ($dir, $file, $fullfile) = @_;
 
-    # FIXME: Once again, special-case `.'.
-    if ($dir eq $relative_dir || $dir eq '.')
+    if ($dir eq $relative_dir)
     {
        &push_dist_common ($file);
     }
@@ -7478,18 +7448,11 @@
 
        $found_it = 0;
        $dangling_sym = 0;
+       
        foreach $dir (@require_file_paths)
        {
-           if ($dir eq '.')
-           {
-               $fullfile = $relative_dir . "/" . $file;
-               $errdir = $relative_dir unless $errdir;
-           }
-           else
-           {
-               $fullfile = $dir . "/" . $file;
-               $errdir = $dir unless $errdir;
-           }
+           $fullfile = $dir . "/" . $file;
+           $errdir = $dir unless $errdir;
 
            # Use different name for "error filename".  Otherwise on
            # an error the bad file will be reported as eg
@@ -7520,7 +7483,7 @@
        {
            if ($strictness >= $mystrict)
            {
-               if ($dangling_sym && ($force_missing || $add_missing))
+               if ($dangling_sym && $add_missing)
                {
                    unlink ($fullfile);
                }
@@ -7573,6 +7536,9 @@
 
                    &maybe_push_required_file (&dirname ($errfile),
                                               $file, $errfile);
+
+                   # Prune the path list.
+                   @require_file_paths = &dirname ($errfile);
                }
 
                local ($save) = $exit_status;
@@ -7596,19 +7562,19 @@
 # configure.in, not the current Makefile.am.
 sub require_file_with_conf_line
 {
-    @require_file_paths = '.';
+    @require_file_paths = $relative_dir;
     &require_file_internal (1, @_);
 }
 
 sub require_file_with_line
 {
-    @require_file_paths = '.';
+    @require_file_paths = $relative_dir;
     &require_file_internal (0, @_);
 }
 
 sub require_file
 {
-    @require_file_paths = '.';
+    @require_file_paths = $relative_dir;
     &require_file_internal (0, '', @_);
 }
 
@@ -7620,14 +7586,8 @@
     &require_file_internal (1, '', @_);
     local ($dir) = $require_file_paths[0];
     @config_aux_path = @require_file_paths;
-    if ($dir eq '.')
-    {
-       $config_aux_dir = '.';
-    }
-    else
-    {
-       $config_aux_dir = '$(top_srcdir)/' . $dir;
-    }
+    # avoid unsightly '/.'s.
+    $config_aux_dir = '$(top_srcdir)' . ($dir eq '.' ? "" : "/$dir");
 }
 
 # Assumes that the line number is in Makefile.am.
@@ -7637,14 +7597,8 @@
     &require_file_internal (0, @_);
     local ($dir) = $require_file_paths[0];
     @config_aux_path = @require_file_paths;
-    if ($dir eq '.')
-    {
-       $config_aux_dir = '.';
-    }
-    else
-    {
-       $config_aux_dir = '$(top_srcdir)/' . $dir;
-    }
+    # avoid unsightly '/.'s.
+    $config_aux_dir = '$(top_srcdir)' . ($dir eq '.' ? "" : "/$dir");
 }
 
 # Assumes that the line number is in configure.in.
@@ -7654,14 +7608,8 @@
     &require_file_internal (1, @_);
     local ($dir) = $require_file_paths[0];
     @config_aux_path = @require_file_paths;
-    if ($dir eq '.')
-    {
-       $config_aux_dir = '.';
-    }
-    else
-    {
-       $config_aux_dir = '$(top_srcdir)/' . $dir;
-    }
+    # avoid unsightly '/.'s.
+    $config_aux_dir = '$(top_srcdir)' . ($dir eq '.' ? "" : "/$dir");
 }
 
 ################################################################
Index: automake/tests/Makefile.am
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.am,v
retrieving revision 1.239
diff -u -r1.239 Makefile.am
--- Makefile.am 2000/11/26 01:37:30     1.239
+++ Makefile.am 2000/12/05 21:34:57
@@ -83,6 +83,7 @@
 dejagnu.test \
 depacl.test \
 depacl2.test \
+depcomp.test \
 depend.test \
 depend3.test \
 discover.test \
Index: automake/tests/Makefile.in
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.in,v
retrieving revision 1.300
diff -u -r1.300 Makefile.in
--- Makefile.in 2000/11/26 01:37:30     1.300
+++ Makefile.in 2000/12/05 21:34:57
@@ -151,6 +151,7 @@
 dejagnu.test \
 depacl.test \
 depacl2.test \
+depcomp.test \
 depend.test \
 depend3.test \
 discover.test \
Index: automake/tests/auxdir.test
===================================================================
RCS file: /cvs/automake/automake/tests/auxdir.test,v
retrieving revision 1.2
diff -u -r1.2 auxdir.test
--- auxdir.test 1996/05/18 00:29:31     1.2
+++ auxdir.test 2000/12/05 21:34:57
@@ -4,19 +4,19 @@
 
 . $srcdir/defs || exit 1
 
-# The "./." is here so we don't have to mess with subdirs.
 cat >> configure.in << 'END'
 PACKAGE=nonesuch
 VERSION=nonesuch
-AC_CONFIG_AUX_DIR(./.)
+AC_CONFIG_AUX_DIR(subdir)
 END
 
+mkdir subdir
+mv depcomp install-sh missing mkinstalldirs subdir
+
 cat > Makefile.am << 'END'
 pkgdata_DATA =
 END
 
-# The "././" prefix confuses Automake into thinking it is doing a
-# subdir build.  Yes, this is hacky.
-$AUTOMAKE ././Makefile || exit 1
+$AUTOMAKE Makefile || exit 1
 
-grep '/\./\./mkinstalldirs' Makefile.in
+grep 'subdir/mkinstalldirs' Makefile.in
Index: automake/tests/confsub.test
===================================================================
RCS file: /cvs/automake/automake/tests/confsub.test,v
retrieving revision 1.9
diff -u -r1.9 confsub.test
--- confsub.test        2000/03/19 23:38:10     1.9
+++ confsub.test        2000/12/05 21:34:57
@@ -27,8 +27,6 @@
 
 : > subdir/config.h.in
 
-mv depcomp subdir
-
 $AUTOMAKE || exit 1
 
 # Make sure subdir Makefile.in doesn't itself look in the subdir.
Index: automake/tests/libobj2.test
===================================================================
RCS file: /cvs/automake/automake/tests/libobj2.test,v
retrieving revision 1.8
diff -u -r1.8 libobj2.test
--- libobj2.test        2000/03/19 23:38:10     1.8
+++ libobj2.test        2000/12/05 21:34:57
@@ -28,8 +28,6 @@
 
 : > subdir/fsusage.c
 
-mv depcomp subdir
-
 $AUTOMAKE || exit 1
 
 grep 'fsusage\.c' subdir/Makefile.in
diff -u /dev/null depcomp.test
--- /dev/null   Thu Aug 24 05:00:32 2000
+++ automake/tests/depcomp.test Tue Dec  5 14:07:38 2000
@@ -0,0 +1,30 @@
+#! /bin/sh
+
+# Test to make sure depcomp is installed and found properly
+# when required for multiple directories
+
+. $srcdir/defs || exit 1
+
+cat > configure.in << 'END'
+AM_INIT_AUTOMAKE(nonesuch, nonesuch)
+PACKAGE=nonesuch
+VERSION=nonesuch
+AC_PROG_CC
+AC_OUTPUT(subdir/Makefile subdir2/Makefile)
+END
+
+rm depcomp
+mkdir subdir
+mkdir subdir2
+
+cat > subdir/Makefile.am << 'END'
+noinst_PROGRAMS = foo
+foo_SOURCES = foo.c
+END
+
+cp subdir/Makefile.am subdir2/Makefile.am
+
+: > subdir/foo.c
+: > subdir2/foo.c
+
+$AUTOMAKE --add-missing || exit 1
Index: automake/ChangeLog
===================================================================
RCS file: /cvs/automake/automake/ChangeLog,v
retrieving revision 1.910
diff -u -r1.910 ChangeLog
--- ChangeLog   2000/11/26 22:11:20     1.910
+++ ChangeLog   2000/12/05 23:24:54
@@ -1,3 +1,26 @@
+2000-12-05  Derek Price  <address@hidden>
+
+       * automake.in (require_file_with_conf_line,
+       require_file_with_line, require_file): pass a @require_file_path
+       of $relative_dir instead of '.' to require_file_internal so that
+       all the special casing of '.' can be removed elsewhere
+       (require_config_file, require_conf_file_with_line,
+       require_conf_file_with_conf_line): remove special casing for '.'
+       and make sure $config_aux_dir is maintained properly
+       (require_file_internal): remove special casing of '.' and set
+       @require_file_path when missing files are added
+       (maybe_push_required_file): remove special casing of '.'
+       (handle_dependencies):  remove a workaround for a bug now fixed
+       and remove $config_aux_dir special casing
+       (handle_configure): remove special casing for $config_aux_dir
+       (handle_python): ditto
+       (yacc_lex_finish_helper): change $config_aux_dir switch to
+       switch on the value of $config_aux_dir_set_in_configure_in
+       (handle_texinfo): ditto
+       (scan_one_configure_file): set $config_aux_dir and
+       $config_aux_dir_set_in_configure_in properly so special casing
+       on the value of $config_aux_dir can be removed elsewhere
+
 2000-11-25  Tom Tromey  <address@hidden>
 
        * automake.in (file_contents_with_transform): Added file name and
Index: automake/tests/ChangeLog
===================================================================
RCS file: /cvs/automake/automake/tests/ChangeLog,v
retrieving revision 1.308
diff -u -r1.308 ChangeLog
--- ChangeLog   2000/11/26 01:37:30     1.308
+++ ChangeLog   2000/12/05 23:24:55
@@ -1,3 +1,13 @@
+2000-12-05  Derek Price  <address@hidden>
+
+       * depcomp.test: New File
+       * confsub.test: look for depcomp in $(top_srcdir) instead of the
+       first subdir containing a C file.
+       * libobj2.test: ditto
+       * auxdir.test: remove a hack that uses a path of ./. to fake a
+       subdir since it doesn't work anymore
+       * Makefile.am (TESTS): Added depcomp.test
+
 2000-11-25  Tom Tromey  <address@hidden>
 
        * space.test: New file.

reply via email to

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