From 17c1fa9d648cd0eafa1013cdb76d5f1538f09068 Mon Sep 17 00:00:00 2001 From: Clemens Wasser Date: Tue, 28 Feb 2023 23:42:12 +0100 Subject: [PATCH] Port bg builtin to Rust (#9621) * bg: Port bg builtin to Rust --- CMakeLists.txt | 3 +- fish-rust/src/builtins/bg.rs | 139 +++++++++++++++++++++++++++++++ fish-rust/src/builtins/mod.rs | 1 + fish-rust/src/builtins/shared.rs | 1 + src/builtin.cpp | 6 +- src/builtin.h | 1 + src/builtins/bg.cpp | 107 ------------------------ src/builtins/bg.h | 11 --- src/parser.cpp | 26 ++++-- src/parser.h | 7 +- src/proc.cpp | 8 ++ src/proc.h | 8 ++ 12 files changed, 189 insertions(+), 129 deletions(-) create mode 100644 fish-rust/src/builtins/bg.rs delete mode 100644 src/builtins/bg.cpp delete mode 100644 src/builtins/bg.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 3fbda66a7..ace81bac0 100644 --- a/CMakeLists.txt +++ b/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 diff --git a/fish-rust/src/builtins/bg.rs b/fish-rust/src/builtins/bg.rs new file mode 100644 index 000000000..65026f5cb --- /dev/null +++ b/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 { + 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 { + 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 = 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; +} diff --git a/fish-rust/src/builtins/mod.rs b/fish-rust/src/builtins/mod.rs index eda5ab03e..9d2b3265e 100644 --- a/fish-rust/src/builtins/mod.rs +++ b/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; diff --git a/fish-rust/src/builtins/shared.rs b/fish-rust/src/builtins/shared.rs index 7ae629a75..9f6719a56 100644 --- a/fish-rust/src/builtins/shared.rs +++ b/fish-rust/src/builtins/shared.rs @@ -119,6 +119,7 @@ pub fn run_builtin( ) -> Option { 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), diff --git a/src/builtin.cpp b/src/builtin.cpp index 2d7aab1c7..bca191139 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -30,7 +30,6 @@ #include #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 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; } diff --git a/src/builtin.h b/src/builtin.h index cb71578a8..5054fa770 100644 --- a/src/builtin.h +++ b/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, diff --git a/src/builtins/bg.cpp b/src/builtins/bg.cpp deleted file mode 100644 index 9a9de959a..000000000 --- a/src/builtins/bg.cpp +++ /dev/null @@ -1,107 +0,0 @@ -// Implementation of the bg builtin. -#include "config.h" // IWYU pragma: keep - -#include "bg.h" - -#include - -#include -#include -#include -#include - -#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 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 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; -} diff --git a/src/builtins/bg.h b/src/builtins/bg.h deleted file mode 100644 index cc4e857bd..000000000 --- a/src/builtins/bg.h +++ /dev/null @@ -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 builtin_bg(parser_t &parser, io_streams_t &streams, const wchar_t **argv); -#endif diff --git a/src/parser.cpp b/src/parser.cpp index 382f28173..c3f319cf4 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -458,7 +458,12 @@ void parser_t::job_add(shared_ptr 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(); } } } diff --git a/src/parser.h b/src/parser.h index 496d04ee7..0636f8d13 100644 --- a/src/parser.h +++ b/src/parser.h @@ -422,7 +422,9 @@ class parser_t : public std::enable_shared_from_this { maybe_t 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 { /// 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. diff --git a/src/proc.cpp b/src/proc.cpp index eb4a18acb..ae6253606 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -176,6 +176,14 @@ RustFFIProcList job_t::ffi_processes() const { return RustFFIProcList{const_cast(processes.data()), processes.size()}; } +const job_group_t& job_t::ffi_group() const { + return *group; +} + +bool job_t::ffi_resume() const { + return const_cast(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); diff --git a/src/proc.h b/src/proc.h index b74096083..5e81b9a77 100644 --- a/src/proc.h +++ b/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;