[SV 45211] Parse MAKEFLAGS immediately when it's reset

When MAKEFLAGS is set in a makefile, reparse it immediately rather
than waiting until after all makefiles have been read and parsed.
This change doesn't actually fix the SV bug referenced because, even
though we do reparse MAKEFLAGS, we don't handle the -r or -R options
immediately.  Doing this will require more effort.

* NEWS: Announce the change.
* src/makeint.h: Publish reset_switches() and decode_env_switches()
from main.c
* src/main.c (main): Don't call construct_include_path(); it will be
invoked decode_switches().
Preserve the old values of builtin_rules, builtin_variables, and
job_slots before we read makefiles since they can be changed now.
(reset_switches): Publish (remove static).  Set the initial value of
the stringlist list to NULL.
(decode_switches): Call construct_include_path() after decoding.
(decode_env_switches): Publish (remove static).
(define_makeflags): Set the MAKEFLAGS variable for special handling.
* src/read.c (eval_makefile): Check for empty include_directories.
(construct_include_path): Clear any old value of .INCLUDE_DIRS before
appending new values.  Free the previous include_directories.
* src/variable.c (lookup_special_var): When MAKEFLAGS is set, first
reset the switches then re-parse the variable.
* tests/run_make_tests.pl: Memo-ize some default variable values.
* tests/scripts/options/dash-r: Create tests for setting -r and -R.
* tests/scripts/variables/MAKEFLAGS: Test that resetting -I from
within the makefile takes effect immediately.
This commit is contained in:
Paul Smith 2021-09-05 17:11:44 -04:00
parent e13fd5c83d
commit f4b8ddf260
10 changed files with 229 additions and 138 deletions

5
NEWS
View File

@ -50,6 +50,11 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=109&se
user-defined function and they will not impact global variable assignments.
Implementation provided by Jouke Witteveen <j.witteveen@gmail.com>
* If the MAKEFLAGS variable is modified in a makefile, it will be re-parsed
immediately rather than after all makefiles have been read. Note that
although all options are parsed immediately, some special effects won't
appear until after all makefiles are read.
* The -I option accepts an argument "-" (e.g., "-I-") which means "reset the
list of search directories to empty". Among other things this can be used
to prevent GNU make from searching in its default list of directories.

View File

@ -668,7 +668,7 @@ static const char *default_variables[] =
#endif /* !VMS */
/* Make this assignment to avoid undefined variable warnings. */
"GNUMAKEFLAGS", "",
GNUMAKEFLAGS_NAME, "",
0, 0
};

View File

