mirrors/make
1
0
Fork 0
mirror of https://git.savannah.gnu.org/git/make.git synced 2024-11-24 12:19:02 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

[SV 64822, SV 36486] Fix appending to a pattern specific variable

Browse source 
Appending to a pattern specific variable produces an incorrect value
in the presence of a command line definition or an env override of
the variable.  Also, fix pattern/target-specific appending to a
variable with origin override.

* At parse time record_target_var sets the value of a pattern
  specific variable to the value defined on command line or to the
  value of the env override.
* Later, at build time, recursively_expand_for_file appends this
  value of the variable (set in record_target_var) to the command
  line value again, regardless of the origin of the variable.

This patch modifies recursively_expand_for_file to avoid appending,
unless the origin of the variable beats or equals the origin of one
of the parent definitions of this variable.

Reported by Rob <robw9739@gmail.com>,
Brian Vandenberg <phantall@gmail.com>,
Markus Oberhumer <markus@oberhumer.com>.

* NEWS: Note the change.
* src/variable.c (do_variable_definition): Avoid merging a
pattern-specific variable with the parent definition when a command
line or env override is present.
* src/expand.c (recursively_expand_for_file): Avoid appending to a
pattern-specific variable, unless the origin of this pattern-specific
variable beats or equals the origin of one of the parent definitions
of this variable.
* doc/make.texi (Override Directive): Add missing cross-reference.
* tests/scripts/variables/append: Add tests.
This commit is contained in:
Dmitry Goncharov 2024-02-04 13:04:05 -05:00 committed by Paul Smith
parent a493d9ab6c
commit 07187db947
5 changed files with 370 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
NEWS
View file

@ -42,6 +42,11 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=111&se
"environment override" even if they are not otherwise set in the makefile.
See https://savannah.gnu.org/bugs/index.php?64803
* WARNING: Backward-incompatibility!
The behavior of appending to pattern-specific variables has been clarified
when combined with command-line settings or -e overrides.
See https://savannah.gnu.org/bugs/index.php?64822
* NOTE: Deprecated behavior.
The check in GNU Make 4.3 for suffix rules with prerequisites didn't check
single-suffix rules, only double-suffix rules. Add the missing check.

13
doc/make.texi
View file

@ -6466,8 +6466,9 @@ switches with a command argument just as usual. You could use this
override CFLAGS += -g
@end example
You can also use @code{override} directives with @code{define} directives.
This is done as you might expect:
You can also use @code{override} directives with @code{define} directives
(@pxref{Multi-Line, , Defining Multi-Line Variables}). This is done as you
might expect:
@example
override define foo =
@ -6475,14 +6476,6 @@ bar
endef
@end example
@noindent
@iftex
See the next section for information about @code{define}.
@end iftex
@ifnottex
@xref{Multi-Line, ,Defining Multi-Line Variables}.
@end ifnottex
@node Multi-Line
@section Defining Multi-Line Variables
@findex define

44
src/expand.c
View file

