diff --git a/CHANGELOG.md b/CHANGELOG.md index 225b2589b..fcb202123 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ _When adding new entries to the changelog, please include issue/PR numbers where - Improved progress reporting during working-copy checkout. [#738](https://github.com/koordinates/kart/pull/738) - Support for changing the primary key column of an existing dataset. [#238](https://github.com/koordinates/kart/issues/238) - Help for the user get the working copy back into a valid state if a crash or similar causes it to become out of sync with the Kart repo. [#751](https://github.com/koordinates/kart/pull/751) +- Enable the background CLI helper on Linux & macOS in CI builds. The helper improves CLI performance significantly. [#776](https://github.com/koordinates/kart/pull/776) ## 0.11.5 diff --git a/CMakeLists.txt b/CMakeLists.txt index 3f2249260..466c6351a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -55,7 +55,11 @@ message(STATUS "Kart build version: ${KART_VERSION}") # option(USE_VCPKG "Use vcpkg for vendor dependencies") -option(CLI_HELPER "Default to the CLI helper on macOS & Linux") + +if(MACOS OR LINUX) + option(CLI_HELPER "Enable the CLI helper on macOS & Linux" OFF) +endif() + set(VENDOR_ARCHIVE "" CACHE @@ -215,6 +219,7 @@ if(BUILD_TESTING) NAME e2e-1 COMMAND tests/scripts/e2e-1.sh WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}) + set_tests_properties(e2e-1 PROPERTIES ENVIRONMENT "KART_USE_HELPER=0") if(CLI_HELPER) # Run the same test under Linux & Mac using the cli helper diff --git a/CMakePresets.json b/CMakePresets.json index fb258475d..85bb1e072 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -24,6 +24,9 @@ "displayName": "CI: Linux", "inherits": "ci-base", "generator": "Unix Makefiles", + "cacheVariables": { + "CLI_HELPER": "ON" + }, "condition": { "type": "equals", "lhs": "${hostSystemName}", @@ -36,6 +39,7 @@ "inherits": "ci-base", "generator": "Unix Makefiles", "cacheVariables": { + "CLI_HELPER": "ON", "MACOS_SIGN_BUNDLE": "$env{MACOS_SIGN_BUNDLE}", "MACOS_SIGN_PKG": "$env{MACOS_SIGN_PKG}", "MACOS_NOTARIZE": "$env{MACOS_NOTARIZE}" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f997eb082..3f02c1fdd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -71,6 +71,10 @@ $ cmake --build build --target bundle $ cmake --install build ``` +Kart includes a background helper for improved command-line performance, but this +feature is disabled by default in development builds. To enable it, configure +Kart with `-DCLI_HELPER=ON`. + ### Downloading vendor dependencies from CI If you're having issues with VCPKG in the above, you can download a recent master-branch vendor CI artifact for your platform (eg: `vendor-macos-X64-py3.10.zip`). diff --git a/cli_helper/CMakeLists.txt b/cli_helper/CMakeLists.txt index fda38a49e..593851576 100644 --- a/cli_helper/CMakeLists.txt +++ b/cli_helper/CMakeLists.txt @@ -1,3 +1,7 @@ add_executable(kart_cli_helper kart.c cJSON.c) set_property(TARGET kart_cli_helper PROPERTY OUTPUT_NAME kart) +set_property(TARGET kart_cli_helper PROPERTY C_STANDARD 11) + +target_compile_options(kart_cli_helper PRIVATE -Wall -Werror) +target_compile_definitions(kart_cli_helper PRIVATE "$<$:DEBUG>") diff --git a/cli_helper/kart.c b/cli_helper/kart.c index 85b9db9da..27c030d10 100644 --- a/cli_helper/kart.c +++ b/cli_helper/kart.c @@ -1,58 +1,226 @@ -#define _XOPEN_SOURCE 1 - -#include -#include -#include +#include +#include +#include +#include +#include #include -#include -#include -#include +#include #include +#include +#include +#include +#include #include -#include +#include +#include #include -#include -#include -#include -#include +#include #include -#include -#include -#include +#include + +#if __APPLE__ +#include +const int SEM_FLAGS = IPC_CREAT | IPC_EXCL | SEM_R | SEM_A; +#elif __linux__ +#define _GNU_SOURCE +const int SEM_FLAGS = IPC_CREAT | IPC_EXCL | S_IRUSR | S_IWUSR; +#endif + int nanosleep(const struct timespec *req, struct timespec *rem); #include "cJSON.h" int semid; +const int SEMNUM = 0; + +#ifndef DEBUG +#define DEBUG 0 +#endif +#define debug(fmt, ...) \ + do { if (DEBUG && getenv("KART_HELPER_DEBUG") != NULL) { \ + fprintf(stderr, "HELPER[%d]:%d:%s(): " fmt, getpid(), \ + __LINE__, __func__, ##__VA_ARGS__); }} while (0) + +/** + * @brief find the path to the current executable + * @param[in] argv process argv + * @param[out] exe_path path to the executable, absolute or relative to cwd. len=PATH_MAX + * @return 0 success, 1 error + */ +int find_executable(char **argv, char *exe_path) +{ + exe_path[0] = '\0'; + +#if __linux__ + ssize_t r = readlink("/proc/self/exe", exe_path, PATH_MAX); + if(r == -1) + { + debug("Error calling readlink(/proc/self/exe): %d\n", errno); + } + else + { + exe_path[r] = '\0'; + debug("readlink(/proc/self/exe)=%s\n", exe_path); + } +#elif __APPLE__ + uint32_t bufsize = PATH_MAX; + int e = _NSGetExecutablePath(exe_path, &bufsize); + if (e) + { + debug("Error calling _NSGetExecutablePath(): %d (bufsize=%d)\n", e, bufsize); + } + else + { + debug("_NSGetExecutablePath=%s\n", exe_path); + } +#endif + + // that didn't work for some reason, try argv[0] + if (!exe_path[0]) + { + if (!realpath(argv[0], exe_path)) + { + debug("Error calling realpath(argv[0]=%s)\n", argv[0]); + return 1; + } + debug("realpath(argv[0])=%s\n", exe_path); + } + + return 0; +} + +/** + * @brief find the path to the kart_cli executable + * @param[in] source absolute or relative path to existing file + * @param[in] name sibling filename + * @param[out] sibling_path sibling path name. len=PATH_MAX + * @return 0 success, 1 error + */ +int find_sibling(char* source, char* name, char* sibling_path) +{ + (void)memset(sibling_path, 0, PATH_MAX); + + // look for a sibling of the executable + char *p = strrchr(source, '/'); + if(p == NULL) + { + (void)strncpy(sibling_path, name, PATH_MAX-1); + } + else + { + char buf[PATH_MAX]; + if (snprintf(buf, PATH_MAX, "/%s", name) < 0) + { + fprintf(stderr, "Error calculating sibling path\n"); + return 1; + } + + (void)strncpy(sibling_path, source, p - source); + (void)strncat(sibling_path, buf, PATH_MAX - strlen(buf) - 1); + } + debug("sibling path: %s\n", sibling_path); + return 0; +} + +/** + * @brief find the path to the kart_cli executable + * @param[in] argv process argv + * @param[out] exe_path path to the executable, absolute or relative to cwd. len=PATH_MAX + * @return 0 success, 1 error + */ +int find_kart_cli(char **argv, char *cmd_path) +{ + char exe_path[PATH_MAX]; + int r; + r = find_executable(argv, exe_path); + if (r) + { + return r; + } + debug("executable=%s\n", exe_path); + + r = find_sibling(exe_path, "kart_cli", cmd_path); + if (r) + { + return r; + } + + if (access(cmd_path, F_OK) == 0) { + // found it + return 0; + } + + // file doesn't exist + debug("%s doesn't exist\n", cmd_path); + + // if kart is a symlink, try resolving it then finding the symlink + char buf[PATH_MAX]; + if (!realpath(exe_path, buf)) + { + fprintf(stderr, "Error resolving kart_cli path\n"); + return 1; + } + debug("realpath(%s)=%s\n", exe_path, buf); + + r = find_sibling(buf, "kart_cli", cmd_path); + if (r) + { + return r; + } + + if (access(cmd_path, F_OK) == 0) { + // found it + return 0; + } + + debug("%s doesn't exist\n", cmd_path); + return 1; +} + +/** + * @brief Check whether helper is enabled via KART_USE_HELPER + * Defaults on, turn off via KART_USE_HELPER=0 + * @return 0 no, 1 yes + */ +int is_helper_enabled() +{ + char *env = getenv("KART_USE_HELPER"); + return (env == NULL || *env != '0'); +} + +/** + * @brief Exit signal handler for SIGALRM + */ void exit_on_alarm(int sig) { - int semval = semctl(semid, 0, GETVAL); + int semval = semctl(semid, SEMNUM, GETVAL); + if (semval < 0) + { + debug("sigalrm: error getting semval, semid=%d, errno=%d\n", semid, errno); + exit(5); + } + int exit_code = semval - 1000; - semctl(semid, 0, IPC_RMID); + semctl(semid, SEMNUM, IPC_RMID); + debug("sigalrm: semid=%d semval=%d exit_code=%d\n", semid, semval, exit_code); exit(exit_code); } int main(int argc, char **argv, char **environ) { - // find the appropriate kart_cli to run based on whether argv[0] - // is absolute, relative or bare, meaning on the path - char *cmd = "kart_cli"; char cmd_path[PATH_MAX]; - char buf[PATH_MAX]; - if (strchr(argv[0], '/')) { - realpath(argv[0], buf); - sprintf(cmd_path, "%s/%s", dirname(&buf[0]), cmd); - } else { - strcpy(cmd_path, cmd); + if (find_kart_cli(argv, cmd_path)) { + fprintf(stderr, "Couldn't find kart_cli\n"); + exit(1); } - char *use_helper = getenv("KART_USE_HELPER"); - - if (use_helper != NULL && *use_helper != '\0' && *use_helper != ' ' && *use_helper != '0') + if (is_helper_enabled()) { + debug("enabled %s, pid=%d\n", cmd_path, getpid()); + // start or use an existing helper process char **env_ptr; @@ -66,19 +234,19 @@ int main(int argc, char **argv, char **environ) cJSON *payload = cJSON_CreateObject(); cJSON_AddNumberToObject(payload, "pid", getpid()); env = cJSON_AddObjectToObject(payload, "environ"); - + int found = 0; // filter the environment so that KART_USE_HELPER isn't passed to the // spawned process and so getting into a loop for (env_ptr = environ; *env_ptr != NULL; env_ptr++) - { + { char *key = malloc(strlen(*env_ptr)); char *val = malloc(strlen(*env_ptr)); if (sscanf(*env_ptr, "%[^=]=%s", key, val) != 2) { // not found with two values in a key=value pair if (sscanf(*env_ptr, "%[^=]=", key) != 1) { - printf("error reading environment variable where only name is present\n"); + fprintf(stderr, "error reading environment variable where only name is present\n"); } } if (strcmp(key, "KART_USE_HELPER")) @@ -107,11 +275,12 @@ int main(int argc, char **argv, char **environ) addr.sun_family = AF_UNIX; strcpy(addr.sun_path, socket_filename); - // if there is no open socket perform a double fork and spawn to + // if there is no open socket perform a double fork and spawn to // detach the helper, wait till the first forked child has completed // then attempt to connect to the socket the helper will open if (connect(socket_fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) { + debug("no open socket found @%s\n", socket_filename); int status; if (fork() == 0) { @@ -121,7 +290,7 @@ int main(int argc, char **argv, char **environ) if (fork() == 0) { setsid(); - + // start helper in background and wait char *helper_argv[] = {&cmd_path[0], "helper", "--socket", socket_filename, NULL}; @@ -130,11 +299,12 @@ int main(int argc, char **argv, char **environ) for (int fd = 0; fd < 3; fd++){ fcntl(fd, F_SETFD, FD_CLOEXEC); } + debug("grandchild: execvp: %s helper --socket %s\n", cmd_path, socket_filename); status = execvp(helper_argv[0], helper_argv); if (status < 0) { - printf("Error running kart helper, %s: %s", cmd_path, strerror(status)); + fprintf(stderr, "Error running kart helper, %s: %s", cmd_path, strerror(status)); exit(1); } } @@ -145,6 +315,8 @@ int main(int argc, char **argv, char **environ) wait(&status); } + debug("parent: waiting for socket\n"); + int rtc, max_retry = 50; struct timespec sleep_for = {0, 250 * 1000 * 1000}; while ((rtc = connect(socket_fd, (struct sockaddr *)&addr, sizeof addr)) != 0 && --max_retry >= 0) @@ -153,20 +325,24 @@ int main(int argc, char **argv, char **environ) } if (rtc < 0) { - printf("Timeout connecting to kart helper\n"); + fprintf(stderr, "Timeout connecting to kart helper\n"); return 2; } + } else { + debug("open socket found @%s\n", socket_filename); } // set up exit code semaphore - if ((semid = semget(IPC_PRIVATE, 1, IPC_CREAT | IPC_EXCL | S_IRUSR | S_IWUSR)) < 0) + if ((semid = semget(IPC_PRIVATE, 1, SEM_FLAGS)) < 0) { - printf("Error setting up result communication with helper %s\n", strerror(errno)); + fprintf(stderr, "Error setting up result communication with helper %s\n", strerror(errno)); return 5; - }; + } cJSON_AddNumberToObject(payload, "semid", semid); - char *payload_string = cJSON_Print(payload); + char *payload_string = cJSON_PrintUnformatted(payload); + + debug("payload (%lub): %s\n", strlen(payload_string), payload_string); struct iovec iov = { .iov_base = payload_string, @@ -198,18 +374,21 @@ int main(int argc, char **argv, char **environ) if (sendmsg(socket_fd, &msg, 0) < 0) { - printf("Error sending command to kart helper %s\n", strerror(errno)); + fprintf(stderr, "Error sending command to kart helper %s\n", strerror(errno)); return 3; - }; - + }; + + debug("complete, sleeping until exit\n"); + // The process needs to sleep for as long as the longest command, clone etc. could take. - sleep(86400); - printf("Timed out, no response from kart helper\n"); + sleep(86400); + fprintf(stderr, "Timed out, no response from kart helper\n"); return 4; } else { + debug("disabled, execvp(%s)\n", cmd_path); // run the full application as normal execvp(&cmd_path[0], argv); } -} \ No newline at end of file +} diff --git a/cmake/KartPy.cmake b/cmake/KartPy.cmake index 492dfcb11..78d1b321d 100644 --- a/cmake/KartPy.cmake +++ b/cmake/KartPy.cmake @@ -7,6 +7,7 @@ if(WIN32) cmake_path(SET KART_EXE_VENV ${VENV_BIN}/kart.exe) cmake_path(SET KART_EXE_BUILD ${CMAKE_CURRENT_BINARY_DIR}/kart.cmd) + set(KART_EXE_NAME "kart") else() set(VENV_BIN ${CMAKE_CURRENT_BINARY_DIR}/venv/bin) # this is needed sometimes for bad setup.py files that invoke Python again seems ok without it on @@ -14,7 +15,14 @@ else() set(VENV_EXEC_ENV ${CMAKE_COMMAND} -E env "PATH=${VENV_BIN}:$ENV{PATH}") set(VENV_PY ${VENV_EXEC_ENV} ${VENV_BIN}/python) cmake_path(SET KART_EXE_VENV ${VENV_BIN}/kart) - cmake_path(SET KART_EXE_BUILD ${CMAKE_CURRENT_BINARY_DIR}/kart) + + if(CLI_HELPER) + set(KART_EXE_NAME "kart_cli") + cmake_path(SET KART_EXE_HELPER ${CMAKE_CURRENT_BINARY_DIR}/kart) + else() + set(KART_EXE_NAME "kart") + endif() + cmake_path(SET KART_EXE_BUILD ${CMAKE_CURRENT_BINARY_DIR}/${KART_EXE_NAME}) endif() set(VENV_PIP_INSTALL ${VENV_PY} -m pip install --isolated --disable-pip-version-check) @@ -64,13 +72,29 @@ add_custom_target( DEPENDS venv/.vendor.stamp ${pydeps} COMMENT "Installing Python dependencies...") +set(CLI_DEPS ${KART_EXE_BUILD}) +if(CLI_HELPER) + add_custom_command( + OUTPUT ${KART_EXE_HELPER} + DEPENDS kart_cli_helper + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} + COMMAND ${CMAKE_COMMAND} "-DTARGET:FILEPATH=$" + "-DLINK_NAME:FILEPATH=kart" -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/link.cmake + COMMAND + ${CMAKE_COMMAND} "-DTARGET:FILEPATH=${KART_EXE_VENV}" + "-DLINK_NAME:FILEPATH=cli_helper/${KART_EXE_NAME}" -P + ${CMAKE_CURRENT_SOURCE_DIR}/cmake/link.cmake + COMMENT "Installing Kart Helper...") + list(APPEND CLI_DEPS ${KART_EXE_HELPER}) +endif() + add_custom_command( OUTPUT ${KART_EXE_VENV} ${KART_EXE_BUILD} DEPENDS py-dependencies setup.py WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} COMMAND ${VENV_PIP_INSTALL} --force-reinstall --no-deps --editable "${CMAKE_CURRENT_SOURCE_DIR}" - COMMAND ${CMAKE_COMMAND} "-DTARGET:FILEPATH=${KART_EXE_VENV}" "-DLINK_NAME:FILEPATH=kart" -P - ${CMAKE_CURRENT_SOURCE_DIR}/cmake/link.cmake + COMMAND ${CMAKE_COMMAND} "-DTARGET:FILEPATH=${KART_EXE_VENV}" + "-DLINK_NAME:FILEPATH=${KART_EXE_NAME}" -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/link.cmake COMMAND ${KART_EXE_BUILD} --version COMMENT "Installing Kart...") @@ -83,5 +107,5 @@ add_custom_command( add_custom_target( cli ALL - DEPENDS ${KART_EXE_BUILD} ${VENV_DOCS} + DEPENDS ${VENV_DOCS} ${CLI_DEPS} COMMENT "Kart CLI") diff --git a/kart/helper.py b/kart/helper.py index 6df242e88..12f1c8d84 100644 --- a/kart/helper.py +++ b/kart/helper.py @@ -1,18 +1,28 @@ -from pathlib import Path -import socket -import signal +import errno import json -import io import os +import signal +import socket import sys import traceback +from datetime import datetime +from pathlib import Path import click -from .socket_utils import recv_fds +from .socket_utils import recv_json_and_fds from .cli import load_commands_from_args, cli, is_windows, is_darwin, is_linux +log_filename = None + + +def _helper_log(msg): + if log_filename: + with open(log_filename, "at") as log_file: + log_file.write(f"{datetime.now()} [{os.getpid()}]: {msg}\n") + + @click.command(context_settings=dict(ignore_unknown_options=True)) @click.pass_context @click.option( @@ -46,6 +56,10 @@ def helper(ctx, socket_filename, timeout, args): load_commands_from_args(["--help"]) + if os.environ.get("KART_HELPER_LOG"): + global log_filename + log_filename = os.path.abspath(os.environ["KART_HELPER_LOG"]) + # stash the required environment for used libs # TODO - this should be checked to ensure it is all that is needed # is there anything beyond these which is needed, eg. PWD, USER, etc. @@ -70,6 +84,7 @@ def helper(ctx, socket_filename, timeout, args): "GIT_TEMPLATE_DIR", "GIT_INDEX_FILE", "SSL_CERT_FILE", + "KART_HELPER_LOG", ] } @@ -82,6 +97,7 @@ def helper(ctx, socket_filename, timeout, args): os.unlink(socket_filename) sock.bind(str(socket_filename)) + _helper_log(f"Bound socket: {socket_filename}") except OSError as e: click.echo(f"Unable to bind to socket [{socket_filename}] [{e.strerror}]") ctx.exit(1) @@ -108,25 +124,29 @@ def helper(ctx, socket_filename, timeout, args): # The helper will exit if no command received within timeout sock.settimeout(timeout) try: + _helper_log(f"socket ready, waiting for messages (timeout={timeout})") client, info = sock.accept() - if os.fork() == 0: - payload, fds = recv_fds(client, 8164, 4) + _helper_log("pre-fork messaged received") + if os.fork() != 0: + # parent + continue + else: + # child + _helper_log("post-fork") + payload, fds = recv_json_and_fds(client, maxfds=4) if not payload or len(fds) != 4: - click.echo("No payload or fds passed from kart_cli_helper") + click.echo( + "No payload or fds passed from kart_cli_helper: exit(-1)" + ) sys.exit(-1) - kart_helper_log = os.environ.get("KART_HELPER_LOG") - if kart_helper_log: - kart_helper_log = open(kart_helper_log, "a") - else: - kart_helper_log = io.StringIO() - # as there is a new process the child could drop permissions here or use a security system to set up # controls, chroot etc. # change to the calling processes working directory # TODO - pass as path for windows os.fchdir(fds[3]) + _helper_log(f"cwd={os.getcwd()}") # set this processes stdin/stdout/stderr to the calling processes passed in fds # TODO - have these passed as named pipes paths, will work on windows as well @@ -137,17 +157,20 @@ def helper(ctx, socket_filename, timeout, args): sys.stdout = os.fdopen(fds[1], "w") sys.stderr = os.fdopen(fds[2], "w") + # re-enable SIGCHLD so subprocess handling works + signal.signal(signal.SIGCHLD, signal.SIG_DFL) + try: calling_environment = json.loads(payload) - except (TypeError, ValueError) as e: - click.echo( - "kart helper: Unable to read command from kart_cli_helper", e + except (TypeError, ValueError, json.decoder.JSONDecodeError) as e: + raise RuntimeError( + "kart helper: Unable to read command from kart_cli_helper", + e, + f"Payload:\n{repr(payload)}", ) else: sys.argv[1:] = calling_environment["argv"][1:] - kart_helper_log.write( - f"PID={calling_environment['pid']} CWD={os.getcwd()} CMD={' '.join(calling_environment['argv'])}\n" - ) + _helper_log(f"cmd={' '.join(calling_environment['argv'])}") os.environ.clear() os.environ.update( { @@ -157,66 +180,114 @@ def helper(ctx, socket_filename, timeout, args): } ) + # setup the semctl() function import ctypes import ctypes.util libc = ctypes.CDLL(ctypes.util.find_library("c"), use_errno=True) - # if is_darwin: SETVAL = 8 + + # union semun { + # int val; /* value for SETVAL */ + # struct semid_ds *buf; /* buffer for IPC_STAT & IPC_SET */ + # u_short *array; /* array for GETALL & SETALL */ + # }; + class C_SEMUN(ctypes.Union): + _fields_ = ( + ("val", ctypes.c_int), + ("semid_ds", ctypes.c_void_p), + ("array", ctypes.POINTER(ctypes.c_ushort)), + ) + elif is_linux: SETVAL = 16 - class struct_semid_ds(ctypes.Structure): - pass + # union semun { + # int val; /* Value for SETVAL */ + # struct semid_ds *buf; /* Buffer for IPC_STAT, IPC_SET */ + # unsigned short *array; /* Array for GETALL, SETALL */ + # struct seminfo *__buf; /* Buffer for IPC_INFO + # }; + class C_SEMUN(ctypes.Union): + _fields_ = ( + ("val", ctypes.c_int), + ("semid_ds", ctypes.c_void_p), + ("array", ctypes.POINTER(ctypes.c_ushort)), + ("seminfo", ctypes.c_void_p), + ) - class struct_semun(ctypes.Union): - _fields_ = [ - ("val", ctypes.c_uint32), - ( - "buf", - ctypes.POINTER(struct_semid_ds), - ), - ( - "array", - ctypes.POINTER(ctypes.c_uint16), - ), - ] - - libc.semctl.argtypes = [ + # arg (union semun) is a variadic arg. On macOS/arm64 the + # calling convention differs between fixed & variadic args + # so we _must_ treat it as variadic. + libc.semctl.argtypes = ( ctypes.c_int, ctypes.c_int, ctypes.c_int, - struct_semun, - ] - libc.semget.argtypes = [ctypes.c_int, ctypes.c_int, ctypes.c_int] + ) + libc.semctl.restype = ctypes.c_int + semid = calling_environment["semid"] + SEMNUM = 0 + + _helper_log(f"semid={semid}") try: + _helper_log("invoking cli()...") cli() except SystemExit as system_exit: """exit is called in the commands but we ignore as we need to clean up the caller""" - # TODO - do we ever see negative exit codes from cli (git etc)? - libc.semctl( - semid, 0, SETVAL, struct_semun(val=system_exit.code + 1000) + _helper_log( + f"SystemExit from cli(): {system_exit.code} semval={1000+system_exit.code}" ) - + if ( + libc.semctl( + semid, + SEMNUM, + SETVAL, + C_SEMUN(val=system_exit.code + 1000), + ) + < 0 + ): + raise RuntimeError( + f"Error setting semid {semid}[{SEMNUM}]=1000+{system_exit.code}: " + f"{errno.errorcode.get(ctypes.get_errno(), ctypes.get_errno())}" + ) except Exception as e: - print( - f"kart helper: unhandled exception" - ) # TODO - should ext-run capture/handle this? + # TODO - should ext-run capture/handle this? + _helper_log( + f"unhandled exception from cli() semval=1001: {traceback.format_exc(e)}" + ) + print("kart helper: unhandled exception", file=sys.stderr) traceback.print_exc(file=sys.stderr) - libc.semctl(semid, 0, SETVAL, struct_semun(val=1001)) + if libc.semctl(semid, SEMNUM, SETVAL, C_SEMUN(val=1001)) < 0: + raise RuntimeError( + f"Error setting semid {semid}[{SEMNUM}]=1001: " + f"{errno.errorcode.get(ctypes.get_errno(), ctypes.get_errno())}" + ) + else: + _helper_log("return from cli() without SystemExit semval=1000") + if libc.semctl(semid, SEMNUM, SETVAL, C_SEMUN(val=1000)) < 0: + raise RuntimeError( + f"Error setting semid {semid}[{SEMNUM}]=1000: " + f"{errno.errorcode.get(ctypes.get_errno(), ctypes.get_errno())}" + ) + try: # send a signal to caller that we are done + _helper_log( + f"sending SIGALRM to pid {calling_environment['pid']}" + ) os.kill(calling_environment["pid"], signal.SIGALRM) except ProcessLookupError as e: + _helper_log(f"error signalling caller: {e}") pass - kart_helper_log.close() + _helper_log("bye(0)") sys.exit() except socket.timeout: + _helper_log("socket timeout, bye (0)") try: os.unlink(socket_filename) except FileNotFoundError: diff --git a/kart/socket_utils.py b/kart/socket_utils.py index cdde97f89..ab8e83984 100644 --- a/kart/socket_utils.py +++ b/kart/socket_utils.py @@ -1,16 +1,28 @@ -import logging - -import time import array import socket -# Function from https://docs.python.org/3/library/socket.html#socket.socket.recvmsg -def recv_fds(sock, msglen, maxfds): +# Setting the message length higher than this has no effect - the message gets chunked by TCP anyway. +MAX_CHUNK_LEN = 8164 + +# Function modified from https://docs.python.org/3/library/socket.html#socket.socket.recvmsg +def recv_json_and_fds(sock, maxfds=0): + chunks = [] fds = array.array("i") # Array of ints - msg, ancdata, flags, addr = sock.recvmsg( - msglen, socket.CMSG_LEN(maxfds * fds.itemsize) - ) - for cmsg_level, cmsg_type, cmsg_data in ancdata: - if cmsg_level == socket.SOL_SOCKET and cmsg_type == socket.SCM_RIGHTS: - fds.frombytes(cmsg_data[: len(cmsg_data) - (len(cmsg_data) % fds.itemsize)]) - return msg, list(fds) + while True: + chunk, ancdata, flags, addr = sock.recvmsg( + MAX_CHUNK_LEN, socket.CMSG_LEN(maxfds * fds.itemsize) + ) + chunks.append(chunk) + for cmsg_level, cmsg_type, cmsg_data in ancdata: + if ( + cmsg_level == socket.SOL_SOCKET + and cmsg_type == socket.SCM_RIGHTS + and cmsg_data + ): + fds.frombytes( + cmsg_data[: len(cmsg_data) - (len(cmsg_data) % fds.itemsize)] + ) + if not chunk or chunk.rstrip().endswith(b"}"): + break + + return b"".join(chunks), list(fds) diff --git a/tests/scripts/e2e-1.sh b/tests/scripts/e2e-1.sh index 4496ac909..77d9bc2d1 100755 --- a/tests/scripts/e2e-1.sh +++ b/tests/scripts/e2e-1.sh @@ -19,8 +19,12 @@ echo "Test data is at: ${TEST_GPKG}" TMP_PATH=$(mktemp -q -d -t "kart-e2e.XXXXXX") echo "Using temp folder: ${TMP_PATH}" +export KART_HELPER_LOG=${TMP_PATH}/kart-helper.log function do_cleanup { + if [ -f "$KART_HELPER_LOG" ]; then + cat "$KART_HELPER_LOG" + fi rm -rf "$TMP_PATH" } trap do_cleanup EXIT @@ -38,7 +42,7 @@ mkdir "${TMP_PATH}/test" cd "${TMP_PATH}/test" set -x -echo "Using helper mode: ${KART_USE_HELPER:-0}" +echo "Using helper mode: ${KART_USE_HELPER:-?}" kart -vvvv install tab-completion --shell auto