@ -104,44 +104,12 @@ double atof ();
static void clean_jobserver (int status);
static void print_data_base (void);
static void print_version (void);
static void reset_switches ();
static void decode_switches (int argc, const char **argv, int env);
static void decode_env_switches (const char *envar, size_t len);
static struct variable *define_makeflags (int all, int makefile);
static char *quote_for_env (char *out, const char *in);
static void initialize_global_hash_tables (void);
/* The structure that describes an accepted command switch. */
struct command_switch
{
int c; /* The switch character. */
enum /* Type of the value. */
{
flag, /* Turn int flag on. */
flag_off, /* Turn int flag off. */
string, /* One string per invocation. */
strlist, /* One string per switch. */
filename, /* A string containing a file name. */
positive_int, /* A positive integer. */
floating, /* A floating-point number (double). */
ignore /* Ignored. */
} type;
void *value_ptr; /* Pointer to the value-holding variable. */
unsigned int env:1; /* Can come from MAKEFLAGS. */
unsigned int toenv:1; /* Should be put in MAKEFLAGS. */
unsigned int no_makefile:1; /* Don't propagate when remaking makefiles. */
const void *noarg_value; /* Pointer to value used if no arg given. */
const void *default_value; /* Pointer to default value. */
const char *long_name; /* Long option name. */
};
/* True if C is a switch value that corresponds to a short option. */
#define short_option(c) ((c) <= CHAR_MAX)
@ -283,7 +251,7 @@ static struct stringlist *directories = 0;
/* List of include directories given with -I switches. */
static struct stringlist *include_directories = 0;
static struct stringlist *include_dirs = 0;
/* List of files given with -o switches. */
@ -418,6 +386,36 @@ static const char *const usage[] =
static int trace_flag = 0;
/* The structure that describes an accepted command switch. */
struct command_switch
{
int c; /* The switch character. */
enum /* Type of the value. */
{
flag, /* Turn int flag on. */
flag_off, /* Turn int flag off. */
string, /* One string per invocation. */
strlist, /* One string per switch. */
filename, /* A string containing a file name. */
positive_int, /* A positive integer. */
floating, /* A floating-point number (double). */
ignore /* Ignored. */
} type;
void *value_ptr; /* Pointer to the value-holding variable. */
unsigned int env:1; /* Can come from MAKEFLAGS. */
unsigned int toenv:1; /* Should be put in MAKEFLAGS. */
unsigned int no_makefile:1; /* Don't propagate when remaking makefiles. */
const void *noarg_value; /* Pointer to value used if no arg given. */
const void *default_value; /* Pointer to default value. */
const char *long_name; /* Long option name. */
};
/* The table of command switches.
Order matters here: this is the order MAKEFLAGS will be constructed.
So be sure all simple flags (single char, no argument) come first. */
@ -452,7 +450,7 @@ static const struct command_switch switches[] =
/* These options take arguments. */
{ 'C', filename, &directories, 0, 0, 0, 0, 0, "directory" },
{ 'f', filename, &makefiles, 0, 0, 0, 0, 0, "file" },
{ 'I', filename, &include_directories, 1, 1, 0, 0, 0,
{ 'I', filename, &include_dirs, 1, 1, 0, 0, 0,
"include-dir" },
{ 'j', positive_int, &arg_job_slots, 1, 1, 0, &inf_jobs, &default_job_slots,
"jobs" },
@ -1429,50 +1427,50 @@ main (int argc, char **argv, char **envp)
}
}
#ifdef WINDOWS32
/* If we didn't find a correctly spelled PATH we define PATH as
* either the first misspelled value or an empty string
*/
if (!unix_path)
define_variable_cname ("PATH", windows32_path ? windows32_path : "",
o_env, 1)->export = v_export;
/* If we didn't find a correctly spelled PATH we define PATH as
* either the first misspelled value or an empty string
*/
if (!unix_path)
define_variable_cname ("PATH", windows32_path ? windows32_path : "",
o_env, 1)->export = v_export;
#endif
#else /* For Amiga, read the ENV: device, ignoring all dirs */
{
BPTR env, file, old;
char buffer[1024];
int len;
__aligned struct FileInfoBlock fib;
{
BPTR env, file, old;
char buffer[1024];
int len;
__aligned struct FileInfoBlock fib;
env = Lock ("ENV:", ACCESS_READ);
if (env)
env = Lock ("ENV:", ACCESS_READ);
if (env)
{
old = CurrentDir (DupLock (env));
Examine (env, &fib);
while (ExNext (env, &fib))
{
old = CurrentDir (DupLock (env));
Examine (env, &fib);
while (ExNext (env, &fib))
if (fib.fib_DirEntryType < 0) /* File */
{
if (fib.fib_DirEntryType < 0) /* File */
{
/* Define an empty variable. It will be filled in
variable_lookup(). Makes startup quite a bit faster. */
define_variable (fib.fib_FileName,
strlen (fib.fib_FileName),
"", o_env, 1)->export = v_export;
}
/* Define an empty variable. It will be filled in
variable_lookup(). Makes startup quite a bit faster. */
define_variable (fib.fib_FileName,
strlen (fib.fib_FileName),
"", o_env, 1)->export = v_export;
}
UnLock (env);
UnLock (CurrentDir (old));
}
}
UnLock (env);
UnLock (CurrentDir (old));
}
}
#endif
/* Decode the switches. */
decode_env_switches (STRING_SIZE_TUPLE ("GNUMAKEFLAGS"));
decode_env_switches (STRING_SIZE_TUPLE (GNUMAKEFLAGS_NAME));
/* Clear GNUMAKEFLAGS to avoid duplication. */
define_variable_cname ("GNUMAKEFLAGS", "", o_env, 0);
define_variable_cname (GNUMAKEFLAGS_NAME, "", o_env, 0);
decode_env_switches (STRING_SIZE_TUPLE ("MAKEFLAGS"));
decode_env_switches (STRING_SIZE_TUPLE (MAKEFLAGS_NAME));
#if 0
/* People write things like:
@ -1649,14 +1647,9 @@ main (int argc, char **argv, char **envp)
* lookups to fail because the current directory (.) was pointing
* at the wrong place when it was first evaluated.
*/
no_default_sh_exe = !find_and_set_default_shell (NULL);
no_default_sh_exe = !find_and_set_default_shell (NULL);
#endif /* WINDOWS32 */
/* Construct the list of include directories to search. */
construct_include_path (include_directories == 0
? 0 : include_directories->list);
/* If we chdir'ed, figure out where we are now. */
if (directories)
{
@ -1955,55 +1948,26 @@ main (int argc, char **argv, char **envp)
define_variable_cname ("-*-eval-flags-*-", value, o_automatic, 0);
}
/* Read all the makefiles. */
read_files = read_all_makefiles (makefiles == 0 ? 0 : makefiles->list);
#ifdef WINDOWS32
/* look one last time after reading all Makefiles */
if (no_default_sh_exe)
no_default_sh_exe = !find_and_set_default_shell (NULL);
#endif /* WINDOWS32 */
#if defined (__MSDOS__) || defined (__EMX__) || defined (VMS)
/* We need to know what kind of shell we will be using. */
{
extern int _is_unixy_shell (const char *_path);
struct variable *shv = lookup_variable (STRING_SIZE_TUPLE ("SHELL"));
extern int unixy_shell;
extern const char *default_shell;
if (shv && *shv->value)
{
char *shell_path = recursively_expand (shv);
if (shell_path && _is_unixy_shell (shell_path))
unixy_shell = 1;
else
unixy_shell = 0;
if (shell_path)
default_shell = shell_path;
}
}
#endif /* __MSDOS__ || __EMX__ */
{
int old_builtin_rules_flag = no_builtin_rules_flag;
int old_builtin_variables_flag = no_builtin_variables_flag;
int old_arg_job_slots = arg_job_slots;
/* Read all the makefiles. */
read_files = read_all_makefiles (makefiles == 0 ? 0 : makefiles->list);
/* Reset switches that are taken from MAKEFLAGS so we don't get dups. */
reset_switches ();
arg_job_slots = INVALID_JOB_SLOTS;
/* Decode switches again, for variables set by the makefile. */
decode_env_switches (STRING_SIZE_TUPLE ("GNUMAKEFLAGS"));
decode_env_switches (STRING_SIZE_TUPLE (GNUMAKEFLAGS_NAME));
/* Clear GNUMAKEFLAGS to avoid duplication. */
define_variable_cname ("GNUMAKEFLAGS", "", o_override, 0);
define_variable_cname (GNUMAKEFLAGS_NAME, "", o_override, 0);
decode_env_switches (STRING_SIZE_TUPLE ("MAKEFLAGS"));
decode_env_switches (STRING_SIZE_TUPLE (MAKEFLAGS_NAME));
#if 0
decode_env_switches (STRING_SIZE_TUPLE ("MFLAGS"));
#endif
@ -2052,6 +2016,34 @@ main (int argc, char **argv, char **envp)
undefine_default_variables ();
}
#ifdef WINDOWS32
/* look one last time after reading all Makefiles */
if (no_default_sh_exe)
no_default_sh_exe = !find_and_set_default_shell (NULL);
#endif /* WINDOWS32 */
#if defined (__MSDOS__) || defined (__EMX__) || defined (VMS)
/* We need to know what kind of shell we will be using. */
{
extern int _is_unixy_shell (const char *_path);
struct variable *shv = lookup_variable (STRING_SIZE_TUPLE ("SHELL"));
extern int unixy_shell;
extern const char *default_shell;
if (shv && *shv->value)
{
char *shell_path = recursively_expand (shv);
if (shell_path && _is_unixy_shell (shell_path))
unixy_shell = 1;
else
unixy_shell = 0;
if (shell_path)
default_shell = shell_path;
}
}
#endif /* __MSDOS__ || __EMX__ */
/* Final jobserver configuration.
If we have jobserver_auth then we are a client in an existing jobserver
@ -2862,9 +2854,9 @@ print_usage (int bad)
}
/* Reset switches that come from MAKEFLAGS and go to MAKEFLAGS.
Before re-parsing MAKEFLAGS after reading makefiles, start from scratch. */
Before re-parsing MAKEFLAGS, start from scratch. */
static void
void
reset_switches ()
{
const struct command_switch *cs;
@ -2898,7 +2890,10 @@ reset_switches ()
/* The strings are in the cache so don't free them. */
struct stringlist *sl = *(struct stringlist **) cs->value_ptr;
if (sl)
sl->idx = 0;
{
sl->idx = 0;
sl->list[0] = 0;
}
}
break;
@ -2934,7 +2929,7 @@ decode_switches (int argc, const char **argv, int env)
const char *coptarg;
/* Parse the next argument. */
c = getopt_long (argc, (char*const*)argv, options, long_options, NULL);
c = getopt_long (argc, (char *const *)argv, options, long_options, NULL);
coptarg = optarg;
if (c == EOF)
/* End of arguments, or "--" marker seen. */
@ -3102,6 +3097,9 @@ decode_switches (int argc, const char **argv, int env)
/* Perform any special switch handling. */
run_silent = silent_flag;
/* Construct the list of include directories to search. */
construct_include_path (include_dirs ? include_dirs->list : NULL);
}
/* Decode switches from environment variable ENVAR (which is LEN chars long).
@ -3109,7 +3107,7 @@ decode_switches (int argc, const char **argv, int env)
dash to the first word if it lacks one, and passing the vector to
decode_switches. */
static void
void
decode_env_switches (const char *envar, size_t len)
{
char *varref = alloca (2 + len + 2);
@ -3136,7 +3134,7 @@ decode_env_switches (const char *envar, size_t len)
/* getopt will look at the arguments starting at ARGV[1].
Prepend a spacer word. */
argv[0] = 0;
argv[0] = "";
argc = 1;
/* We need a buffer to copy the value into while we split it into words
@ -3206,6 +3204,7 @@ define_makeflags (int all, int makefile)
const char posixref[] = "-*-command-variables-*-";
const char evalref[] = "$(-*-eval-flags-*-)";
const struct command_switch *cs;
struct variable *v;
char *flagstring;
char *p;
@ -3400,7 +3399,7 @@ define_makeflags (int all, int makefile)
const char *r = posix_pedantic ? posixref : ref;
size_t l = strlen (r);
struct variable *v = lookup_variable (r, l);
v = lookup_variable (r, l);
if (v && v->value && v->value[0] != '\0')
{
@ -3425,8 +3424,11 @@ define_makeflags (int all, int makefile)
lost when users added -e, causing a previous MAKEFLAGS env. var. to take
precedence over the new one. Of course, an override or command
definition will still take precedence. */
return define_variable_cname ("MAKEFLAGS", flagstring,
env_overrides ? o_env_override : o_file, 1);
v = define_variable_cname (MAKEFLAGS_NAME, flagstring,
env_overrides ? o_env_override : o_file, 1);
v->special = 1;
return v;
}
/* Print version information. */

