[SV 60795] Don't remake phony included makefiles and show errors

Change the handling of included makefiles which are phony targets to
be similar to double-colon rules with no prerequisites: simply don't
build them at all during the remake a makefile phase.

Ensure that any included makefile which is needed but not built
results in an error.

Update the documentation to make this clear.
Add tests to verify this behavior.

* doc/make.texi (Remaking Makefiles): Clarify double-colon exception.
Document that phony targets are handled the same way.
(Phony Targets): Ditto.
* src/main.c (main): Check for phony targets when skipping goals.
Rather than throwing out skipped but failed goals keep them
separately then report them as errors.
* src/read.c (eval): Set the file location on included makefiles even
when there's no error.
* tests/scripts/features/include: Add tests for handling included
makefiles with both phony and double-colon rules to rebuild them.
This commit is contained in:
Paul Smith 2021-09-06 17:47:04 -04:00
parent 214df0e92a
commit 0c2fc00544
4 changed files with 199 additions and 85 deletions

View file

@ -1379,16 +1379,20 @@ of preventing implicit rule look-up to do so. For example, you can
write an explicit rule with the makefile as the target, and an empty
recipe (@pxref{Empty Recipes, ,Using Empty Recipes}).
If the makefiles specify a double-colon rule to remake a file with
a recipe but no prerequisites, that file will always be remade
(@pxref{Double-Colon}). In the case of makefiles, a makefile that has a
double-colon rule with a recipe but no prerequisites will be remade every
time @code{make} is run, and then again after @code{make} starts over
and reads the makefiles in again. This would cause an infinite loop:
@code{make} would constantly remake the makefile, and never do anything
else. So, to avoid this, @code{make} will @strong{not} attempt to
remake makefiles which are specified as targets of a double-colon rule
with a recipe but no prerequisites.@refill
If the makefiles specify a double-colon rule to remake a file with a recipe
but no prerequisites, that file will always be remade (@pxref{Double-Colon}).
In the case of makefiles, a makefile that has a double-colon rule with a
recipe but no prerequisites will be remade every time @code{make} is run, and
then again after @code{make} starts over and reads the makefiles in again.
This would cause an infinite loop: @code{make} would constantly remake the
makefile and restart, and never do anything else. So, to avoid this,
@code{make} will @strong{not} attempt to remake makefiles which are specified
as targets of a double-colon rule with a recipe but no prerequisites.
Phony targets (@pxref{Phony Targets}) have the same issue: they are never
considered up-to-date and so an included file marked as phony would cause
@code{make} to restart continuously. To avoid this @code{make} will not
attempt to remake makefiles which are marked phony.
If you do not specify any makefiles to be read with @samp{-f} or
@samp{--file} options, @code{make} will try the default makefile names;
@ -2760,15 +2764,15 @@ subdirs:
@end group
@end example
There are problems with this method, however. First, any error
detected in a sub-make is ignored by this rule, so it will continue
to build the rest of the directories even when one fails. This can be
overcome by adding shell commands to note the error and exit, but then
it will do so even if @code{make} is invoked with the @code{-k}
option, which is unfortunate. Second, and perhaps more importantly,
you cannot take advantage of @code{make}'s ability to build targets in
parallel (@pxref{Parallel, ,Parallel Execution}), since there is only
one rule.
There are problems with this method, however. First, any error detected in a
sub-make is ignored by this rule, so it will continue to build the rest of the
directories even when one fails. This can be overcome by adding shell
commands to note the error and exit, but then it will do so even if
@code{make} is invoked with the @code{-k} option, which is unfortunate.
Second, and perhaps more importantly, you cannot take full advantage of
@code{make}'s ability to build targets in parallel (@pxref{Parallel, ,Parallel
Execution}), since there is only one rule. Each individual makefile's targets
will be built in parallel, but only one sub-directory will be built at a time.
By declaring the sub-directories as @code{.PHONY} targets (you must do
this as the sub-directory obviously always exists; otherwise it won't
@ -2799,12 +2803,18 @@ The implicit rule search (@pxref{Implicit Rules}) is skipped for
@code{.PHONY} is good for performance, even if you are not worried
about the actual file existing.
A phony target should not be a prerequisite of a real target file; if it
is, its recipe will be run every time @code{make} goes to update that
file. As long as a phony target is never a prerequisite of a real
target, the phony target recipe will be executed only when the phony
target is a specified goal (@pxref{Goals, ,Arguments to Specify the
Goals}).
A phony target should not be a prerequisite of a real target file; if it is,
its recipe will be run every time @code{make} considers that file. As long as
a phony target is never a prerequisite of a real target, the phony target
recipe will be executed only when the phony target is a specified goal
(@pxref{Goals, ,Arguments to Specify the Goals}).
You should not declare an included makefile as phony. Phony targets are not
intended to represent real files, and because the target is always considered
out of date make will always rebuild it then re-execute itself
(@pxref{Remaking Makefiles, ,How Makefiles Are Remade}). To avoid this,
@code{make} will not re-execute itself if an included file marked as phony is
re-built.
Phony targets can have prerequisites. When one directory contains multiple
programs, it is most convenient to describe all of the programs in one
@ -13203,3 +13213,7 @@ tar.zoo: $(SRCS) $(AUX)
@printindex fn
@bye
@c Local Variables:
@c eval: (setq fill-column 78)
@c End:

View file

@ -2192,8 +2192,10 @@ main (int argc, char **argv, char **envp)
/* Update any makefiles if necessary. */
FILE_TIMESTAMP *makefile_mtimes;
struct goaldep *skipped_makefiles = NULL;
char **aargv = NULL;
const char **nargv;
int any_failed = 0;
int nargc;
enum update_status status;
@ -2226,21 +2228,34 @@ main (int argc, char **argv, char **envp)
while (d != 0)
{
struct file *f;
int skip = 0;
struct file *f = d->file;
for (f = d->file->double_colon; f != NULL; f = f->prev)
if (f->deps == 0 && f->cmds != 0)
break;
/* Check for makefiles that are either phony or a :: target with
commands, but no dependencies. These will always be remade,
which will cause an infinite restart loop, so don't try to
remake it (this will only happen if your makefiles are written
exceptionally stupidly; but if you work for Athena, that's how
you write your makefiles.) */
if (f)
if (f->phony)
skip = 1;
else
for (f = f->double_colon; f != NULL; f = f->prev)
if (f->deps == NULL && f->cmds != NULL)
{
skip = 1;
break;
}
if (!skip)
{
makefile_mtimes[mm_idx++] = file_mtime_no_search (d->file);
last = d;
d = d->next;
}
else
{
/* This makefile is a :: target with commands, but no
dependencies. So, it will always be remade. This might
well cause an infinite loop, so don't try to remake it.
(This will only happen if your makefiles are written
exceptionally stupidly; but if you work for Athena, that's
how you write your makefiles.) */
DB (DB_VERBOSE,
(_("Makefile '%s' might loop; not remaking it.\n"),
f->name));
@ -2250,17 +2265,19 @@ main (int argc, char **argv, char **envp)
else
read_files = d->next;
/* Free the storage. */
free_goaldep (d);
if (d->error && ! (d->flags & RM_DONTCARE))
{
/* This file won't be rebuilt, was not found, and we care,
so remember it to report later. */
d->next = skipped_makefiles;
skipped_makefiles = d;
any_failed = 1;
}
else
free_goaldep (d);
d = last ? last->next : read_files;
}
else
{
makefile_mtimes[mm_idx++] = file_mtime_no_search (d->file);
last = d;
d = d->next;
}
}
}
@ -2280,6 +2297,23 @@ main (int argc, char **argv, char **envp)
db_level = orig_db_level;
}
/* Report errors for makefiles that needed to be remade but were not. */
while (skipped_makefiles != NULL)
{
struct goaldep *d = skipped_makefiles;
const char *err = strerror (d->error);
OSS (error, &d->floc, _("%s: %s"), dep_name (d), err);
skipped_makefiles = skipped_makefiles->next;
free_goaldep (d);
}
/* If we couldn't build something we need but otherwise we succeeded,
reset the status. */
if (any_failed && status == us_success)
status = us_none;
switch (status)
{
case us_question:
@ -2293,20 +2327,16 @@ main (int argc, char **argv, char **envp)
/* No makefiles needed to be updated. If we couldn't read some
included file that we care about, fail. */
{
int any_failed = 0;
struct goaldep *d;
for (d = read_files; d != 0; d = d->next)
if (d->error && ! (d->flags & RM_DONTCARE))
{
/* This makefile couldn't be loaded, and we care. */
OSS (error, &d->floc,
_("%s: %s"), dep_name (d), strerror (d->error));
const char *err = strerror (d->error);
OSS (error, &d->floc, _("%s: %s"), dep_name (d), err);
any_failed = 1;
}
if (any_failed)
die (MAKE_FAILURE);
break;
}
@ -2315,9 +2345,6 @@ main (int argc, char **argv, char **envp)
{
/* Nonzero if any makefile was successfully remade. */
int any_remade = 0;
/* Nonzero if any makefile we care about failed
in updating or could not be found at all. */
int any_failed = 0;
unsigned int i;
struct goaldep *d;
@ -2327,11 +2354,9 @@ main (int argc, char **argv, char **envp)
{
/* This makefile was updated. */
if (d->file->update_status == us_success)
{
/* It was successfully updated. */
any_remade |= (file_mtime_no_search (d->file)
!= makefile_mtimes[i]);
}
/* It was successfully updated. */
any_remade |= (file_mtime_no_search (d->file)
!= makefile_mtimes[i]);
else if (! (d->flags & RM_DONTCARE))
{
FILE_TIMESTAMP mtime;
@ -2346,33 +2371,30 @@ main (int argc, char **argv, char **envp)
makefile_status = MAKE_FAILURE;
}
}
else
/* This makefile was not found at all. */
if (! (d->flags & RM_DONTCARE))
{
const char *dnm = dep_name (d);
size_t l = strlen (dnm);
/* This is a makefile we care about. See how much. */
if (d->flags & RM_INCLUDED)
/* An included makefile. We don't need to die, but we
do want to complain. */
error (&d->floc, l,
_("Included makefile '%s' was not found."), dnm);
else
{
/* A normal makefile. We must die later. */
error (NILF, l,
_("Makefile '%s' was not found"), dnm);
any_failed = 1;
}
}
/* This makefile was not found at all. */
else if (! (d->flags & RM_DONTCARE))
{
const char *dnm = dep_name (d);
/* This is a makefile we care about. See how much. */
if (d->flags & RM_INCLUDED)
/* An included makefile. We don't need to die, but we
do want to complain. */
OS (error, &d->floc,
_("Included makefile '%s' was not found."), dnm);
else
{
/* A normal makefile. We must die later. */
OS (error, NILF, _("Makefile '%s' was not found"), dnm);
any_failed = 1;
}
}
}
if (any_remade)
goto re_exec;
if (any_failed)
die (MAKE_FAILURE);
break;
}
@ -2535,6 +2557,9 @@ main (int argc, char **argv, char **envp)
free (aargv);
break;
}
if (any_failed)
die (MAKE_FAILURE);
}
/* Set up 'MAKEFLAGS' again for the normal targets. */

