diff --git a/Userland/DevTools/HackStudio/HackStudioWidget.cpp b/Userland/DevTools/HackStudio/HackStudioWidget.cpp index a86e7da8e6..b10bbc7364 100644 --- a/Userland/DevTools/HackStudio/HackStudioWidget.cpp +++ b/Userland/DevTools/HackStudio/HackStudioWidget.cpp @@ -1341,7 +1341,8 @@ NonnullRefPtr HackStudioWidget::create_stop_action() { auto action = GUI::Action::create("&Stop", Gfx::Bitmap::try_load_from_file("/res/icons/16x16/program-stop.png").release_value_but_fixme_should_propagate_errors(), [this](auto&) { if (!Debugger::the().session()) { - m_terminal_wrapper->kill_running_command(); + if (auto result = m_terminal_wrapper->kill_running_command(); result.is_error()) + warnln("{}", result.error()); return; } diff --git a/Userland/DevTools/HackStudio/ProjectBuilder.cpp b/Userland/DevTools/HackStudio/ProjectBuilder.cpp index 2148f43c86..25124923a1 100644 --- a/Userland/DevTools/HackStudio/ProjectBuilder.cpp +++ b/Userland/DevTools/HackStudio/ProjectBuilder.cpp @@ -28,11 +28,12 @@ ErrorOr ProjectBuilder::build(StringView active_file) return Error::from_string_literal("no active file"sv); if (active_file.ends_with(".js")) { - m_terminal->run_command(String::formatted("js -A {}", active_file)); + TRY(m_terminal->run_command(String::formatted("js -A {}", active_file))); return {}; } + if (m_is_serenity == IsSerenityRepo::No) { - m_terminal->run_command("make"); + TRY(m_terminal->run_command("make")); return {}; } @@ -47,12 +48,12 @@ ErrorOr ProjectBuilder::run(StringView active_file) return Error::from_string_literal("no active file"sv); if (active_file.ends_with(".js")) { - m_terminal->run_command(String::formatted("js {}", active_file)); + TRY(m_terminal->run_command(String::formatted("js {}", active_file))); return {}; } if (m_is_serenity == IsSerenityRepo::No) { - m_terminal->run_command("make run"); + TRY(m_terminal->run_command("make run")); return {}; } @@ -64,7 +65,7 @@ ErrorOr ProjectBuilder::run(StringView active_file) ErrorOr ProjectBuilder::run_serenity_component() { auto relative_path_to_dir = LexicalPath::relative_path(LexicalPath::dirname(m_serenity_component_cmake_file), m_project_root); - m_terminal->run_command(LexicalPath::join(relative_path_to_dir, m_serenity_component_name).string(), build_directory()); + TRY(m_terminal->run_command(LexicalPath::join(relative_path_to_dir, m_serenity_component_name).string(), build_directory())); return {}; } @@ -89,10 +90,8 @@ ErrorOr ProjectBuilder::update_active_file(StringView active_file) ErrorOr ProjectBuilder::build_serenity_component() { - m_terminal->run_command(String::formatted("make {}", m_serenity_component_name), build_directory(), TerminalWrapper::WaitForExit::Yes); - if (m_terminal->child_exit_status() == 0) - return {}; - return Error::from_string_literal("Make failed"sv); + TRY(m_terminal->run_command(String::formatted("make {}", m_serenity_component_name), build_directory(), TerminalWrapper::WaitForExit::Yes, "Make failed"sv)); + return {}; } ErrorOr ProjectBuilder::component_name(StringView cmake_file_path) @@ -122,14 +121,12 @@ ErrorOr ProjectBuilder::initialize_build_directory() auto cmake_file = TRY(Core::File::open(cmake_file_path, Core::OpenMode::WriteOnly)); cmake_file->write(generate_cmake_file_content()); - m_terminal->run_command(String::formatted("cmake -S {} -DHACKSTUDIO_BUILD=ON -DHACKSTUDIO_BUILD_CMAKE_FILE={}" - " -DENABLE_UNICODE_DATABASE_DOWNLOAD=OFF", - m_project_root, cmake_file_path), - build_directory(), TerminalWrapper::WaitForExit::Yes); + TRY(m_terminal->run_command(String::formatted("cmake -S {} -DHACKSTUDIO_BUILD=ON -DHACKSTUDIO_BUILD_CMAKE_FILE={}" + " -DENABLE_UNICODE_DATABASE_DOWNLOAD=OFF", + m_project_root, cmake_file_path), + build_directory(), TerminalWrapper::WaitForExit::Yes, "CMake error"sv)); - if (m_terminal->child_exit_status() == 0) - return {}; - return Error::from_string_literal("CMake error"sv); + return {}; } Optional ProjectBuilder::find_cmake_file_for(StringView file_path) const diff --git a/Userland/DevTools/HackStudio/TerminalWrapper.cpp b/Userland/DevTools/HackStudio/TerminalWrapper.cpp index e45d087b35..de85ad006d 100644 --- a/Userland/DevTools/HackStudio/TerminalWrapper.cpp +++ b/Userland/DevTools/HackStudio/TerminalWrapper.cpp @@ -7,7 +7,7 @@ #include "TerminalWrapper.h" #include -#include +#include #include #include #include @@ -24,52 +24,41 @@ namespace HackStudio { -void TerminalWrapper::run_command(const String& command, Optional working_directory, WaitForExit wait_for_exit) +ErrorOr TerminalWrapper::run_command(const String& command, Optional working_directory, WaitForExit wait_for_exit, Optional failure_message) { if (m_pid != -1) { GUI::MessageBox::show(window(), "A command is already running in this TerminalWrapper", "Can't run command", GUI::MessageBox::Type::Error); - return; + return {}; } - auto ptm_res = setup_master_pseudoterminal(); - if (ptm_res.is_error()) { - perror("setup_master_pseudoterminal"); - return; - } + auto ptm_fd = TRY(setup_master_pseudoterminal()); - int ptm_fd = ptm_res.value(); m_child_exited = false; m_child_exit_status.clear(); - m_pid = fork(); - if (m_pid < 0) { - perror("fork"); - return; - } + m_pid = TRY(Core::System::fork()); if (m_pid > 0) { if (wait_for_exit == WaitForExit::Yes) { GUI::Application::the()->event_loop().spin_until([this]() { return m_child_exited; }); + + VERIFY(m_child_exit_status.has_value()); + if (m_child_exit_status.value() != 0) + return Error::from_string_literal(failure_message.value_or("Command execution failed"sv)); } - return; + + return {}; } - if (working_directory.has_value()) { - if (chdir(working_directory->characters())) { - perror("chdir"); - exit(1); - } - } + if (working_directory.has_value()) + TRY(Core::System::chdir(working_directory->view())); - if (setup_slave_pseudoterminal(ptm_fd).is_error()) { - perror("setup_pseudoterminal"); - exit(1); - } + TRY(setup_slave_pseudoterminal(ptm_fd)); auto parts = command.split(' '); VERIFY(!parts.is_empty()); @@ -77,6 +66,7 @@ void TerminalWrapper::run_command(const String& command, Optional workin for (size_t i = 0; i < parts.size(); i++) { args[i] = parts[i].characters(); } + auto rc = execvp(args[0], const_cast(args)); if (rc < 0) { perror("execve"); @@ -87,29 +77,28 @@ void TerminalWrapper::run_command(const String& command, Optional workin ErrorOr TerminalWrapper::setup_master_pseudoterminal(WaitForChildOnExit wait_for_child) { - int ptm_fd = posix_openpt(O_RDWR | O_CLOEXEC); - if (ptm_fd < 0) { - perror("posix_openpt"); - VERIFY_NOT_REACHED(); - } - if (grantpt(ptm_fd) < 0) { - perror("grantpt"); - VERIFY_NOT_REACHED(); - } - if (unlockpt(ptm_fd) < 0) { - perror("unlockpt"); - VERIFY_NOT_REACHED(); - } + int ptm_fd = TRY(Core::System::posix_openpt(O_RDWR | O_CLOEXEC)); + bool error_happened = true; + + ScopeGuard close_ptm { [&]() { + if (error_happened) { + if (auto result = Core::System::close(ptm_fd); result.is_error()) + warnln("{}", result.release_error()); + } + } }; + + TRY(Core::System::grantpt(ptm_fd)); + TRY(Core::System::unlockpt(ptm_fd)); m_terminal_widget->set_pty_master_fd(ptm_fd); m_terminal_widget->on_command_exit = [this, wait_for_child] { if (wait_for_child == WaitForChildOnExit::Yes) { - int wstatus; - int rc = waitpid(m_pid, &wstatus, 0); - if (rc < 0) { - perror("waitpid"); + auto result = Core::System::waitpid(m_pid, 0); + if (result.is_error()) { + warnln("{}", result.error()); VERIFY_NOT_REACHED(); } + int wstatus = result.release_value().status; if (WIFEXITED(wstatus)) { m_terminal_widget->inject_string(String::formatted("\033[{};1m(Command exited with code {})\033[0m\r\n", wstatus == 0 ? 32 : 31, WEXITSTATUS(wstatus))); @@ -130,6 +119,8 @@ ErrorOr TerminalWrapper::setup_master_pseudoterminal(WaitForChildOnExit wai terminal().scroll_to_bottom(); + error_happened = false; + return ptm_fd; } @@ -137,55 +128,41 @@ ErrorOr TerminalWrapper::setup_slave_pseudoterminal(int master_fd) { setsid(); - const char* tty_name = ptsname(master_fd); - if (!tty_name) - return Error::from_errno(errno); + auto tty_name = TRY(Core::System::ptsname(master_fd)); close(master_fd); - int pts_fd = open(tty_name, O_RDWR); - if (pts_fd < 0) - return Error::from_errno(errno); + + int pts_fd = TRY(Core::System::open(tty_name, O_RDWR)); tcsetpgrp(pts_fd, getpid()); // NOTE: It's okay if this fails. - int rc = ioctl(0, TIOCNOTTY); + ioctl(0, TIOCNOTTY); close(0); close(1); close(2); - rc = dup2(pts_fd, 0); - if (rc < 0) - return Error::from_errno(errno); + TRY(Core::System::dup2(pts_fd, 0)); + TRY(Core::System::dup2(pts_fd, 1)); + TRY(Core::System::dup2(pts_fd, 2)); - rc = dup2(pts_fd, 1); - if (rc < 0) - return Error::from_errno(errno); + TRY(Core::System::close(pts_fd)); - rc = dup2(pts_fd, 2); - if (rc < 0) - return Error::from_errno(errno); - - rc = close(pts_fd); - if (rc < 0) - return Error::from_errno(errno); - - rc = ioctl(0, TIOCSCTTY); - if (rc < 0) - return Error::from_errno(errno); + TRY(Core::System::ioctl(0, TIOCSCTTY)); setenv("TERM", "xterm", true); return {}; } -void TerminalWrapper::kill_running_command() +ErrorOr TerminalWrapper::kill_running_command() { VERIFY(m_pid != -1); // Kill our child process and its whole process group. - [[maybe_unused]] auto rc = killpg(m_pid, SIGTERM); + TRY(Core::System::killpg(m_pid, SIGTERM)); + return {}; } void TerminalWrapper::clear_including_history() @@ -199,9 +176,11 @@ TerminalWrapper::TerminalWrapper(bool user_spawned) set_layout(); m_terminal_widget = add(-1, false); - - if (user_spawned) - run_command("Shell"); + if (user_spawned) { + auto maybe_error = run_command("Shell"); + if (maybe_error.is_error()) + warnln("{}", maybe_error.release_error()); + } } int TerminalWrapper::child_exit_status() const diff --git a/Userland/DevTools/HackStudio/TerminalWrapper.h b/Userland/DevTools/HackStudio/TerminalWrapper.h index d06e213175..128aff05e2 100644 --- a/Userland/DevTools/HackStudio/TerminalWrapper.h +++ b/Userland/DevTools/HackStudio/TerminalWrapper.h @@ -21,8 +21,8 @@ public: No, Yes }; - void run_command(const String&, Optional working_directory = {}, WaitForExit = WaitForExit::No); - void kill_running_command(); + ErrorOr run_command(const String&, Optional working_directory = {}, WaitForExit = WaitForExit::No, Optional failure_message = {}); + ErrorOr kill_running_command(); void clear_including_history(); bool user_spawned() const { return m_user_spawned; }