Preserve the real value of -jN in MAKEFLAGS using jobserver.

Previously if the jobserver was active, MAKEFLAGS would contain only
the -j option but not the number (not -j5 or whatever) so users
could not discover that value.  Allow that value to be provided in
MAKEFLAGS without error but still give warnings if -jN is provided
on the command line if the jobserver is already activated.

* NEWS: Discuss the new behavior.
* os.h, posixos.c, w32/w32os.c: Return success/failure from
jobserver_setup() and jobserver_parse_auth().
* main.c (main): Separate the command line storage of job slots (now
in arg_job_slots) from the control storage (in job_slots).  Make a
distinction between -jN flags read from MAKEFLAGS and those seen
on the command line: for the latter if the jobserver is enabled then
warn and disable it, as before.
* tests/scripts/features/jobserver: Add new testing.
This commit is contained in:
Paul Smith 2016-04-04 01:23:04 -04:00
parent 65115e7095
commit 5bd7ad2b22
9 changed files with 179 additions and 108 deletions

4
NEWS
View file

@ -30,6 +30,10 @@ http://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=106&set
WARNING: Backward-incompatibility! The internal-only command line option
--jobserver-fds has been renamed for publishing, to --jobserver-auth.
* The amount of parallelism can be determined by querying MAKEFLAGS, even when
the job server is enabled (previously MAKEFLAGS would always contain only
"-j" when job server was enabled).
* VMS-specific changes:
* Perl test harness now works.

View file

@ -4721,9 +4721,9 @@ doesn't count against the total jobs (otherwise we could get @samp{N}
sub-@code{make}s running and have no slots left over for any real work!)
If your operating system doesn't support the above communication, then
@samp{-j 1} is always put into @code{MAKEFLAGS} instead of the value you
specified. This is because if the @w{@samp{-j}} option were passed down
to sub-@code{make}s, you would get many more jobs running in parallel
no @samp{-j} is added to @code{MAKEFLAGS}, so that sub-@code{make}s
run in non-parallel mode. If the @w{@samp{-j}} option were passed down
to sub-@code{make}s you would get many more jobs running in parallel
than you asked for. If you give @samp{-j} with no numeric argument,
meaning to run as many jobs as possible in parallel, this is passed
down, since multiple infinities are no more than one.@refill

167
main.c
View file

