diff --git a/NEWS b/NEWS index c2475e3c..4a38e4fe 100644 --- a/NEWS +++ b/NEWS @@ -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. diff --git a/doc/make.texi b/doc/make.texi index b4ce44ae..2dc907a3 100644 --- a/doc/make.texi +++ b/doc/make.texi @@ -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 diff --git a/main.c b/main.c index bf8fb0dc..ee08720f 100644 --- a/main.c +++ b/main.c @@ -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 (); } } diff --git a/os.h b/os.h index f8a5a233..ac5350b1 100644 --- a/os.h +++ b/os.h @@ -23,10 +23,10 @@ this program. If not, see . */ 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) diff --git a/posixos.c b/posixos.c index 94083059..8c270010 100644 --- a/posixos.c +++ b/posixos.c @@ -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; diff --git a/tests/scripts/features/jobserver b/tests/scripts/features/jobserver index b5549cc9..7da4a654 100644 --- a/tests/scripts/features/jobserver +++ b/tests/scripts/features/jobserver @@ -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=,$(MAKEFLAGS)) +recurse: ; @echo $@: "/$(SHOW)/"; $(MAKE) -f #MAKEFILE# all +all:;@echo $@: "/$(SHOW)/" +!, + "-j2 $np", "recurse: /-j2 --jobserver-auth= $np/\nall: /-j2 --jobserver-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=,$(MAKEFLAGS)) +recurse: ; @echo $@: "/$(SHOW)/"; $(MAKE) -f #MAKEFILE# all +all:;@echo $@: "/$(SHOW)/" +!, + '', "recurse: /-j2 --jobserver-auth= $np/\nall: /-j2 --jobserver-auth= $np/\n"); +delete $extraENV{MAKEFLAGS}; + +# Test override of -jN +$extraENV{MAKEFLAGS} = "-j9 $np"; +run_make_test(q! +SHOW = $(patsubst --jobserver-auth=%,--jobserver-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= $np/\n#MAKE#[1]: warning: -jN forced in submake: disabling jobserver mode.\nrecurse2: /-j3 --jobserver-auth= $np/\nall: /-j3 --jobserver-auth= $np/\n"); +delete $extraENV{MAKEFLAGS}; + +# Test override of -jN with -j +run_make_test(q! +SHOW = $(patsubst --jobserver-auth=%,--jobserver-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= $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'."); diff --git a/tests/scripts/features/output-sync b/tests/scripts/features/output-sync index ee3e5ad8..64d6fe08 100644 --- a/tests/scripts/features/output-sync +++ b/tests/scripts/features/output-sync @@ -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. diff --git a/tests/scripts/variables/MAKEFLAGS b/tests/scripts/variables/MAKEFLAGS index 8a5d0f6a..0fac74a3 100644 --- a/tests/scripts/variables/MAKEFLAGS +++ b/tests/scripts/variables/MAKEFLAGS @@ -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: diff --git a/w32/w32os.c b/w32/w32os.c index 170c6d4c..533b9100 100644 --- a/w32/w32os.c +++ b/w32/w32os.c @@ -33,7 +33,7 @@ this program. If not, see . */ 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];