[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.

diff --git a/NEWS b/NEWS
index 676794d6c6177957f0c02f17f5f20a64784d73f1..96d8ce3370525e2b0545312e5d9d764928beb755 100644 (file)
--- 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 99011d9e8d2eca3bb0bdb3ea174f80cdf729fece..15104212511884952fe7bde7386679017be310d9 100644 (file)
--- 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 9d44cd4ad6da43976dacb69f3708294ba8f0db2b..2026e987b7e1fc62e6d4078a445d624290cb4b48 100644 (file)
--- 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);
+}
 \f
 /* 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 f6f0cb32088e052771bd6d0ed445079eb74760ab..9cedebcdbc68df49f5afbe7ce19f614f3e5ec9f6 100644 (file)
--- 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 27dae15abf8cdba4c518d10b806bd9f73d69f8fc..3af37a2308061834acec345d89394cf769d35107 100644 (file)
--- 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 d2e4d8227d3e407627c81c32bc7de5d5b78e96ca..8c6c1bcb86d292df48a5983f5118dc80bf984c04 100644 (file)
--- 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 9b9fd56a16c18bf5af3f9ac3d12d704d12890a8c..78e0e6f08cef583200c06728152065156a0278f3 100644 (file)
--- 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 e4659fb2b2a6955456e0ff2cad739aaec4b08deb..423081e3ce6da32c7a66d6ca34c0d7290d7d286f 100644 (file)
--- 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;