View File

@ -527,6 +527,8 @@ void out_of_memory () NORETURN;
#define ONS(_t,_a,_f,_n,_s) _t((_a), INTSTR_LENGTH + strlen (_s), \
(_f), (_n), (_s))
void reset_switches ();
void decode_env_switches (const char*, size_t line);
void die (int) NORETURN;
void pfatal_with_name (const char *) NORETURN;
void perror_with_name (const char *, const char *);
@ -696,9 +698,12 @@ extern const char *default_shell;
/* can we run commands via 'sh -c xxx' or must we use batch files? */
extern int batch_mode_shell;
#define GNUMAKEFLAGS_NAME "GNUMAKEFLAGS"
#define MAKEFLAGS_NAME "MAKEFLAGS"
/* Resetting the command script introduction prefix character. */
#define RECIPEPREFIX_NAME ".RECIPEPREFIX"
#define RECIPEPREFIX_DEFAULT '\t'
#define RECIPEPREFIX_NAME ".RECIPEPREFIX"
#define RECIPEPREFIX_DEFAULT '\t'
extern char cmd_prefix;
extern unsigned int job_slots;

View File

@ -373,21 +373,18 @@ eval_makefile (const char *filename, unsigned short flags)
/* If the makefile wasn't found and it's either a makefile from
the 'MAKEFILES' variable or an included makefile,
search the included makefile search path for this makefile. */
if (ebuf.fp == 0 && (flags & RM_INCLUDED) && *filename != '/')
{
unsigned int i;
for (i = 0; include_directories[i] != 0; ++i)
{
const char *included = concat (3, include_directories[i],
"/", filename);
ebuf.fp = fopen (included, "r");
if (ebuf.fp)
{
filename = included;
break;
}
}
}
if (ebuf.fp == NULL && (flags & RM_INCLUDED) && *filename != '/'
&& include_directories)
for (const char **dir = include_directories; *dir != NULL; ++dir)
{
const char *included = concat (3, *dir, "/", filename);
ebuf.fp = fopen (included, "r");
if (ebuf.fp)
{
filename = included;
break;
}
}
/* Enter the final name for this makefile as a goaldep. */
filename = strcache_add (filename);
@ -3027,10 +3024,12 @@ construct_include_path (const char **arg_dirs)
/* Now add each dir to the .INCLUDE_DIRS variable. */
do_variable_definition (NILF, ".INCLUDE_DIRS", "", o_default, f_simple, 0);
for (cpp = dirs; *cpp != 0; ++cpp)
do_variable_definition (NILF, ".INCLUDE_DIRS", *cpp,
o_default, f_append, 0);
free (include_directories);
include_directories = dirs;
}

