Switch last_pid from the pgroup to the actual last pid

When a job is placed in the background, fish will set the `$last_pid`
variable. Prior to this change, `$last_pid` was set to the process group
leader of the job. However this caussed problems when the job ran in
fish's process group, because then fish itself would be the process group
leader and commands like `wait` would not work.

Switch `$last_pid` to be the actual last pid of the pipeline. This brings
it in line with the `$!` variable from zsh and bash.

This is technically a breaking change, but it is unlikely to cause
problems, because `$last_pid` was already rather broken.

Fixes #5036
Fixes #5832
Fixes #7721
This commit is contained in:
ridiculousfish 2021-05-18 12:35:37 -07:00
parent f2448e3f0e
commit f3d78e21d1
4 changed files with 20 additions and 3 deletions

View File

@ -27,6 +27,7 @@ Scripting improvements
- ``fish_indent`` allows to write inline variable assignments on multiple lines (ending in a backslash), instead of joining them into one line (:issue:`7955`).
- fish gained a ``--no-config`` option to disable reading the configuration. This applies both to the user's and the admin's config.fish (typically in /etc/fish/config.fish) and snippets and also sets $fish_function_path to just the functions shipped with fish and disables universal variables and history. (:issue:`7921`, :issue:`1256`).
- When universal variables are unavailable for some reason, setting a universal variable now sets a global variable instead (:issue:`7921`).
- ``$last_pid`` now reports the pid of the last process in the pipeline, allowing it to be used in scripts (:issue:`5036`, :issue:`5832`, :issue:`7721`).
- ``process-exit`` event handlers now receive the same value as ``$status`` in all cases, instead of receiving -1 when the exit was due to a signal.
- ``process-exit`` event handlers for pid 0 also received ``JOB_EXIT`` events; this has been fixed.

View File

@ -1123,9 +1123,12 @@ bool exec_job(parser_t &parser, const shared_ptr<job_t> &j, const io_chain_t &bl
// If exec_error then a backgrounded job would have been terminated before it was ever assigned
// a pgroup, so error out before setting last_pid.
auto pgid = j->get_pgid();
if (!j->is_foreground() && pgid.has_value()) {
parser.vars().set_one(L"last_pid", ENV_GLOBAL, to_string(*pgid));
if (!j->is_foreground()) {
if (maybe_t<pid_t> last_pid = j->get_last_pid()) {
parser.vars().set_one(L"last_pid", ENV_GLOBAL, to_string(*last_pid));
} else {
parser.vars().set_empty(L"last_pid", ENV_GLOBAL);
}
}
j->continue_job(parser, !j->is_initially_background());

View File

@ -953,6 +953,14 @@ bool job_t::is_foreground() const { return group->is_foreground(); }
maybe_t<pid_t> job_t::get_pgid() const { return group->get_pgid(); }
maybe_t<pid_t> job_t::get_last_pid() const {
for (auto iter = processes.rbegin(); iter != processes.rend(); ++iter) {
const process_t *proc = iter->get();
if (proc->pid > 0) return proc->pid;
}
return none();
}
job_id_t job_t::job_id() const { return group->get_id(); }
void job_t::continue_job(parser_t &parser, bool in_foreground) {

View File

@ -392,6 +392,11 @@ class job_t {
/// This may also be fish itself.
maybe_t<pid_t> get_pgid() const;
/// \return the pid of the last external process in the job.
/// This may be none if the job consists of just internal fish functions or builtins.
/// This will never be fish's own pid.
maybe_t<pid_t> get_last_pid() const;
/// The id of this job.
/// This is user-visible, is recycled, and may be -1.
job_id_t job_id() const;