]> git.ipfire.org Git - thirdparty/git.git/commitdiff
t/helper/simple-ipc: convert test-simple-ipc to use start_bg_command
authorJeff Hostetler <jeffhost@microsoft.com>
Mon, 20 Sep 2021 15:36:18 +0000 (15:36 +0000)
committerJunio C Hamano <gitster@pobox.com>
Mon, 20 Sep 2021 15:57:58 +0000 (08:57 -0700)
Convert test helper to use `start_bg_command()` when spawning a server
daemon in the background rather than blocks of platform-specific code.

Also, while here, remove _() translation around error messages since
this is a test helper and not Git code.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/helper/test-simple-ipc.c

index 9134518075097ec2f7572d31be64361f6133f6a7..28365ff85b69bbda4ca47b5284b2cbd7bebea9e9 100644 (file)
@@ -9,6 +9,7 @@
 #include "parse-options.h"
 #include "thread-utils.h"
 #include "strvec.h"
+#include "run-command.h"
 
 #ifndef SUPPORTS_SIMPLE_IPC
 int cmd__simple_ipc(int argc, const char **argv)
@@ -267,185 +268,71 @@ static int daemon__run_server(void)
         */
        ret = ipc_server_run(cl_args.path, &opts, test_app_cb, (void*)&my_app_data);
        if (ret == -2)
-               error(_("socket/pipe already in use: '%s'"), cl_args.path);
+               error("socket/pipe already in use: '%s'", cl_args.path);
        else if (ret == -1)
-               error_errno(_("could not start server on: '%s'"), cl_args.path);
+               error_errno("could not start server on: '%s'", cl_args.path);
 
        return ret;
 }
 
