Support "unexport" in target-specific variables.

Rewrite the environment variable algorithm to correctly inherit
export settings from parent variable sets.  The new algorithm
for computing the table of environment variables is:

- Start with the most local variable set and proceed to global.
- If the variable already exists in the table and we don't know
  its export status, update it with the current variable's status.
- If the variable is not in the table and it's not global, add it
  regardless of its status so if it's unexported we remember that.
- If the variable is not in the table and is global, check its
  export status and don't add it if we won't export it.

Then when generating the environment variables, check the export
status of each variable in case it was a target-specific variable
and we have determined it should not be exported.

Rework SHELL handling to check at the end whether we added it or
not and if we didn't, add the value from the environment.

* NEWS: Announce support for target-specific "unexport"."
* doc/make.texi (Target-specific): Document the support.
* src/variable.h (enum variable_export): Make into a global type.
* src/read.c (struct vmodifiers): Use enum variable_export rather
than individual booleans.
(parse_var_assignment): Parse the "unexport" keyword.
(eval): Remember the vmodifier value in the variable.
(record_target_var): Ditto.
* src/variable.c (should_export): Check if the variable should be
exported.
(target_environment): Implement the above algorithm.
* tests/scripts/features/export: Test export/unexport with variable
assignments on the same line.
* tests/scripts/features/targetvars: Add a comprehensive suite of
tests for different types of target-specific export / unexport.
* tests/scripts/variables/SHELL: Update the comment.

NEWS

@@ -34,6 +34,11 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=109&se
   https://www.gnu.org/software/gnulib/manual/html_node/C99-features-assumed.html
   The configure script should verify the compiler has these features.
 
+* Target-specific variables can now be marked "unexport".
+
+* Exporting / unexporting target-specific variables is handled correctly, so
+  that the attribute of the most specific variable setting is used.
+
 * Special targets like .POSIX are detected upon definition, ensuring that any
   change in behavior takes effect immediately, before the next line is parsed.
 

doc/make.texi

@@ -6283,8 +6283,9 @@ Set a target-specific variable value like this:
 @end example
 
 Target-specific variable assignments can be prefixed with any or all of the
-special keywords @code{export}, @code{override}, or @code{private};
+special keywords @code{export}, @code{unexport}, @code{override}, or
-these apply their normal behavior to this instance of the variable only.
+@code{private}; these apply their normal behavior to this instance of the
+variable only.
 
 Multiple @var{target} values create a target-specific variable value for
 each member of the target list individually.

src/read.c

@@ -63,9 +63,9 @@ struct vmodifiers
     unsigned int assign_v:1;
     unsigned int define_v:1;
     unsigned int undefine_v:1;
-    unsigned int export_v:1;
     unsigned int override_v:1;
     unsigned int private_v:1;
+    enum variable_export export_v ENUM_BITFIELD (2);
   };
 
 /* Types of "words" that can be read in a makefile.  */
@@ -517,7 +517,9 @@ parse_var_assignment (const char *line, struct vmodifiers *vmod)
       wlen = p2 - p;
 
       if (word1eq ("export"))
-        vmod->export_v = 1;
+        vmod->export_v = v_export;
+      else if (word1eq ("unexport"))
+        vmod->export_v = v_noexport;
       else if (word1eq ("override"))
         vmod->override_v = 1;
       else if (word1eq ("private"))
@@ -743,8 +745,8 @@ eval (struct ebuffer *ebuf, int set_default)
 
           assert (v != NULL);
 
-          if (vmod.export_v)
+          if (vmod.export_v != v_default)
-            v->export = v_export;
+            v->export = vmod.export_v;
           if (vmod.private_v)
             v->private_var = 1;
 
