[SV 12078, SV 62809] Rebuild grouped targets if any is missing

If any of a set of grouped targets is missing or out of date, even
if make is not trying to build that target, rebuild them all.
Ensure this is true for explicit grouped targets as well as pattern
rule grouped targets.

Original patch by Jonathan Gravel <jo@stashed.dev>

* src/remake.c (update_file_1): After matching any pattern rules,
go through the also_make targets and set noexist as needed.  Also
compute the oldest this_mtime.
* tests/scripts/features/grouped_targets: Add regression tests.
* tests/scripts/features/patternrules: Ditto.
* tests/features/vpath: Rewrite to use modern run_make_test().
Add a test that we check for VPATH before implicit rule search.
Move the tests in vpath2 and vpath3 into this suite.
* tests/features/vpathplus: Rewrite to use modern run_make_test().

9 files changed, 233 insertions, 217 deletions

diff --git a/src/implicit.c b/src/implicit.c
index 43f0cd24..a9c763a6 100644
--- a/src/implicit.c
+++ b/src/implicit.c
@@ -997,9 +997,9 @@ pattern_search (struct file *file, int archive,
           f->cmds = imf->cmds;
           f->stem = imf->stem;
           /* Setting target specific variables for a file causes the file to be
-           * entered to the database as a prerequisite. Implicit search then
-           * treats this file as explicitly mentioned. Preserve target specific
-           * variables of this file.  */
+             entered to the database as a prerequisite. Implicit search then
+             treats this file as explicitly mentioned. Preserve target specific
+             variables of this file.  */
           merge_variable_set_lists(&f->variables, imf->variables);
           f->pat_variables = imf->pat_variables;
           f->pat_searched = imf->pat_searched;
diff --git a/src/remake.c b/src/remake.c
index 59bd644c..414a5410 100644
--- a/src/remake.c
+++ b/src/remake.c
@@ -503,10 +503,8 @@ update_file_1 (struct file *file, unsigned int depth)
       this_mtime += FILE_TIMESTAMPS_PER_S - 1 - ns;
     }
 
-  must_make = noexist;
-
-  /* If file was specified as a target with no commands,
-     come up with some default commands.  */
+  /* If file was specified as a target with no commands, come up with some
+     default commands.  This may also add more also_make files.  */
 
   if (!file->phony && file->cmds == 0 && !file->tried_implicit)
     {
@@ -520,6 +518,26 @@ update_file_1 (struct file *file, unsigned int depth)
       file->cmds = default_file->cmds;
     }
 
