Rename populate_group_for_job to resolve_group_for_job

Factor it to allows the function to not modify the job.
This commit is contained in:
ridiculousfish 2020-09-01 15:09:53 -07:00
parent 6c4d6dc4a9
commit 760b6e76cc
3 changed files with 32 additions and 29 deletions

View file

@ -47,8 +47,10 @@ void job_group_t::set_pgid(pid_t pgid) {
maybe_t<pid_t> job_group_t::get_pgid() const { return pgid_; } maybe_t<pid_t> job_group_t::get_pgid() const { return pgid_; }
void job_group_t::populate_group_for_job(job_t *job, const job_group_ref_t &proposed) { // static
assert(!job->group && "Job already has a group"); job_group_ref_t job_group_t::resolve_group_for_job(const job_t &job,
const job_group_ref_t &proposed) {
assert(!job.group && "Job already has a group");
// Note there's three cases to consider: // Note there's three cases to consider:
// nullptr -> this is a root job, there is no inherited job group // nullptr -> this is a root job, there is no inherited job group
// internal -> the parent is running as part of a simple function execution // internal -> the parent is running as part of a simple function execution
@ -56,10 +58,10 @@ void job_group_t::populate_group_for_job(job_t *job, const job_group_ref_t &prop
// non-internal -> we are running as part of a real pipeline // non-internal -> we are running as part of a real pipeline
// Decide if this job can use an internal group. // Decide if this job can use an internal group.
// This is true if it's a simple foreground execution of an internal proc. // This is true if it's a simple foreground execution of an internal proc.
bool initial_bg = job->is_initially_background(); bool initial_bg = job.is_initially_background();
bool first_proc_internal = job->processes.front()->is_internal(); bool first_proc_internal = job.processes.front()->is_internal();
bool can_use_internal = bool can_use_internal =
!initial_bg && job->processes.size() == 1 && job->processes.front()->is_internal(); !initial_bg && job.processes.size() == 1 && job.processes.front()->is_internal();
bool needs_new_group = false; bool needs_new_group = false;
if (!proposed) { if (!proposed) {
@ -73,27 +75,25 @@ void job_group_t::populate_group_for_job(job_t *job, const job_group_ref_t &prop
needs_new_group = true; needs_new_group = true;
} }
job->mut_flags().is_group_root = needs_new_group; if (!needs_new_group) return proposed;
if (!needs_new_group) { // We will need to create a new group.
job->group = proposed; properties_t props{};
} else { props.job_control = job.wants_job_control();
properties_t props{}; props.wants_terminal = job.wants_job_control() && !job.from_event_handler();
props.job_control = job->wants_job_control(); props.is_internal = can_use_internal;
props.wants_terminal = job->wants_job_control() && !job->from_event_handler(); props.job_id = can_use_internal ? -1 : acquire_job_id();
props.is_internal = can_use_internal; job_group_ref_t result{new job_group_t(props, job.command())};
props.job_id = can_use_internal ? -1 : acquire_job_id();
job->group.reset(new job_group_t(props, job->command()));
// Mark if it's foreground. // Mark if it's foreground.
job->group->set_is_foreground(!initial_bg); result->set_is_foreground(!initial_bg);
// Perhaps this job should immediately live in fish's pgroup. // Perhaps this job should immediately live in fish's pgroup.
// There's two reasons why it may be so: // There's two reasons why it may be so:
// 1. The job doesn't need job control. // 1. The job doesn't need job control.
// 2. The first process in the job is internal to fish; this needs to own the tty. // 2. The first process in the job is internal to fish; this needs to own the tty.
if (!can_use_internal && (!props.job_control || first_proc_internal)) { if (!can_use_internal && (!props.job_control || first_proc_internal)) {
job->group->set_pgid(getpgrp()); result->set_pgid(getpgrp());
}
} }
return result;
} }

View file

@ -74,9 +74,11 @@ class job_group_t {
void mark_root_constructed() { root_constructed_ = true; }; void mark_root_constructed() { root_constructed_ = true; };
bool is_root_constructed() const { return root_constructed_; } bool is_root_constructed() const { return root_constructed_; }
/// Given a job and a proposed job group (possibly null), populate the job's group field. /// Given a job and a proposed job group (possibly null), return a group for the job.
/// The proposed group is the group from the parent job, or null if this is a root. /// The proposed group is the group from the parent job, or null if this is a root.
static void populate_group_for_job(job_t *job, const job_group_ref_t &proposed_tree); /// This never returns null.
static job_group_ref_t resolve_group_for_job(const job_t &job,
const job_group_ref_t &proposed_group);
~job_group_t(); ~job_group_t();

View file

@ -1342,9 +1342,10 @@ end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &jo
// Clean up the job on failure or cancellation. // Clean up the job on failure or cancellation.
if (pop_result == end_execution_reason_t::ok) { if (pop_result == end_execution_reason_t::ok) {
// Set the pgroup assignment mode and job group, now that the job is populated. // Resolve the job's group and mark if this job is the first to get it.
job_group_t::populate_group_for_job(job.get(), ctx.job_group); job->group = job_group_t::resolve_group_for_job(*job, ctx.job_group);
assert(job->group && "Should have a job group"); assert(job->group && "Should not have a null group");
job->mut_flags().is_group_root = (job->group != ctx.job_group);
// Success. Give the job to the parser - it will clean it up. // Success. Give the job to the parser - it will clean it up.
parser->job_add(job); parser->job_add(job);