Skip to content

Commit 592408f

Browse files
author
Yannic Charlon
committed
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 <justyannicc.8tlny@simplelogin.com>
1 parent 47fdf22 commit 592408f

8 files changed

Lines changed: 357 additions & 4 deletions

File tree

‎GIT-BUILD-OPTIONS.in‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,5 +44,6 @@ TEST_OUTPUT_DIRECTORY=@TEST_OUTPUT_DIRECTORY@
4444
TEST_SHELL_PATH=@TEST_SHELL_PATH@
4545
USE_GETTEXT_SCHEME=@USE_GETTEXT_SCHEME@
4646
USE_LIBPCRE2=@USE_LIBPCRE2@
47+
USE_POSIX_SPAWN=@USE_POSIX_SPAWN@
4748
WITH_BREAKING_CHANGES=@WITH_BREAKING_CHANGES@
4849
X=@X@

‎Makefile‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,9 @@ include shared.mak
149149
#
150150
# Define NO_PTHREADS if you do not have or do not want to use Pthreads.
151151
#
152+
# Define USE_POSIX_SPAWN to use posix_spawn() for start_command(). This is
153+
# currently supported on macOS only.
154+
#
152155
# Define NO_PREAD if you have a problem with pread() system call (e.g.
153156
# cygwin1.dll before v1.5.22).
154157
#
@@ -889,6 +892,10 @@ TEST_PROGRAMS_NEED_X += test-tool
889892

890893
TEST_PROGRAMS = $(patsubst %,t/helper/%$X,$(TEST_PROGRAMS_NEED_X))
891894

895+
ifdef USE_POSIX_SPAWN
896+
TEST_HELPER_LIBS += t/helper/test-fork-interposer.dylib
897+
endif
898+
892899
# List built-in command $C whose implementation cmd_$C() is not in
893900
# builtin/$C.o but is linked in as part of some other command.
894901
BUILT_INS += $(patsubst builtin/%.o,git-%$X,$(BUILTIN_OBJS))
@@ -2247,6 +2254,10 @@ else
22472254
EXTLIBS += $(PTHREAD_LIBS)
22482255
endif
22492256

2257+
ifdef USE_POSIX_SPAWN
2258+
BASIC_CFLAGS += -DUSE_POSIX_SPAWN
2259+
endif
2260+
22502261
ifdef HAVE_PATHS_H
22512262
BASIC_CFLAGS += -DHAVE_PATHS_H
22522263
endif
@@ -3387,6 +3398,7 @@ GIT-BUILD-OPTIONS: FORCE
33873398
-e "s|@TEST_SHELL_PATH@|\'$(TEST_SHELL_PATH_SQ)\'|" \
33883399
-e "s|@USE_GETTEXT_SCHEME@|\'$(USE_GETTEXT_SCHEME)\'|" \
33893400
-e "s|@USE_LIBPCRE2@|\'$(USE_LIBPCRE2)\'|" \
3401+
-e "s|@USE_POSIX_SPAWN@|\'$(USE_POSIX_SPAWN)\'|" \
33903402
-e "s|@WITH_BREAKING_CHANGES@|\'$(WITH_BREAKING_CHANGES)\'|" \
33913403
-e "s|@X@|\'$(X)\'|" \
33923404
GIT-BUILD-OPTIONS.in >$@+
@@ -3408,7 +3420,7 @@ endif
34083420

34093421
test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
34103422

3411-
all:: $(TEST_PROGRAMS) $(test_bindir_programs) $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG)
3423+
all:: $(TEST_PROGRAMS) $(TEST_HELPER_LIBS) $(test_bindir_programs) $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG)
34123424

34133425
$(test_bindir_programs): bin-wrappers/%: bin-wrappers/wrap-for-bin.sh
34143426
$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
@@ -3442,11 +3454,16 @@ perf: all
34423454

34433455
.PRECIOUS: $(TEST_OBJS)
34443456

3445-
t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) $(UNIT_TEST_DIR)/test-lib.o
3457+
t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) $(UNIT_TEST_DIR)/test-lib.o $(TEST_HELPER_LIBS)
34463458

34473459
t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)
34483460
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
34493461

3462+
ifdef USE_POSIX_SPAWN
3463+
t/helper/test-fork-interposer.dylib: t/helper/test-fork-interposer.c
3464+
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -dynamiclib -o $@ $(ALL_LDFLAGS) $<
3465+
endif
3466+
34503467
check-sha1:: t/helper/test-tool$X
34513468
t/helper/test-sha1.sh
34523469

