diff options
| -rw-r--r-- | NEWS | 6 | ||||
| -rw-r--r-- | doc/make.texi | 17 | ||||
| -rw-r--r-- | src/function.c | 43 | ||||
| -rw-r--r-- | src/job.c | 23 | ||||
| -rw-r--r-- | src/job.h | 1 | ||||
| -rw-r--r-- | src/main.c | 1 | ||||
| -rw-r--r-- | src/variable.c | 39 | ||||
| -rw-r--r-- | tests/scripts/functions/shell | 55 |
8 files changed, 124 insertions, 61 deletions
@@ -33,6 +33,12 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=109&se variable that was visible while parsing makefiles. Now, all options are available in MAKEFLAGS. +* WARNING: Backward-incompatibility! + Previously makefile variables marked as export were not exported to commands + started by the $(shell ...) function. Now, all exported variables are + exported to $(shell ...). + To detect this change search for 'shell-export' in the .FEATURES variable. + * WARNING: New build requirement GNU make utilizes facilities from GNU Gnulib: Gnulib requires certain C99 features in the C compiler and so these features are required by GNU make: diff --git a/doc/make.texi b/doc/make.texi index 943c0941..085e9c24 100644 --- a/doc/make.texi +++ b/doc/make.texi @@ -8616,6 +8616,23 @@ using a very strange shell, this has the same result as @w{@samp{$(wildcard *.c)}} (as long as at least one @samp{.c} file exists).@refill +All variables that are marked as @code{export} will also be passed to the +shell started by the @code{shell} function. It is possible to create a +variable expansion loop: consider this @file{makefile}: + +@example +export HI = $(shell echo hi) +all: ; @@echo $$HI +@end example + +When @code{make} wants to run the recipe it must add the variable @var{HI} to +the environment; to do so it must be expanded. The value of this variable +requires an invocation of the @code{shell} function, and to invoke it we must +create its environment. Since @var{HI} is exported, we need to expand it to +create its environment. And so on. In this obscure case @code{make} will not +add recursively-expanded variables to the @code{shell} environment rather than +fail with an error. + @node Guile Function, , Shell Function, Functions @section The @code{guile} Function @findex guile diff --git a/src/function.c b/src/function.c index 89449a8f..9247fc3f 100644 --- a/src/function.c +++ b/src/function.c @@ -1725,12 +1725,6 @@ windows32_openpipe (int *pipedes, int errfd, pid_t *pid_p, char **command_argv, return -1; } - /* make sure that CreateProcess() has Path it needs */ - sync_Path_environment (); - /* 'sync_Path_environment' may realloc 'environ', so take note of - the new value. */ - envp = environ; - if (! process_begin (hProcess, command_argv, envp, command_argv[0], NULL)) { /* register process for wait */ @@ -1845,13 +1839,13 @@ func_shell_base (char *o, char **argv, int trim_newlines) char * func_shell_base (char *o, char **argv, int trim_newlines) { + struct childbase child = {0}; char *batch_filename = NULL; int errfd; #ifdef __MSDOS__ FILE *fpipe; #endif char **command_argv = NULL; - char **envp; int pipedes[2]; pid_t pid; @@ -1876,26 +1870,14 @@ func_shell_base (char *o, char **argv, int trim_newlines) } #endif /* !__MSDOS__ */ - /* Using a target environment for 'shell' loses in cases like: - export var = $(shell echo foobie) - bad := $(var) - because target_environment hits a loop trying to expand $(var) to put it - in the environment. This is even more confusing when 'var' was not - explicitly exported, but just appeared in the calling environment. - - See Savannah bug #10593. - - envp = target_environment (NULL); - */ - - envp = environ; - /* Set up the output in case the shell writes something. */ output_start (); errfd = (output_context && output_context->err >= 0 ? output_context->err : FD_STDERR); + child.environment = target_environment (NULL); + #if defined(__MSDOS__) fpipe = msdos_openpipe (pipedes, &pid, argv[0]); if (pipedes[0] < 0) @@ -1906,7 +1888,7 @@ func_shell_base (char *o, char **argv, int trim_newlines) } #elif defined(WINDOWS32) - windows32_openpipe (pipedes, errfd, &pid, command_argv, envp); + windows32_openpipe (pipedes, errfd, &pid, command_argv, child.environment); /* Restore the value of just_print_flag. */ just_print_flag = j_p_f; @@ -1931,18 +1913,11 @@ func_shell_base (char *o, char **argv, int trim_newlines) fd_noinherit (pipedes[1]); fd_noinherit (pipedes[0]); - { - struct childbase child; - child.cmd_name = NULL; - child.output.syncout = 1; - child.output.out = pipedes[1]; - child.output.err = errfd; - child.environment = envp; + child.output.syncout = 1; + child.output.out = pipedes[1]; + child.output.err = errfd; - pid = child_execute_job (&child, 1, command_argv); - - free (child.cmd_name); - } + pid = child_execute_job (&child, 1, command_argv); if (pid < 0) { @@ -2029,6 +2004,8 @@ func_shell_base (char *o, char **argv, int trim_newlines) free (command_argv); } + free_childbase (&child); + return o; } @@ -1117,6 +1117,20 @@ reap_children (int block, int err) /* Free the storage allocated for CHILD. */ +void +free_childbase (struct childbase *child) +{ + if (child->environment != 0) + { + char **ep = child->environment; + while (*ep != 0) + free (*ep++); + free (child->environment); + } + + free (child->cmd_name); +} + static void free_child (struct child *child) { @@ -1149,15 +1163,8 @@ free_child (struct child *child) free (child->command_lines); } - if (child->environment != 0) - { - char **ep = child->environment; - while (*ep != 0) - free (*ep++); - free (child->environment); - } + free_childbase ((struct childbase*)child); - free (child->cmd_name); free (child); } @@ -73,6 +73,7 @@ int is_bourne_compatible_shell(const char *path); void new_job (struct file *file); void reap_children (int block, int err); void start_waiting_jobs (void); +void free_childbase (struct childbase* child); char **construct_command_argv (char *line, char **restp, struct file *file, int cmd_flags, char** batch_file); @@ -1344,6 +1344,7 @@ main (int argc, char **argv, char **envp) const char *features = "target-specific order-only second-expansion" " else-if shortest-stem undefine oneshell nocomment" " grouped-target extra-prereqs notintermediate" + " shell-export" #ifndef NO_ARCHIVES " archives" #endif diff --git a/src/variable.c b/src/variable.c index 3cddfd6d..c53ce5a7 100644 --- a/src/variable.c +++ b/src/variable.c @@ -19,6 +19,7 @@ this program. If not, see <http://www.gnu.org/licenses/>. */ #include <assert.h> #include "filedef.h" +#include "debug.h" #include "dep.h" #include "job.h" #include "commands.h" @@ -1099,17 +1100,24 @@ target_environment (struct file *file) /* 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. */ - if (v->recursive - && v->origin != o_env && v->origin != o_env_override) + if (v->recursive && v->origin != o_env && v->origin != o_env_override) { - char *value = recursively_expand_for_file (v, file); + /* If V is being recursively expanded and this is for a shell + function, just skip it. */ + if (v->expanding && file == NULL) + DB (DB_VERBOSE, (_("%s:%lu: Skipping export of %s to shell function due to recursive expansion"), + v->fileinfo.filenm, v->fileinfo.lineno, v->name)); + else + { + char *value = recursively_expand_for_file (v, file); #ifdef WINDOWS32 - if (strcmp (v->name, "Path") == 0 || - strcmp (v->name, "PATH") == 0) - convert_Path_to_windows32 (value, ';'); + if (strcmp (v->name, "Path") == 0 || + strcmp (v->name, "PATH") == 0) + convert_Path_to_windows32 (value, ';'); #endif - *result++ = xstrdup (concat (3, v->name, "=", value)); - free (value); + *result++ = xstrdup (concat (3, v->name, "=", value)); + free (value); + } } else { @@ -1858,21 +1866,20 @@ print_target_variables (const struct file *file) #ifdef WINDOWS32 void -sync_Path_environment (void) +sync_Path_environment () { - char *path = allocated_variable_expand ("$(PATH)"); static char *environ_path = NULL; + char *oldpath = environ_path; + char *path = allocated_variable_expand ("PATH=$(PATH)"); if (!path) return; - /* If done this before, free the previous entry before allocating new one. */ - free (environ_path); - - /* Create something WINDOWS32 world can grok. */ + /* Convert PATH into something WINDOWS32 world can grok. */ convert_Path_to_windows32 (path, ';'); - environ_path = xstrdup (concat (3, "PATH", "=", path)); + + environ_path = path; putenv (environ_path); - free (path); + free (oldpath); } #endif diff --git a/tests/scripts/functions/shell b/tests/scripts/functions/shell index 63320a2b..80425172 100644 --- a/tests/scripts/functions/shell +++ b/tests/scripts/functions/shell @@ -32,14 +32,61 @@ foo: ; echo '$(FOO)' !, '', "echo '#'\n#\n"); -# Test shells inside exported environment variables. -# This is the test that fails if we try to put make exported variables into -# the environment for a $(shell ...) call. +# Test that exported variables are passed to $(shell ...) +$ENV{FOO} = 'baz'; +run_make_test(q! +OUT = $(shell echo $$FOO) +foo: ; @echo '$(OUT)' +!, + '', 'baz'); + +$ENV{FOO} = 'baz'; +run_make_test(q! +FOO = bar +OUT = $(shell echo $$FOO) +foo: ; @echo '$(OUT)' +!, + '', 'bar'); + +run_make_test(q! +export FOO = bar +OUT = $(shell echo $$FOO) +foo: ; @echo '$(OUT)' +!, + '', 'bar'); + +# Test shells inside exported environment variables, simply expanded. +run_make_test(' +export HI := $(shell echo hi) +.PHONY: all +all: ; @echo $$HI +', + '','hi'); + +# Test shells inside exported environment variables. See SV 10593 run_make_test(' export HI = $(shell echo hi) .PHONY: all all: ; @echo $$HI - ','','hi'); +', + '',"hi"); + +$ENV{HI} = 'foo'; +run_make_test(' +HI = $(shell echo hi) +.PHONY: all +all: ; @echo $$HI +', + '',"hi"); + +$ENV{HI} = 'foo'; +run_make_test(' +HI = $(shell echo hi) +bad := $(HI) +.PHONY: all +all: ; @echo $$HI $(bad) +', + '',"hi hi"); if ($port_type ne 'W32') { # Test shell errors in recipes including offset |