+  /* If any also_make target doesn't exist, we must remake this one too.
+     If they do exist choose the oldest mtime so they will rebuild.  */
+
+  for (ad = file->also_make; ad && !noexist; ad = ad->next)
+    {
+      struct file *adfile = ad->file;
+      FILE_TIMESTAMP fmtime = file_mtime (adfile);
+
+      check_renamed (adfile);
+      noexist = fmtime == NONEXISTENT_MTIME;
+      if (noexist)
+        DBS (DB_BASIC,
+             (_("Grouped target peer '%s' of file '%s' does not exist.\n"),
+              adfile->name, file->name));
+      else if (fmtime < this_mtime)
+        this_mtime = fmtime;
+    }
+
+  must_make = noexist;
+
   /* 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
diff --git a/tests/run_make_tests.pl b/tests/run_make_tests.pl
index 95c07db7..59e2a811 100644
--- a/tests/run_make_tests.pl
+++ b/tests/run_make_tests.pl
@@ -251,6 +251,7 @@ sub subst_make_string
     s/#MAKE#/$make_name/g;
     s/#PERL#/$perl_name/g;
     s/#PWD#/$cwdpath/g;
+    s/#WORK#/$workdir/g;
     # If we're using a shell
     s/#HELPER#/$perl_name $helptool/g;
     return $_;
diff --git a/tests/scripts/features/grouped_targets b/tests/scripts/features/grouped_targets
index f6a17214..6b3c5617 100644
--- a/tests/scripts/features/grouped_targets
+++ b/tests/scripts/features/grouped_targets
@@ -165,5 +165,43 @@ unrelated: hello.x
 unlink('hello.z');
 unlink('hello.q');
 
+# SV 62809: Missing grouped target peer causes remake regardless of which
+# target caused the rule to run.
+touch(qw(gta));  # but not gtb
+run_make_test(q!
+gta gtb &: ; touch gta gtb
+!,
+              'gta', "touch gta gtb\n");
+unlink(qw(gta gtb));
+
+# Ensure both goal targets are built if they depend on a grouped prereq
+touch(qw(gta));  # but not gtb
+run_make_test(q!
+x1 x2: ; touch $@
+
+x1: gta
+x2: gtb
+
+gta gtb &: ; touch gta gtb
+!,
+              'x1 x2', "touch gta gtb\ntouch x1\ntouch x2\n");
+
+# Now everything should be up to date
+run_make_test(undef, 'x1 x2',
+              "#MAKE#: 'x1' is up to date.\n#MAKE#: 'x2' is up to date.");
+
+unlink(qw(x1 x2 gta gtb));
+
+# If an also-make file is older than a prerequisite build both
+
+utouch(-20, 'gtb');
+utouch(-10, 'pre');
+touch(qw(gta));
+run_make_test(q!
+gta gtb &: pre ; touch gta gtb
+!,
+              'gta', "touch gta gtb\n");
+unlink(qw(pre gta gtb));
+
 # This tells the test driver that the perl test script executed properly.
 1;
diff --git a/tests/scripts/features/patternrules b/tests/scripts/features/patternrules
index c7ded311..76f1f92b 100644
--- a/tests/scripts/features/patternrules
+++ b/tests/scripts/features/patternrules
@@ -474,6 +474,61 @@ unlink('1.all', '1.q', '1.r');
 my @dir = ('', 'lib/'); # With and without last slash.
 my @secondexpansion = ('', '.SECONDEXPANSION:');
 
+# SV 62809: Missing grouped pattern peer causes remake regardless of which
+# target caused the rule to run.
+touch(qw(gta));  # but not gtb
+run_make_test(q!
+%a %b : ; touch $*a $*b
+!,
+              'gta', "touch gta gtb\n");
+unlink(qw(gta gtb));
+
+# Ensure both goal targets are built if they depend on a grouped pattern
+touch(qw(gta));  # but not gtb
+run_make_test(q!
+x y: ; touch $@
+
+x: gta
+y: gtb
+
+%a %b : ; touch $*a $*b
+!,
+              'x y', "touch gta gtb\ntouch x\ntouch y\n");
+
+# Now everything should be up to date
+run_make_test(undef, 'x y',
+              "#MAKE#: 'x' is up to date.\n#MAKE#: 'y' is up to date.");
+
+unlink(qw(x y gta gtb));
+
+# sv 12078 : make sure we notice when all targets need to be rebuilt
+# a.1st exists but b.1st doesn't: make sure a.2nd is out of date as well
+
+utouch(-20, 'a.1st');
+utouch(-10, 'a.2nd', 'b.2nd');
+
+run_make_test(q!
+1st := a.1st b.1st
+2nd := ${1st:.1st=.2nd}
+.PHONY: all
+all: ${2nd}
+a.% b.% : ; touch a.$* b.$*
+${2nd}: %.2nd: %.1st ; cp $< $@
+!
+              , '', "touch a.1st b.1st\ncp a.1st a.2nd\ncp b.1st b.2nd\n");
+
+unlink(qw(a.1st b.1st a.2nd b.2nd));
+
+# Variation: b.1st exists but is newer
+
+utouch(-20, 'a.1st');
+utouch(-10, 'a.2nd', 'b.2nd');
+touch(qw(b.1st));
+
+run_make_test(undef, '', "cp b.1st b.2nd\n");
+
+unlink(qw(a.1st b.1st a.2nd b.2nd));
+
 # The following combinations are generated with and without second expansion.
 # 1.
 # all: bye.x
@@ -548,7 +603,7 @@ $prereqs = "${pdir}bye1%2% ${pdir}bye ${pdir}3bye4%5 ${pdir}6bye ${pdir}bye7%8 $
 
 # Multiple funcs, each has multiple words, each word has multiple %, sole %,
 # various corner cases.
-# Make should substitude the first % and only the first % in each word with the
+# Make should substitute the first % and only the first % in each word with the
 # stem.
 run_make_test("
 $se
diff --git a/tests/scripts/features/vpath b/tests/scripts/features/vpath
index ec24165f..7c034b5e 100644
--- a/tests/scripts/features/vpath
+++ b/tests/scripts/features/vpath
@@ -1,65 +1,34 @@
 #                                                                     -*-perl-*-
 
-$description = "The following test creates a makefile to test the \n"
-              ."vpath directive which allows you to specify a search \n"
-              ."path for a particular class of filenames, those that\n"
-              ."match a particular pattern.";
-
-$details = "This tests the vpath directive by specifying search directories\n"
-         ."for one class of filenames with the form: vpath pattern directories"
-         ."\nIn this test, we specify the working directory for all files\n"
-         ."that end in c or h.  We also test the variables $@ (which gives\n"
-         ."target name) and $^ (which is a list of all dependencies \n"
-         ."including the directories in which they were found).  It also\n"
-         ."uses the function firstword used to extract just the first\n"
-         ."dependency from the entire list.";
-
-open(MAKEFILE,"> $makefile");
-
-# The Contents of the MAKEFILE ...
-
-print MAKEFILE "vpath %.c foo\n";
-print MAKEFILE "vpath %.c $workdir\n";
-print MAKEFILE "vpath %.h $workdir\n";
-print MAKEFILE "objects = main.o kbd.o commands.o display.o insert.o\n";
-print MAKEFILE "edit:  \$(objects)\n";
-print MAKEFILE "\t\@echo cc -o \$@ \$^\n";
-print MAKEFILE "main.o : main.c defs.h\n";
-print MAKEFILE "\t\@echo cc -c \$(firstword \$^)\n";
-print MAKEFILE "kbd.o : kbd.c defs.h command.h\n";
-print MAKEFILE "\t\@echo cc -c kbd.c\n";
-print MAKEFILE "commands.o : command.c defs.h command.h\n";
-print MAKEFILE "\t\@echo cc -c commands.c\n";
-print MAKEFILE "display.o : display.c defs.h buffer.h\n";
-print MAKEFILE "\t\@echo cc -c display.c\n";
-print MAKEFILE "insert.o : insert.c defs.h buffer.h\n";
-print MAKEFILE "\t\@echo cc -c insert.c\n";
-
-# END of Contents of MAKEFILE
-
-close(MAKEFILE);
+$description = "Test vpath for particular classes of filenames.";
 
+$details = "";
 
 @files_to_touch = ("$workdir${pathsep}main.c","$workdir${pathsep}defs.h",
-               "$workdir${pathsep}kbd.c","$workdir${pathsep}command.h",
-               "$workdir${pathsep}commands.c","$workdir${pathsep}display.c",
-               "$workdir${pathsep}buffer.h","$workdir${pathsep}insert.c",
-	       "$workdir${pathsep}command.c");
+                   "$workdir${pathsep}kbd.c","$workdir${pathsep}command.h",
+                   "$workdir${pathsep}commands.c","$workdir${pathsep}display.c",
+                   "$workdir${pathsep}buffer.h","$workdir${pathsep}insert.c",
+                   "$workdir${pathsep}command.c");
 
 &touch(@files_to_touch);
 
-&run_make_with_options($makefile,"",&get_logfile);
-
-# Create the answer to what should be produced by this Makefile
-$answer = "cc -c $workdir${pathsep}main.c\ncc -c kbd.c\ncc -c commands.c\n"
-         ."cc -c display.c\n"
-         ."cc -c insert.c\ncc -o edit main.o kbd.o commands.o display.o "
-         ."insert.o\n";
+run_make_test(q!
+vpath %.c foo
+vpath %.c #WORK#
+vpath %.h #WORK#
+objects = main.o kbd.o commands.o display.o insert.o
+edit: $(objects) ; @echo cc -o $@ $^
+main.o : main.c defs.h ; @echo cc -c $(firstword $^)
+kbd.o : kbd.c defs.h command.h ; @echo cc -c kbd.c
+commands.o : command.c defs.h command.h ; @echo cc -c commands.c
+display.o : display.c defs.h buffer.h ; @echo cc -c display.c
+insert.o : insert.c defs.h buffer.h ; @echo cc -c insert.c
+!,
+    '', "cc -c $workdir${pathsep}main.c\ncc -c kbd.c\ncc -c commands.c\n"
+        ."cc -c display.c\ncc -c insert.c\n"
+        ."cc -o edit main.o kbd.o commands.o display.o insert.o\n");
 
-if (&compare_output($answer,&get_logfile(1)))
-{
-  unlink @files_to_touch;
-}
+unlink(@files_to_touch);
 
 # TEST 2: after vpath lookup ensure we don't get incorrect circular dependency
 # warnings due to change of struct file ptr.  Savannah bug #13529.
@@ -78,4 +47,68 @@ vpath-d/fail.te:
 
 rmdir('vpath-d');
 
+# Test VPATH vs vpath
+
+run_make_test(q!
+VPATH = #WORK#:#PWD#
+vpath %.c foo
+vpath %.c #WORK#
+vpath %.c #PWD#
+vpath %.h #WORK#
+vpath %.c
+vpath
+all: ; @echo ALL IS WELL
+!,
+              '', "ALL IS WELL\n");
+
+# Test interaction of -lfoo and vpath
+
+my @dirs_to_make = qw(a1 b1 a2 b2 b3);
+for my $d (@dirs_to_make) {
+    mkdir($d, 0777);
+}
+
+my @files_to_touch = ("a1${pathsep}lib1.a",
+                      "a1${pathsep}libc.a",
+                      "b1${pathsep}lib1.so",
+                      "a2${pathsep}lib2.a",
+                      "b2${pathsep}lib2.so",
+                      "lib3.a",
+                      "b3${pathsep}lib3.so");
+&touch(@files_to_touch);
+
+my $answer = "a1${pathsep}lib1.a a1${pathsep}libc.a " .
+             "a2${pathsep}lib2.a lib3.a\n";
+if ($port_type eq 'VMS-DCL') {
+    $answer =~ s/ /,/g;
+}
+
+run_make_test('
+vpath %.h b3
+vpath %.a a1
+vpath %.so b1
+vpath % a2 b2
+vpath % b3
+all: -l1 -lc -l2 -l3; @echo $^
+',
+              '', $answer);
+
+unlink(@files_to_touch);
+for my $d (@dirs_to_make) {
+    rmdir($d);
+}
+
+# Check that if we find find files with VPATH, we don't do pattern search
+
+mkdir("vpa");
+
+run_make_test(q!
+VPATH = vpa
+%.x: ; @echo pattern $@
+vpa/foo.x: ; @echo vpath $@
+!,
+              'foo.x', "vpath vpa/foo.x\n");
+
+rmdir("vpa");
+
 1;
diff --git a/tests/scripts/features/vpath2 b/tests/scripts/features/vpath2
deleted file mode 100644
index c8de29bc..00000000
--- a/tests/scripts/features/vpath2
+++ /dev/null
@@ -1,45 +0,0 @@
-$description = "This is part 2 in a series to test the vpath directive\n"
-              ."It tests the three forms of the directive:\n"
-              ."     vpath pattern directive\n"
-              ."     vpath pattern  (clears path associated with pattern)\n"
-              ."     vpath          (clears all paths specified with vpath)\n";
-
-$details = "This test simply adds many search paths using various vpath\n"
-          ."directive forms and clears them afterwards.  It has a simple\n"
-          ."rule to print a message at the end to confirm that the makefile\n"
-          ."ran with no errors.\n";
-
-open(MAKEFILE,"> $makefile");
-
-# The Contents of the MAKEFILE ...
-
-print MAKEFILE "VPATH = $workdir:$scriptdir\n";
-print MAKEFILE "vpath %.c foo\n";
-print MAKEFILE "vpath %.c $workdir\n";
-print MAKEFILE "vpath %.c $scriptdir\n";
-print MAKEFILE "vpath %.h $workdir\n";
-print MAKEFILE "vpath %.c\n";
-print MAKEFILE "vpath\n";
-print MAKEFILE "all:\n";
-print MAKEFILE "\t\@echo ALL IS WELL\n";
-# END of Contents of MAKEFILE
-
-close(MAKEFILE);
-
-&run_make_with_options($makefile,"",&get_logfile);
-
-# Create the answer to what should be produced by this Makefile
-$answer = "ALL IS WELL\n";
-
-&compare_output($answer,&get_logfile(1));
-
-1;
-
-
-
-
-
-
-
-
-
diff --git a/tests/scripts/features/vpath3 b/tests/scripts/features/vpath3
deleted file mode 100644
index 839fb723..00000000
--- a/tests/scripts/features/vpath3
+++ /dev/null
@@ -1,41 +0,0 @@
-#                                                                    -*-perl-*-
-
-$description = "Test the interaction of the -lfoo feature and vpath";
-$details = "";
-
-my @dirs_to_make = qw(a1 b1 a2 b2 b3);
-for my $d (@dirs_to_make) {
-    mkdir($d, 0777);
-}
-
-my @files_to_touch = ("a1${pathsep}lib1.a",
-                      "a1${pathsep}libc.a",
-                      "b1${pathsep}lib1.so",
-                      "a2${pathsep}lib2.a",
-                      "b2${pathsep}lib2.so",
-                      "lib3.a",
-                      "b3${pathsep}lib3.so");
-&touch(@files_to_touch);
-
-my $answer = "a1${pathsep}lib1.a a1${pathsep}libc.a " .
-             "a2${pathsep}lib2.a lib3.a\n";
-if ($port_type eq 'VMS-DCL') {
-    $answer =~ s/ /,/g;
-}
-
-run_make_test('
-vpath %.h b3
-vpath %.a a1
-vpath %.so b1
-vpath % a2 b2
-vpath % b3
-all: -l1 -lc -l2 -l3; @echo $^
-',
-              '', $answer);
-
-unlink(@files_to_touch);
-for my $d (@dirs_to_make) {
-    rmdir($d);
-}
-
-1;
diff --git a/tests/scripts/features/vpathplus b/tests/scripts/features/vpathplus
index 978aecb8..da169839 100644
--- a/tests/scripts/features/vpathplus
+++ b/tests/scripts/features/vpathplus
@@ -5,13 +5,23 @@ $details = "";
 
 $VP = "$workdir$pathsep";
 
-open(MAKEFILE,"> $makefile");
+@touchedfiles = ();
+
+$off = -500;
 
-# The Contents of the MAKEFILE ...
+sub touchfiles {
+  foreach (@_) {
+    &utouch($off, $_);
+    $off += 10;
+    push(@touchedfiles, $_);
+  }
+}
 
-print MAKEFILE "VPATH = $VP\n";
+&touchfiles("$VP/foo.d", "$VP/bar.d", "$VP/foo.c", "$VP/bar.c", "foo.b", "bar.d");
 
-print MAKEFILE <<'EOMAKE';
+# Run the general-case test
+
+run_make_test(qq!VPATH = $VP! . q!
 .SUFFIXES: .a .b .c .d
 .PHONY: general rename notarget intermediate
 
@@ -20,12 +30,9 @@ print MAKEFILE <<'EOMAKE';
 %.c:
 %.d:
 
-%.a : %.b
-	cat $^ > $@
-%.b : %.c
-	cat $^ > $@ 2>/dev/null || exit 1
-%.c :: %.d
-	cat $^ > $@
+%.a : %.b ; cat $^ > $@
+%.b : %.c ; cat $^ > $@ 2>/dev/null || exit 1
+%.c :: %.d ; cat $^ > $@
 
 # General testing info:
 
@@ -39,71 +46,32 @@ rename: $(VPATH)/foo.c foo.d
 # Target not made testing info:
 
 notarget: notarget.b
-notarget.c: notarget.d
-	-@echo "not creating $@ from $^"
+notarget.c: notarget.d ; -@echo "not creating $@ from $^"
 
 # Intermediate files:
 
 intermediate: inter.a
-
-EOMAKE
-
-close(MAKEFILE);
-
-@touchedfiles = ();
-
-$off = -500;
-
-sub touchfiles {
-  foreach (@_) {
-    &utouch($off, $_);
-    $off += 10;
-    push(@touchedfiles, $_);
-  }
-}
-
-# Run the general-case test
-
-&touchfiles("$VP/foo.d", "$VP/bar.d", "$VP/foo.c", "$VP/bar.c", "foo.b", "bar.d");
-
-&run_make_with_options($makefile,"general",&get_logfile);
+!,
+    'general', "cat bar.d > bar.c\ncat ${VP}foo.c bar.c > foo.b 2>/dev/null || exit 1\n");
 
 push(@touchedfiles, "bar.c");
 
-$answer = "cat bar.d > bar.c
-cat ${VP}foo.c bar.c > foo.b 2>/dev/null || exit 1
-";
-&compare_output($answer,&get_logfile(1));
-
 # Test rules that don't make the target correctly
 
 &touchfiles("$VP/notarget.c", "notarget.b", "notarget.d");
 
-&run_make_with_options($makefile,"notarget",&get_logfile,512);
-
-$answer = "not creating notarget.c from notarget.d
-cat notarget.c > notarget.b 2>/dev/null || exit 1
-$make_name: *** [$makefile:13: notarget.b] Error 1
-";
-
-&compare_output($answer,&get_logfile(1));
+run_make_test(undef, 'notarget', "not creating notarget.c from notarget.d\ncat notarget.c > notarget.b 2>/dev/null || exit 1\n#MAKE#: *** [#MAKEFILE#:11: notarget.b] Error 1\n", 512);
 
 # Test intermediate file handling (part 1)
 
 &touchfiles("$VP/inter.d");
 
-&run_make_with_options($makefile,"intermediate",&get_logfile);
-
-push(@touchedfiles, "inter.a", "inter.b");
-
 my $be = pack("L", 1) eq pack("N", 1);
 my $intfiles = $be ? "inter.c inter.b" : "inter.b inter.c";
-$answer = "cat ${VP}inter.d > inter.c
-cat inter.c > inter.b 2>/dev/null || exit 1
-cat inter.b > inter.a
-rm $intfiles
-";
-&compare_output($answer,&get_logfile(1));
+
+run_make_test(undef, 'intermediate', "cat ${VP}inter.d > inter.c\ncat inter.c > inter.b 2>/dev/null || exit 1\ncat inter.b > inter.a\nrm $intfiles\n");
+
+push(@touchedfiles, "inter.a", "inter.b");
 
 # Test intermediate file handling (part 2)
 
@@ -111,21 +79,10 @@ rm $intfiles
 &utouch(-10, "$VP/inter.b");
 &touch("$VP/inter.d");
 
-push(@touchedfiles, "$VP/inter.b", "$VP/inter.d");
-
-&run_make_with_options($makefile,"intermediate",&get_logfile);
+run_make_test(undef, 'intermediate', "cat ${VP}inter.d > inter.c\ncat inter.c > inter.b 2>/dev/null || exit 1\ncat inter.b > inter.a\nrm inter.c\n");
 
-$answer = "cat ${VP}inter.d > inter.c
-cat inter.c > inter.b 2>/dev/null || exit 1
-cat inter.b > inter.a
-rm inter.c
-";
-&compare_output($answer,&get_logfile(1));
+push(@touchedfiles, "$VP/inter.b", "$VP/inter.d");
 
 unlink @touchedfiles unless $keep;
 
 1;
-
-### Local Variables:
-### eval: (setq whitespace-action (delq 'auto-cleanup whitespace-action))
-### End: