Re: bug: depcomp misplaced

["Derek R. Price" <derek dot price at openavenue dot com>]

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:dprice@openavenue.com     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:dprice@openavenue.com     OpenAvenue ( http://OpenAvenue.com )
> --
> 82. Hold a hard drive to your ear -- listen to the C.
>
> Raja R Harinath wrote:
>
> > "Derek R. Price" <derek.price@openavenue.com> 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 ------------------------------ harinath@cs.umn.edu
> > "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  <dprice@openavenue.com>
+
+	* 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  <tromey@cygnus.com>
 
 	* 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  <derek.price@openavenue.com>
+
+	* 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  <tromey@cygnus.com>
 
 	* space.test: New file.