@ -148,13 +148,14 @@ recursively_expand_for_file (struct variable *v, struct file *file)
const floc **saved_varp;
struct variable_set_list *savev = 0;
int set_reading = 0;
size_t nl = strlen (v->name);
struct variable *parent = NULL;
/* If we're expanding to put into the environment of a shell function then
ignore any recursion issues: for backward-compatibility we will use
the value of the environment variable we were started with. */
if (v->expanding && env_recursion)
{
size_t nl = strlen (v->name);
char **ep;
DB (DB_VERBOSE,
(_("%s:%lu: not recursively expanding %s to export to shell function\n"),
@ -203,8 +204,49 @@ recursively_expand_for_file (struct variable *v, struct file *file)
v->expanding = 1;
if (v->append)
{
/* Find a parent definition which is marked override. */
struct variable_set_list *sl;
for (sl = current_variable_set_list; sl && !parent; sl = sl->next)
{
struct variable *vp = lookup_variable_in_set (v->name, nl, sl->set);
if (vp && vp != v && vp->origin == o_override)
parent = vp;
}
}
if (parent)
/* PARENT is an override, V is appending. If V is also an override:
override hello := first
al%: override hello += second
Then construct the value from its appended parts in the parent sets.
Else if V is not an override:
override hello := first
al%: hello += second
Then ignore the value of V and use the value of PARENT. */
value = v->origin == o_override
? allocated_variable_append (v)
: xstrdup (parent->value);
else if (v->origin == o_command || v->origin == o_env_override)
/* Avoid appending to a pattern-specific variable, unless the origin of this
pattern-specific variable beats or equals the origin of one of the parent
definitions of this variable.
This is needed, because if there is a command line definition or an env
override, then the value defined in the makefile should only be appended
in the case of a file override.
In the presence of command line definition or env override and absence of
makefile override, the value should be expanded, rather than appended. In
this case, at parse time record_target_var already set the value of this
pattern-specific variable to the value defined on the command line or to
the env override value.
User provided a command line definition or an env override.
PARENT does not have an override directive, so ignore it. */
value = allocated_expand_string (v->value);
else if (v->append)
/* Construct the value from its appended parts in the parent sets. */
value = allocated_variable_append (v);
else
/* A definition without appending. */
value = allocated_expand_string (v->value);
v->expanding = 0;

51
src/variable.c
View file

@ -1432,29 +1432,56 @@ do_variable_definition (const floc *flocp, const char *varname, const char *valu
case f_append:
case f_append_value:
{
/* If we have += but we're in a target variable context, we want to
append only with other variables in the context of this target. */
if (scope)
int override = 0;
if (scope == s_global)
v = lookup_variable (varname, strlen (varname));
else
{
/* When appending in a target/pattern variable context, we want to
append only with other variables in the context of this
target/pattern. */
append = 1;
v = lookup_variable_in_set (varname, strlen (varname),
current_variable_set_list->set);
if (v)
{
/* Don't append from the global set if a previous non-appending
target/pattern-specific variable definition exists. */
if (!v->append)
append = 0;
/* Don't append from the global set if a previous non-appending
target-specific variable definition exists. */
if (v && !v->append)
append = 0;
if (scope == s_pattern &&
(v->origin == o_env_override || v->origin == o_command))
{
/* This is the case of multiple target/pattern specific
definitions/appends, e.g.
al%: hello := first
al%: hello += second
in the presence of a command line definition or an
env override. Do not merge x->value and value here.
For pattern-specific variables the values are merged in
recursively_expand_for_file. */
override = 1;
append = 1;
}
}
}
else
v = lookup_variable (varname, strlen (varname));
if (v == 0)
if (!v)
{
/* There was no old value.
This becomes a normal recursive definition. */
/* There was no old value: make this a recursive definition. */
newval = value;
flavor = f_recursive;
}
else if (override)
{
/* Command line definition / env override takes precedence over
a pattern/target-specific append. */
newval = value;
/* Set flavor to f_recursive to recursively expand this variable
at build time in recursively_expand_for_file. */
flavor = f_recursive;
}
else
{
/* Paste the old and new values together in VALUE. */

280
tests/scripts/variables/append Normal file
View file

@ -0,0 +1,280 @@
# -*-perl-*-
use warnings;
my $description = "Test appending to variables of various origins.";
# sv 64822, sv 36486.
# Test various combinations of appends in the presence and absence of command
# line definitions, env overrides and makefile overrides.
# Test the following statements.
# A target or pattern specific definition or append is available at build time
# only.
# A definition with a makefile override can only be appended to with another
# override.
# A definition with a makefile override can only be redefined with another
# override.
# A target or pattern specific definition takes precedence over a global
# definition for that target.
# A global variable is immune to a target or pattern specific definition or
# append.
# A target or pattern specific definition is immune to another target or pattern
# specific definition or append.
# A prerequisite inherits a target or pattern specific value from its target.
# "phobos" inherits the value of "hello" from "mars".
my @goals = ('phobos', 'mars');
my @specificities = ('', 's: ', '%: ');
my @inits = ('', 'hello=file', 'hello:=file', 'override hello=file');
my @global_append = ('', 'hello+=red');
my @blue_override = ('', 'override ');
my @yellow_override = ('', 'override ');
my @cmdline = ('', 'hello=cmd', 'hello:=cmd hello+=cmd2');
my @env_ovr = ('', '-e');
my @envs = ('', 'env');
for my $goal (@goals) {
for my $spec (@specificities) {
for my $init (@inits) {
for my $ga (@global_append) {
for my $bovr (@blue_override) {
for my $yovr (@yellow_override) {
for my $dashe (@env_ovr) {
for my $env (@envs) {
for my $cmd (@cmdline) {
if ($env) {
$ENV{'hello'} = 'env';
} else {
delete $ENV{'hello'};
}
my $parse_time_answer = '';
my $build_time_answer = '';
# For goal "phobos" target is "", "phobos: " and "phobo%: ".
# For goal "mars" target is "", "mars: " and "mar%: ".
my $target = '';
if ($spec) {
$target = $goal;
chop($target);
$target .= $spec;
}
if ($init =~ 'override') {
$build_time_answer = 'file';
} elsif ($cmd) {
$build_time_answer = $cmd =~ 'cmd2' ? 'cmd cmd2' : 'cmd';
} elsif ($dashe and $env) {
$build_time_answer = 'env';
} elsif ($init and !$target and $ga) {
$build_time_answer = 'file red';
} elsif ($init) {
$build_time_answer = 'file';
} elsif ($env and $ga) {
$build_time_answer = 'env red';
} elsif ($env) {
$build_time_answer = 'env';
} elsif ($ga) {
$build_time_answer = 'red';
}
if ($bovr and $yovr) {
$build_time_answer .= ' ' if ($build_time_answer);
$build_time_answer .= 'blue yellow';
} elsif ($bovr) {
$build_time_answer .= ' ' if ($build_time_answer);
$build_time_answer .= 'blue';
} elsif ($yovr and !($init =~ 'override') and !$cmd and !($dashe and $env)) {
$build_time_answer .= ' ' if ($build_time_answer);
$build_time_answer .= 'blue yellow';
} elsif ($yovr) {
$build_time_answer .= ' ' if ($build_time_answer);
$build_time_answer .= 'yellow';
} elsif ($init =~ 'override') {
} elsif ($cmd) {
} elsif ($dashe and $env) {
} else {
$build_time_answer .= ' ' if ($build_time_answer);
$build_time_answer .= 'blue yellow';
}
if ($cmd and $target) {
$parse_time_answer = $cmd =~ 'cmd2' ? 'cmd cmd2' : 'cmd';
} elsif ($env and !$dashe and $target and $ga) {
$parse_time_answer = 'env red';
} elsif ($env and $target) {
$parse_time_answer = 'env';
} elsif ($target and $ga) {
$parse_time_answer = 'red';
} elsif ($target) {
$parse_time_answer = '';
} else {
$parse_time_answer = $build_time_answer;
}
my $i = $init ? "$target$init" : '';
# moon and satur% specific settings test that target and pattern
# settings specific to one target do not affect another target.
my $answer = $goal eq "mars" ?
"$parse_time_answer\n$build_time_answer\n" # From parse time and from "phobos" recipe.
. "$build_time_answer\n#MAKE#: 'mars' is up to date.\n" : # From "mars" recipe.
"$parse_time_answer\n$build_time_answer\n#MAKE#: 'phobos' is up to date.\n";
run_make_test("
# goal = $goal
# init = $init
# target = $target
# ga = $ga
# bovr = $bovr
# yovr = $yovr
# cmd = $cmd
# env = $env
# dashe = $dashe
moon: hello=1
moon: override hello+=2
$i
$ga
$target${bovr}hello+=blue
$target${yovr}hello+=yellow
satur%: override hello:=3
satur%: hello+=4
\$(info \$(hello))
phobos:; \$(info \$(hello))
mars: phobos; \$(info \$(hello))
saturn:; \$(info \$(hello))
", "$dashe $cmd $goal", "$answer");
}
}
}
}
}
}
}
}
}
# Preferably, we would want to run the tests below for all the combinations,
# generated by the loop above. However, that causes the test to take a lot of
# time.
# The fix for sv 64822 went to recursively_expand_for_file.
# There are three code paths that lead to recursively_expand_for_file.
# - export of a variable to target environment.
# - expansion of a substitution reference.
# - other expansions of a variable that was appended to.
# Test target env, substitution reference in parse and build modes.;
# Also test a mix of pattern and target specific definitions and appends.
run_make_test(q!
al%: hello=file
al%: hello+=one
all: hello+=two
$(info $(hello))
all:; $(info $(hello))
!, "", "\nfile one two\n#MAKE#: 'all' is up to date.\n");
run_make_test(q!
hello=file
al%: hello+=one
all: hello+=two
$(info $(hello:X=Y))
all:; $(info $(hello:X=Y))
!, "", "file\nfile one two\n#MAKE#: 'all' is up to date.\n");
run_make_test(q!
hello=file
al%: hello+=one
all: hello+=two
export hello
$(info $(shell echo $$hello))
all:; $(info $(shell echo $$hello))
!, "", "file\nfile one two\n#MAKE#: 'all' is up to date.\n");
# "phobos" inherits the value of "hello" from "mars". On top of that there are
# also "phobos" specific appends.
for my $goal (@goals) {
my $answer = $goal eq "mars" ?
"\nminit mone mtwo pone ptwo\n" # From parse time and from "phobos" recipe.
. "minit mone mtwo\n#MAKE#: 'mars' is up to date.\n" : # From "mars" recipe.
"\npone ptwo\n#MAKE#: 'phobos' is up to date.\n"; # From parse time and from "phobos" recipe.
run_make_test("
mar%: hello=minit
mar%: hello+=mone
mars: hello+=mtwo
phobo%: hello+=pone
phobos: hello+=ptwo
\$(info \$(hello))
phobos:; \$(info \$(hello))
mars: phobos; \$(info \$(hello))
", "$goal", "$answer");
}
# This test is similar to the one above. The difference is that here there is a
# pattern-specific definition of "hello" that matches "phobos".
run_make_test(q!
mar%: hello:=minit
mar%: hello+=mone
mars: hello+=mtwo
phobo%: hello:=pinit
phobo%: hello+=pone
phobos: hello+=ptwo
$(info $(hello))
phobos:; $(info $(hello))
mars: phobos; $(info $(hello))
!, 'phobos', "\npinit pone ptwo\n#MAKE#: 'phobos' is up to date.\n");
# Test pattern and target specific appends to a global variable that has origin override.
# sv 36486.
my @ops = ('=', '+=', ':=');
@inits = ('', 'override ', 'al%: override ');
@specificities = ('', 'all: ', 'al%: ');
for my $init (@inits) {
for my $spec (@specificities) {
for my $op (@ops) {
my $build_time_answer = '';
if ($init =~ ':' and $op eq '+=' and !$spec) {
# This is the case where global variable obtains value 'one two three' at
# parse time and later at build time a pattern or target specific
# 'hello+=file' appends 'file'.
# e.g.
# al%: override hello+=file
# hello+=one
# hello+=two
# hello+=three
$build_time_answer = 'one two three file';
} elsif ($init =~ 'override') {
$build_time_answer = 'file';
} else {
$build_time_answer = 'file one two three';
}
my $parse_time_answer = $init =~ ':' ? '' : 'file';
if (!$spec and ($init ne 'override ')) {
$parse_time_answer .= ' ' if $parse_time_answer;
$parse_time_answer .= 'one two three';
}
run_make_test("
${init}hello${op}file
${spec}hello+=one
${spec}hello+=two
${spec}hello+=three
\$(info \$(hello))
all:; \$(info \$(hello))
", "", "$parse_time_answer\n$build_time_answer\n#MAKE#: 'all' is up to date.\n");
}
}
}
1;