View File

@ -381,10 +381,10 @@ lookup_special_var (struct variable *var)
/* This one actually turns out to be very hard, due to the way the parser
records targets. The way it works is that target information is collected
internally until make knows the target is completely specified. It unitl
it sees that some new construct (a new target or variable) is defined that
it knows the previous one is done. In short, this means that if you do
this:
internally until make knows the target is completely specified. Only when
it sees that some new construct (a new target or variable) is defined does
make know that the previous one is done. In short, this means that if
you do this:
all:
@ -1144,6 +1144,11 @@ set_special_var (struct variable *var)
properly. */
cmd_prefix = var->value[0]=='\0' ? RECIPEPREFIX_DEFAULT : var->value[0];
}
else if (streq (var->name, MAKEFLAGS_NAME))
{
reset_switches ();
decode_env_switches (STRING_SIZE_TUPLE(MAKEFLAGS_NAME));
}
return var;
}

View File

@ -45,6 +45,10 @@ $cwdpath = cwd();
$has_POSIX = eval { require "POSIX.pm" };
%FEATURES = ();
%DEFVARS = (
AR => undef,
CC => undef
);
$valgrind = 0; # invoke make with valgrind
$valgrind_args = '';
@ -649,6 +653,18 @@ sub set_more_defaults
%FEATURES = map { $_ => 1 } split /\s+/, `$make_path -sf features.mk`;
unlink('features.mk');
# Find the default values for different built-in variables
my $s = "all:;\n";
foreach (keys %DEFVARS) {
$s .= "\$(info $_=\$($_))\n";
}
create_file('defvars.mk', $s);
foreach (split "\n", `$make_path -sf defvars.mk`) {
my @e = split /=/, $_, 2;
$DEFVARS{$e[0]} = $e[1];
}
unlink('defvars.mk');
# Set up for valgrind, if requested.
@make_command = ($make_path);