@ -259,19 +259,23 @@ struct rlimit stack_limit;
#endif
/* Number of job slots (commands that can be run at once). */
/* Number of job slots for parallelism. */
unsigned int job_slots = 1;
unsigned int default_job_slots = 1;
unsigned int job_slots;
#define INVALID_JOB_SLOTS (-1)
static unsigned int master_job_slots = 0;
static int arg_job_slots = INVALID_JOB_SLOTS;
static const int default_job_slots = INVALID_JOB_SLOTS;
/* Value of job_slots that means no limit. */
static unsigned int inf_jobs = 0;
static const int inf_jobs = 0;
/* Authorization for the jobserver. */
char *jobserver_auth = NULL;
static char *jobserver_auth = NULL;
/* Handle for the mutex used on Windows to synchronize output of our
children under -O. */
@ -456,7 +460,7 @@ static const struct command_switch switches[] =
{ 'f', filename, &makefiles, 0, 0, 0, 0, 0, "file" },
{ 'I', filename, &include_directories, 1, 1, 0, 0, 0,
"include-dir" },
{ 'j', positive_int, &job_slots, 1, 1, 0, &inf_jobs, &default_job_slots,
{ 'j', positive_int, &arg_job_slots, 1, 1, 0, &inf_jobs, &default_job_slots,
"jobs" },
#ifndef NO_FLOAT
{ 'l', floating, &max_load_average, 1, 1, 0, &default_load_average,
@ -1057,8 +1061,13 @@ msdos_return_to_initial_directory (void)
}
#endif /* __MSDOS__ */
static void
reset_jobserver ()
{
jobserver_clear ();
free (jobserver_auth);
jobserver_auth = NULL;
}
#ifdef _AMIGA
int
@ -1074,6 +1083,7 @@ main (int argc, char **argv, char **envp)
PATH_VAR (current_directory);
unsigned int restarts = 0;
unsigned int syncing = 0;
int argv_slots;
#ifdef WINDOWS32
const char *unix_path = NULL;
const char *windows32_path = NULL;
@ -1483,24 +1493,34 @@ main (int argc, char **argv, char **envp)
|| output_sync == OUTPUT_SYNC_TARGET);
OUTPUT_SET (&make_sync);
decode_switches (argc, (const char **)argv, 0);
/* Remember the job slots set through the environment vs. command line. */
{
int env_slots = arg_job_slots;
arg_job_slots = INVALID_JOB_SLOTS;
/* Set a variable specifying whether stdout/stdin is hooked to a TTY. */
decode_switches (argc, (const char **)argv, 0);
argv_slots = arg_job_slots;
if (arg_job_slots == INVALID_JOB_SLOTS)
arg_job_slots = env_slots;
}
/* Set a variable specifying whether stdout/stdin is hooked to a TTY. */
#ifdef HAVE_ISATTY
if (isatty (fileno (stdout)))
if (! lookup_variable (STRING_SIZE_TUPLE ("MAKE_TERMOUT")))
{
const char *tty = TTYNAME (fileno (stdout));
define_variable_cname ("MAKE_TERMOUT", tty ? tty : DEFAULT_TTYNAME,
o_default, 0)->export = v_export;
}
if (isatty (fileno (stderr)))
if (! lookup_variable (STRING_SIZE_TUPLE ("MAKE_TERMERR")))
{
const char *tty = TTYNAME (fileno (stderr));
define_variable_cname ("MAKE_TERMERR", tty ? tty : DEFAULT_TTYNAME,
o_default, 0)->export = v_export;
}
if (isatty (fileno (stdout)))
if (! lookup_variable (STRING_SIZE_TUPLE ("MAKE_TERMOUT")))
{
const char *tty = TTYNAME (fileno (stdout));
define_variable_cname ("MAKE_TERMOUT", tty ? tty : DEFAULT_TTYNAME,
o_default, 0)->export = v_export;
}
if (isatty (fileno (stderr)))
if (! lookup_variable (STRING_SIZE_TUPLE ("MAKE_TERMERR")))
{
const char *tty = TTYNAME (fileno (stderr));
define_variable_cname ("MAKE_TERMERR", tty ? tty : DEFAULT_TTYNAME,
o_default, 0)->export = v_export;
}
#endif
/* Reset in case the switches changed our minds. */
@ -1600,37 +1620,43 @@ main (int argc, char **argv, char **envp)
/* We may move, but until we do, here we are. */
starting_directory = current_directory;
#ifdef MAKE_JOBSERVER
/* If the jobserver_auth option is seen, make sure that -j is reasonable.
This can't be usefully set in the makefile, and we want to verify the
authorization is valid before any other aspect of make has a chance to
start using it for something else. */
/* Set up the job_slots value and the jobserver. This can't be usefully set
in the makefile, and we want to verify the authorization is valid before
make has a chance to start using it for something else. */
if (jobserver_auth)
{
/* The combination of jobserver_auth and !job_slots means we're using
the jobserver. If !job_slots and no jobserver_auth, we can start
infinite jobs. If we see both jobserver_auth and job_slots >0 that
means the user set -j explicitly. This is broken; in this case obey
the user (ignore the jobserver for this make) but print a message.
If we've restarted, we already printed this the first time. */
if (argv_slots == INVALID_JOB_SLOTS)
{
if (jobserver_parse_auth (jobserver_auth))
{
/* Success! Use the jobserver. */
job_slots = 0;
goto job_setup_complete;
}
if (!job_slots)
jobserver_parse_auth (jobserver_auth);
O (error, NILF, _("warning: jobserver unavailable: using -j1. Add '+' to parent make rule."));
arg_job_slots = 1;
}
else if (! restarts)
/* The user provided a -j setting on the command line: use it. */
else if (!restarts)
/* If restarts is >0 we already printed this message. */
O (error, NILF,
_("warning: -jN forced in submake: disabling jobserver mode."));
if (job_slots > 0)
{
/* If job_slots is set now then we're not using jobserver */
jobserver_clear ();
free (jobserver_auth);
jobserver_auth = NULL;
}
/* We failed to use our parent's jobserver. */
reset_jobserver ();
job_slots = (unsigned int)arg_job_slots;
}
#endif
else if (arg_job_slots == INVALID_JOB_SLOTS)
/* The default is one job at a time. */
job_slots = 1;
else
/* Use whatever was provided. */
job_slots = (unsigned int)arg_job_slots;
job_setup_complete:
/* The extra indirection through $(MAKE_COMMAND) is done
for hysterical raisins. */
@ -2024,7 +2050,7 @@ main (int argc, char **argv, char **envp)
}
#if defined (__MSDOS__) || defined (__EMX__) || defined (VMS)
if (job_slots != 1
if (arg_job_slots != 1
# ifdef __EMX__
&& _osmode != OS2_MODE /* turn off -j if we are in DOS mode */
# endif
@ -2033,30 +2059,29 @@ main (int argc, char **argv, char **envp)
O (error, NILF,
_("Parallel jobs (-j) are not supported on this platform."));
O (error, NILF, _("Resetting to single job (-j1) mode."));
job_slots = 1;
arg_job_slots = job_slots = 1;
}
#endif
#ifdef MAKE_JOBSERVER
if (job_slots > 1)
/* If we have >1 slot at this point, then we're a top-level make.
Set up the jobserver.
Every make assumes that it always has one job it can run. For the
submakes it's the token they were given by their parent. For the top
make, we just subtract one from the number the user wants. */
if (job_slots > 1 && jobserver_setup (job_slots - 1))
{
/* If we have >1 slot at this point, then we're a top-level make.
Set up the jobserver.
Every make assumes that it always has one job it can run. For the
submakes it's the token they were given by their parent. For the
top make, we just subtract one from the number the user wants. */
jobserver_setup (job_slots - 1);
/* We're using the jobserver so set job_slots to 0. */
master_job_slots = job_slots;
job_slots = 0;
/* Fill in the jobserver_auth for our children. */
jobserver_auth = jobserver_get_auth ();
if (jobserver_auth)
{
/* We're using the jobserver so set job_slots to 0. */
master_job_slots = job_slots;
job_slots = 0;
}
}
#endif
/* If we're not using parallel jobs, then we don't need output sync.
This is so people can enable output sync in GNUMAKEFLAGS or similar, but
@ -2365,11 +2390,6 @@ main (int argc, char **argv, char **envp)
++restarts;
/* If we're re-exec'ing the first make, put back the number of
job slots so define_makefiles() will get it right. */
if (master_job_slots)
job_slots = master_job_slots;
if (ISDB (DB_BASIC))
{
const char **p;
@ -3400,14 +3420,7 @@ clean_jobserver (int status)
"INTERNAL: Exiting with %u jobserver tokens available; should be %u!",
tokens, master_job_slots);
jobserver_clear ();
/* Clean out jobserver_auth so we don't pass this information to any
sub-makes. Also reset job_slots since it will be put on the command
line, not in MAKEFLAGS. */
job_slots = default_job_slots;
free (jobserver_auth);
jobserver_auth = NULL;
reset_jobserver ();
}
}

