[SV 63098] Enhance detection of missing peer also-make targets

The previous attempt to detect missing peer targets for implicit
rules had some holes.  Move the detection to notice_finished_file().

* src/remake.c (check_also_make): If we don't have the current mtime
for the file, obtain it.
(update_goal_chain): Don't call check_also_make() here.
(check_dep): Ditto.
(notice_finished_file): If we finished running an implicit rule that
has also_make targets, invoke check_also_make().

diff --git a/src/remake.c b/src/remake.c
index 77a3b0517d2976c1d77ecb7ac7bb2b79365d553e..4c87f783baa6590e5df0d5f7df03661320ac3068 100644 (file)
--- a/src/remake.c
+++ b/src/remake.c
@@ -81,19 +81,20 @@ static const char *library_search (const char *lib, FILE_TIMESTAMP *mtime_ptr);
 static void
 check_also_make (const struct file *file)
 {
-  /* If the target was created by an implicit rule, and it exists and was
-     updated, warn about any of its also_make targets that don't exist.  */
-  if (file->tried_implicit && is_ordinary_mtime (file->last_mtime)
-      && file->last_mtime > file->mtime_before_update)
-    {
-      struct dep *ad;
+  struct dep *ad;
+  FILE_TIMESTAMP mtime = file->last_mtime;
 
-      for (ad = file->also_make; ad; ad = ad->next)
-        if (ad->file->last_mtime == NONEXISTENT_MTIME)
-          OS (error, file->cmds ? &file->cmds->fileinfo : NILF,
-              _("warning: pattern recipe did not update peer target '%s'."),
-              ad->file->name);
-    }
+  if (mtime == UNKNOWN_MTIME)
+    mtime = name_mtime (file->name);
+
+  /* If we updated the file, check its also-make files.  */
+
+  if (is_ordinary_mtime (mtime) && mtime > file->mtime_before_update)
+    for (ad = file->also_make; ad; ad = ad->next)
+      if (ad->file->last_mtime == NONEXISTENT_MTIME)
+        OS (error, file->cmds ? &file->cmds->fileinfo : NILF,
+            _("warning: pattern recipe did not update peer target '%s'."),
+            ad->file->name);
 }
 
 /* Remake all the goals in the 'struct dep' chain GOALS.  Return update_status
@@ -205,8 +206,6 @@ update_goal_chain (struct goaldep *goaldeps)
                       FILE_TIMESTAMP mtime = MTIME (file);
                       check_renamed (file);
 
-                      check_also_make (file);
-
                       if (file->updated && mtime != file->mtime_before_update)
                         {
                           /* Updating was done.  If this is a makefile and
@@ -1039,23 +1038,30 @@ notice_finished_file (struct file *file)
     }
 
   if (ran && file->update_status != us_none)
-    /* We actually tried to update FILE, which has
-       updated its also_make's as well (if it worked).
-       If it didn't work, it wouldn't work again for them.
-       So mark them as updated with the same status.  */
-    for (d = file->also_make; d != 0; d = d->next)
-      {
-        d->file->command_state = cs_finished;
-        d->file->updated = 1;
-        d->file->update_status = file->update_status;
-
-        if (ran && !d->file->phony)
-          /* Fetch the new modification time.
-             We do this instead of just invalidating the cached time
-             so that a vpath_search can happen.  Otherwise, it would
-             never be done because the target is already updated.  */
-          f_mtime (d->file, 0);
-      }
+    {
+      /* We actually tried to update FILE, which has
+         updated its also_make's as well (if it worked).
+         If it didn't work, it wouldn't work again for them.
+         So mark them as updated with the same status.  */
+      for (d = file->also_make; d != 0; d = d->next)
+        {
+          d->file->command_state = cs_finished;
+          d->file->updated = 1;
+          d->file->update_status = file->update_status;
+
+          if (ran && !d->file->phony)
+            /* Fetch the new modification time.
+               We do this instead of just invalidating the cached time
+               so that a vpath_search can happen.  Otherwise, it would
+               never be done because the target is already updated.  */
+            f_mtime (d->file, 0);
+        }
+
+      /* If the target was created by an implicit rule, and it was updated,
+         warn about any of its also_make targets that don't exist.  */
+      if (file->tried_implicit && file->also_make)
+        check_also_make (file);
+    }
   else if (file->update_status == us_none)
     /* Nothing was done for FILE, but it needed nothing done.
        So mark it now as "succeeded".  */
@@ -1094,7 +1100,6 @@ check_dep (struct file *file, unsigned int depth,
       check_renamed (file);
       if (mtime == NONEXISTENT_MTIME || mtime > this_mtime)
         *must_make_ptr = 1;
-      check_also_make (file);
     }
   else
     {

diff --git a/tests/scripts/features/patternrules b/tests/scripts/features/patternrules
index a4107432c0650fbdaf82d915b768732ae7ef2a29..b5b70347b71e5e8e40b63f7eea9a991a7f20a9e1 100644 (file)
--- a/tests/scripts/features/patternrules
+++ b/tests/scripts/features/patternrules
@@ -478,6 +478,20 @@ run_make_test(q!
               'gta', "touch gta\n#MAKEFILE#:2: warning: pattern recipe did not update peer target 'gtb'.\n");
 unlink(qw(gta));
 
+# We don't warn if we didn't update the file
+utouch(-10, qw(gta));
+run_make_test(q!
+%a %b : xyzzy ; $(OP)
+xyzzy: ;
+ifdef RUN
+OP = @echo no
+endif
+!,
+              '-rR gta', "#MAKE#: 'gta' is up to date.\n");
+
+run_make_test(undef, '-rR gta RUN=1', "no\n");
+unlink(qw(gta));
+
 run_make_test(q!
 all:;
 include gta
@@ -486,6 +500,14 @@ include gta
               '', "touch gta\n#MAKEFILE#:4: warning: pattern recipe did not update peer target 'gtb'.\n#MAKE#: 'all' is up to date.");
 unlink(qw(gta));
 
+run_make_test(q!
+%.c %.h : %.y; touch $*.c
+%.o: %.c; touch $@
+foo.y: ; touch $@
+!,
+              'foo.o', "touch foo.y\ntouch foo.c\n#MAKEFILE#:2: warning: pattern recipe did not update peer target 'foo.h'.\ntouch foo.o\nrm foo.c");
+unlink(qw(foo.y foo.c foo.o));
+
 if (0) {
 # SV 12078: Missing grouped pattern peer causes remake regardless of which
 # target caused the rule to run.