@@ -1832,8 +1834,8 @@ record_target_var (struct nameseq *filenames, char *defn,
       /* Set up the variable to be *-specific.  */
       v->per_target = 1;
       v->private_var = vmod->private_v;
-      if (vmod->export_v)
+      if (vmod->export_v != v_default)
-        v->export = v_export;
+        v->export = vmod->export_v;
 
       /* If it's not an override, check to see if there was a command-line
          setting.  If so, reset the value.  */

src/variable.c

@@ -980,6 +980,41 @@ define_automatic_variables (void)
 
 int export_all_variables;
 
+static int
+should_export (const struct variable *v)
+{
+  switch (v->export)
+    {
+    case v_export:
+      break;
+
+    case v_noexport:
+      return 0;
+
+    case v_ifset:
+      if (v->origin == o_default)
+        return 0;
+      break;
+
+    case v_default:
+      if (v->origin == o_default || v->origin == o_automatic)
+        /* Only export default variables by explicit request.  */
+        return 0;
+
+      /* The variable doesn't have a name that can be exported.  */
+      if (! v->exportable)
+        return 0;
+
+      if (! export_all_variables
+          && v->origin != o_command
+          && v->origin != o_env && v->origin != o_env_override)
+        return 0;
+      break;
+    }
+
+  return 1;
+}
+
 /* Create a new environment for FILE's commands.
    If FILE is nil, this is for the 'shell' function.
    The child's MAKELEVEL variable is incremented.  */
@@ -995,6 +1030,8 @@ target_environment (struct file *file)
   struct variable makelevel_key;
   char **result_0;
   char **result;
+  /* If we got no value from the environment then never add the default.  */
+  int added_SHELL = shell_var.value == 0;
 
   if (file == 0)
     set_list = current_variable_set_list;
@@ -1004,74 +1041,35 @@ target_environment (struct file *file)
   hash_init (&table, VARIABLE_BUCKETS,
              variable_hash_1, variable_hash_2, variable_hash_cmp);
 
-  /* Run through all the variable sets in the list,
+  /* Run through all the variable sets in the list, accumulating variables
-     accumulating variables in TABLE.  */
+     in TABLE.  We go from most specific to least, so the first variable we
+     encounter is the keeper.  */
   for (s = set_list; s != 0; s = s->next)
     {
       struct variable_set *set = s->set;
+      int isglobal = set == &global_variable_set;
+
       v_slot = (struct variable **) set->table.ht_vec;
       v_end = v_slot + set->table.ht_size;
       for ( ; v_slot < v_end; v_slot++)
         if (! HASH_VACANT (*v_slot))
           {
-            struct variable **new_slot;
+            struct variable **evslot;
             struct variable *v = *v_slot;
 
-            /* If this is a per-target variable and it hasn't been touched
+            evslot = (struct variable **) hash_find_slot (&table, v);
-               already then look up the global version and take its export
+
-               value.  */
+            if (HASH_VACANT (*evslot))
-            if (v->per_target && v->export == v_default)
               {
-                struct variable *gv;
+                /* If we're not global, or we are and should export, add it.  */
-
+                if (!isglobal || should_export (v))
-                gv = lookup_variable_in_set (v->name, strlen (v->name),
+                  hash_insert_at (&table, v, evslot);
-                                             &global_variable_set);
-                if (gv)
-                  v->export = gv->export;
               }
-
+            else if ((*evslot)->export == v_default)
-            switch (v->export)
               {
-              case v_default:
+                /* We already have a variable but we don't know its status.  */
-                if (v->origin == o_default || v->origin == o_automatic)
+                (*evslot)->export = v->export;
-                  /* Only export default variables by explicit request.  */
-                  continue;
-
-                /* The variable doesn't have a name that can be exported.  */
-                if (! v->exportable)
-                  continue;
-
-                if (! export_all_variables
-                    && v->origin != o_command
-                    && v->origin != o_env && v->origin != o_env_override)
-                  continue;
-                break;
-
-              case v_export:
-                break;
-
-              case v_noexport:
-                {
-                  /* If this is the SHELL variable and it's not exported,
-                     then add the value from our original environment, if
-                     the original environment defined a value for SHELL.  */
-                  if (streq (v->name, "SHELL") && shell_var.value)
-                    {
-                      v = &shell_var;
-                      break;
-                    }
-                  continue;
-                }
-
-              case v_ifset:
-                if (v->origin == o_default)
-                  continue;
-                break;
               }
-
-            new_slot = (struct variable **) hash_find_slot (&table, v);
-            if (HASH_VACANT (*new_slot))
-              hash_insert_at (&table, v, new_slot);
           }
     }
 
@@ -1079,7 +1077,7 @@ target_environment (struct file *file)
   makelevel_key.length = MAKELEVEL_LENGTH;
   hash_delete (&table, &makelevel_key);
 
-  result = result_0 = xmalloc ((table.ht_fill + 2) * sizeof (char *));
+  result = result_0 = xmalloc ((table.ht_fill + 3) * sizeof (char *));
 
   v_slot = (struct variable **) table.ht_vec;
   v_end = v_slot + table.ht_size;
@@ -1088,6 +1086,15 @@ target_environment (struct file *file)
       {
         struct variable *v = *v_slot;
 
+        /* This might be here because it was a target-specific variable that
+           we didn't know the status of when we added it.  */
+        if (! should_export (v))
+          continue;
+
+        /* If this is the SHELL variable remember we already added it.  */
+        if (!added_SHELL && streq (v->name, "SHELL"))
+          added_SHELL = 1;
+
         /* If V is recursively expanded and didn't come from the environment,
            expand its value.  If it came from the environment, it should
            go back into the environment unchanged.  */
@@ -1114,6 +1121,9 @@ target_environment (struct file *file)
           }
       }
 
