On 3/2/21 4:44 AM, Jeff King wrote:
On Wed, Feb 17, 2021 at 09:48:48PM +0000, Jeff Hostetler via GitGitGadget wrote:
Create t/helper/test-simple-ipc test tool to exercise the "simple-ipc"
functions.
BTW, one oddity I noticed in this (because of my -Wunused-parameters
branch):
+#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(const char *path,
+ const struct ipc_server_opts *opts,
+ pid_t *pid)
+{
+ *pid = fork();
+
+ switch (*pid) {
+ case 0:
+ if (setsid() == -1)
+ error_errno(_("setsid failed"));
+ close(0);
+ close(1);
+ close(2);
+ sanitize_stdfds();
+
+ return ipc_server_run(path, opts, test_app_cb, (void*)&my_app_data);
+
+ case -1:
+ return error_errno(_("could not spawn daemon in the background"));
+
+ default:
+ return 0;
+ }
+}
In the non-Windows version, we spawn a server using the "path" parameter
we got from the caller.
But in the Windows version:
+#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(const char *path,
+ const struct ipc_server_opts *opts,
+ 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, "--threads=%d", opts->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
We ignore the "path" parameter entirely. Should we be passing it along
as an option to the child process? I think it doesn't really matter at
this point because both the parent and child processes will use the
hard-coded string "ipc-test", but it seems like something the test
script might want to be able to specify.
-Peff
Yeah, since it was a test helper I hesitated to add a command line
arg to pass it to the child process (when all callers were right here
and using the same default value). However it would be good to do so
in case we want to write more complicated tests.
Jeff
