From 7296991d6c2feec34099934af75c35b5c0a47e3a Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sun, 2 Oct 2022 10:18:21 -0400 Subject: [PATCH] [SV 63098] Temporarily revert the change to pattern rule behavior The fix for SV 12078 caused a backward-compatibility issue with some makefiles. In order to allow users to resolve this issue, revert that change for this release cycle: it will be reinstated in the next release cycle. Introduce a warning if we detect that the recipe of a multi-target pattern rule doesn't create all the targets. * NEWS: Announce the future backward-incompatibility. * doc/make.texi (Pattern Intro): Describe the behavior and that it will change in the future. * src/remake.c (check_also_make): Check for also_make targets that were not created and generate a warning. (update_goal_chain): Call the new function. (check_dep): Ditto. (update_file_1): Defer implicit rule detection until after we check all the also_make files (as it used to be). * tests/scripts/features/patternrules: Add tests of the new warning. Skip the tests for SV 12078. --- NEWS | 22 +++++++--- doc/make.texi | 9 +++++ src/remake.c | 63 ++++++++++++++++++++--------- tests/scripts/features/patternrules | 27 +++++++++++-- 4 files changed, 91 insertions(+), 30 deletions(-) diff --git a/NEWS b/NEWS index f53e7691..2beec358 100644 --- a/NEWS +++ b/NEWS @@ -22,6 +22,16 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=109&se In the NEXT release of GNU make, support for these systems will be removed. If you want to see them continue to be supported, contact . +* WARNING: Future backward-incompatibility! + In the NEXT release of GNU make, pattern rules will implement the same + behavior change for multiple targets as explicit grouped targets, below: if + any target of the rule is needed by the build, the recipe will be invoked if + any target of the rule is missing or out of date. During testing some + makefiles were found to contain pattern rules that do not build all targets; + this can cause issues so we are delaying this change for one release cycle + to allow these makefiles to be updated. GNU make show a warning if it + detects this situation: "pattern recipe did not update peer target". + * WARNING: Backward-incompatibility! GNU make now uses temporary files in more situations than previous releases. If your build system sets TMPDIR (or TMP or TEMP on Windows) and deletes the @@ -32,12 +42,12 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=109&se needs to find its temporary directory before the makefiles are parsed. * WARNING: Backward-incompatibility! - Previously each target in a grouped target rule (pattern or explicit) was - considered individually: if the targets needed by the build were not out of - date the recipe was not run even if other targets in the group were out of - date. Now if any of the grouped targets are needed by the build, then if - any of the grouped targets are out of date the recipe is run and all targets - in the group are considered updated. + Previously each target in a explicit grouped target rule was considered + individually: if the targets needed by the build were not out of date the + recipe was not run even if other targets in the group were out of date. Now + if any of the grouped targets are needed by the build, then if any of the + grouped targets are out of date the recipe is run and all targets in the + group are considered updated. * WARNING: Backward-incompatibility! Previously if --no-print-directory was seen anywhere in the environment or diff --git a/doc/make.texi b/doc/make.texi index 4bcd4fbe..79dc34dc 100644 --- a/doc/make.texi +++ b/doc/make.texi @@ -10623,6 +10623,15 @@ always treated as grouped targets (@pxref{Multiple Targets, , Multiple Targets in a Rule}) regardless of whether they use the @code{:} or @code{&:} separator. +There is one exception: if a pattern target is out of date or does +not exist and the makefile does not need to build it, then it will not cause +the other targets to be considered out of date. Note that this historical +exception will be removed in future versions of GNU @code{make} and should not +be relied on. If this situation is detected @code{make} will generate a +warning @emph{pattern recipe did not update peer target}; however, @code{make} +cannot detect all such situations. Please be sure that your recipe updates +@emph{all} the target patterns when it runs. + @node Pattern Examples, Automatic Variables, Pattern Intro, Pattern Rules @subsection Pattern Rule Examples diff --git a/src/remake.c b/src/remake.c index a97431b9..e536c8b5 100644 --- a/src/remake.c +++ b/src/remake.c @@ -78,6 +78,24 @@ static FILE_TIMESTAMP name_mtime (const char *name); 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; + + 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 representing the totality of the status of the goals. @@ -187,6 +205,8 @@ 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 @@ -502,6 +522,28 @@ update_file_1 (struct file *file, unsigned int depth) this_mtime += FILE_TIMESTAMPS_PER_S - 1 - ns; } + /* 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); + + noexist = fmtime == NONEXISTENT_MTIME; + if (noexist) + { + check_renamed (adfile); + 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; + /* If file was specified as a target with no commands, come up with some default commands. This may also add more also_make files. */ @@ -517,26 +559,6 @@ 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 @@ -1072,6 +1094,7 @@ 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 76f1f92b..a4107432 100644 --- a/tests/scripts/features/patternrules +++ b/tests/scripts/features/patternrules @@ -469,12 +469,25 @@ run_make_test(q! unlink('1.all', '1.q', '1.r'); -# sv 62206. +# SV 63098: Verify that missing also_made in pattern rules gives a warning but +# doesn't fail. -my @dir = ('', 'lib/'); # With and without last slash. -my @secondexpansion = ('', '.SECONDEXPANSION:'); +run_make_test(q! +%a %b : ; touch $*a +!, + 'gta', "touch gta\n#MAKEFILE#:2: warning: pattern recipe did not update peer target 'gtb'.\n"); +unlink(qw(gta)); -# SV 62809: Missing grouped pattern peer causes remake regardless of which +run_make_test(q! +all:; +include gta +%a %b : ; touch $*a +!, + '', "touch gta\n#MAKEFILE#:4: warning: pattern recipe did not update peer target 'gtb'.\n#MAKE#: 'all' is up to date."); +unlink(qw(gta)); + +if (0) { +# SV 12078: 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! @@ -528,6 +541,9 @@ touch(qw(b.1st)); run_make_test(undef, '', "cp b.1st b.2nd\n"); unlink(qw(a.1st b.1st a.2nd b.2nd)); +} + +# sv 62206. # The following combinations are generated with and without second expansion. # 1. @@ -548,6 +564,9 @@ unlink(qw(a.1st b.1st a.2nd b.2nd)); # all: bye.x # lib/%.x: ... +my @dir = ('', 'lib/'); # With and without last slash. +my @secondexpansion = ('', '.SECONDEXPANSION:'); + for my $se (@secondexpansion) { for my $d (@dir) { # The directory of the prerequisite of 'all'. for my $r (@dir) { # The directory of the target in the rule definition.