@@ -3918,7 +3935,7 @@ clean: profile-clean coverage-clean cocciclean
39183935
$(RM) headless-git.o
39193936
$(RM) $(LIB_FILE)
39203937
$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS)
3921-
$(RM) $(TEST_PROGRAMS)
3938+
$(RM) $(TEST_PROGRAMS) t/helper/test-fork-interposer.dylib
39223939
$(RM) $(FUZZ_PROGRAMS)
39233940
$(RM) $(SP_OBJ)
39243941
$(RM) $(HCC)

‎meson.build‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -790,6 +790,7 @@ build_options_config.set('GIT_TEST_PERL_FATAL_WARNINGS', '')
790790
build_options_config.set_quoted('GIT_TEST_UTF8_LOCALE', get_option('test_utf8_locale'))
791791
build_options_config.set_quoted('LOCALEDIR', fs.as_posix(get_option('prefix') / get_option('localedir')))
792792
build_options_config.set_quoted('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb'))
793+
build_options_config.set('USE_POSIX_SPAWN', '')
793794

794795
if get_option('sane_tool_path').length() != 0
795796
sane_tool_path = (host_machine.system() == 'windows' ? ';' : ':').join(get_option('sane_tool_path'))

‎run-command.c‎

Lines changed: 243 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,28 @@
1818
#include "packfile.h"
1919
#include "compat/nonblock.h"
2020

21+
#ifdef USE_POSIX_SPAWN
22+
#include <spawn.h>
23+
#include <AvailabilityMacros.h>
24+
25+
#ifndef __APPLE__
26+
#error "USE_POSIX_SPAWN currently requires the macOS spawn file actions"
27+
#endif
28+
29+
static int spawn_add_fchdir(posix_spawn_file_actions_t *actions, int fd)
30+
{
31+
#if MAC_OS_X_VERSION_MIN_REQUIRED >= 260000
32+
return posix_spawn_file_actions_addfchdir(actions, fd);
33+
#else
34+
#pragma clang diagnostic push
35+
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
36+
int error = posix_spawn_file_actions_addfchdir_np(actions, fd);
37+
#pragma clang diagnostic pop
38+
return error;
39+
#endif
40+
}
41+
#endif
42+
2143
void child_process_init(struct child_process *child)
2244
{
2345
struct child_process blank = CHILD_PROCESS_INIT;
@@ -309,7 +331,9 @@ static const char **prepare_shell_cmd(struct strvec *out, const char **argv)
309331
}
310332

311333
#ifndef GIT_WINDOWS_NATIVE
334+
#ifndef USE_POSIX_SPAWN
312335
static int child_notifier = -1;
336+
#endif
313337

314338
enum child_errcode {
315339
CHILD_ERR_CHDIR,
@@ -325,6 +349,7 @@ struct child_err {
325349
int syserr; /* errno */
326350
};
327351

352+
#ifndef USE_POSIX_SPAWN
328353
static void child_die(enum child_errcode err)
329354
{
330355
struct child_err buf;
@@ -373,6 +398,7 @@ static void NORETURN child_die_fn(const char *err UNUSED, va_list params UNUSED)
373398
xwrite(2, msg, sizeof(msg) - 1);
374399
_exit(2);
375400
}
401+
#endif
376402

377403
/* this runs in the parent process */
378404
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)
496522
return childenv;
497523
}
498524