View File

@ -0,0 +1,36 @@
# -*-perl-*-
$description = "Test removing default rules and variables";
$details = "DETAILS";
touch('xxx.c');
# Simple check
run_make_test('', '-r COMPILE.c=echo xxx.o',
"#MAKE#: *** No rule to make target 'xxx.o'. Stop.", 512);
# Make sure we can set it from within the makefile too
run_make_test(q!
COMPILE.c = echo
MAKEFLAGS += -r
!,
'xxx.o',
"#MAKE#: *** No rule to make target 'xxx.o'. Stop.", 512);
unlink('xxx.c');
# Simple check for -R
run_make_test(q!
all:;$(info CC='$(CC)')
!,
'-sR', "CC=''");
# Make sure we can set -R from within the makefile too
run_make_test(q!
MAKEFLAGS += -R
all:;$(info CC='$(CC)')
!,
'-s', "CC=''");
1;

View File

@ -31,7 +31,7 @@ $(warning dir is empty)
endif
ifeq ($(filter $(dir),$(.INCLUDE_DIRS)),)
$(warning .INCLUDE_DIRS does not contain $(dir))
$(warning .INCLUDE_DIRS does not contain $(dir): $(.INCLUDE_DIRS))
endif
.PHONY: all

View File

@ -39,4 +39,27 @@ print:
jump Works: MAKEFLAGS=e --no-print-directory
print Works: MAKEFLAGS=e --no-print-directory');
# Ensure MAKEFLAGS updates are handled immediately rather than later
mkdir('foo', 0777);
mkdir('bar', 0777);
run_make_test(q!
$(info MAKEFLAGS=$(MAKEFLAGS))
$(info INCLUDE_DIRS=$(.INCLUDE_DIRS))
MAKEFLAGS += -Ibar
$(info MAKEFLAGS=$(MAKEFLAGS))
$(info INCLUDE_DIRS=$(.INCLUDE_DIRS))
.PHONY: all
all: ; @echo 'MAKEFLAGS=$(MAKEFLAGS)' "\$$MAKEFLAGS=$$MAKEFLAGS"
!,
'-I- -Ifoo', 'MAKEFLAGS= -I- -Ifoo
INCLUDE_DIRS=foo
MAKEFLAGS= -I- -Ifoo -Ibar
INCLUDE_DIRS=foo bar
MAKEFLAGS= -I- -Ifoo -Ibar $MAKEFLAGS= -I- -Ifoo -Ibar');
rmdir('foo');
rmdir('bar');
1;