-#ifndef GIT_WINDOWS_NATIVE
-/*
- * This is adapted from `daemonize()`.  Use `fork()` to directly create and
- * run the daemon in a child process.
- */
-static int spawn_server(pid_t *pid)
-{
-       struct ipc_server_opts opts = {
-               .nr_threads = cl_args.nr_threads,
-       };
+static start_bg_wait_cb bg_wait_cb;
 
-       *pid = fork();
-
-       switch (*pid) {
-       case 0:
-               if (setsid() == -1)
-                       error_errno(_("setsid failed"));
-               close(0);
-               close(1);
-               close(2);
-               sanitize_stdfds();
+static int bg_wait_cb(const struct child_process *cp, void *cb_data)
+{
+       int s = ipc_get_active_state(cl_args.path);
 
-               return ipc_server_run(cl_args.path, &opts, test_app_cb,
-                                     (void*)&my_app_data);
+       switch (s) {
+       case IPC_STATE__LISTENING:
+               /* child is "ready" */
+               return 0;
 
-       case -1:
-               return error_errno(_("could not spawn daemon in the background"));
+       case IPC_STATE__NOT_LISTENING:
+       case IPC_STATE__PATH_NOT_FOUND:
+               /* give child more time */
+               return 1;
 
        default:
-               return 0;
+       case IPC_STATE__INVALID_PATH:
+       case IPC_STATE__OTHER_ERROR:
+               /* all the time in world won't help */
+               return -1;
        }
 }
-#else
-/*
- * Conceptually like `daemonize()` but different because Windows does not
- * have `fork(2)`.  Spawn a normal Windows child process but without the
- * limitations of `start_command()` and `finish_command()`.
- */
-static int spawn_server(pid_t *pid)
-{
-       char test_tool_exe[MAX_PATH];
-       struct strvec args = STRVEC_INIT;
-       int in, out;
-
-       GetModuleFileNameA(NULL, test_tool_exe, MAX_PATH);
-
-       in = open("/dev/null", O_RDONLY);
-       out = open("/dev/null", O_WRONLY);
-
-       strvec_push(&args, test_tool_exe);
-       strvec_push(&args, "simple-ipc");
-       strvec_push(&args, "run-daemon");
-       strvec_pushf(&args, "--name=%s", cl_args.path);
-       strvec_pushf(&args, "--threads=%d", cl_args.nr_threads);
-
-       *pid = mingw_spawnvpe(args.v[0], args.v, NULL, NULL, in, out, out);
-       close(in);
-       close(out);
-
-       strvec_clear(&args);
 
-       if (*pid < 0)
-               return error(_("could not spawn daemon in the background"));
-
-       return 0;
-}
-#endif
-
-/*
- * This is adapted from `wait_or_whine()`.  Watch the child process and
- * let it get started and begin listening for requests on the socket
- * before reporting our success.
- */
-static int wait_for_server_startup(pid_t pid_child)
+static int daemon__start_server(void)
 {
-       int status;
-       pid_t pid_seen;
-       enum ipc_active_state s;
-       time_t time_limit, now;
+       struct child_process cp = CHILD_PROCESS_INIT;
+       enum start_bg_result sbgr;
 
-       time(&time_limit);
-       time_limit += cl_args.max_wait_sec;
+       strvec_push(&cp.args, "test-tool");
+       strvec_push(&cp.args, "simple-ipc");
+       strvec_push(&cp.args, "run-daemon");
+       strvec_pushf(&cp.args, "--name=%s", cl_args.path);
+       strvec_pushf(&cp.args, "--threads=%d", cl_args.nr_threads);
 
-       for (;;) {
-               pid_seen = waitpid(pid_child, &status, WNOHANG);
+       cp.no_stdin = 1;
+       cp.no_stdout = 1;
+       cp.no_stderr = 1;
 
-               if (pid_seen == -1)
-                       return error_errno(_("waitpid failed"));
+       sbgr = start_bg_command(&cp, bg_wait_cb, NULL, cl_args.max_wait_sec);
 
-               else if (pid_seen == 0) {
-                       /*
-                        * The child is still running (this should be
-                        * the normal case).  Try to connect to it on
-                        * the socket and see if it is ready for
-                        * business.
-                        *
-                        * If there is another daemon already running,
-                        * our child will fail to start (possibly
-                        * after a timeout on the lock), but we don't
-                        * care (who responds) if the socket is live.
-                        */
-                       s = ipc_get_active_state(cl_args.path);
-                       if (s == IPC_STATE__LISTENING)
-                               return 0;
-
-                       time(&now);
-                       if (now > time_limit)
-                               return error(_("daemon not online yet"));
-
-                       continue;
-               }
+       switch (sbgr) {
+       case SBGR_READY:
+               return 0;
 
-               else if (pid_seen == pid_child) {
-                       /*
-                        * The new child daemon process shutdown while
-                        * it was starting up, so it is not listening
-                        * on the socket.
-                        *
-                        * Try to ping the socket in the odd chance
-                        * that another daemon started (or was already
-                        * running) while our child was starting.
-                        *
-                        * Again, we don't care who services the socket.
-                        */
-                       s = ipc_get_active_state(cl_args.path);
-                       if (s == IPC_STATE__LISTENING)
-                               return 0;
+       default:
+       case SBGR_ERROR:
+       case SBGR_CB_ERROR:
+               return error("daemon failed to start");
 
-                       /*
-                        * We don't care about the WEXITSTATUS() nor
-                        * any of the WIF*(status) values because
-                        * `cmd__simple_ipc()` does the `!!result`
-                        * trick on all function return values.
-                        *
-                        * So it is sufficient to just report the
-                        * early shutdown as an error.
-                        */
-                       return error(_("daemon failed to start"));
-               }
+       case SBGR_TIMEOUT:
+               return error("daemon not online yet");
 
-               else
-                       return error(_("waitpid is confused"));
+       case SBGR_DIED:
+               return error("daemon terminated");
        }
 }
 
-/*
- * This process will start a simple-ipc server in a background process and
- * wait for it to become ready.  This is like `daemonize()` but gives us
- * more control and better error reporting (and makes it easier to write
- * unit tests).
- */
-static int daemon__start_server(void)
-{
-       pid_t pid_child;
-       int ret;
-
-       /*
-        * Run the actual daemon in a background process.
-        */
-       ret = spawn_server(&pid_child);
-       if (pid_child <= 0)
-               return ret;
-
-       /*
-        * Let the parent wait for the child process to get started
-        * and begin listening for requests on the socket.
-        */
-       ret = wait_for_server_startup(pid_child);
-
-       return ret;
-}
-
 /*
  * This process will run a quick probe to see if a simple-ipc server
  * is active on this path.
@@ -548,7 +435,7 @@ static int client__stop_server(void)
 
                time(&now);
                if (now > time_limit)
-                       return error(_("daemon has not shutdown yet"));
+                       return error("daemon has not shutdown yet");
        }
 }