525+
#ifdef USE_POSIX_SPAWN
526+
static int spawn_add_dup2(posix_spawn_file_actions_t *actions, int from,
527+
int to, unsigned char stdio_touched[3])
528+
{
529+
int error = posix_spawn_file_actions_adddup2(actions, from, to);
530+
if (!error && to < 3)
531+
stdio_touched[to] = 1;
532+
return error;
533+
}
534+
535+
static int spawn_add_close(posix_spawn_file_actions_t *actions, int fd,
536+
unsigned char stdio_touched[3])
537+
{
538+
int error = posix_spawn_file_actions_addclose(actions, fd);
539+
if (!error && fd < 3)
540+
stdio_touched[fd] = 1;
541+
return error;
542+
}
543+
544+
static int spawn_add_open(posix_spawn_file_actions_t *actions, int fd,
545+
unsigned char stdio_touched[3])
546+
{
547+
int error = posix_spawn_file_actions_addopen(actions, fd, "/dev/null",
548+
O_RDWR, 0);
549+
if (!error && fd < 3)
550+
stdio_touched[fd] = 1;
551+
return error;
552+
}
553+
554+
static pid_t spawn_command(const struct child_process *cmd, const char **argv,
555+
char **childenv, int need_in, int fdin[2],
556+
int need_out, int fdout[2],
557+
int need_err, int fderr[2],
558+
enum child_errcode *error_kind)
559+
{
560+
posix_spawn_file_actions_t actions;
561+
posix_spawnattr_t attr;
562+
const posix_spawnattr_t *attrp = NULL;
563+
unsigned char stdio_touched[3] = { 0 };
564+
int actions_initialized = 0, attr_initialized = 0;
565+
sigset_t signal_mask, signal_defaults;
566+
struct sigaction sa;
567+
short flags = POSIX_SPAWN_SETSIGMASK | POSIX_SPAWN_SETSIGDEF;
568+
int error;
569+
pid_t pid = -1;
570+
int dir_fd = -1;
571+
int sig, fd;
572+
573+
*error_kind = CHILD_ERR_ERRNO;
574+
575+
#define ADD_SPAWN_ACTION(action) \
576+
do { \
577+
error = (action); \
578+
if (error) \
579+
goto done; \
580+
} while (0)
581+
582+
error = posix_spawn_file_actions_init(&actions);
583+
if (error)
584+
goto done;
585+
actions_initialized = 1;
586+
error = posix_spawnattr_init(&attr);
587+
if (error)
588+
goto done;
589+
attr_initialized = 1;
590+
attrp = &attr;
591+
592+
#ifdef NO_PTHREADS
593+
if (sigprocmask(SIG_SETMASK, NULL, &signal_mask) < 0) {
594+
error = errno;
595+
goto done;
596+
}
597+
#else
598+
error = pthread_sigmask(SIG_SETMASK, NULL, &signal_mask);
599+
if (error)
600+
goto done;
601+
#endif
602+
if (sigemptyset(&signal_defaults) < 0) {
603+
error = errno;
604+
goto done;
605+
}
606+
for (sig = 1; sig < NSIG; sig++) {
607+
if (!sigaction(sig, NULL, &sa) && sa.sa_handler != SIG_IGN)
608+
sigaddset(&signal_defaults, sig);
609+
}
610+
ADD_SPAWN_ACTION(posix_spawnattr_setsigmask(&attr, &signal_mask));
611+
ADD_SPAWN_ACTION(posix_spawnattr_setsigdefault(&attr,
612+
&signal_defaults));
613+
614+
if (cmd->no_stdin)
615+
ADD_SPAWN_ACTION(spawn_add_open(&actions, 0, stdio_touched));
616+
else if (need_in) {
617+
ADD_SPAWN_ACTION(spawn_add_dup2(&actions, fdin[0], 0,
618+
stdio_touched));
619+
ADD_SPAWN_ACTION(spawn_add_close(&actions, fdin[0], stdio_touched));
620+
ADD_SPAWN_ACTION(spawn_add_close(&actions, fdin[1], stdio_touched));
621+
} else if (cmd->in) {
622+
ADD_SPAWN_ACTION(spawn_add_dup2(&actions, cmd->in, 0,
623+
stdio_touched));
624+
ADD_SPAWN_ACTION(spawn_add_close(&actions, cmd->in, stdio_touched));
625+
}
626+
627+
if (cmd->no_stderr)
628+
ADD_SPAWN_ACTION(spawn_add_open(&actions, 2, stdio_touched));
629+
else if (need_err) {
630+
ADD_SPAWN_ACTION(spawn_add_dup2(&actions, fderr[1], 2,
631+
stdio_touched));
632+
ADD_SPAWN_ACTION(spawn_add_close(&actions, fderr[0], stdio_touched));
633+
ADD_SPAWN_ACTION(spawn_add_close(&actions, fderr[1], stdio_touched));
634+
} else if (cmd->err > 1) {
635+
ADD_SPAWN_ACTION(spawn_add_dup2(&actions, cmd->err, 2,
636+
stdio_touched));
637+
ADD_SPAWN_ACTION(spawn_add_close(&actions, cmd->err, stdio_touched));
638+
}
639+
640+
if (cmd->no_stdout)
641+
ADD_SPAWN_ACTION(spawn_add_open(&actions, 1, stdio_touched));
642+
else if (cmd->stdout_to_stderr)
643+
ADD_SPAWN_ACTION(spawn_add_dup2(&actions, 2, 1, stdio_touched));
644+
else if (need_out) {
645+
ADD_SPAWN_ACTION(spawn_add_dup2(&actions, fdout[1], 1,
646+
stdio_touched));
647+
ADD_SPAWN_ACTION(spawn_add_close(&actions, fdout[0], stdio_touched));
648+
ADD_SPAWN_ACTION(spawn_add_close(&actions, fdout[1], stdio_touched));
649+
} else if (cmd->out > 1) {
650+
ADD_SPAWN_ACTION(spawn_add_dup2(&actions, cmd->out, 1,
651+
stdio_touched));
652+
ADD_SPAWN_ACTION(spawn_add_close(&actions, cmd->out, stdio_touched));
653+
}
654+
655+
if (cmd->dir) {
656+
dir_fd = open(cmd->dir, O_SEARCH | O_CLOEXEC);
657+
if (dir_fd < 0) {
658+
*error_kind = CHILD_ERR_CHDIR;
659+
error = errno;
660+
goto done;
661+
}
662+
ADD_SPAWN_ACTION(posix_spawn_file_actions_addinherit_np(
663+
&actions, dir_fd));
664+
ADD_SPAWN_ACTION(spawn_add_fchdir(&actions, dir_fd));
665+
ADD_SPAWN_ACTION(posix_spawn_file_actions_addclose(&actions,
666+
dir_fd));
667+
}
668+
669+
if (cmd->close_fd_above_stderr) {
670+
flags |= POSIX_SPAWN_CLOEXEC_DEFAULT;
671+
672+
for (fd = 0; fd < 3; fd++) {
673+
int fd_flags;
674+
if (stdio_touched[fd])
675+
continue;
676+
fd_flags = fcntl(fd, F_GETFD);
677+
if (fd_flags < 0 || (fd_flags & FD_CLOEXEC))
678+
continue;
679+
ADD_SPAWN_ACTION(posix_spawn_file_actions_addinherit_np(
680+
&actions, fd));
681+
}
682+
}
683+
ADD_SPAWN_ACTION(posix_spawnattr_setflags(&attr, flags));
684+
685+
error = posix_spawn(&pid, argv[1], &actions, attrp,
686+
(char *const *)argv + 1, childenv);
687+
if (error == ENOEXEC)
688+
error = posix_spawn(&pid, argv[0], &actions, attrp,
689+
(char *const *)argv, childenv);
690+
if (error)
691+
pid = -1;
692+
693+
done:
694+
if (attr_initialized)
695+
posix_spawnattr_destroy(&attr);
696+
if (actions_initialized)
697+
posix_spawn_file_actions_destroy(&actions);
698+
if (dir_fd >= 0)
699+
close(dir_fd);
700+
#undef ADD_SPAWN_ACTION
701+
if (error)
702+
errno = error;
703+
return pid;
704+
}
705+
#endif
706+
707+
#ifndef USE_POSIX_SPAWN
499708
struct atfork_state {
500709
#ifndef NO_PTHREADS
501710
int cs;
@@ -545,6 +754,7 @@ static void atfork_parent(struct atfork_state *as)
545754
"restoring signal mask");
546755
#endif
547756
}
757+
#endif
548758

