From f3d78e21d1d1a4d72392f16b2b20eaa61d1d3b33 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Tue, 18 May 2021 12:35:37 -0700 Subject: [PATCH] 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 --- CHANGELOG.rst | 1 + src/exec.cpp | 9 ++++++--- src/proc.cpp | 8 ++++++++ src/proc.h | 5 +++++ 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4d7a65966..86c4dae97 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -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. diff --git a/src/exec.cpp b/src/exec.cpp index 53e688199..9a7172974 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -1123,9 +1123,12 @@ bool exec_job(parser_t &parser, const shared_ptr &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 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()); diff --git a/src/proc.cpp b/src/proc.cpp index 3dc511a81..6823e251a 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -953,6 +953,14 @@ bool job_t::is_foreground() const { return group->is_foreground(); } maybe_t job_t::get_pgid() const { return group->get_pgid(); } +maybe_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) { diff --git a/src/proc.h b/src/proc.h index aaac1a960..289cab57d 100644 --- a/src/proc.h +++ b/src/proc.h @@ -392,6 +392,11 @@ class job_t { /// This may also be fish itself. maybe_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 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;