--- src/commands.c | 206 ++++++++++++++++++++++++++++++++++--------------- src/commands.h | 18 ++++- src/default.c | 3 + src/file.c | 11 +++ src/filedef.h | 2 + src/job.c | 111 +++++++++++++++++++++----- src/main.c | 9 ++- src/makeint.h | 2 +- src/read.c | 8 -- src/remake.c | 34 ++++++-- 10 files changed, 301 insertions(+), 103 deletions(-)
diff --git a/src/commands.c b/src/commands.c index 343c1cb5..ea5a8164 100644 --- a/src/commands.c +++ b/src/commands.c @@ -317,22 +317,46 @@ set_file_variables (struct file *file, const char *stem) } /* Chop CMDS up into individual command lines if necessary. - Also set the 'lines_flags' and 'any_recurse' members. */ - + Also set the 'lines_flags' and 'any_recurse' members. ONESHELL_TARGET + indicates that the commands are being prepared for a target that is a + prerequisite of .ONESHELL: */ + +// FIXME: above phrase is garbage. The extra fields in struct commands +// (e.g. oneshell_command_lines) are weird and confusing. They should +// probably at least be packed into their own structure called maybe +// commands_per_target_oneshell or something. The roots of all this mess +// are the strategy of mutating struct commands, and the lack of any OO +// to support mutation. The mutability could in theory be changed by +// demand-computing command_lines or something, but that cold be pretty +// inefficient. I guess it could be memoized or something but that sounds +// sort of weird. Actually I think we want struct commands_traditional +// and struct commands_oneshell, and struct commands has pointers to both, +// and either or both can be non-NULL. But that would involve significant +// changes even when no per-target ONESHELL was happening. Also notable +// that struct commands_traditional and struct commands_oneshell would have +// all the same fields, really those should be different pointers to the +// same structure type. Hmmm +// +// FIXME: doc the oneshell_target arg, and consider it's name further +// UPDATE: should it be support oneshell_targets or just support_oneshell? +// I think I like support_oneshell_targets, but that's still misleading +// becaues what we really mean given how things work at the moment is +// support_per_target_oneshell_targets void -chop_commands (struct commands *cmds) +chop_commands (struct commands *cmds, int support_oneshell_target) { unsigned short nlines; unsigned short i; char **lines; - /* If we don't have any commands, or we already parsed them, never mind. */ - if (!cmds || cmds->command_lines != NULL) + /* If we don't have any commands, or we already parsed them for non-oneshell + * and possible one-shell use as requested, never mind. */ + if (!cmds || ((cmds->command_lines != NULL) && ((!support_oneshell_target) || (cmds->oneshell_command_lines != NULL)))) return; /* Chop CMDS->commands up into lines in CMDS->command_lines. */ - if (one_shell) + if (all_one_shell) { size_t l = strlen (cmds->commands); @@ -346,49 +370,103 @@ chop_commands (struct commands *cmds) } else { - const char *p = cmds->commands; - size_t max = 5; - - nlines = 0; - lines = xmalloc (max * sizeof (char *)); - while (*p != '\0') + if ( support_oneshell_target ) { - const char *end = p; - find_end:; - end = strchr (end, '\n'); - if (end == NULL) - end = p + strlen (p); - else if (end > p && end[-1] == '\\') + // FIXME: this paragraph and next are common functionality with the + // code in all_one_shell and if everything works should perhaps be + // modularized somehow. Or this entire function rewritten + size_t l = strlen (cmds->commands); + unsigned short oneshell_nlines = 1; + // FIXME: needs corresponding free if make frees things + char **oneshell_lines = xmalloc (oneshell_nlines * sizeof (char *)); + oneshell_lines[0] = xstrdup (cmds->commands); + + /* Strip the trailing newline. */ + if (l > 0 && oneshell_lines[0][l-1] == '\n') + oneshell_lines[0][l-1] = '\0'; + + cmds->oneshell_ncommand_lines = oneshell_nlines; + cmds->oneshell_command_lines = oneshell_lines; + + cmds->oneshell_any_recurse = 0; + // FIXME: needs corresponding free if make frees things + cmds->oneshell_lines_flags = xmalloc (oneshell_nlines); + + for (i = 0; i < oneshell_nlines; ++i) { - int backslash = 1; - if (end > p + 1) - { - const char *b; - for (b = end - 2; b >= p && *b == '\\'; --b) - backslash = !backslash; - } - if (backslash) + unsigned char flags = 0; + const char *p = oneshell_lines[i]; + + while (ISBLANK (*p) || *p == '-' || *p == '@' || *p == '+') + switch (*(p++)) { - ++end; - goto find_end; + case '+': + flags |= COMMANDS_RECURSE; + break; + case '@': + flags |= COMMANDS_SILENT; + break; + case '-': + flags |= COMMANDS_NOERROR; + break; } - } - if (nlines == USHRT_MAX) - ON (fatal, &cmds->fileinfo, - _("recipe has too many lines (limit %hu)"), nlines); + /* If no explicit '+' was given, look for MAKE variable + * references. */ + if (! ANY_SET (flags, COMMANDS_RECURSE) + && (strstr (p, "$(MAKE)") != 0 || strstr (p, "${MAKE}") != 0)) + flags |= COMMANDS_RECURSE; - if (nlines == max) - { - max += 2; - lines = xrealloc (lines, max * sizeof (char *)); + cmds->oneshell_lines_flags[i] = flags; + cmds->oneshell_any_recurse |= ANY_SET (flags, COMMANDS_RECURSE) ? 1 : 0; } + } - lines[nlines++] = xstrndup (p, (size_t) (end - p)); - p = end; - if (*p != '\0') - ++p; - } + { + const char *p = cmds->commands; + size_t max = 5; + + nlines = 0; + lines = xmalloc (max * sizeof (char *)); + while (*p != '\0') + { + const char *end = p; + find_end:; + end = strchr (end, '\n'); + if (end == NULL) + end = p + strlen (p); + else if (end > p && end[-1] == '\\') + { + int backslash = 1; + if (end > p + 1) + { + const char *b; + for (b = end - 2; b >= p && *b == '\\'; --b) + backslash = !backslash; + } + if (backslash) + { + ++end; + goto find_end; + } + } + + if (nlines == USHRT_MAX) + ON (fatal, &cmds->fileinfo, + _("recipe has too many lines (limit %hu)"), nlines); + + if (nlines == max) + { + max += 2; + lines = xrealloc (lines, max * sizeof (char *)); + } + + lines[nlines++] = xstrndup (p, (size_t) (end - p)); + p = end; + if (*p != '\0') + ++p; + } + } } /* Finally, set the corresponding CMDS->lines_flags elements and the @@ -401,32 +479,32 @@ chop_commands (struct commands *cmds) cmds->lines_flags = xmalloc (nlines); for (i = 0; i < nlines; ++i) - { - unsigned char flags = 0; - const char *p = lines[i]; + { + unsigned char flags = 0; + const char *p = lines[i]; - while (ISBLANK (*p) || *p == '-' || *p == '@' || *p == '+') - switch (*(p++)) - { - case '+': - flags |= COMMANDS_RECURSE; - break; - case '@': - flags |= COMMANDS_SILENT; - break; - case '-': - flags |= COMMANDS_NOERROR; - break; - } + while (ISBLANK (*p) || *p == '-' || *p == '@' || *p == '+') + switch (*(p++)) + { + case '+': + flags |= COMMANDS_RECURSE; + break; + case '@': + flags |= COMMANDS_SILENT; + break; + case '-': + flags |= COMMANDS_NOERROR; + break; + } - /* If no explicit '+' was given, look for MAKE variable references. */ - if (! ANY_SET (flags, COMMANDS_RECURSE) - && (strstr (p, "$(MAKE)") != 0 || strstr (p, "${MAKE}") != 0)) - flags |= COMMANDS_RECURSE; + /* If no explicit '+' was given, look for MAKE variable references. */ + if (! ANY_SET (flags, COMMANDS_RECURSE) + && (strstr (p, "$(MAKE)") != 0 || strstr (p, "${MAKE}") != 0)) + flags |= COMMANDS_RECURSE; - cmds->lines_flags[i] = flags; - cmds->any_recurse |= ANY_SET (flags, COMMANDS_RECURSE) ? 1 : 0; - } + cmds->lines_flags[i] = flags; + cmds->any_recurse |= ANY_SET (flags, COMMANDS_RECURSE) ? 1 : 0; + } } /* Execute the commands to remake FILE. If they are currently executing, diff --git a/src/commands.h b/src/commands.h index 9451b68b..b26c38ec 100644 --- a/src/commands.h +++ b/src/commands.h @@ -27,6 +27,22 @@ struct commands char recipe_prefix; /* Recipe prefix for this command set. */ unsigned int any_recurse:1; /* Nonzero if any 'lines_flags' elt has */ /* the COMMANDS_RECURSE bit set. */ + /* These are like the correspondingly namned variables above, but are + * needed only when commands that are recipes for targets that are + * prerequisites of the .ONESHELL: special target are to be run. They + * aren't used when all_one_shell is used, because in that case we just + * keep the one shell version in the above variables. They're needed + * when per-target .ONESHELL is used because the commands structures for + * implicit rules, .DEFAULT: rules, and suffix rules are shared and + * both oneshell and non-oneshell targets might be using this structure. + */ + char **oneshell_command_lines;/* ONLY for .ONESHELL: with prereqs */ + unsigned char *oneshell_lines_flags;/* ONLY for .ONESHELL: with prereqs */ + // FIXME: this next one is especially silly in the oneline-specific world. + // probaly all we ultimately need in oneshell world is + // oneshell_command_no_newline or so + unsigned short oneshell_ncommand_lines;/* ONLY for .ONESHELL: with prereqs */ + unsigned int oneshell_any_recurse:1; /* ONLY for .ONESHELL: with prereqs */ }; /* Bits in 'lines_flags'. */ @@ -41,5 +57,5 @@ void fatal_error_signal (int sig); void execute_file_commands (struct file *file); void print_commands (const struct commands *cmds); void delete_child_targets (struct child *child); -void chop_commands (struct commands *cmds); +void chop_commands (struct commands *cmds, int support_oneshell_target); void set_file_variables (struct file *file, const char *stem); diff --git a/src/default.c b/src/default.c index c731817e..85a8e137 100644 --- a/src/default.c +++ b/src/default.c @@ -715,6 +715,9 @@ install_default_suffix_rules () for (s = default_suffix_rules; *s != 0; s += 2) { struct file *f = enter_file (strcache_add (s[0])); + // FIXME: The comment above this function says we're doing this before + // reading makefiles, so how can there be a user-defined rule here? I + // think it must mean before *re*-reading makefiles. /* Install the default rule only if there is no user defined rule. */ if (!f->cmds) { diff --git a/src/file.c b/src/file.c index 6f816c8a..2b9a46ef 100644 --- a/src/file.c +++ b/src/file.c @@ -830,6 +830,17 @@ snap_deps (void) else no_intermediates = 1; +#if !MK_OS_DOS && !MK_OS_OS2 + for (f = lookup_file (".ONESHELL"); f != 0; f = f->prev) + if (f->deps) + for (d = f->deps; d != 0; d = d->next) + for (f2 = d->file; f2 != 0; f2 = f2->prev) + f2->oneshell = 1; + /* .ONESHELL with no deps marks all files as one_shell. (*/ + else + all_one_shell = 1; +#endif + /* The same file cannot be both .INTERMEDIATE and .NOTINTERMEDIATE. However, it is possible for a file to be .INTERMEDIATE and also match a .NOTINTERMEDIATE pattern. In that case, the intermediate file has diff --git a/src/filedef.h b/src/filedef.h index b2ef1a16..8dd3fe08 100644 --- a/src/filedef.h +++ b/src/filedef.h @@ -102,6 +102,8 @@ struct file not delete it. */ unsigned int notintermediate:1; /* Nonzero means a file is a prereq to .NOTINTERMEDIATE. */ + unsigned int oneshell:1; /* Nonzero means entire recipe should be run in + a single shell. */ unsigned int dontcare:1; /* Nonzero if no complaint is to be made if this target cannot be remade. */ unsigned int ignore_vpath:1;/* Nonzero if we threw out VPATH name. */ diff --git a/src/job.c b/src/job.c index e54a9340..f0a7f6cb 100644 --- a/src/job.c +++ b/src/job.c @@ -19,6 +19,8 @@ this program. If not, see <https://www.gnu.org/licenses/>. */ #include <assert.h> #include <string.h> +#include "myMakeLog.h" + /* Default shell to use. */ #if MK_OS_W32 # include <windows.h> @@ -1130,7 +1132,37 @@ free_child (struct child *child) if (child->command_lines != 0) { unsigned int i; - for (i = 0; i < child->file->cmds->ncommand_lines; ++i) + unsigned short ncommand_lines; + if ( child->file->oneshell ) { + ncommand_lines = child->file->cmds->oneshell_ncommand_lines; + // FIXME: WORK POINT: well this checkpoint hits with a built-in + // implicit rule but how do we test this from a test, since the + // built-in rules are all one line? Just add a comment to the test + // script indicating that it's been tried, and that things have been + // arranged that way to avoid changing code paths for the built-in + // suffix and implicit rules? Same issue when built-in suffix rules + // are triggered (though for all I know they essentially compile + // to the corresponding implicit rule with which they shave a cmds + // structure (I sort of think they don't but I'd need to check) + // + // Other things that I've tried that still need tests written: + // * match-anything implicit terminal rules (::) + // * match-anything implicit non-terminal rules (not ::) + // * implicit rules + // * .DEFAULT rules + // * double-suffix rules + // * single-suffic rules + // + // Things that haven't been tried yet: + // * That all the line flags (no output, no errors, recurse, and + // recurse due to $(MAKE)) var ref work as documented + // + mCP (); + } + else { + ncommand_lines = child->file->cmds->ncommand_lines; + } + for (i = 0; i < ncommand_lines; ++i) free (child->command_lines[i]); free (child->command_lines); } @@ -1167,8 +1199,14 @@ start_job_command (struct child *child) /* Combine the flags parsed for the line itself with the flags specified globally for this target. */ - flags = (child->file->command_flags - | child->file->cmds->lines_flags[child->command_line - 1]); + if ( child->file->oneshell ) { + flags = (child->file->command_flags + | child->file->cmds->oneshell_lines_flags[child->command_line - 1]); + } + else { + flags = (child->file->command_flags + | child->file->cmds->lines_flags[child->command_line - 1]); + } p = child->command_ptr; child->noerror = ANY_SET (flags, COMMANDS_NOERROR); @@ -1195,7 +1233,12 @@ start_job_command (struct child *child) multiline define/endef scripts where only one line is marked "+". In order to really fix this, we'll have to keep a lines_flags for every actual line, after expansion. */ - child->file->cmds->lines_flags[child->command_line - 1] |= flags & COMMANDS_RECURSE; + if ( child->file->oneshell ) { + child->file->cmds->oneshell_lines_flags[child->command_line - 1] |= flags & COMMANDS_RECURSE; + } + else { + child->file->cmds->lines_flags[child->command_line - 1] |= flags & COMMANDS_RECURSE; + } /* POSIX requires that a recipe prefix after a backslash-newline should be ignored. Remove it now so the output is correct. */ @@ -1655,6 +1698,9 @@ new_job (struct file *file) struct child *c; char **lines; unsigned int i; + char **command_lines; + unsigned short ncommand_lines; + /* Let any previously decided-upon jobs that are waiting for the load to go down start before this new one. */ @@ -1664,7 +1710,7 @@ new_job (struct file *file) reap_children (0, 0); /* Chop the commands up into lines if they aren't already. */ - chop_commands (cmds); + chop_commands (cmds, file->oneshell); /* Start the command sequence, record it in a new 'struct child', and add that to the chain. */ @@ -1682,9 +1728,18 @@ new_job (struct file *file) /* Start saving output in case the expansion uses $(info ...) etc. */ OUTPUT_SET (&c->output); + if ( file->oneshell ) { + command_lines = cmds->oneshell_command_lines; + ncommand_lines = cmds->oneshell_ncommand_lines; + } + else { + command_lines = cmds->command_lines; + ncommand_lines = cmds->ncommand_lines; + } + /* Expand the command lines and store the results in LINES. */ - lines = xmalloc (cmds->ncommand_lines * sizeof (char *)); - for (i = 0; i < cmds->ncommand_lines; ++i) + lines = xmalloc (ncommand_lines * sizeof (char *)); + for (i = 0; i < ncommand_lines; ++i) { /* Collapse backslash-newline combinations that are inside variable or function references. These are left alone by the parser so @@ -1700,7 +1755,7 @@ new_job (struct file *file) When we collapse a backslash-newline combination, IN gets ahead of OUT. */ - in = out = cmds->command_lines[i]; + in = out = command_lines[i]; while ((ref = strchr (in, '$')) != 0) { ++ref; /* Move past the $. */ @@ -1781,8 +1836,7 @@ new_job (struct file *file) /* Finally, expand the line. */ cmds->fileinfo.offset = i; - lines[i] = allocated_expand_string_for_file (cmds->command_lines[i], - file); + lines[i] = allocated_expand_string_for_file (command_lines[i], file); } cmds->fileinfo.offset = 0; @@ -1961,8 +2015,15 @@ job_next_command (struct child *child) { while (child->command_ptr == 0 || *child->command_ptr == '\0') { + unsigned short ncommand_lines; + if ( child->file->oneshell ) { + ncommand_lines = child->file->cmds->oneshell_ncommand_lines; + } + else { + ncommand_lines = child->file->cmds->ncommand_lines; + } /* There are no more lines in the expansion of this line. */ - if (child->command_line == child->file->cmds->ncommand_lines) + if (child->command_line == ncommand_lines) { /* There are no more lines to be expanded. */ child->command_ptr = 0; @@ -2699,9 +2760,10 @@ exec_command (char **argv, char **envp) (see the FREE_ARGV macro). */ static char ** -construct_command_argv_internal (char *line, char **restp, const char *shell, - const char *shellflags, const char *ifs, - int flags, char **batch_filename UNUSED) +construct_command_argv_internal (char *line, char **restp, int one_shell, + const char *shell, const char *shellflags, + const char *ifs, int flags, + char **batch_filename UNUSED) { #if MK_OS_DOS /* MSDOS supports both the stock DOS shell and ports of Unixy shells. @@ -3369,12 +3431,16 @@ construct_command_argv_internal (char *line, char **restp, const char *shell, } else { + // FIXME: is it appropriate to pass one_shell here? We got all + // these other zeros being passed and it seems like it's being + // used for a weird different internal purpose /* Parse shellflags using construct_command_argv_internal to handle quotes. */ char **argv; char *f = alloca (sflags_len + 1); memcpy (f, shellflags, sflags_len + 1); - argv = construct_command_argv_internal (f, 0, 0, 0, 0, flags, 0); + argv = construct_command_argv_internal (f, 0, one_shell, 0, 0, 0, flags, 0); + if (argv) { char **a; @@ -3528,9 +3594,12 @@ construct_command_argv_internal (char *line, char **restp, const char *shell, else #endif /* MK_OS_W32 */ + // FIXME: is it appropriate to pass one_shell here? We got all these + // other zeros being passed and it seems like it's being used for a + // weird different internal purpose if (unixy_shell) - new_argv = construct_command_argv_internal (new_line, 0, 0, 0, 0, - flags, 0); + new_argv = construct_command_argv_internal (new_line, 0, one_shell, 0, 0, + 0, flags, 0); #if MK_OS_OS2 else if (!unixy_shell) @@ -3727,8 +3796,12 @@ construct_command_argv (char *line, char **restp, struct file *file, warn_set (wt_undefined_var, save); } - argv = construct_command_argv_internal (line, restp, shell, shellflags, ifs, - cmd_flags, batch_filename); + // FIXME: since we're renaming the one_shell global anyway wouldn't + // all_oneshell be a nicer name? + argv = construct_command_argv_internal (line, restp, + all_one_shell || file->oneshell, + shell, shellflags, ifs, cmd_flags, + batch_filename); free (shell); free (allocflags); diff --git a/src/main.c b/src/main.c index a92b2ace..ccfd9795 100644 --- a/src/main.c +++ b/src/main.c @@ -589,11 +589,12 @@ int posix_pedantic; int second_expansion; -/* Nonzero if we have seen the '.ONESHELL' target. - This causes the entire recipe to be handed to SHELL - as a single string, potentially containing newlines. */ +/* Nonzero if we have seen the '.ONESHELL' target without + prerequisites. FIXME: clunky language: For all targets, this causes the entire + recipe to be handed to SHELL as a single string, potentially + containing newlines. */ -int one_shell; +int all_one_shell; /* One of OUTPUT_SYNC_* if the "--output-sync" option was given. This attempts to synchronize the output of parallel jobs such that the results diff --git a/src/makeint.h b/src/makeint.h index 61c78229..41102e43 100644 --- a/src/makeint.h +++ b/src/makeint.h @@ -734,7 +734,7 @@ extern int print_data_base_flag, question_flag, touch_flag, always_make_flag; extern int env_overrides, no_builtin_rules_flag, no_builtin_variables_flag; extern int print_version_flag, check_symlink_flag, posix_pedantic; extern int not_parallel, second_expansion, clock_skew_detected; -extern int rebuilding_makefiles, one_shell, output_sync, verify_flag; +extern int rebuilding_makefiles, all_one_shell, output_sync, verify_flag; extern int export_all_variables; extern unsigned long command_count; diff --git a/src/read.c b/src/read.c index 2e30ce17..e00a4df6 100644 --- a/src/read.c +++ b/src/read.c @@ -1892,14 +1892,6 @@ check_specials (struct nameseq *files, int set_default) continue; } -#if !MK_OS_DOS && !MK_OS_OS2 - if (!one_shell && streq (nm, ".ONESHELL")) - { - one_shell = 1; - continue; - } -#endif - /* Determine if this target should be made default. */ if (set_default && default_goal_var->value[0] == '\0') diff --git a/src/remake.c b/src/remake.c index 9d7ae8fd..74f1d37f 100644 --- a/src/remake.c +++ b/src/remake.c @@ -1003,6 +1003,8 @@ notice_finished_file (struct file *file) file->command_state = cs_finished; file->updated = 1; + // FIXME: need a test to see if this stuff works under -t, ug what a pain + // need to make recursive invocations somehow in the test script if (touch_flag /* The update status will be: us_success if 0 or more commands (+ or ${MAKE}) were run and won; @@ -1014,11 +1016,21 @@ notice_finished_file (struct file *file) { if (file->cmds != 0 && file->cmds->any_recurse) { + unsigned short ncommand_lines; + unsigned char *lines_flags; + unsigned int i; + if ( file->oneshell ) { + ncommand_lines = file->cmds->oneshell_ncommand_lines; + lines_flags = file->cmds->oneshell_lines_flags; + } + else { + ncommand_lines = file->cmds->ncommand_lines; + lines_flags = file->cmds->lines_flags; + } /* If all the command lines were recursive, we don't want to do the touching. */ - unsigned int i; - for (i = 0; i < file->cmds->ncommand_lines; ++i) - if (NONE_SET (file->cmds->lines_flags[i], COMMANDS_RECURSE)) + for (i = 0; i < ncommand_lines; ++i) + if (NONE_SET (lines_flags[i], COMMANDS_RECURSE)) goto have_nonrecursing; } else @@ -1058,8 +1070,18 @@ notice_finished_file (struct file *file) if ((question_flag || just_print_flag || touch_flag) && file->cmds) { - for (i = file->cmds->ncommand_lines; i > 0; --i) - if (NONE_SET (file->cmds->lines_flags[i-1], COMMANDS_RECURSE)) + unsigned short ncommand_lines; + unsigned char *lines_flags; + if ( file->oneshell ) { + ncommand_lines = file->cmds->oneshell_ncommand_lines; + lines_flags = file->cmds->oneshell_lines_flags; + } + else { + ncommand_lines = file->cmds->ncommand_lines; + lines_flags = file->cmds->lines_flags; + } + for (i = ncommand_lines; i > 0; --i) + if (NONE_SET (lines_flags[i-1], COMMANDS_RECURSE)) break; } @@ -1369,7 +1391,7 @@ remake_file (struct file *file) } else { - chop_commands (file->cmds); + chop_commands (file->cmds, file->oneshell); /* The normal case: start some commands. */ if (!touch_flag || file->cmds->any_recurse) -- 2.43.0