549759
#endif /* GIT_WINDOWS_NATIVE */
550760

@@ -746,7 +956,37 @@ int start_command(struct child_process *cmd)
746956
odb_close(cmd->odb_to_close);
747957

748958
#ifndef GIT_WINDOWS_NATIVE
959+
#ifdef USE_POSIX_SPAWN
749960
{
961+
struct strvec argv = STRVEC_INIT;
962+
struct child_err spawn_error;
963+
char **childenv;
964+
965+
if (prepare_cmd(&argv, cmd) < 0) {
966+
failed_errno = errno;
967+
cmd->pid = -1;
968+
if (!cmd->silent_exec_failure)
969+
error_errno("cannot run %s", cmd->args.v[0]);
970+
} else {
971+
trace_argv_printf(&argv.v[1], "trace: start_command:");
972+
childenv = prep_childenv(cmd->env.v);
973+
cmd->pid = spawn_command(cmd, argv.v, childenv,
974+
need_in, fdin, need_out, fdout,
975+
need_err, fderr, &spawn_error.err);
976+
failed_errno = errno;
977+
if (cmd->pid < 0) {
978+
spawn_error.syserr = failed_errno;
979+
if (!cmd->silent_exec_failure || failed_errno != ENOENT ||
980+
spawn_error.err == CHILD_ERR_CHDIR)
981+
child_err_spew(cmd, &spawn_error);
982+
} else if (cmd->clean_on_exit)
983+
mark_child_for_cleanup(cmd->pid, cmd);
984+
free(childenv);
985+
}
986+
strvec_clear(&argv);
987+
}
988+
#else
989+
{
750990
int notify_pipe[2];
751991
int null_fd = -1;
752992
char **childenv;
@@ -905,8 +1145,10 @@ int start_command(struct child_process *cmd)
9051145
close(null_fd);
9061146
strvec_clear(&argv);
9071147
free(childenv);
908-
}
9091148
end_of_spawn:
1149+
;
1150+
}
1151+
#endif
9101152

9111153
#else
9121154
{

‎t/helper/.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
/test-tool
22
/test-fake-ssh
3+
/test-fork-interposer.dylib

0 commit comments

Comments
 (0)