10
os.h
View file

@ -23,10 +23,10 @@ this program. If not, see <http://www.gnu.org/licenses/>. */
unsigned int jobserver_enabled ();
/* Called in the master instance to set up the jobserver initially. */
void jobserver_setup (int job_slots);
unsigned int jobserver_setup (int job_slots);
/* Called in a child instance to connect to the jobserver. */
void jobserver_parse_auth (const char* auth);
unsigned int jobserver_parse_auth (const char* auth);
/* Returns an allocated buffer used to pass to child instances. */
char *jobserver_get_auth ();
@ -57,13 +57,13 @@ void jobserver_pre_acquire ();
in this case we won't wait forever, so we can check the load.
Returns 1 if we got a token, or 0 if we stopped waiting due to a child
exiting or a timeout. */
int jobserver_acquire (int timeout);
unsigned int jobserver_acquire (int timeout);
#else
#define jobserver_enabled() (0)
#define jobserver_setup(_slots) (void)(0)
#define jobserver_parse_auth(_auth) (void)(0)
#define jobserver_setup(_slots) (0)
#define jobserver_parse_auth(_auth) (0)
#define jobserver_get_auth() (NULL)
#define jobserver_clear() (void)(0)
#define jobserver_release(_fatal) (void)(0)

View file

@ -59,7 +59,7 @@ make_job_rfd ()
#endif
}
void
unsigned int
jobserver_setup (int slots)
{
int r;
@ -77,9 +77,11 @@ jobserver_setup (int slots)
if (r != 1)
pfatal_with_name (_("init jobserver pipe"));
}
return 1;
}
void
unsigned int
jobserver_parse_auth (const char *auth)
{
/* Given the command-line parameter, parse it. */
@ -106,12 +108,12 @@ jobserver_parse_auth (const char *auth)
if (errno != EBADF)
pfatal_with_name (_("jobserver pipeline"));
O (error, NILF,
_("warning: jobserver unavailable: using -j1. Add '+' to parent make rule."));
job_slots = 1;
job_fds[0] = job_fds[1] = -1;
return 0;
}
return 1;
}
char *
@ -175,7 +177,8 @@ jobserver_acquire_all ()
}
/* Prepare the jobserver to start a child process. */
void jobserver_pre_child (int recursive)
void
jobserver_pre_child (int recursive)
{
/* If it's not a recursive make, avoid polutting the jobserver pipes. */
if (!recursive && job_fds[0] >= 0)
@ -185,7 +188,8 @@ void jobserver_pre_child (int recursive)
}
}
void jobserver_post_child (int recursive)
void
jobserver_post_child (int recursive)
{
#if defined(F_GETFD) && defined(F_SETFD)
if (!recursive && job_fds[0] >= 0)
@ -229,7 +233,7 @@ jobserver_pre_acquire ()
and only unblocked (atomically) within the pselect() call, so we can
never miss a SIGCHLD.
*/
int
unsigned int
jobserver_acquire (int timeout)
{
sigset_t empty;
@ -273,7 +277,7 @@ jobserver_acquire (int timeout)
/* What does it mean if read() returns 0? It shouldn't happen because only
the master make can reap all the tokens and close the write side...?? */
return r;
return r > 0;
}
#else
@ -358,7 +362,7 @@ set_child_handler_action_flags (int set_handler, int set_alarm)
#endif
}
int
unsigned int
jobserver_acquire (int timeout)
{
char intake;

View file

@ -12,6 +12,48 @@ if (!$parallel_jobs) {
return -1;
}
# Shorthand
my $np = '--no-print-directory';
# Simple test of MAKEFLAGS settings
run_make_test(q!
SHOW = $(patsubst --jobserver-auth=%,--jobserver-auth=<auth>,$(MAKEFLAGS))
recurse: ; @echo $@: "/$(SHOW)/"; $(MAKE) -f #MAKEFILE# all
all:;@echo $@: "/$(SHOW)/"
!,
"-j2 $np", "recurse: /-j2 --jobserver-auth=<auth> $np/\nall: /-j2 --jobserver-auth=<auth> $np/\n");
# Setting parallelism with the environment
# Command line should take precedence over the environment
$extraENV{MAKEFLAGS} = "-j2 $np";
run_make_test(q!
SHOW = $(patsubst --jobserver-auth=%,--jobserver-auth=<auth>,$(MAKEFLAGS))
recurse: ; @echo $@: "/$(SHOW)/"; $(MAKE) -f #MAKEFILE# all
all:;@echo $@: "/$(SHOW)/"
!,
'', "recurse: /-j2 --jobserver-auth=<auth> $np/\nall: /-j2 --jobserver-auth=<auth> $np/\n");
delete $extraENV{MAKEFLAGS};
# Test override of -jN
$extraENV{MAKEFLAGS} = "-j9 $np";
run_make_test(q!
SHOW = $(patsubst --jobserver-auth=%,--jobserver-auth=<auth>,$(MAKEFLAGS))
recurse: ; @echo $@: "/$(SHOW)/"; $(MAKE) -j3 -f #MAKEFILE# recurse2
recurse2: ; @echo $@: "/$(SHOW)/"; $(MAKE) -f #MAKEFILE# all
all:;@echo $@: "/$(SHOW)/"
!,
"-j2 $np", "recurse: /-j2 --jobserver-auth=<auth> $np/\n#MAKE#[1]: warning: -jN forced in submake: disabling jobserver mode.\nrecurse2: /-j3 --jobserver-auth=<auth> $np/\nall: /-j3 --jobserver-auth=<auth> $np/\n");
delete $extraENV{MAKEFLAGS};
# Test override of -jN with -j
run_make_test(q!
SHOW = $(patsubst --jobserver-auth=%,--jobserver-auth=<auth>,$(MAKEFLAGS))
recurse: ; @echo $@: "/$(SHOW)/"; $(MAKE) -j -f #MAKEFILE# recurse2
recurse2: ; @echo $@: "/$(SHOW)/"; $(MAKE) -f #MAKEFILE# all
all:;@echo $@: "/$(SHOW)/"
!,
"-j2 $np", "recurse: /-j2 --jobserver-auth=<auth> $np/\n#MAKE#[1]: warning: -jN forced in submake: disabling jobserver mode.\nrecurse2: /-j $np/\nall: /-j $np/\n");
# Don't put --jobserver-auth into a re-exec'd MAKEFLAGS.
# We can't test this directly because there's no way a makefile can
# show the value of MAKEFLAGS we were re-exec'd with. We can intuit it
@ -34,7 +76,7 @@ inc.mk:
# @echo 'MAKEFLAGS = $(MAKEFLAGS)'
@echo 'FOO = bar' > $@
!,
'--no-print-directory -j2', "#MAKE#[1]: warning: -jN forced in submake: disabling jobserver mode.\nall\n");
"$np -j2", "#MAKE#[1]: warning: -jN forced in submake: disabling jobserver mode.\nall\n");
unlink('inc.mk');
@ -52,7 +94,7 @@ close(MAKEFILE);
run_make_test(q!
default: ; @ #MAKEPATH# -f Makefile2
!,
'-j2 --no-print-directory',
"-j2 $np",
"#MAKE#[1]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule.
#MAKE#[1]: Nothing to be done for 'foo'.");

View file

@ -140,7 +140,7 @@ bar: start
bar: end
baz: start
baz: end
#MAKE#[1]: Leaving directory '#PWD#/bar'\n", 0, 6);
#MAKE#[1]: Leaving directory '#PWD#/bar'\n", 0, 10);
# Test per-target synchronization.
# Note we have to sleep again here after starting the foo makefile before
@ -171,7 +171,7 @@ foo: end
#MAKE#[1]: Entering directory '#PWD#/bar'
baz: start
baz: end
#MAKE#[1]: Leaving directory '#PWD#/bar'\n", 0, 6);
#MAKE#[1]: Leaving directory '#PWD#/bar'\n", 0, 10);
# Rerun but this time suppress the directory tracking
unlink(@syncfiles);
@ -183,7 +183,7 @@ bar: end
foo: start
foo: end
baz: start
baz: end\n", 0, 6);
baz: end\n", 0, 10);
# Test that messages from make itself are enclosed with
# "Entering/Leaving directory" messages.
@ -236,7 +236,7 @@ bar: end
#MAKE#[1]: Leaving directory '#PWD#/bar'
#MAKE#[1]: Entering directory '#PWD#/foo'
foo: end
#MAKE#[1]: Leaving directory '#PWD#/foo'\n", 0, 6);
#MAKE#[1]: Leaving directory '#PWD#/foo'\n", 0, 10);
# Remove temporary directories and contents.

View file

@ -39,3 +39,7 @@ jump Works: MAKEFLAGS=e --no-print-directory
print Works: MAKEFLAGS=e --no-print-directory');
1;
### Local Variables:
### eval: (setq whitespace-action (delq 'auto-cleanup whitespace-action))
### End:

View file

@ -33,7 +33,7 @@ this program. If not, see <http://www.gnu.org/licenses/>. */
static char jobserver_semaphore_name[MAX_PATH + 1];
static HANDLE jobserver_semaphore = NULL;
void
unsigned int
jobserver_setup (int slots)
{
/* sub_proc.c cannot wait for more than MAXIMUM_WAIT_OBJECTS objects
@ -61,15 +61,17 @@ jobserver_setup (int slots)
ONS (fatal, NILF,
_("creating jobserver semaphore: (Error %ld: %s)"), err, estr);
}
return 1;
}
void
unsigned int
jobserver_parse_auth (const char *auth)
{
jobserver_semaphore = OpenSemaphore (
SEMAPHORE_ALL_ACCESS, /* Semaphore access setting */
FALSE, /* Child processes DON'T inherit */
auth); /* Semaphore name */
SEMAPHORE_ALL_ACCESS, /* Semaphore access setting */
FALSE, /* Child processes DON'T inherit */
auth); /* Semaphore name */
if (jobserver_semaphore == NULL)
{
@ -80,6 +82,8 @@ jobserver_parse_auth (const char *auth)
auth, err, estr);
}
DB (DB_JOBS, (_("Jobserver client (semaphore %s)\n"), auth));
return 1;
}
char *
@ -161,7 +165,7 @@ jobserver_pre_acquire ()
/* Returns 1 if we got a token, or 0 if a child has completed.
The Windows implementation doesn't support load detection. */
int
unsigned int
jobserver_acquire (int timeout)
{
HANDLE handles[MAXIMUM_WAIT_OBJECTS];