Merge pull request #31456 from azat/client-editor

Fix waiting of the editor during interactive query editing

base/base/ReplxxLineReader.cpp

@@ -25,6 +25,16 @@ void trim(String & s)
     s.erase(std::find_if(s.rbegin(), s.rend(), [](int ch) { return !std::isspace(ch); }).base(), s.end());
 }
 
+std::string getEditor()
+{
+    const char * editor = std::getenv("EDITOR");
+
+    if (!editor || !*editor)
+        editor = "vim";
+
+    return editor;
+}
+
 /// Copied from replxx::src/util.cxx::now_ms_str() under the terms of 3-clause BSD license of Replxx.
 /// Copyright (c) 2017-2018, Marcin Konarski (amok at codestation.org)
 /// Copyright (c) 2010, Salvatore Sanfilippo (antirez at gmail dot com)
@@ -123,6 +133,7 @@ ReplxxLineReader::ReplxxLineReader(
     Patterns delimiters_,
     replxx::Replxx::highlighter_callback_t highlighter_)
     : LineReader(history_file_path_, multiline_, std::move(extenders_), std::move(delimiters_)), highlighter(std::move(highlighter_))
+    , editor(getEditor())
 {
     using namespace std::placeholders;
     using Replxx = replxx::Replxx;
@@ -236,14 +247,13 @@ void ReplxxLineReader::addToHistory(const String & line)
         rx.print("Unlock of history file failed: %s\n", errnoToString(errno).c_str());
 }
 
-int ReplxxLineReader::execute(const std::string & command)
+/// See comments in ShellCommand::executeImpl()
+/// (for the vfork via dlsym())
+int ReplxxLineReader::executeEditor(const std::string & path)
 {
-    std::vector<char> argv0("sh", &("sh"[3]));
+    std::vector<char> argv0(editor.data(), editor.data() + editor.size() + 1);
-    std::vector<char> argv1("-c", &("-c"[3]));
+    std::vector<char> argv1(path.data(), path.data() + path.size() + 1);
-    std::vector<char> argv2(command.data(), command.data() + command.size() + 1);
+    char * const argv[] = {argv0.data(), argv1.data(), nullptr};
-
-    const char * filename = "/bin/sh";
-    char * const argv[] = {argv0.data(), argv1.data(), argv2.data(), nullptr};
 
     static void * real_vfork = dlsym(RTLD_DEFAULT, "vfork");
     if (!real_vfork)
@@ -260,6 +270,7 @@ int ReplxxLineReader::execute(const std::string & command)
         return -1;
     }
 
+    /// Child
     if (0 == pid)
     {
         sigset_t mask;
@@ -267,16 +278,26 @@ int ReplxxLineReader::execute(const std::string & command)
         sigprocmask(0, nullptr, &mask);
         sigprocmask(SIG_UNBLOCK, &mask, nullptr);
 
-        execv(filename, argv);
+        execvp(editor.c_str(), argv);
+        rx.print("Cannot execute %s: %s\n", editor.c_str(), errnoToString(errno).c_str());
         _exit(-1);
     }
 
     int status = 0;
-    if (-1 == waitpid(pid, &status, 0))
+    do
     {
+        int exited_pid = waitpid(pid, &status, 0);
+        if (exited_pid == -1)
+        {
+            if (errno == EINTR)
+                continue;
+
             rx.print("Cannot waitpid: %s\n", errnoToString(errno).c_str());
             return -1;
         }
+        else
+            break;
+    } while (true);
     return status;
 }
 
@@ -290,10 +311,6 @@ void ReplxxLineReader::openEditor()
         return;
     }
 
-    const char * editor = std::getenv("EDITOR");
-    if (!editor || !*editor)
-        editor = "vim";
-
     replxx::Replxx::State state(rx.get_state());
 
     size_t bytes_written = 0;
@@ -316,7 +333,7 @@ void ReplxxLineReader::openEditor()
         return;
     }
 
-    if (0 == execute(fmt::format("{} {}", editor, filename)))
+    if (0 == executeEditor(filename))
     {
         try
         {

base/base/ReplxxLineReader.h

@@ -22,7 +22,7 @@ public:
 private:
     InputStatus readOneLine(const String & prompt) override;
     void addToHistory(const String & line) override;
-    int execute(const std::string & command);
+    int executeEditor(const std::string & path);
     void openEditor();
 
     replxx::Replxx rx;
@@ -31,4 +31,6 @@ private:
     // used to call flock() to synchronize multiple clients using same history file
     int history_file_fd = -1;
     bool bracketed_paste_enabled = false;
+
+    std::string editor;
 };

tests/queries/0_stateless/01610_client_spawn_editor.sh

@@ -7,7 +7,7 @@ match_max 100000
 
 if ![info exists env(CLICKHOUSE_PORT_TCP)] {set env(CLICKHOUSE_PORT_TCP) 9000}
 
-set env(EDITOR) [file dirname [file normalize [info script]]]"/01610_client_spawn_editor_open.editor"
+set env(EDITOR) [file dirname [file normalize [info script]]]/01610_client_spawn_editor_open.editor
 
 spawn clickhouse-client --disable_suggestion
 expect ":) "