From 592408fa43ef85ebd7de8255db93f8529b385045 Mon Sep 17 00:00:00 2001 From: Yannic Charlon Date: Sun, 18 Oct 2026 20:33:02 +0200 Subject: [PATCH] run-command: add an opt-in posix_spawn backend on macOS Platforms that can start child programs but cannot duplicate the parent process cannot use the POSIX start_command() path. It always calls fork(), even when the platform supplies a spawn API. Add USE_POSIX_SPAWN as an opt-in macOS prototype. Keep the existing fork backend as the default. Reuse command preparation, environment deltas and child cleanup, and translate the child setup into spawn file actions and attributes. Preserve the ENOEXEC shell fallback and the synchronous error reporting expected by start_command() callers. The macOS implementation provides a concrete place to discuss the backend boundary before adding another target. It relies on macOS spawn extensions for cwd and descriptor inheritance. It is not a WASM port: NO_PTHREADS start_async() still calls fork(), and no wasm child process has been run with this change. The RFC question is whether a posix_spawn backend is the right direction for platforms without fork, or whether Git would prefer a small platform hook like the existing Windows launch path. With the opt-in backend, t0061 preloads a macOS test library that makes fork() return ENOSYS. A canary requires that failure before the launch, cwd, environment, pipe, error and fallback cases run. The tests call binaries directly because macOS strips DYLD_* from shell wrappers. On macOS arm64, DEVELOPER=1 builds and t0061 pass with both backends: 26 cases, with three MINGW skips for spawn and an additional canary skip for the default backend. Without the test library, the canary fails as expected. A separate external-alias control also fails on the default backend when fork() returns ENOSYS. Meson keeps the prototype disabled. Provide its empty build-option substitution so the shared template also configures under Meson's fatal-warning checks. Meson 1.10.1 configuration passes with those checks. AI assistance: GPT-6.1 Sol and GPT-6 Luna through the Codex harness. Signed-off-by: Yannic Charlon --- GIT-BUILD-OPTIONS.in | 1 + Makefile | 23 ++- meson.build | 1 + run-command.c | 244 +++++++++++++++++++++++++++++++- t/helper/.gitignore | 1 + t/helper/test-fork-interposer.c | 17 +++ t/helper/test-run-command.c | 45 ++++++ t/t0061-run-command.sh | 29 ++++ 8 files changed, 357 insertions(+), 4 deletions(-) create mode 100644 t/helper/test-fork-interposer.c diff --git a/GIT-BUILD-OPTIONS.in b/GIT-BUILD-OPTIONS.in index 0a9884e0ade556..19d5e8d410c400 100644 --- a/GIT-BUILD-OPTIONS.in +++ b/GIT-BUILD-OPTIONS.in @@ -44,5 +44,6 @@ TEST_OUTPUT_DIRECTORY=@TEST_OUTPUT_DIRECTORY@ TEST_SHELL_PATH=@TEST_SHELL_PATH@ USE_GETTEXT_SCHEME=@USE_GETTEXT_SCHEME@ USE_LIBPCRE2=@USE_LIBPCRE2@ +USE_POSIX_SPAWN=@USE_POSIX_SPAWN@ WITH_BREAKING_CHANGES=@WITH_BREAKING_CHANGES@ X=@X@ diff --git a/Makefile b/Makefile index a96be506b59f2d..842bf4b2743b82 100644 --- a/Makefile +++ b/Makefile @@ -149,6 +149,9 @@ include shared.mak # # Define NO_PTHREADS if you do not have or do not want to use Pthreads. # +# Define USE_POSIX_SPAWN to use posix_spawn() for start_command(). This is +# currently supported on macOS only. +# # Define NO_PREAD if you have a problem with pread() system call (e.g. # cygwin1.dll before v1.5.22). # @@ -889,6 +892,10 @@ TEST_PROGRAMS_NEED_X += test-tool TEST_PROGRAMS = $(patsubst %,t/helper/%$X,$(TEST_PROGRAMS_NEED_X)) +ifdef USE_POSIX_SPAWN +TEST_HELPER_LIBS += t/helper/test-fork-interposer.dylib +endif + # List built-in command $C whose implementation cmd_$C() is not in # builtin/$C.o but is linked in as part of some other command. BUILT_INS += $(patsubst builtin/%.o,git-%$X,$(BUILTIN_OBJS)) @@ -2247,6 +2254,10 @@ else EXTLIBS += $(PTHREAD_LIBS) endif +ifdef USE_POSIX_SPAWN + BASIC_CFLAGS += -DUSE_POSIX_SPAWN +endif + ifdef HAVE_PATHS_H BASIC_CFLAGS += -DHAVE_PATHS_H endif @@ -3387,6 +3398,7 @@ GIT-BUILD-OPTIONS: FORCE -e "s|@TEST_SHELL_PATH@|\'$(TEST_SHELL_PATH_SQ)\'|" \ -e "s|@USE_GETTEXT_SCHEME@|\'$(USE_GETTEXT_SCHEME)\'|" \ -e "s|@USE_LIBPCRE2@|\'$(USE_LIBPCRE2)\'|" \ + -e "s|@USE_POSIX_SPAWN@|\'$(USE_POSIX_SPAWN)\'|" \ -e "s|@WITH_BREAKING_CHANGES@|\'$(WITH_BREAKING_CHANGES)\'|" \ -e "s|@X@|\'$(X)\'|" \ GIT-BUILD-OPTIONS.in >$@+ @@ -3408,7 +3420,7 @@ endif test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X)) -all:: $(TEST_PROGRAMS) $(test_bindir_programs) $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG) +all:: $(TEST_PROGRAMS) $(TEST_HELPER_LIBS) $(test_bindir_programs) $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG) $(test_bindir_programs): bin-wrappers/%: bin-wrappers/wrap-for-bin.sh $(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ @@ -3442,11 +3454,16 @@ perf: all .PRECIOUS: $(TEST_OBJS) -t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) $(UNIT_TEST_DIR)/test-lib.o +t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) $(UNIT_TEST_DIR)/test-lib.o $(TEST_HELPER_LIBS) t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS) +ifdef USE_POSIX_SPAWN +t/helper/test-fork-interposer.dylib: t/helper/test-fork-interposer.c + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -dynamiclib -o $@ $(ALL_LDFLAGS) $< +endif + check-sha1:: t/helper/test-tool$X t/helper/test-sha1.sh @@ -3918,7 +3935,7 @@ clean: profile-clean coverage-clean cocciclean $(RM) headless-git.o $(RM) $(LIB_FILE) $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) - $(RM) $(TEST_PROGRAMS) + $(RM) $(TEST_PROGRAMS) t/helper/test-fork-interposer.dylib $(RM) $(FUZZ_PROGRAMS) $(RM) $(SP_OBJ) $(RM) $(HCC) diff --git a/meson.build b/meson.build index fa104a3efda7df..e6f7f6edc3adf4 100644 --- a/meson.build +++ b/meson.build @@ -790,6 +790,7 @@ build_options_config.set('GIT_TEST_PERL_FATAL_WARNINGS', '') build_options_config.set_quoted('GIT_TEST_UTF8_LOCALE', get_option('test_utf8_locale')) build_options_config.set_quoted('LOCALEDIR', fs.as_posix(get_option('prefix') / get_option('localedir'))) build_options_config.set_quoted('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb')) +build_options_config.set('USE_POSIX_SPAWN', '') if get_option('sane_tool_path').length() != 0 sane_tool_path = (host_machine.system() == 'windows' ? ';' : ':').join(get_option('sane_tool_path')) diff --git a/run-command.c b/run-command.c index ce84db87824262..48c7a9cc8923ad 100644 --- a/run-command.c +++ b/run-command.c @@ -18,6 +18,28 @@ #include "packfile.h" #include "compat/nonblock.h" +#ifdef USE_POSIX_SPAWN +#include +#include + +#ifndef __APPLE__ +#error "USE_POSIX_SPAWN currently requires the macOS spawn file actions" +#endif + +static int spawn_add_fchdir(posix_spawn_file_actions_t *actions, int fd) +{ +#if MAC_OS_X_VERSION_MIN_REQUIRED >= 260000 + return posix_spawn_file_actions_addfchdir(actions, fd); +#else +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + int error = posix_spawn_file_actions_addfchdir_np(actions, fd); +#pragma clang diagnostic pop + return error; +#endif +} +#endif + void child_process_init(struct child_process *child) { struct child_process blank = CHILD_PROCESS_INIT; @@ -309,7 +331,9 @@ static const char **prepare_shell_cmd(struct strvec *out, const char **argv) } #ifndef GIT_WINDOWS_NATIVE +#ifndef USE_POSIX_SPAWN static int child_notifier = -1; +#endif enum child_errcode { CHILD_ERR_CHDIR, @@ -325,6 +349,7 @@ struct child_err { int syserr; /* errno */ }; +#ifndef USE_POSIX_SPAWN static void child_die(enum child_errcode err) { struct child_err buf; @@ -373,6 +398,7 @@ static void NORETURN child_die_fn(const char *err UNUSED, va_list params UNUSED) xwrite(2, msg, sizeof(msg) - 1); _exit(2); } +#endif /* this runs in the parent process */ static void child_err_spew(struct child_process *cmd, struct child_err *cerr) @@ -496,6 +522,189 @@ static char **prep_childenv(const char *const *deltaenv) return childenv; } +#ifdef USE_POSIX_SPAWN +static int spawn_add_dup2(posix_spawn_file_actions_t *actions, int from, + int to, unsigned char stdio_touched[3]) +{ + int error = posix_spawn_file_actions_adddup2(actions, from, to); + if (!error && to < 3) + stdio_touched[to] = 1; + return error; +} + +static int spawn_add_close(posix_spawn_file_actions_t *actions, int fd, + unsigned char stdio_touched[3]) +{ + int error = posix_spawn_file_actions_addclose(actions, fd); + if (!error && fd < 3) + stdio_touched[fd] = 1; + return error; +} + +static int spawn_add_open(posix_spawn_file_actions_t *actions, int fd, + unsigned char stdio_touched[3]) +{ + int error = posix_spawn_file_actions_addopen(actions, fd, "/dev/null", + O_RDWR, 0); + if (!error && fd < 3) + stdio_touched[fd] = 1; + return error; +} + +static pid_t spawn_command(const struct child_process *cmd, const char **argv, + char **childenv, int need_in, int fdin[2], + int need_out, int fdout[2], + int need_err, int fderr[2], + enum child_errcode *error_kind) +{ + posix_spawn_file_actions_t actions; + posix_spawnattr_t attr; + const posix_spawnattr_t *attrp = NULL; + unsigned char stdio_touched[3] = { 0 }; + int actions_initialized = 0, attr_initialized = 0; + sigset_t signal_mask, signal_defaults; + struct sigaction sa; + short flags = POSIX_SPAWN_SETSIGMASK | POSIX_SPAWN_SETSIGDEF; + int error; + pid_t pid = -1; + int dir_fd = -1; + int sig, fd; + + *error_kind = CHILD_ERR_ERRNO; + +#define ADD_SPAWN_ACTION(action) \ + do { \ + error = (action); \ + if (error) \ + goto done; \ + } while (0) + + error = posix_spawn_file_actions_init(&actions); + if (error) + goto done; + actions_initialized = 1; + error = posix_spawnattr_init(&attr); + if (error) + goto done; + attr_initialized = 1; + attrp = &attr; + +#ifdef NO_PTHREADS + if (sigprocmask(SIG_SETMASK, NULL, &signal_mask) < 0) { + error = errno; + goto done; + } +#else + error = pthread_sigmask(SIG_SETMASK, NULL, &signal_mask); + if (error) + goto done; +#endif + if (sigemptyset(&signal_defaults) < 0) { + error = errno; + goto done; + } + for (sig = 1; sig < NSIG; sig++) { + if (!sigaction(sig, NULL, &sa) && sa.sa_handler != SIG_IGN) + sigaddset(&signal_defaults, sig); + } + ADD_SPAWN_ACTION(posix_spawnattr_setsigmask(&attr, &signal_mask)); + ADD_SPAWN_ACTION(posix_spawnattr_setsigdefault(&attr, + &signal_defaults)); + + if (cmd->no_stdin) + ADD_SPAWN_ACTION(spawn_add_open(&actions, 0, stdio_touched)); + else if (need_in) { + ADD_SPAWN_ACTION(spawn_add_dup2(&actions, fdin[0], 0, + stdio_touched)); + ADD_SPAWN_ACTION(spawn_add_close(&actions, fdin[0], stdio_touched)); + ADD_SPAWN_ACTION(spawn_add_close(&actions, fdin[1], stdio_touched)); + } else if (cmd->in) { + ADD_SPAWN_ACTION(spawn_add_dup2(&actions, cmd->in, 0, + stdio_touched)); + ADD_SPAWN_ACTION(spawn_add_close(&actions, cmd->in, stdio_touched)); + } + + if (cmd->no_stderr) + ADD_SPAWN_ACTION(spawn_add_open(&actions, 2, stdio_touched)); + else if (need_err) { + ADD_SPAWN_ACTION(spawn_add_dup2(&actions, fderr[1], 2, + stdio_touched)); + ADD_SPAWN_ACTION(spawn_add_close(&actions, fderr[0], stdio_touched)); + ADD_SPAWN_ACTION(spawn_add_close(&actions, fderr[1], stdio_touched)); + } else if (cmd->err > 1) { + ADD_SPAWN_ACTION(spawn_add_dup2(&actions, cmd->err, 2, + stdio_touched)); + ADD_SPAWN_ACTION(spawn_add_close(&actions, cmd->err, stdio_touched)); + } + + if (cmd->no_stdout) + ADD_SPAWN_ACTION(spawn_add_open(&actions, 1, stdio_touched)); + else if (cmd->stdout_to_stderr) + ADD_SPAWN_ACTION(spawn_add_dup2(&actions, 2, 1, stdio_touched)); + else if (need_out) { + ADD_SPAWN_ACTION(spawn_add_dup2(&actions, fdout[1], 1, + stdio_touched)); + ADD_SPAWN_ACTION(spawn_add_close(&actions, fdout[0], stdio_touched)); + ADD_SPAWN_ACTION(spawn_add_close(&actions, fdout[1], stdio_touched)); + } else if (cmd->out > 1) { + ADD_SPAWN_ACTION(spawn_add_dup2(&actions, cmd->out, 1, + stdio_touched)); + ADD_SPAWN_ACTION(spawn_add_close(&actions, cmd->out, stdio_touched)); + } + + if (cmd->dir) { + dir_fd = open(cmd->dir, O_SEARCH | O_CLOEXEC); + if (dir_fd < 0) { + *error_kind = CHILD_ERR_CHDIR; + error = errno; + goto done; + } + ADD_SPAWN_ACTION(posix_spawn_file_actions_addinherit_np( + &actions, dir_fd)); + ADD_SPAWN_ACTION(spawn_add_fchdir(&actions, dir_fd)); + ADD_SPAWN_ACTION(posix_spawn_file_actions_addclose(&actions, + dir_fd)); + } + + if (cmd->close_fd_above_stderr) { + flags |= POSIX_SPAWN_CLOEXEC_DEFAULT; + + for (fd = 0; fd < 3; fd++) { + int fd_flags; + if (stdio_touched[fd]) + continue; + fd_flags = fcntl(fd, F_GETFD); + if (fd_flags < 0 || (fd_flags & FD_CLOEXEC)) + continue; + ADD_SPAWN_ACTION(posix_spawn_file_actions_addinherit_np( + &actions, fd)); + } + } + ADD_SPAWN_ACTION(posix_spawnattr_setflags(&attr, flags)); + + error = posix_spawn(&pid, argv[1], &actions, attrp, + (char *const *)argv + 1, childenv); + if (error == ENOEXEC) + error = posix_spawn(&pid, argv[0], &actions, attrp, + (char *const *)argv, childenv); + if (error) + pid = -1; + +done: + if (attr_initialized) + posix_spawnattr_destroy(&attr); + if (actions_initialized) + posix_spawn_file_actions_destroy(&actions); + if (dir_fd >= 0) + close(dir_fd); +#undef ADD_SPAWN_ACTION + if (error) + errno = error; + return pid; +} +#endif + +#ifndef USE_POSIX_SPAWN struct atfork_state { #ifndef NO_PTHREADS int cs; @@ -545,6 +754,7 @@ static void atfork_parent(struct atfork_state *as) "restoring signal mask"); #endif } +#endif #endif /* GIT_WINDOWS_NATIVE */ @@ -746,7 +956,37 @@ int start_command(struct child_process *cmd) odb_close(cmd->odb_to_close); #ifndef GIT_WINDOWS_NATIVE +#ifdef USE_POSIX_SPAWN { + struct strvec argv = STRVEC_INIT; + struct child_err spawn_error; + char **childenv; + + if (prepare_cmd(&argv, cmd) < 0) { + failed_errno = errno; + cmd->pid = -1; + if (!cmd->silent_exec_failure) + error_errno("cannot run %s", cmd->args.v[0]); + } else { + trace_argv_printf(&argv.v[1], "trace: start_command:"); + childenv = prep_childenv(cmd->env.v); + cmd->pid = spawn_command(cmd, argv.v, childenv, + need_in, fdin, need_out, fdout, + need_err, fderr, &spawn_error.err); + failed_errno = errno; + if (cmd->pid < 0) { + spawn_error.syserr = failed_errno; + if (!cmd->silent_exec_failure || failed_errno != ENOENT || + spawn_error.err == CHILD_ERR_CHDIR) + child_err_spew(cmd, &spawn_error); + } else if (cmd->clean_on_exit) + mark_child_for_cleanup(cmd->pid, cmd); + free(childenv); + } + strvec_clear(&argv); +} +#else + { int notify_pipe[2]; int null_fd = -1; char **childenv; @@ -905,8 +1145,10 @@ int start_command(struct child_process *cmd) close(null_fd); strvec_clear(&argv); free(childenv); -} end_of_spawn: +; +} +#endif #else { diff --git a/t/helper/.gitignore b/t/helper/.gitignore index 8c2ddcce95f7aa..d551a0c1e4a42e 100644 --- a/t/helper/.gitignore +++ b/t/helper/.gitignore @@ -1,2 +1,3 @@ /test-tool /test-fake-ssh +/test-fork-interposer.dylib diff --git a/t/helper/test-fork-interposer.c b/t/helper/test-fork-interposer.c new file mode 100644 index 00000000000000..36c62fed5f8b40 --- /dev/null +++ b/t/helper/test-fork-interposer.c @@ -0,0 +1,17 @@ +#include +#include +#include + +static pid_t fork_returns_enosys(void) +{ + errno = ENOSYS; + return -1; +} + +__attribute__((used)) static struct { + pid_t (*replacement)(void); + pid_t (*replacee)(void); +} interpose __attribute__((section("__DATA,__interpose"))) = { + fork_returns_enosys, + fork, +}; diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index 4a56456894ccff..a931874cc79d9f 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -19,6 +19,25 @@ #include "thread-utils.h" #include "wildmatch.h" +#ifdef USE_POSIX_SPAWN +#include + +static int fork_interposer_canary(void) +{ + pid_t pid = fork(); + + if (pid < 0 && errno == ENOSYS) + return 0; + if (pid < 0) + return error_errno("fork interposer canary"); + if (!pid) + _exit(0); + if (waitpid(pid, NULL, 0) < 0) + return error_errno("waitpid in fork interposer canary"); + return error("fork interposer is not active"); +} +#endif + static int number_callbacks; static int parallel_next(struct child_process *cp, struct strbuf *err, @@ -444,6 +463,8 @@ int cmd__run_command(int argc, const char **argv) struct child_process proc = CHILD_PROCESS_INIT; int jobs; int ret; + struct strbuf cwd = STRBUF_INIT; + struct strbuf cwd_after = STRBUF_INIT; struct run_process_parallel_opts opts = { .data = &proc, }; @@ -454,6 +475,10 @@ int cmd__run_command(int argc, const char **argv) return inherit_handle(argv[0]); if (!strcmp(argv[1], "inherited-handle-child")) return inherit_handle_child(); +#ifdef USE_POSIX_SPAWN + if (argc > 1 && !strcmp(argv[1], "fork-interposer-canary")) + return fork_interposer_canary(); +#endif if (argc >= 2 && !strcmp(argv[1], "quote-stress-test")) return !!quote_stress_test(argc - 1, argv + 1); @@ -463,6 +488,26 @@ int cmd__run_command(int argc, const char **argv) if (argc < 3) return 1; + if (!strcmp(argv[1], "run-command-with-dir")) { + if (argc < 5) + return 1; + proc.dir = argv[2]; + strvec_push(&proc.env, argv[3]); + strvec_pushv(&proc.args, argv + 4); + proc.close_fd_above_stderr = 1; + if (strbuf_getcwd(&cwd) < 0) + die_errno("getcwd"); + ret = run_command(&proc); + if (!ret) { + if (strbuf_getcwd(&cwd_after) < 0) + die_errno("getcwd"); + if (strcmp(cwd.buf, cwd_after.buf)) + die("working directory changed"); + } + strbuf_release(&cwd); + strbuf_release(&cwd_after); + goto cleanup; + } while (!strcmp(argv[1], "env")) { if (!argv[2]) die("env specifier without a value"); diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index dc9d77b0e14ab4..ec749608f14756 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -7,6 +7,20 @@ test_description='Test run command' . ./test-lib.sh +if test -n "$USE_POSIX_SPAWN" && test -z "$GIT_TEST_INSTALLED" +then + test_set_prereq POSIX_SPAWN + DYLD_INSERT_LIBRARIES="$GIT_BUILD_DIR/t/helper/test-fork-interposer.dylib${DYLD_INSERT_LIBRARIES:+:$DYLD_INSERT_LIBRARIES}" + export DYLD_INSERT_LIBRARIES + # macOS strips DYLD_* from shell wrappers, so exercise the binaries directly. + PATH="$GIT_BUILD_DIR:$GIT_BUILD_DIR/t/helper:$PATH" + export PATH +fi + +test_expect_success POSIX_SPAWN 'fork interposer is active' ' + test-tool run-command fork-interposer-canary +' + cat >hello-script <<-EOF #!$SHELL_PATH cat hello-script @@ -35,6 +49,21 @@ test_expect_success 'run_command can run a command' ' test_must_be_empty err ' +test_expect_success 'run_command preserves environment and working directory' ' + mkdir child && + test_when_finished "chmod 755 child && rm -rf child check-env.sh" && + : >child/child-marker && + write_script check-env.sh <<-\EOF && + test -f child-marker && + printf "%s\\n" "$GIT_SPAWN_TEST" + EOF + chmod 111 child && + echo spawn-ok >expect && + test-tool run-command run-command-with-dir child GIT_SPAWN_TEST=spawn-ok \ + "$SHELL_PATH" ../check-env.sh >actual && + test_cmp expect actual +' + test_lazy_prereq RUNS_COMMANDS_FROM_PWD ' write_script runs-commands-from-pwd <<-\EOF &&