From bb0c05a7f0329dd2ea38021e9c0b9e74e0cdb7de Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sat, 22 Oct 2022 19:09:44 -0400 Subject: [PATCH] [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(). --- src/remake.c | 67 ++++++++++++++++------------- tests/scripts/features/patternrules | 22 ++++++++++ 2 files changed, 58 insertions(+), 31 deletions(-) diff --git a/src/remake.c b/src/remake.c index 77a3b051..4c87f783 100644 --- 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; + { + /* 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 (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 a4107432..b5b70347 100644 --- 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.