+  if (!added_SHELL)
+    *result++ = xstrdup (concat (3, shell_var.name, "=", shell_var.value));
+
   *result = xmalloc (100);
   sprintf (*result, "%s=%u", MAKELEVEL_NAME, makelevel + 1);
   *++result = 0;

src/variable.h

@@ -41,6 +41,14 @@ enum variable_flavor
     f_append_value      /* Append unexpanded value */
   };
 
+enum variable_export
+{
+    v_default = 0,      /* Decide in target_environment.  */
+    v_export,           /* Export this variable.  */
+    v_noexport,         /* Don't export this variable.  */
+    v_ifset             /* Export it if it has a non-default value.  */
+};
+
 /* Structure that represents one variable definition.
    Each bucket of the hash table is a chain of these,
    chained through 'next'.  */
@@ -73,12 +81,7 @@ struct variable
     enum variable_origin
       origin ENUM_BITFIELD (3); /* Variable origin.  */
     enum variable_export
-      {
+      export ENUM_BITFIELD (2); /* Export control. */
-        v_export,               /* Export this variable.  */
-        v_noexport,             /* Don't export this variable.  */
-        v_ifset,                /* Export it if it has a non-default value.  */
-        v_default               /* Decide in target_environment.  */
-      } export ENUM_BITFIELD (2);
   };
 
 /* Structure that represents a variable set.  */

tests/scripts/features/export

@@ -178,7 +178,7 @@ a: ; @echo "\$$(export)=$(export) / \$$export=$$export"
 ',
                '', "\$(export)=456 / \$export=456\n");
 
-# TEST 9: Check "export" as a target
+# TEST 10: Check "export" as a target
 
 &run_make_test('
 a: export
@@ -186,5 +186,25 @@ export: ; @echo "$@"
 ',
                '', "export\n");
 
+# Check export and assignment of a variable on the same line
+
+$ENV{hello} = 'moon';
+
+run_make_test(q!
+all: ; @echo hello=$(hello) hello=$$hello
+export hello=sun
+!,
+              '', "hello=sun hello=sun\n");
+
+# Check unexport and assignment of a variable on the same line
+
+$ENV{hello} = 'moon';
+
+run_make_test(q!
+all: ; @echo hello=$(hello) hello=$$hello
+unexport hello=sun
+!,
+              '', "hello=sun hello=\n");
+
 # This tells the test driver that the perl test script executed properly.
 1;

tests/scripts/features/targetvars

@@ -12,8 +12,9 @@ export BAR = bar
 one: override FOO = one
 one two: ; @echo $(FOO) $(BAR)
 two: BAR = two
+.RECIPEPREFIX = >
 three: ; BAR=1000
-	@echo $(FOO) $(BAR)
+> @echo $(FOO) $(BAR)
 # Some things that shouldn not be target vars
 funk : override
 funk : override adelic
@@ -301,6 +302,117 @@ dummy: hello?=world
 !,
               '', 'hello=sun');
 
