Port bg builtin to Rust (#9621)

* bg: Port bg builtin to Rust

CMakeLists.txt

@@ -99,8 +99,7 @@ endif()
 
 # List of sources for builtin functions.
 set(FISH_BUILTIN_SRCS
-    src/builtin.cpp src/builtins/argparse.cpp
-    src/builtins/bg.cpp src/builtins/bind.cpp
+    src/builtin.cpp src/builtins/argparse.cpp src/builtins/bind.cpp
     src/builtins/builtin.cpp src/builtins/cd.cpp src/builtins/command.cpp
     src/builtins/commandline.cpp src/builtins/complete.cpp
     src/builtins/disown.cpp

fish-rust/src/builtins/bg.rs

@@ -0,0 +1,139 @@
+// Implementation of the bg builtin.
+
+use std::pin::Pin;
+
+use super::shared::{builtin_print_help, io_streams_t, STATUS_CMD_ERROR, STATUS_INVALID_ARGS};
+use crate::{
+    builtins::shared::{HelpOnlyCmdOpts, STATUS_CMD_OK},
+    ffi::{self, parser_t, Repin},
+    wchar::wstr,
+    wchar_ffi::{c_str, WCharFromFFI, WCharToFFI},
+    wutil::{fish_wcstoi, wgettext_fmt},
+};
+use libc::c_int;
+
+/// Helper function for builtin_bg().
+fn send_to_bg(
+    parser: &mut parser_t,
+    streams: &mut io_streams_t,
+    cmd: &wstr,
+    job_pos: usize,
+) -> Option<c_int> {
+    let job = parser.get_jobs()[job_pos]
+        .as_ref()
+        .expect("job_pos must be valid");
+    if !job.wants_job_control() {
+        let err = wgettext_fmt!(
+            "%ls: Can't put job %d, '%ls' to background because it is not under job control\n",
+            cmd,
+            job.job_id().0,
+            job.command().from_ffi()
+        );
+        ffi::builtin_print_help(
+            parser.pin(),
+            streams.ffi_ref(),
+            c_str!(cmd),
+            err.to_ffi().as_ref()?,
+        );
+        return STATUS_CMD_ERROR;
+    }
+
+    streams.err.append(wgettext_fmt!(
+        "Send job %d '%ls' to background\n",
+        job.job_id().0,
+        job.command().from_ffi()
+    ));
+
+    unsafe {
+        std::mem::transmute::<&ffi::job_group_t, &crate::job_group::JobGroup>(job.ffi_group())
+    }
+    .set_is_foreground(false);
+
+    if !job.ffi_resume() {
+        return STATUS_CMD_ERROR;
+    }
+    parser.pin().job_promote_at(job_pos);
+
+    return STATUS_CMD_OK;
+}
+
+/// Builtin for putting a job in the background.
+pub fn bg(parser: &mut parser_t, streams: &mut io_streams_t, args: &mut [&wstr]) -> Option<c_int> {
+    let opts = match HelpOnlyCmdOpts::parse(args, parser, streams) {
+        Ok(opts) => opts,
+        Err(err @ Some(_)) if err != STATUS_CMD_OK => return err,
+        Err(err) => panic!("Illogical exit code from parse_options(): {err:?}"),
+    };
+
+    let cmd = args[0];
+    if opts.print_help {
+        builtin_print_help(parser, streams, args.get(0)?);
+        return STATUS_CMD_OK;
+    }
+
+    if opts.optind == args.len() {
+        // No jobs were specified so use the most recent (i.e., last) job.
+        let jobs = parser.get_jobs();
+        let job_pos = jobs.iter().position(|job| {
+            if let Some(job) = job.as_ref() {
+                return job.is_stopped() && job.wants_job_control() && !job.is_completed();
+            }
+
+            false
+        });
+
+        let Some(job_pos) = job_pos else {
+            streams
+                    .err
+                    .append(wgettext_fmt!("%ls: There are no suitable jobs\n", cmd));
+                return STATUS_CMD_ERROR;
+        };
+
+        return send_to_bg(parser, streams, cmd, job_pos);
+    }
+
+    // The user specified at least one job to be backgrounded.
+
+    // If one argument is not a valid pid (i.e. integer >= 0), fail without backgrounding anything,
+    // but still print errors for all of them.
+    let mut retval = STATUS_CMD_OK;
+    let pids: Vec<i64> = args[opts.optind..]
+        .iter()
+        .map(|arg| {
+            fish_wcstoi(arg.chars()).unwrap_or_else(|_| {
+                streams.err.append(wgettext_fmt!(
+                    "%ls: '%ls' is not a valid job specifier\n",
+                    cmd,
+                    *arg
+                ));
+                retval = STATUS_INVALID_ARGS;
+                0
+            })
+        })
+        .collect();
+
+    if retval != STATUS_CMD_OK {
+        return retval;
+    }
+
+    // Background all existing jobs that match the pids.
+    // Non-existent jobs aren't an error, but information about them is useful.
+    for pid in pids {
+        let mut job_pos = 0;
+        let job = unsafe {
+            parser
+                .job_get_from_pid1(pid, Pin::new(&mut job_pos))
+                .as_ref()
+        };
+
+        if job.is_some() {
+            send_to_bg(parser, streams, cmd, job_pos);
+        } else {
+            streams
+                .err
+                .append(wgettext_fmt!("%ls: Could not find job '%d'\n", cmd, pid));
+        }
+    }
+
+    return STATUS_CMD_OK;
+}

fish-rust/src/builtins/mod.rs

@@ -1,6 +1,7 @@
 pub mod shared;
 
 pub mod abbr;
+pub mod bg;
 pub mod block;
 pub mod contains;
 pub mod echo;

fish-rust/src/builtins/shared.rs

@@ -119,6 +119,7 @@ pub fn run_builtin(
 ) -> Option<c_int> {
     match builtin {
         RustBuiltin::Abbr => super::abbr::abbr(parser, streams, args),
+        RustBuiltin::Bg => super::bg::bg(parser, streams, args),
         RustBuiltin::Block => super::block::block(parser, streams, args),
         RustBuiltin::Contains => super::contains::contains(parser, streams, args),
         RustBuiltin::Echo => super::echo::echo(parser, streams, args),

src/builtin.cpp

@@ -30,7 +30,6 @@
 #include <string>
 
 #include "builtins/argparse.h"
-#include "builtins/bg.h"
 #include "builtins/bind.h"
 #include "builtins/builtin.h"
 #include "builtins/cd.h"
@@ -361,7 +360,7 @@ static constexpr builtin_data_t builtin_datas[] = {
     {L"and", &builtin_generic, N_(L"Run command if last command succeeded")},
     {L"argparse", &builtin_argparse, N_(L"Parse options in fish script")},
     {L"begin", &builtin_generic, N_(L"Create a block of code")},
-    {L"bg", &builtin_bg, N_(L"Send job to background")},
+    {L"bg", &implemented_in_rust, N_(L"Send job to background")},
     {L"bind", &builtin_bind, N_(L"Handle fish key bindings")},
     {L"block", &implemented_in_rust, N_(L"Temporarily block delivery of events")},
     {L"break", &builtin_break_continue, N_(L"Stop the innermost loop")},
@@ -524,6 +523,9 @@ static maybe_t<RustBuiltin> try_get_rust_builtin(const wcstring &cmd) {
     if (cmd == L"abbr") {
         return RustBuiltin::Abbr;
     }
+    if (cmd == L"bg") {
+        return RustBuiltin::Bg;
+    }
     if (cmd == L"block") {
         return RustBuiltin::Block;
     }

src/builtin.h

@@ -110,6 +110,7 @@ int parse_help_only_cmd_opts(help_only_cmd_opts_t &opts, int *optind, int argc,
 /// An enum of the builtins implemented in Rust.
 enum RustBuiltin : int32_t {
     Abbr,
+    Bg,
     Block,
     Contains,
     Echo,

src/builtins/bg.cpp

@@ -1,107 +0,0 @@
-// Implementation of the bg builtin.
-#include "config.h"  // IWYU pragma: keep
-
-#include "bg.h"
-
-#include <sys/types.h>
-
-#include <cerrno>
-#include <deque>
-#include <memory>
-#include <vector>
-
-#include "../builtin.h"
-#include "../common.h"
-#include "../fallback.h"  // IWYU pragma: keep
-#include "../io.h"
-#include "../maybe.h"
-#include "../parser.h"
-#include "../proc.h"
-#include "../wutil.h"  // IWYU pragma: keep
-#include "job_group.rs.h"
-
-/// Helper function for builtin_bg().
-static int send_to_bg(parser_t &parser, io_streams_t &streams, job_t *j) {
-    assert(j != nullptr);
-    if (!j->wants_job_control()) {
-        wcstring error_message = format_string(
-            _(L"%ls: Can't put job %d, '%ls' to background because it is not under job control\n"),
-            L"bg", j->job_id(), j->command_wcstr());
-        builtin_print_help(parser, streams, L"bg", error_message);
-        return STATUS_CMD_ERROR;
-    }
-
-    streams.err.append_format(_(L"Send job %d '%ls' to background\n"), j->job_id(),
-                              j->command_wcstr());
-    j->group->set_is_foreground(false);
-    if (!j->resume()) {
-        return STATUS_CMD_ERROR;
-    }
-    parser.job_promote(j);
-    return STATUS_CMD_OK;
-}
-
-/// Builtin for putting a job in the background.
-maybe_t<int> builtin_bg(parser_t &parser, io_streams_t &streams, const wchar_t **argv) {
-    const wchar_t *cmd = argv[0];
-    int argc = builtin_count_args(argv);
-    help_only_cmd_opts_t opts;
-
-    int optind;
-    int retval = parse_help_only_cmd_opts(opts, &optind, argc, argv, parser, streams);
-    if (retval != STATUS_CMD_OK) return retval;
-
-    if (opts.print_help) {
-        builtin_print_help(parser, streams, cmd);
-        return STATUS_CMD_OK;
-    }
-
-    if (optind == argc) {
-        // No jobs were specified so use the most recent (i.e., last) job.
-        job_t *job = nullptr;
-        for (const auto &j : parser.jobs()) {
-            if (j->is_stopped() && j->wants_job_control() && (!j->is_completed())) {
-                job = j.get();
-                break;
-            }
-        }
-
-        if (!job) {
-            streams.err.append_format(_(L"%ls: There are no suitable jobs\n"), cmd);
-            retval = STATUS_CMD_ERROR;
-        } else {
-            retval = send_to_bg(parser, streams, job);
-        }
-
-        return retval;
-    }
-
-    // The user specified at least one job to be backgrounded.
-    std::vector<pid_t> pids;
-
-    // If one argument is not a valid pid (i.e. integer >= 0), fail without backgrounding anything,
-    // but still print errors for all of them.
-    for (int i = optind; argv[i]; i++) {
-        int pid = fish_wcstoi(argv[i]);
-        if (errno || pid < 0) {
-            streams.err.append_format(_(L"%ls: '%ls' is not a valid job specifier\n"), L"bg",
-                                      argv[i]);
-            retval = STATUS_INVALID_ARGS;
-        }
-        pids.push_back(pid);
-    }
-
-    if (retval != STATUS_CMD_OK) return retval;
-
-    // Background all existing jobs that match the pids.
-    // Non-existent jobs aren't an error, but information about them is useful.
-    for (auto p : pids) {
-        if (job_t *j = parser.job_get_from_pid(p)) {
-            retval |= send_to_bg(parser, streams, j);
-        } else {
-            streams.err.append_format(_(L"%ls: Could not find job '%d'\n"), cmd, p);
-        }
-    }
-
-    return retval;
-}

src/builtins/bg.h

@@ -1,11 +0,0 @@
-// Prototypes for executing builtin_bg function.
-#ifndef FISH_BUILTIN_BG_H
-#define FISH_BUILTIN_BG_H
-
-#include "../maybe.h"
-
-class parser_t;
-struct io_streams_t;
-
-maybe_t<int> builtin_bg(parser_t &parser, io_streams_t &streams, const wchar_t **argv);
-#endif

src/parser.cpp

@@ -458,7 +458,12 @@ void parser_t::job_add(shared_ptr<job_t> job) {
     job_list.insert(job_list.begin(), std::move(job));
 }
 
-void parser_t::job_promote(job_t *job) {
+void parser_t::job_promote(job_list_t::iterator job_it) {
+    // Move the job to the beginning.
+    std::rotate(job_list.begin(), job_it, std::next(job_it));
+}
+
+void parser_t::job_promote(const job_t *job) {
     job_list_t::iterator loc;
     for (loc = job_list.begin(); loc != job_list.end(); ++loc) {
         if (loc->get() == job) {
@@ -466,9 +471,12 @@ void parser_t::job_promote(job_t *job) {
         }
     }
     assert(loc != job_list.end());
+    job_promote(loc);
+}
 
-    // Move the job to the beginning.
-    std::rotate(job_list.begin(), loc, std::next(loc));
+void parser_t::job_promote_at(size_t job_pos) {
+    assert(job_pos < job_list.size());
+    job_promote(job_list.begin() + job_pos);
 }
 
 const job_t *parser_t::job_with_id(job_id_t id) const {
@@ -479,10 +487,16 @@ const job_t *parser_t::job_with_id(job_id_t id) const {
 }
 
 job_t *parser_t::job_get_from_pid(pid_t pid) const {
-    for (const auto &job : jobs()) {
-        for (const process_ptr_t &p : job->processes) {
+    size_t job_pos{};
+    return job_get_from_pid(pid, job_pos);
+}
+
+job_t *parser_t::job_get_from_pid(int64_t pid, size_t& job_pos) const {
+    for (auto it = job_list.begin(); it != job_list.end(); ++it) {
+        for (const process_ptr_t &p : (*it)->processes) {
             if (p->pid == pid) {
-                return job.get();
+                job_pos = it - job_list.begin();
+                return (*it).get();
             }
         }
     }

src/parser.h

@@ -422,7 +422,9 @@ class parser_t : public std::enable_shared_from_this<parser_t> {
     maybe_t<wcstring> get_function_name(int level = 1);
 
     /// Promotes a job to the front of the list.
-    void job_promote(job_t *job);
+    void job_promote(job_list_t::iterator job_it);
+    void job_promote(const job_t *job);
+    void job_promote_at(size_t job_pos);
 
     /// Return the job with the specified job id. If id is 0 or less, return the last job used.
     const job_t *job_with_id(job_id_t job_id) const;
@@ -430,6 +432,9 @@ class parser_t : public std::enable_shared_from_this<parser_t> {
     /// Returns the job with the given pid.
     job_t *job_get_from_pid(pid_t pid) const;
 
+    /// Returns the job and position with the given pid.
+    job_t *job_get_from_pid(int64_t pid, size_t& job_pos) const;
+
     /// Returns a new profile item if profiling is active. The caller should fill it in.
     /// The parser_t will deallocate it.
     /// If profiling is not active, this returns nullptr.

src/proc.cpp

@@ -176,6 +176,14 @@ RustFFIProcList job_t::ffi_processes() const {
     return RustFFIProcList{const_cast<process_ptr_t *>(processes.data()), processes.size()};
 }
 
+const job_group_t& job_t::ffi_group() const {
+    return *group;
+}
+
+bool job_t::ffi_resume() const {
+    return const_cast<job_t*>(this)->resume();
+}
+
 void internal_proc_t::mark_exited(proc_status_t status) {
     assert(!exited() && "Process is already exited");
     status_.store(status, std::memory_order_relaxed);

src/proc.h

@@ -540,6 +540,14 @@ class job_t : noncopyable_t {
 
     /// autocxx junk.
     RustFFIProcList ffi_processes() const;
+
+    /// autocxx junk.
+    const job_group_t &ffi_group() const;
+
+    /// autocxx junk.
+    /// The const is a lie and is only necessary since at the moment cxx's SharedPtr doesn't support
+    /// getting a mutable reference.
+    bool ffi_resume() const;
 };
 using job_ref_t = std::shared_ptr<job_t>;