From a382ac6cd17eedf535407d29bcfc3cef04bbc3bc Mon Sep 17 00:00:00 2001
From: Dmitry Goncharov <dgoncharov@users.sf.net>
Date: Sun, 4 Feb 2024 11:16:40 -0500
Subject: [PATCH] [SV 64803] Set origin for unmodified variables after -e
Ensure the origin of all variables inherited from the environment is
"environment override" if -e is given. Previously only variables
that were set in the makefile had this origin.
PDS: Most of these changes are from Dmitry but I slightly modified
the algorithm: instead of rearranging the way in which MAKEFLAGS is
parsed we reset the env_override value to the default before we
re-parse MAKEFLAGS, then we set the origin of all env vars to the
correct value based on the new setting.
* NEWS: Mention the change for backward-compatibility.
* src/main.c (main): Ensure MAKEFLAGS is always marked special.
(reset_makeflags): Set env_overrides back to default before parsing
MAKEFLAGS.
(decode_switches): Call reset_env_override() to check for changes.
* src/variable.h (reset_env_override): Declare a new function.
* src/variable.c (reset_env_override): Go through all env variables
and ensure the origin is correct based on env_overrides.
(set_env_override): Helper function for the hash.
* tests/scripts/functions/foreach: Fix tests.
* tests/scripts/functions/let: Ditto.
* tests/scripts/functions/origin: Ditto.
* tests/scripts/options/dash-e: Add tests.
---
NEWS | 5 ++
src/main.c | 12 +++-
src/variable.c | 34 +++++----
src/variable.h | 1 +
tests/scripts/functions/foreach | 4 +-
tests/scripts/functions/let | 2 +-
tests/scripts/functions/origin | 4 +-
tests/scripts/options/dash-e | 121 ++++++++++++++++++++++++++++++++
8 files changed, 163 insertions(+), 20 deletions(-)
diff --git a/NEWS b/NEWS
index 676794d6..96d8ce33 100644
--- a/NEWS
+++ b/NEWS
@@ -37,6 +37,11 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=111&se
things like "ifeq ((foo,bar),)" are now syntax errors. Use a variable to
hide the comma if needed: "COMMA = ," / "ifeq ((foo$(COMMA)bar),)".
+* WARNING: Backward-incompatibility!
+ If -e is given all environment variables will now have an origin of
+ "environment override" even if they are not otherwise set in the makefile.
+ See https://savannah.gnu.org/bugs/index.php?64803
+
* 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.
diff --git a/src/main.c b/src/main.c
index 99011d9e..15104212 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1445,6 +1445,7 @@ main (int argc, char **argv, char **envp)
#endif
/* Initialize the special variables. */
+ define_variable_cname ("MAKEFLAGS", "", o_default, 0)->special = 1;
define_variable_cname (".VARIABLES", "", o_default, 0)->special = 1;
/* define_variable_cname (".TARGETS", "", o_default, 0)->special = 1; */
define_variable_cname (".RECIPEPREFIX", "", o_default, 0)->special = 1;
@@ -3090,6 +3091,8 @@ handle_non_switch_argument (const char *arg, enum variable_origin origin)
void
reset_makeflags (enum variable_origin origin)
{
+ /* Reset to default values. */
+ env_overrides = 0;
decode_env_switches (STRING_SIZE_TUPLE(MAKEFLAGS_NAME), origin);
construct_include_path (include_dirs ? include_dirs->list : NULL);
disable_builtins ();
@@ -3363,6 +3366,9 @@ decode_switches (int argc, const char **argv, enum variable_origin origin)
/* Perform any special switch handling. */
run_silent = silent_flag;
+
+ /* Check variable priorities. */
+ reset_env_override ();
}
/* Decode switches from environment variable ENVAR (which is LEN chars long).
@@ -3453,8 +3459,8 @@ quote_for_env (char *out, const char *in)
}
/* Disable builtin variables and rules, if -R or -r is specified.
- * This function is called at parse time whenever MAKEFLAGS is modified and
- * also when the parsing phase is over. */
+ This function is called at parse time whenever MAKEFLAGS is modified and
+ also when the parsing phase is over. */
static
void disable_builtins ()
@@ -3837,7 +3843,7 @@ die (int status)
verify_file_data_base ();
/* Unload plugins before jobserver integrity check in case a plugin
- * participates in jobserver. */
+ participates in jobserver. */
unload_all ();
clean_jobserver (status);
diff --git a/src/variable.c b/src/variable.c
index 9d44cd4a..2026e987 100644
--- a/src/variable.c
+++ b/src/variable.c
@@ -564,9 +564,7 @@ lookup_variable (const char *name, size_t length)
*nptr++ = '$';
}
else
- {
- *nptr++ = *sptr;
- }
+ *nptr++ = *sptr;
sptr++;
}
@@ -704,12 +702,10 @@ initialize_file_variables (struct file *file, int reading)
v->flavor = f_simple;
}
else
- {
- v = do_variable_definition (
- &p->variable.fileinfo, p->variable.name, p->variable.value,
- p->variable.origin, p->variable.flavor,
- p->variable.conditional, s_pattern);
- }
+ v = do_variable_definition (
+ &p->variable.fileinfo, p->variable.name, p->variable.value,
+ p->variable.origin, p->variable.flavor,
+ p->variable.conditional, s_pattern);
/* Also mark it as a per-target and copy export status. */
v->per_target = p->variable.per_target;
@@ -1911,6 +1907,23 @@ warn_undefined (const char *name, size_t len)
(int)len, name));
}
}
+
+static void
+set_env_override (const void *item, void *arg UNUSED)
+{
+ struct variable *v = (struct variable *)item;
+ enum variable_origin old = env_overrides ? o_env : o_env_override;
+ enum variable_origin new = env_overrides ? o_env_override : o_env;
+
+ if (v->origin == old)
+ v->origin = new;
+}
+
+void
+reset_env_override ()
+{
+ hash_map_arg (&global_variable_set.table, set_env_override, NULL);
+}
/* Print information for variable V, prefixing it with PREFIX. */
@@ -1985,7 +1998,6 @@ print_variable (const void *item, void *arg)
}
}
-
static void
print_auto_variable (const void *item, void *arg)
{
@@ -1995,7 +2007,6 @@ print_auto_variable (const void *item, void *arg)
print_variable (item, arg);
}
-
static void
print_noauto_variable (const void *item, void *arg)
{
@@ -2005,7 +2016,6 @@ print_noauto_variable (const void *item, void *arg)
print_variable (item, arg);
}
-
/* Print all the variables in SET. PREFIX is printed before
the actual variable definitions (everything else is comments). */
diff --git a/src/variable.h b/src/variable.h
index f6f0cb32..9cedebcd 100644
--- a/src/variable.h
+++ b/src/variable.h
@@ -205,6 +205,7 @@ struct variable *define_variable_in_set (const char *name, size_t length,
struct variable_set *set,
const floc *flocp);
void warn_undefined (const char* name, size_t length);
+void reset_env_override (void);
/* Define a variable in the current variable set. */
diff --git a/tests/scripts/functions/foreach b/tests/scripts/functions/foreach
index 27dae15a..3af37a23 100644
--- a/tests/scripts/functions/foreach
+++ b/tests/scripts/functions/foreach
@@ -28,8 +28,8 @@ all: auto for2
auto : ; @echo $(av)
for2: ; @echo $(fe)',
'-e WHITE=WHITE CFLAGS=',
- "undefined file default file environment default file command line override automatic automatic
-foo.o bletch.o null.o @.o garf.o .o .o undefined.o file.o default.o file.o environment.o default.o file.o command.o line.o override.o automatic.o automatic.o");
+ "undefined file default file environment override default file command line override automatic automatic
+foo.o bletch.o null.o @.o garf.o .o .o undefined.o file.o default.o file.o environment.o override.o default.o file.o command.o line.o override.o automatic.o automatic.o");
# TEST 1: Test that foreach variables take precedence over global
# variables in a global scope (like inside an eval). Tests bug #11913
diff --git a/tests/scripts/functions/let b/tests/scripts/functions/let
index d2e4d822..8c6c1bcb 100644
--- a/tests/scripts/functions/let
+++ b/tests/scripts/functions/let
@@ -85,7 +85,7 @@ all: auto target
auto: ; @echo $(let $(auto_var),,$(av)) $(av)
$(let AR foo,bar foo ,$(eval $(value mktarget)))',
'-e WHITE=WHITE CFLAGS=',
- "automatic automatic automatic automatic automatic automatic automatic automatic automatic undefined default environment default file command line override automatic automatic
+ "automatic automatic automatic automatic automatic automatic automatic automatic automatic undefined default environment override default file command line override automatic automatic
ar_foo _
");
diff --git a/tests/scripts/functions/origin b/tests/scripts/functions/origin
index 9b9fd56a..78e0e6f0 100644
--- a/tests/scripts/functions/origin
+++ b/tests/scripts/functions/origin
@@ -36,10 +36,10 @@ all: auto
auto :
> @echo $(av)',
'-e WHITE=WHITE CFLAGS=',
- 'undefined default environment default file command line override automatic
+ 'undefined default environment override default file command line override automatic
undefined
default
-environment
+environment override
default
file
command line
diff --git a/tests/scripts/options/dash-e b/tests/scripts/options/dash-e
index e4659fb2..423081e3 100644
--- a/tests/scripts/options/dash-e
+++ b/tests/scripts/options/dash-e
@@ -21,4 +21,125 @@ recurse: ; @$(MAKE) -f #MAKEFILE#
'-e --no-print-directory FOO=1 recurse',
"FOO [command line]: 1\nFOO [command line]: 1\n#MAKE#[1]: 'all' is up to date.");
+# SV 64803.
+# Test that the origin of an env variable is 'enviroment override' when -e
+# is set and the makefile does not modify the variable.
+# First run the test without -e and then with -e.
+
+mkdir('lib', 0777);
+
+create_file('lib/makefile',
+'$(info in submake value=$(hello), origin=$(origin hello))
+all:; @echo "value=$(hello), origin=$(origin hello)"'."\n");
+
+# No -e.
+$ENV{hello} = 'world';
+run_make_test(q!
+.RECIPEPREFIX = >
+$(info value=$(hello), origin=$(origin hello))
+all:
+> @echo "value=$(hello), origin=$(origin hello)"
+> @$(MAKE) -C lib
+.PHONY: lib all
+!,
+ '-s',
+ "value=world, origin=environment\nvalue=world, origin=environment\n".
+ "in submake value=world, origin=environment\nvalue=world, origin=environment");
+
+# -e is specified on the command line.
+my @opts = ('-e', '--environment-overrides');
+for my $opt (@opts) {
+$ENV{hello} = 'world';
+run_make_test(q!
+.RECIPEPREFIX = >
+$(info value=$(hello), origin=$(origin hello))
+all:
+> @echo "value=$(hello), origin=$(origin hello)"
+> @$(MAKE) -C lib
+.PHONY: lib all
+!,
+ "-s $opt",
+ "value=world, origin=environment override\nvalue=world, origin=environment override\n".
+ "in submake value=world, origin=environment override\nvalue=world, origin=environment override");
+}
+
+# MAKEFLAGS from env affects top level make.
+$ENV{hello} = 'world';
+$ENV{MAKEFLAGS} = 'e';
+run_make_test(q!
+.RECIPEPREFIX = >
+$(info value=$(hello), origin=$(origin hello))
+all:
+> @echo "value=$(hello), origin=$(origin hello)"
+> @$(MAKE) -C lib
+.PHONY: lib all
+!,
+ "-s",
+ "value=world, origin=environment override\nvalue=world, origin=environment override\n".
+ "in submake value=world, origin=environment override\nvalue=world, origin=environment override");
+
+# -e is passed to submake on the command line.
+$ENV{hello} = 'world';
+run_make_test(q!
+.RECIPEPREFIX = >
+$(info value=$(hello), origin=$(origin hello))
+all:
+> @echo "value=$(hello), origin=$(origin hello)"
+> @$(MAKE) -e -C lib
+.PHONY: lib all
+!,
+ "-s",
+ "value=world, origin=environment\nvalue=world, origin=environment\n".
+ "in submake value=world, origin=environment override\nvalue=world, origin=environment override");
+
+# MAKEFLAGS is reset for submake.
+$ENV{hello} = 'world';
+run_make_test(q!
+.RECIPEPREFIX = >
+$(info value=$(hello), origin=$(origin hello))
+all:
+> @echo "value=$(hello), origin=$(origin hello)"
+> @$(MAKE) -C lib MAKEFLAGS=
+.PHONY: lib all
+!,
+ "-se",
+ "value=world, origin=environment override\nvalue=world, origin=environment override\n".
+ "in submake value=world, origin=environment\nvalue=world, origin=environment");
+
+# Some MAKEFLAGS in top make env.
+# This MAKEFLAGS does not conform to the format that make itself produces for
+# submake. However, make still parses and honors this MAKEFLAGS.
+# This test checks that make does not confuse 'e' in 'extramk' with '-e'.
+$ENV{MAKEFLAGS} = 'r -Iextramk -k bye=moon';
+$ENV{hello} = 'world';
+run_make_test(q!
+.RECIPEPREFIX = >
+$(info value=$(hello), origin=$(origin hello))
+all:
+> @echo "value=$(hello), origin=$(origin hello)"
+> @$(MAKE) -C lib
+.PHONY: lib all
+!,
+ "-s",
+ "value=world, origin=environment\nvalue=world, origin=environment\n".
+ "in submake value=world, origin=environment\nvalue=world, origin=environment");
+
+# Some MAKEFLAGS in top make env.
+# This MAKEFLAGS does not conform to the format that make itself produces for
+# submake. However, make still parses and honors this MAKEFLAGS.
+# This test checks that make detects '-e' in this MAKEFLAGS.
+$ENV{MAKEFLAGS} = 'r -Iextramk -ke bye=moon';
+$ENV{hello} = 'world';
+run_make_test(q!
+.RECIPEPREFIX = >
+$(info value=$(hello), origin=$(origin hello))
+all:
+> @echo "value=$(hello), origin=$(origin hello)"
+> @$(MAKE) -C lib
+.PHONY: lib all
+!,
+ "-s",
+ "value=world, origin=environment override\nvalue=world, origin=environment override\n".
+ "in submake value=world, origin=environment override\nvalue=world, origin=environment override");
+
1;