View file

@ -904,9 +904,7 @@ eval (struct ebuffer *ebuf, int set_default)
| (set_default ? 0 : RM_NO_DEFAULT_GOAL));
struct goaldep *d = eval_makefile (files->name, flags);
if (errno)
d->floc = *fstart;
d->floc = *fstart;
free_ns (files);
files = next;

View file

@ -235,6 +235,29 @@ inc1: foo; echo > $@
'', "#MAKEFILE#:3: inc1: $ERR_no_such_file\n#MAKE#: *** No rule to make target 'foo', needed by 'inc1'. Stop.\n", 512);
rmfiles('inc1');
# Check that included double-colon targets with no prerequisites aren't
# built. This should fail as hello.mk doesn't exist
run_make_test(q!
.PHONY: default
default:;@echo 'FOO=$(FOO)'
include hello.mk
hello.mk:: ; echo 'FOO=bar' > $@
!,
'', "#MAKEFILE#:4: hello.mk: $ERR_no_such_file", 512);
# Check that included phony targets aren't built.
# This should fail as hello.mk doesn't exist
run_make_test(q!
.PHONY: default
default:;@echo 'FOO=$(FOO)'
include hello.mk
hello.mk: ; echo 'FOO=bar' > $@
.PHONY: hello.mk
!,
'', "#MAKEFILE#:4: hello.mk: $ERR_no_such_file", 512);
}
if (defined $ERR_unreadable_file) {
@ -276,15 +299,69 @@ all:;
# https://savannah.gnu.org/bugs/?57676
run_make_test(q!
default:; @echo $(hello)
-include hello.mk
$(shell echo hello=world >hello.mk)
include hello.mk
default:; @echo $(hello)
!,
'', "world\n");
unlink('hello.mk');
# Check that included double-colon targets with no prerequisites aren't built.
# This should succeed since hello.mk already exists
touch('hello.mk');
run_make_test(q!
.PHONY: default
default:;@echo 'FOO=$(FOO)'
include hello.mk
hello.mk:: ; echo 'FOO=bar' > $@
!,
'', 'FOO=');
unlink('hello.mk');
# Check that included double-colon targets with no prerequisites aren't built.
# This should succeed due to -include
run_make_test(q!
.PHONY: default
default:;@echo 'FOO=$(FOO)'
-include hello.mk
hello.mk:: ; echo 'FOO=bar' > $@
!,
'', 'FOO=');
# Check that phony targets aren't built.
# This should succeed since hello.mk already exists
touch('hello.mk');
run_make_test(q!
.PHONY: default
default:;@echo 'FOO=$(FOO)'
include hello.mk
hello.mk: ; echo 'FOO=bar' > $@
.PHONY: hello.mk
!,
'', 'FOO=');
unlink('hello.mk');
# Check that included double-colon targets with no prerequisites aren't built.
# This should succeed due to -include
run_make_test(q!
.PHONY: default
default:;@echo 'FOO=$(FOO)'
-include hello.mk
hello.mk: ; echo 'FOO=bar' > $@
.PHONY: hello.mk
!,
'', 'FOO=');
# Check the default makefiles... this requires us to invoke make with no
# arguments. Also check MAKEFILES