+# Support target-specific unexport
+
+$ENV{hello} = "moon";
+run_make_test(q!
+unexport hello=sun
+all: base exp
+base exp: ; @echo hello=$$hello
+exp: export hello=world
+!,
+              '', "hello=\nhello=world\n");
+
+$ENV{hello} = "moon";
+run_make_test(q!
+hello=sun
+all: base exp
+base exp: ; @echo hello=$$hello
+exp: unexport hello=world
+!,
+              '', "hello=sun\nhello=\n");
+
+run_make_test(q!
+all:; @echo hello=$$hello
+unexport hello=sun
+dummy: hello?=world
+!,
+              '', 'hello=');
+
+$ENV{hello} = "moon";
+run_make_test(q!
+all:; @echo hello=$$hello
+hello=sun
+dummy: unexport hello=world
+!,
+              '', 'hello=sun');
+
+run_make_test(q!
+all: mid
+mid: base
+
+ifeq ($(midexport),export)
+mid: export hello=mid
+else ifeq ($(midexport),unexport)
+mid: unexport hello=mid
+else
+mid: hello=mid
+endif
+
+ifeq ($(baseexport),export)
+base: export hello=base
+else ifeq ($(baseexport),unexport)
+base: unexport hello=base
+else
+base: hello=base
+endif
+
+all mid base:; @echo $@ make=$(hello) shell=$$hello
+!,
+              '', "base make=base shell=\nmid make=mid shell=\nall make= shell=\n");
+
+# Test base settings with env var
+$ENV{hello} = "environ";
+run_make_test(undef,
+              '', "base make=base shell=base\nmid make=mid shell=mid\nall make=environ shell=environ\n");
+
+$ENV{hello} = "environ";
+run_make_test(undef,
+              'baseexport=export', "base make=base shell=base\nmid make=mid shell=mid\nall make=environ shell=environ\n");
+
+$ENV{hello} = "environ";
+run_make_test(undef,
+              'baseexport=unexport', "base make=base shell=\nmid make=mid shell=mid\nall make=environ shell=environ\n");
+
+# Test mid settings with env var
+$ENV{hello} = "environ";
+run_make_test(undef,
+              'midexport=export', "base make=base shell=base\nmid make=mid shell=mid\nall make=environ shell=environ\n");
+
+$ENV{hello} = "environ";
+run_make_test(undef,
+              'midexport=export baseexport=unexport', "base make=base shell=\nmid make=mid shell=mid\nall make=environ shell=environ\n");
+
+$ENV{hello} = "environ";
+run_make_test(undef,
+              'midexport=unexport', "base make=base shell=\nmid make=mid shell=\nall make=environ shell=environ\n");
+
+$ENV{hello} = "environ";
+run_make_test(undef,
+              'midexport=unexport baseexport=export', "base make=base shell=base\nmid make=mid shell=\nall make=environ shell=environ\n");
+
+# Test base settings without env var
+run_make_test(undef,
+              'baseexport=export', "base make=base shell=base\nmid make=mid shell=\nall make= shell=\n");
+
+run_make_test(undef,
+              'baseexport=unexport', "base make=base shell=\nmid make=mid shell=\nall make= shell=\n");
+
+# Test mid settings with env var
+run_make_test(undef,
+              'midexport=export', "base make=base shell=base\nmid make=mid shell=mid\nall make= shell=\n");
+
+run_make_test(undef,
+              'midexport=export baseexport=unexport', "base make=base shell=\nmid make=mid shell=mid\nall make= shell=\n");
+
+run_make_test(undef,
+              'midexport=unexport', "base make=base shell=\nmid make=mid shell=\nall make= shell=\n");
+
+run_make_test(undef,
+              'midexport=unexport baseexport=export', "base make=base shell=base\nmid make=mid shell=\nall make= shell=\n");
+
+
+
 # TEST #19: Test define/endef variables as target-specific vars
 
 # run_make_test('
@@ -316,7 +428,3 @@ dummy: hello?=world
 #               '', "local\n");
 
 1;
-
-### Local Variables:
-### eval: (setq whitespace-action (delq 'auto-cleanup whitespace-action))
-### End:

tests/scripts/variables/SHELL

@@ -18,16 +18,15 @@ $mshell = $sh_name;
 $ENV{SHELL} = '/dev/null';
 run_make_test('all:;@echo "$(SHELL)"', '', $mshell);
 
-# According to POSIX, any value of SHELL set in the makefile should _NOT_ be
+# According to POSIX, any value of SHELL set in the makefile should not be
-# exported to the subshell!  I wanted to set SHELL to be $^X (perl) in the
+# exported to the subshell.  A more portable option might be to set SHELL to
-# makefile, but make runs $(SHELL) -c 'commandline' and that doesn't work at
+# be $^X (perl) in the makefile, and set .SHELLFLAGS to -e.
-# all when $(SHELL) is perl :-/.  So, we just add an extra initial /./ which
-# works well on UNIX and seems to work OK on at least some non-UNIX systems.
 
 $ENV{SHELL} = $mshell;
 
 my $altshell = "/./$mshell";
 my $altshell2 = "/././$mshell";
+
 if ($mshell =~ m,^([a-zA-Z]:)([\\/])(.*),) {
     $altshell = "$1$2.$2$3";
     $altshell2 = "$1$2.$2.$2$3";