- Fix Savannah bug #19108

- Fix Savannah bug #17752
- Test suite:
  * When tests fail keep a "run" file containing the command invoked.
  * Support for the Valgrind "memcheck" and "massif" tools.

8 files changed, 200 insertions, 105 deletions

diff --git a/ChangeLog b/ChangeLog
index 61b2466..5d1765e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2009-06-09  Paul Smith  <psmith@gnu.org>
 
+	* remake.c (update_file_1): When rebuilding deps of FILE, also try
+	to rebuild the deps of all the also_make targets for that file.
+	Fixes Savannah bug #19108.
+
+	* implicit.c (pattern_search): Undo test for is_target, added by
+	BorisK on 21 Sep 2004.  This goes against step 5c in the "Implicit
+	Rule Search Algorithm".  Fixes Savannah bug #17752.
+
 	* main.c (clean_jobserver): Clear the jobserver_fds options and
 	set job_slots to the default when we clean up.
 	(define_makeflags): Return the new MAKEFLAGS value.
diff --git a/implicit.c b/implicit.c
index 02b1a91..b536a58 100644
--- a/implicit.c
+++ b/implicit.c
@@ -423,7 +423,6 @@ pattern_search (struct file *file, int archive,
 
       for (ri = 0; ri < nrules; ri++)
 	{
-          struct file *f;
           unsigned int failed = 0;
 	  int check_lastslash;
           int file_variables_set = 0;
@@ -678,7 +677,7 @@ pattern_search (struct file *file, int archive,
                  so it might actually exist.  */
 
               /* @@ dep->changed check is disabled. */
-              if (((f = lookup_file (name)) != 0 && f->is_target)
+              if (lookup_file (name) != 0
                   /*|| ((!dep->changed || check_lastslash) && */
                   || file_exists_p (name))
                 continue;
diff --git a/remake.c b/remake.c
index 0d940d6..513cb56 100644
--- a/remake.c
+++ b/remake.c
@@ -370,10 +370,11 @@ complain (const struct file *file)
 static int
 update_file_1 (struct file *file, unsigned int depth)
 {
-  register FILE_TIMESTAMP this_mtime;
+  FILE_TIMESTAMP this_mtime;
   int noexist, must_make, deps_changed;
   int dep_status = 0;
-  register struct dep *d, *lastd;
+  struct dep *d, *ad;
+  struct dep amake;
   int running = 0;
 
   DBF (DB_VERBOSE, _("Considering target file `%s'.\n"));
@@ -464,83 +465,94 @@ update_file_1 (struct file *file, unsigned int depth)
       file->cmds = default_file->cmds;
     }
 
-  /* Update all non-intermediate files we depend on, if necessary,
-     and see whether any of them is more recent than this file.  */
+  /* Update all non-intermediate files we depend on, if necessary, and see
+     whether any of them is more recent than this file.  We need to walk our
+     deps, AND the deps of any also_make targets to ensure everything happens
+     in the correct order.  */
 
-  lastd = 0;
-  d = file->deps;
-  while (d != 0)
+  amake.file = file;
+  amake.next = file->also_make;
+  ad = &amake;
+  while (ad)
     {
-      FILE_TIMESTAMP mtime;
-      int maybe_make;
-      int dontcare = 0;
+      struct dep *lastd = 0;
 
-      check_renamed (d->file);
+      /* Find the deps we're scanning */
+      d = ad->file->deps;
+      ad = ad->next;
 
-      mtime = file_mtime (d->file);
-      check_renamed (d->file);
+      while (d)
+        {
+          FILE_TIMESTAMP mtime;
+          int maybe_make;
+          int dontcare = 0;
 
-      if (is_updating (d->file))
-	{
-	  error (NILF, _("Circular %s <- %s dependency dropped."),
-		 file->name, d->file->name);
-	  /* We cannot free D here because our the caller will still have
-	     a reference to it when we were called recursively via
-	     check_dep below.  */
-	  if (lastd == 0)
-	    file->deps = d->next;
-	  else
-	    lastd->next = d->next;
-	  d = d->next;
-	  continue;
-	}
+          check_renamed (d->file);
 
-      d->file->parent = file;
-      maybe_make = must_make;
+          mtime = file_mtime (d->file);
+          check_renamed (d->file);
 
-      /* Inherit dontcare flag from our parent. */
-      if (rebuilding_makefiles)
-        {
-          dontcare = d->file->dontcare;
-          d->file->dontcare = file->dontcare;
-        }
+          if (is_updating (d->file))
+            {
+              error (NILF, _("Circular %s <- %s dependency dropped."),
+                     file->name, d->file->name);
+              /* We cannot free D here because our the caller will still have
+                 a reference to it when we were called recursively via
+                 check_dep below.  */
+              if (lastd == 0)
+                file->deps = d->next;
+              else
+                lastd->next = d->next;
+              d = d->next;
+              continue;
+            }
 
+          d->file->parent = file;
+          maybe_make = must_make;
 
-      dep_status |= check_dep (d->file, depth, this_mtime, &maybe_make);
+          /* Inherit dontcare flag from our parent. */
+          if (rebuilding_makefiles)
+            {
+              dontcare = d->file->dontcare;
+              d->file->dontcare = file->dontcare;
+            }
 
-      /* Restore original dontcare flag. */
-      if (rebuilding_makefiles)
-        d->file->dontcare = dontcare;
+          dep_status |= check_dep (d->file, depth, this_mtime, &maybe_make);
 
-      if (! d->ignore_mtime)
-        must_make = maybe_make;
+          /* Restore original dontcare flag. */
+          if (rebuilding_makefiles)
+            d->file->dontcare = dontcare;
 
-      check_renamed (d->file);
+          if (! d->ignore_mtime)
+            must_make = maybe_make;
 
-      {
-	register struct file *f = d->file;
-	if (f->double_colon)
-	  f = f->double_colon;
-	do
-	  {
-	    running |= (f->command_state == cs_running
-			|| f->command_state == cs_deps_running);
-	    f = f->prev;
-	  }
-	while (f != 0);
-      }
+          check_renamed (d->file);
+
+          {
+            register struct file *f = d->file;
+            if (f->double_colon)
+              f = f->double_colon;
+            do
+              {
+                running |= (f->command_state == cs_running
+                            || f->command_state == cs_deps_running);
+                f = f->prev;
+              }
+            while (f != 0);
+          }
 
-      if (dep_status != 0 && !keep_going_flag)
-	break;
+          if (dep_status != 0 && !keep_going_flag)
+            break;
 
-      if (!running)
-        /* The prereq is considered changed if the timestamp has changed while
-           it was built, OR it doesn't exist.  */
-	d->changed = ((file_mtime (d->file) != mtime)
-                      || (mtime == NONEXISTENT_MTIME));
+          if (!running)
+            /* The prereq is considered changed if the timestamp has changed while
+               it was built, OR it doesn't exist.  */
+            d->changed = ((file_mtime (d->file) != mtime)
+                          || (mtime == NONEXISTENT_MTIME));
 
-      lastd = d;
-      d = d->next;
+          lastd = d;
+          d = d->next;
+        }
     }
 
   /* Now we know whether this target needs updating.
@@ -981,7 +993,7 @@ check_dep (struct file *file, unsigned int depth,
           /* Otherwise, update all non-intermediate files we depend on, if
              necessary, and see whether any of them is more recent than the
              file on whose behalf we are checking.  */
-	  struct dep *lastd;
+	  struct dep *ld;
           int deps_running = 0;
 
           /* If this target is not running, set it's state so that we check it
@@ -990,7 +1002,7 @@ check_dep (struct file *file, unsigned int depth,
           if (file->command_state != cs_running)
             set_command_state (file, cs_not_started);
 
-	  lastd = 0;
+	  ld = 0;
 	  d = file->deps;
 	  while (d != 0)
 	    {
@@ -1000,7 +1012,7 @@ check_dep (struct file *file, unsigned int depth,
 		{
 		  error (NILF, _("Circular %s <- %s dependency dropped."),
 			 file->name, d->file->name);
-		  if (lastd == 0)
+		  if (ld == 0)
 		    {
 		      file->deps = d->next;
                       free_dep (d);
@@ -1008,9 +1020,9 @@ check_dep (struct file *file, unsigned int depth,
 		    }
 		  else
 		    {
-		      lastd->next = d->next;
+		      ld->next = d->next;
                       free_dep (d);
-		      d = lastd->next;
+		      d = ld->next;
 		    }
 		  continue;
 		}
@@ -1029,7 +1041,7 @@ check_dep (struct file *file, unsigned int depth,
 		  || d->file->command_state == cs_deps_running)
 		deps_running = 1;
 
-	      lastd = d;
+	      ld = d;
 	      d = d->next;
 	    }
 
diff --git a/tests/ChangeLog b/tests/ChangeLog
index a0e83e0..948d570 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,5 +1,21 @@
 2009-06-09  Paul Smith  <psmith@gnu.org>
 
+	* scripts/features/patternrules: Test that dependencies of
+	"also_make" targets are created properly.  Savannah bug #19108.
+
+	* test_driver.pl (compare_output): Create a "run" file for failed
+	tests containing the command that was run.
+	(get_runfile): New function.
+
+	* run_make_tests.pl (valid_option): Enhanced support for valgrind:
+	allow memcheck and massif tools.
+
+	* scripts/features/patternrules: Have to comment out a line in the
+	first test due to backing out a change that broke the implicit
+	rule search algorithm.  Savannah bug #17752.
+	* scripts/misc/general4: Remove a test that is redundant with
+	patternrules.
+
 	* scripts/features/parallelism: Add a test for re-exec with
 	jobserver master override.  Savannah bug #18124.
 
diff --git a/tests/run_make_tests.pl b/tests/run_make_tests.pl
index 0adb172..092991b 100755
--- a/tests/run_make_tests.pl
+++ b/tests/run_make_tests.pl
@@ -12,7 +12,7 @@
 #                        (and others)
 
 # Copyright (C) 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001,
-# 2002, 2003, 2004, 2005, 2006, 2007 Free Software Foundation, Inc.
+# 2002, 2003, 2004, 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
 # This file is part of GNU Make.
 #
 # GNU Make is free software; you can redistribute it and/or modify it under
@@ -28,10 +28,15 @@
 # You should have received a copy of the GNU General Public License along with
 # this program.  If not, see <http://www.gnu.org/licenses/>.
 
+
 $valgrind = 0;              # invoke make with valgrind
-$valgrind_args = '--num-callers=15 --tool=memcheck --leak-check=full';
+$valgrind_args = '';
+$memcheck_args = '--num-callers=15 --tool=memcheck --leak-check=full';
+$massif_args = '--num-callers=15 --tool=massif --alloc-fn=xmalloc --alloc-fn=xrealloc --alloc-fn=xstrdup --alloc-fn=xstrndup';
 $pure_log = undef;
 
+$command_string = '';
+
 require "test_driver.pl";
 
 # Some target systems might not have the POSIX module...
@@ -54,9 +59,16 @@ sub valid_option
       return 1;
    }
 
-   if ($option =~ /^-valgrind$/i) {
-     $valgrind = 1;
-     return 1;
+   if ($option =~ /^-(valgrind|memcheck)$/i) {
+       $valgrind = 1;
+       $valgrind_args = $memcheck_args;
+       return 1;
+   }
+
+   if ($option =~ /^-massif$/i) {
+       $valgrind = 1;
+       $valgrind_args = $massif_args;
+       return 1;
    }
 
 # This doesn't work--it _should_!  Someone badly needs to fix this.
@@ -148,6 +160,8 @@ sub run_make_with_options {
     $command .= " $options";
   }
 
+  $command_string = "$command\n";
+
   if ($valgrind) {
     print VALGRIND "\n\nExecuting: $command\n";
   }
@@ -155,7 +169,10 @@ sub run_make_with_options {
 
   {
       my $old_timeout = $test_timeout;
-      $test_timeout = $timeout if $timeout;
+      $timeout and $test_timeout = $timeout;
+
+      # If valgrind is enabled, turn off the timeout check
+      $valgrind and $test_timeout = 0;
 
       $code = &run_command_with_output($logname,$command);
 
@@ -183,10 +200,12 @@ sub run_make_with_options {
   if ($code != $expected_code) {
     print "Error running $make_path (expected $expected_code; got $code): $command\n";
     $test_passed = 0;
+    $runf = &get_runfile;
+    &create_file (&get_runfile, $command_string);
     # If it's a SIGINT, stop here
     if ($code & 127) {
       print STDERR "\nCaught signal ".($code & 127)."!\n";
-      exit($code);
+      ($code & 127) == 2 and exit($code);
     }
     return 0;
   }
@@ -195,19 +214,28 @@ sub run_make_with_options {
     system "add_profile $make_path";
   }
 
-  1;
+  return 1;
 }
 
 sub print_usage
 {
    &print_standard_usage ("run_make_tests",
-                          "[-make_path make_pathname] [-valgrind]",);
+                          "[-make_path make_pathname] [-memcheck] [-massif]",);
 }
 
 sub print_help
 {
-   &print_standard_help ("-make_path",
-          "\tYou may specify the pathname of the copy of make to run.");
+   &print_standard_help (
+        "-make_path",
+        "\tYou may specify the pathname of the copy of make to run.",
+        "-valgrind",
+        "-memcheck",
+        "\tRun the test suite under valgrind's memcheck tool.",
+        "\tChange the default valgrind args with the VALGRIND_ARGS env var.",
+        "-massif",
+        "\tRun the test suite under valgrind's massif toool.",
+        "\tChange the default valgrind args with the VALGRIND_ARGS env var."
+       );
 }
 
 sub get_this_pwd {
@@ -334,11 +362,12 @@ sub set_more_defaults
    # Set up for valgrind, if requested.
 
    if ($valgrind) {
+     my $args = $valgrind_args;
      open(VALGRIND, "> valgrind.out")
        || die "Cannot open valgrind.out: $!\n";
      #  -q --leak-check=yes
-     exists $ENV{VALGRIND_ARGS} and $valgrind_args = $ENV{VALGRIND_ARGS};
-     $make_path = "valgrind --log-fd=".fileno(VALGRIND)." $valgrind_args $make_path";
+     exists $ENV{VALGRIND_ARGS} and $args = $ENV{VALGRIND_ARGS};
+     $make_path = "valgrind --log-fd=".fileno(VALGRIND)." $args $make_path";
      # F_SETFD is 2
      fcntl(VALGRIND, 2, 0) or die "fcntl(setfd) failed: $!\n";
      system("echo Starting on `date` 1>&".fileno(VALGRIND));
diff --git a/tests/scripts/features/patternrules b/tests/scripts/features/patternrules
index 35f155a..c5bc3db 100644
--- a/tests/scripts/features/patternrules
+++ b/tests/scripts/features/patternrules
@@ -15,11 +15,13 @@ $dir =~ s,.*/([^/]+)$,../$1,;
 #          to match properly.
 #
 
-run_make_test('
+run_make_test(q!
 .PHONY: all
 
 all: case.1 case.2 case.3
-a: void
+
+# We can't have this, due to "Implicit Rule Search Algorithm" step 5c
+#xxx: void
 
 # 1 - existing file
 %.1: void
@@ -41,9 +43,7 @@ a: void
 	@exit 0
 
 3.implicit-phony:
-',
-'',
-'');
+!, '', '');
 
 # TEST #1: make sure files that are built via implicit rules are marked
 #          as targets (Savannah bug #12202).
@@ -160,5 +160,35 @@ p1.% p2.%: %.orig
 ',
               '', "foo.s1\np1.foo\n");
 
+# TEST 6: Make sure that non-target files are still eligible to be created
+# as part of implicit rule chaining.  Savannah bug #17752.
+
+run_make_test(q!
+BIN = xyz
+COPY = $(BIN).cp
+SRC = $(BIN).c
+allbroken: $(COPY) $(BIN) ; @echo ok
+$(SRC): ; @echo 'main(){}' > $@
+%.cp: % ; @cp $< $@
+% : %.c ; @cp $< $@
+clean: ; @rm -rf $(SRC) $(COPY) $(BIN)
+!,
+              '', "ok\n");
+
+unlink(qw(xyz xyz.cp xyz.c));
+
+# TEST 7: Make sure that all prereqs of all "also_make" targets get created
+# before any of the things that depend on any of them.  Savannah bug #19108.
+
+run_make_test(q!
+final: x ; @echo $@
+x: x.t1 x.t2 ; @echo $@
+x.t2: dep
+dep: ; @echo $@
+%.t1 %.t2: ; @echo $*.t1 ; echo $*.t2
+!,
+              '', "dep\nx.t1\nx.t2\nx\nfinal\n");
+
+
 # This tells the test driver that the perl test script executed properly.
 1;
diff --git a/tests/scripts/misc/general4 b/tests/scripts/misc/general4
index e67323b..6d42a16 100644
--- a/tests/scripts/misc/general4
+++ b/tests/scripts/misc/general4
@@ -28,20 +28,6 @@ cc foo.o -o foo');
 unlink('foo.c');
 
 
-# Test other implicit rule searching
-
-&touch('bar');
-run_make_test('
-test.foo:
-%.foo : baz ; @echo done $<
-%.foo : bar ; @echo done $<
-fox: baz
-',
-              '',
-              'done bar');
-unlink('bar');
-
-
 # Test implicit rules with '$' in the name (see se_implicit)
 
 run_make_test(q!
diff --git a/tests/test_driver.pl b/tests/test_driver.pl
index dd30320..d38868c 100644
--- a/tests/test_driver.pl
+++ b/tests/test_driver.pl
@@ -6,7 +6,7 @@
 # Modified 92-02-11 through 92-02-22 by Chris Arthur to further generalize.
 #
 # Copyright (C) 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 1999, 2000,
-# 2001, 2002, 2003, 2004, 2005, 2006, 2007 Free Software Foundation, Inc.
+# 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
 # This file is part of GNU Make.
 #
 # GNU Make is free software; you can redistribute it and/or modify it under
@@ -435,16 +435,19 @@ sub run_each_test
       $logext = 'l';
       $diffext = 'd';
       $baseext = 'b';
+      $runext = 'r';
       $extext = '';
     } else {
       $logext = 'log';
       $diffext = 'diff';
       $baseext = 'base';
+      $runext = 'run';
       $extext = '.';
     }
     $log_filename = "$testpath.$logext";
     $diff_filename = "$testpath.$diffext";
     $base_filename = "$testpath.$baseext";
+    $run_filename = "$testpath.$runext";
     $tmp_filename = "$testpath.$tmpfilesuffix";
 
     &setup_for_test;          # suite-defined
@@ -691,6 +694,7 @@ sub compare_output
     print "DIFFERENT OUTPUT\n" if $debug;
 
     &create_file (&get_basefile, $answer);
+    &create_file (&get_runfile, $command_string);
 
     print "\nCreating Difference File ...\n" if $debug;
 
@@ -698,6 +702,8 @@ sub compare_output
 
     local($command) = "diff -c " . &get_basefile . " " . $logfile;
     &run_command_with_output(&get_difffile,$command);
+  } else {
+      &rmfiles ();
   }
 
   $suite_passed = 0;
@@ -828,7 +834,7 @@ sub run_command_with_output
 {
   my $filename = shift;
 
-  print "\nrun_command_with_output($filename): @_\n" if $debug;
+  print "\nrun_command_with_output($filename,$runname): @_\n" if $debug;
   &attach_default_output ($filename);
   my $code = _run_command(@_);
   &detach_default_output;
@@ -1201,6 +1207,15 @@ sub get_difffile
   return ($diff_filename . &num_suffix ($num_of_logfiles));
 }
 
+# This subroutine returns a command filename with a number appended
+# to the end corresponding to how many logfiles (and thus command files)
+# have been created in the current running test.
+
+sub get_runfile
+{
+  return ($run_filename . &num_suffix ($num_of_logfiles));
+}
+
 # just like logfile, only a generic tmp filename for use by the test.
 # they are automatically cleaned up unless -keep was used, or the test fails.
 # Pass an argument of 1 to return the same filename as the previous call.