From 539f513773b2e651d987a7bdbdffd8b5164d58cf Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sun, 28 Nov 2004 23:11:23 +0000 Subject: [PATCH] Fix for bug #1276: Handle SHELL according to POSIX requirements. POSIX requires that the value of SHELL in the makefile NOT be exported to sub-commands. Instead, the value in the environment when make was invoked should be passed to the environment of sub-commands. Note that make still uses SHELL to _run_ sub-commands; it just doesn't change the value of the SHELL variable in the environment of sub-commands. As an extension to POSIX, if the makefile explicitly exports SHELL then GNU make _will_ use it in the environment of sub-commands. --- ChangeLog | 12 +++++++ doc/make.texi | 56 ++++++++++++++++++++----------- main.c | 43 +++++++++++++++--------- make.h | 2 ++ tests/ChangeLog | 5 +++ tests/scripts/variables/MAKELEVEL | 3 +- tests/scripts/variables/SHELL | 49 +++++++++++++++++++++++++++ variable.c | 26 +++++++++----- 8 files changed, 150 insertions(+), 46 deletions(-) create mode 100644 tests/scripts/variables/SHELL diff --git a/ChangeLog b/ChangeLog index 6ec8a1d5..b1dcefd7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2004-11-28 Paul D. Smith + + Fix for bug #1276: Handle SHELL according to POSIX requirements. + + * main.c (main): Set SHELL to v_noexport by default. Remember the + original environment setting of SHELL in the env_shell variable. + * main.h: Export new env_shell variable. + * variable.c (target_environment): If we find a v_noexport + variable for SHELL, add a SHELL variable with the env_shell value. + * doc/make.texi (Quick Reference): Document the POSIX behavior. + * doc/make.texi (Variables/Recursion): Ditto. + 2004-11-28 Paul D. Smith * main.c (find_and_set_default_shell) [WINDOWS32]: check for diff --git a/doc/make.texi b/doc/make.texi index a31c3ad9..14d72781 100644 --- a/doc/make.texi +++ b/doc/make.texi @@ -3732,9 +3732,15 @@ line, and if its name consists only of letters, numbers, and underscores. Some shells cannot cope with environment variable names consisting of characters other than letters, numbers, and underscores. -The special variables @code{SHELL} and @code{MAKEFLAGS} are always -exported (unless you unexport them). -@code{MAKEFILES} is exported if you set it to anything. +@cindex SHELL, exported value +The value of the @code{make} variable @code{SHELL} is not exported. +Instead, the value of the @code{SHELL} variable from the invoking +environment is passed to the sub-@code{make}. You can force +@code{make} to export its value for @code{SHELL} by using the +@code{export} directive, described below. + +The special variable @code{MAKEFLAGS} is always exported (unless you +unexport it). @code{MAKEFILES} is exported if you set it to anything. @code{make} automatically passes down variable values that were defined on the command line, by putting them in the @code{MAKEFLAGS} variable. @@ -5137,28 +5143,28 @@ endef @cindex variables, environment @cindex environment Variables in @code{make} can come from the environment in which -@code{make} is run. Every environment variable that @code{make} sees when -it starts up is transformed into a @code{make} variable with the same name -and value. But an explicit assignment in the makefile, or with a command -argument, overrides the environment. (If the @samp{-e} flag is specified, -then values from the environment override assignments in the makefile. -@xref{Options Summary, ,Summary of Options}. -But this is not recommended practice.) +@code{make} is run. Every environment variable that @code{make} sees +when it starts up is transformed into a @code{make} variable with the +same name and value. However, an explicit assignment in the makefile, +or with a command argument, overrides the environment. (If the +@samp{-e} flag is specified, then values from the environment override +assignments in the makefile. @xref{Options Summary, ,Summary of +Options}. But this is not recommended practice.) Thus, by setting the variable @code{CFLAGS} in your environment, you can cause all C compilations in most makefiles to use the compiler switches you prefer. This is safe for variables with standard or conventional meanings -because you know that no makefile will use them for other things. (But +because you know that no makefile will use them for other things. (Note this is not totally reliable; some makefiles set @code{CFLAGS} explicitly and therefore are not affected by the value in the environment.) -When @code{make} is invoked recursively, variables defined in the -outer invocation can be passed to inner invocations through the -environment (@pxref{Recursion, ,Recursive Use of @code{make}}). By -default, only variables that came from the environment or the command -line are passed to recursive invocations. You can use the -@code{export} directive to pass other variables. -@xref{Variables/Recursion, , Communicating Variables to a +When @code{make} runs a command script, variables defined in the +makefile are placed into the environment of that command. This allows +you to pass values to sub-@code{make} invocations. (@pxref{Recursion, +,Recursive Use of @code{make}}). By default, only variables that came +from the environment or the command line are passed to recursive +invocations. You can use the @code{export} directive to pass other +variables. @xref{Variables/Recursion, , Communicating Variables to a Sub-@code{make}}, for full details. Other use of variables from the environment is not recommended. It is not @@ -5167,6 +5173,7 @@ set up outside their control, since this would cause different users to get different results from the same makefile. This is against the whole purpose of most makefiles. +@cindex SHELL, import from environment Such problems would be especially likely with the variable @code{SHELL}, which is normally present in the environment to specify the user's choice of interactive shell. It would be very undesirable for this choice to @@ -5175,6 +5182,15 @@ affect @code{make}. So @code{make} ignores the environment value of usually not set. @xref{Execution, ,Special handling of SHELL on MS-DOS}.)@refill +@cindex SHELL, export to environment +The @code{SHELL} variable is special in another way: just as the value +of the @code{make} variable @code{SHELL} is not taken from the +environment, so also it is not placed into the environment of commands +that @code{make} invokes. Instead, the value of @code{SHELL} from the +invoking environment is provided to the command. You can use +@code{export SHELL} to force the value of the @code{make} variable +@code{SHELL} to be placed in the environment of commands. + @node Target-specific, Pattern-specific, Environment, Using Variables @section Target-specific Variable Values @cindex target-specific variables @@ -9865,7 +9881,9 @@ Directory search path for files not found in the current directory.@* The name of the system default command interpreter, usually @file{/bin/sh}. You can set @code{SHELL} in the makefile to change the shell used to run -commands. @xref{Execution, ,Command Execution}. +commands. @xref{Execution, ,Command Execution}. The @code{SHELL} +variable is handled specially when importing from and exporting to the +environment. @xref{Environment, ,Using Variable from the Environment}. @item MAKESHELL diff --git a/main.c b/main.c index ca6318bd..f0229eef 100644 --- a/main.c +++ b/main.c @@ -264,6 +264,10 @@ int always_make_flag = 0; int rebuilding_makefiles = 0; +/* Remember the original value of the SHELL variable, from the environment. */ + +const char *env_shell = 0; + /* The usage output. We write it this way to make life easier for the translators, especially those trying to translate to right-to-left @@ -1045,11 +1049,8 @@ main (int argc, char **argv, char **envp) #ifndef _AMIGA for (i = 0; envp[i] != 0; ++i) { - int do_not_define; - register char *ep = envp[i]; - - /* by default, everything gets defined and exported */ - do_not_define = 0; + int do_not_define = 0; + char *ep = envp[i]; while (*ep != '=') ++ep; @@ -1065,17 +1066,27 @@ main (int argc, char **argv, char **envp) machines where ptrdiff_t is a different size that doesn't widen the same. */ if (!do_not_define) - define_variable (envp[i], (unsigned int) (ep - envp[i]), - ep + 1, o_env, 1) - /* Force exportation of every variable culled from the environment. - We used to rely on target_environment's v_default code to do this. - But that does not work for the case where an environment variable - is redefined in a makefile with `override'; it should then still - be exported, because it was originally in the environment. - Another wrinkle is that POSIX says the value of SHELL set in the - makefile should not change the value of SHELL given to - subprocesses, which seems silly to me but... */ - ->export = strncmp(envp[i], "SHELL=", 6) ? v_noexport : v_export; + { + struct variable *v; + + v = define_variable (envp[i], (unsigned int) (ep - envp[i]), + ep + 1, o_env, 1); + /* Force exportation of every variable culled from the environment. + We used to rely on target_environment's v_default code to do this. + But that does not work for the case where an environment variable + is redefined in a makefile with `override'; it should then still + be exported, because it was originally in the environment. */ + v->export = v_export; + + /* Another wrinkle is that POSIX says the value of SHELL set in the + makefile should not change the value of SHELL given to + subprocesses, which seems silly to me but... */ + if (strncmp (envp[i], "SHELL=", 6) == 0) + { + v->export = v_noexport; + env_shell = xstrdup (ep + 1); + } + } } #ifdef WINDOWS32 /* diff --git a/make.h b/make.h index ad46e1f8..55dcc215 100644 --- a/make.h +++ b/make.h @@ -496,6 +496,8 @@ extern int print_version_flag, print_directory_flag; extern int warn_undefined_variables_flag, posix_pedantic, not_parallel; extern int clock_skew_detected, rebuilding_makefiles; +extern const char *env_shell; + /* can we run commands via 'sh -c xxx' or must we use batch files? */ extern int batch_mode_shell; diff --git a/tests/ChangeLog b/tests/ChangeLog index 8cc25dba..a6423b07 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,8 @@ +2004-11-28 Paul D. Smith + + * scripts/variables/SHELL: New file: test proper handling of SHELL + according to POSIX rules. Fixes bug #1276. + 2004-10-21 Boris Kolpackov * scripts/functions/word: Test $(firstword ) and $(lastword ). diff --git a/tests/scripts/variables/MAKELEVEL b/tests/scripts/variables/MAKELEVEL index 79a184e4..96a4e740 100644 --- a/tests/scripts/variables/MAKELEVEL +++ b/tests/scripts/variables/MAKELEVEL @@ -1,4 +1,4 @@ -# -*-perl-mode-*- +# -*-perl-*- $description = "The following test creates a makefile to test makelevels in Make. It prints \$(MAKELEVEL) and then @@ -9,7 +9,6 @@ open(MAKEFILE,"> $makefile"); # The Contents of the MAKEFILE ... print MAKEFILE <export = v_export; /* Always export SHELL. */ - /* On MSDOS we do use SHELL from environment, since - it isn't a standard environment variable on MSDOS, - so whoever sets it, does that on purpose. - On OS/2 we do not use SHELL from environment but - we have already handled that problem above. */ + /* On MSDOS we do use SHELL from environment, since it isn't a standard + environment variable on MSDOS, so whoever sets it, does that on purpose. + On OS/2 we do not use SHELL from environment but we have already handled + that problem above. */ #if !defined(__MSDOS__) && !defined(__EMX__) /* Don't let SHELL come from the environment. */ if (*v->value == '\0' || v->origin == o_env || v->origin == o_env_override) @@ -809,6 +807,11 @@ target_environment (struct file *file) struct variable makelevel_key; char **result_0; char **result; + struct variable ev; + + /* Set up a fake variable struct for the original SHELL value. */ + ev.name = "SHELL"; + ev.value = env_shell; if (file == 0) set_list = current_variable_set_list; @@ -865,7 +868,12 @@ target_environment (struct file *file) break; case v_noexport: - continue; + if (!streq (v->name, "SHELL")) + continue; + /* If this is the SHELL variable and it's not exported, then + add the value from our original environment. */ + v = &ev; + break; case v_ifset: if (v->origin == o_default)