diff --git a/src/exec.cpp b/src/exec.cpp index d451ed688..5d36f5ca9 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -983,7 +983,7 @@ bool exec_job(parser_t &parser, const shared_ptr &j, const io_chain_t &bl // A false return tells the caller to remove the job from the list. return false; } - cleanup_t timer = push_timer(j->flags().has_time_prefix && !no_exec()); + cleanup_t timer = push_timer(j->wants_timing() && !no_exec()); // Get the deferred process, if any. We will have to remember its pipes. autoclose_pipes_t deferred_pipes; diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 786e9948b..6d17c152d 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -998,12 +998,6 @@ end_execution_reason_t parse_execution_context_t::populate_not_process( job_t *job, process_t *proc, const ast::not_statement_t ¬_statement) { auto &flags = job->mut_flags(); flags.negate = !flags.negate; - if (not_statement.time) { - flags.has_time_prefix = true; - if (job->is_initially_background()) { - return this->report_error(STATUS_INVALID_ARGS, not_statement, ERROR_TIME_BACKGROUND); - } - } return this->populate_job_process(job, proc, not_statement.contents, not_statement.variables); } @@ -1151,12 +1145,6 @@ end_execution_reason_t parse_execution_context_t::populate_job_from_job_node( // Create processes. Each one may fail. process_list_t processes; processes.emplace_back(new process_t()); - if (job_node.time) { - j->mut_flags().has_time_prefix = true; - if (job_node.bg) { - return this->report_error(STATUS_INVALID_ARGS, job_node, ERROR_TIME_BACKGROUND); - } - } end_execution_reason_t result = this->populate_job_process( j, processes.back().get(), job_node.statement, job_node.variables); @@ -1210,6 +1198,32 @@ static bool remove_job(parser_t &parser, job_t *job) { return false; } +/// Decide if a job node should be 'time'd. +/// For historical reasons the 'not' and 'time' prefix are "inside out". That is, it's +/// 'not time cmd'. Note that a time appearing anywhere in the pipeline affects the whole job. +/// `sleep 1 | not time true` will time the whole job! +static bool job_node_wants_timing(const ast::job_t &job_node) { + // Does our job have the job-level time prefix? + if (job_node.time) return true; + + // Helper to return true if a node is 'not time ...' or 'not not time...' or... + auto is_timed_not_statement = [](const ast::statement_t &stat) { + const auto *ns = stat.contents->try_as(); + while (ns) { + if (ns->time) return true; + ns = ns->contents.try_as(); + } + return false; + }; + + // Do we have a 'not time ...' anywhere in our pipeline? + if (is_timed_not_statement(job_node.statement)) return true; + for (const ast::job_continuation_t &jc : job_node.continuation) { + if (is_timed_not_statement(jc.statement)) return true; + } + return false; +} + end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &job_node, const block_t *associated_block) { if (auto ret = check_end_execution()) { @@ -1306,6 +1320,12 @@ end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &jo ld.is_subshell || ld.is_block || ld.is_event || !parser->is_interactive(); props.from_event_handler = ld.is_event; props.job_control = wants_job_control; + props.wants_timing = job_node_wants_timing(job_node); + + // It's an error to have 'time' in a background job. + if (props.wants_timing && props.initial_background) { + return this->report_error(STATUS_INVALID_ARGS, job_node, ERROR_TIME_BACKGROUND); + } shared_ptr job = std::make_shared(props, get_source(job_node)); diff --git a/src/proc.h b/src/proc.h index cd6852fe8..73973cd04 100644 --- a/src/proc.h +++ b/src/proc.h @@ -318,6 +318,9 @@ class job_t { /// initial state should be. bool initial_background{}; + /// Whether the job has the 'time' prefix and so we should print timing for this job. + bool wants_timing{}; + /// Whether this job was created as part of an event handler. bool from_event_handler{}; @@ -409,9 +412,6 @@ class job_t { /// This job is disowned, and should be removed from the active jobs list. bool disown_requested{false}; - /// Whether to print timing for this job. - bool has_time_prefix{false}; - // Indicates that we are the "group root." Any other jobs using this tree are nested. bool is_group_root{false}; @@ -423,6 +423,9 @@ class job_t { /// Access mutable job flags. flags_t &mut_flags() { return job_flags; } + // \return whether we should print timing information. + bool wants_timing() const { return properties.wants_timing; } + /// \return if we want job control. bool wants_job_control() const { return properties.job_control; }