Add execOne() to simplify exec for build, documentation, and testing.

The core Exec object is efficient but geared toward the specific needs of core and not ease-of-use as required for build, documentation, and testing.

execOne() works similarly to system() except that it automatically redirects stderr to stdout and captures the output.

src/build/common/exec.c

@@ -0,0 +1,87 @@
+/***********************************************************************************************************************************
+Execute Process Extensions
+***********************************************************************************************************************************/
+// Include core module
+#include "common/exec.c"
+
+#include "build/common/exec.h"
+#include "common/io/fd.h"
+
+/**********************************************************************************************************************************/
+static String *
+execProcess(Exec *const this)
+{
+    FUNCTION_LOG_BEGIN(logLevelDebug);
+        FUNCTION_LOG_PARAM(EXEC, this);
+    FUNCTION_LOG_END();
+
+    String *const result = strNew();
+
+    MEM_CONTEXT_TEMP_BEGIN()
+    {
+        int processStatus;
+        int processResult;
+
+        ioReadOpen(this->ioReadFd);
+        strCat(result, strNewBuf(ioReadBuf(this->ioReadFd)));
+
+        THROW_ON_SYS_ERROR(
+            (processResult = waitpid(this->processId, &processStatus, 0)) == -1, ExecuteError,
+            "unable to wait on child process");
+
+        // Clear the process id so we don't try to wait for this process on free
+        this->processId = 0;
+
+        // If the process exited normally but without a success status
+        if (WIFEXITED(processStatus))
+        {
+            if (WEXITSTATUS(processStatus) != 0)
+                execCheckStatusError(this, processStatus, strTrim(result));
+        }
+        // Else if the process did not exit normally then it must have been a signal
+        else
+            execCheckSignalError(this, processStatus);
+    }
+    MEM_CONTEXT_TEMP_END();
+
+    FUNCTION_LOG_RETURN(STRING, result);
+}
+
+/**********************************************************************************************************************************/
+FN_EXTERN String *
+execOne(const String *const command, const ExecOneParam param)
+{
+    FUNCTION_LOG_BEGIN(logLevelDebug);
+        FUNCTION_LOG_PARAM(STRING, command);
+        (void)param;
+    FUNCTION_LOG_END();
+
+    String *result = NULL;
+
+    MEM_CONTEXT_TEMP_BEGIN()
+    {
+        const StringList *const shellList = strLstNewSplitZ(param.shell != NULL ? param.shell : STRDEF("sh -c"), " ");
+        StringList *const param = strLstNew();
+
+        ASSERT(strLstSize(shellList) != 0);
+
+        for (unsigned int shellIdx = 1; shellIdx < strLstSize(shellList); shellIdx++)
+            strLstAdd(param, strLstGet(shellList, shellIdx));
+
+        strLstAddFmt(param, "%s 2>&1", strZ(command));
+        strLstAddZ(param, "2>&1");
+
+        Exec *const exec = execNew(strLstGet(shellList, 0), param, command, ioTimeoutMs());
+
+        execOpen(exec);
+
+        MEM_CONTEXT_PRIOR_BEGIN()
+        {
+            result = execProcess(exec);
+        }
+        MEM_CONTEXT_PRIOR_END();
+    }
+    MEM_CONTEXT_TEMP_END();
+
+    FUNCTION_LOG_RETURN(STRING, result);
+}

src/build/common/exec.h

@@ -0,0 +1,27 @@
+/***********************************************************************************************************************************
+Execute Process Extensions
+
+Functions intended to simplify exec'ing and getting output. The core Exec object is efficient but it does not work well for the
+requirements of the build, test, and doc which prefer ease of use.
+***********************************************************************************************************************************/
+#ifndef BUILD_COMMON_EXEC_H
+#define BUILD_COMMON_EXEC_H
+
+#include "common/exec.h"
+
+/***********************************************************************************************************************************
+Functions
+***********************************************************************************************************************************/
+// Execute a command similar to system() while also capturing output. Note that stderr is redirected to stdout.
+typedef struct ExecOneParam
+{
+    VAR_PARAM_HEADER;
+    const String *shell;                                            // Shell command to use for exec (default is sh -c)
+} ExecOneParam;
+
+#define execOneP(command, ...)                                                                                                     \
+    execOne(command, (ExecOneParam){VAR_PARAM_INIT, __VA_ARGS__})
+
+FN_EXTERN String *execOne(const String *command, ExecOneParam param);
+
+#endif

src/common/exec.c

@@ -27,7 +27,6 @@ Object type
 struct Exec
 {
     ExecPub pub;                                                    // Publicly accessible variables
-    String *command;                                                // Command to execute
     StringList *param;                                              // List of parameters to pass to command
     const String *name;                                             // Name to display in log/error messages
     TimeMSec timeout;                                               // Timeout for any i/o operation (read, write, etc.)
@@ -129,7 +128,10 @@ execNew(const String *command, const StringList *param, const String *name, Time
     {
         *this = (Exec)
         {
-            .command = strDup(command),
+            .pub =
+            {
+                .command = strDup(command),
+            },
             .name = strDup(name),
             .timeout = timeout,
 
@@ -138,7 +140,7 @@ execNew(const String *command, const StringList *param, const String *name, Time
         };
 
         // The first parameter must be the command
-        strLstInsert(this->param, 0, this->command);
+        strLstInsert(this->param, 0, execCommand(this));
     }
     OBJ_NEW_END();
 
@@ -151,6 +153,31 @@ Check if the process is still running
 This should be called when anything unexpected happens while reading or writing, including errors and eof. If this function returns
 then the original error should be rethrown.
 ***********************************************************************************************************************************/
+// Helper to throw status error
+static void
+execCheckStatusError(Exec *const this, const int status, const String *const output)
+{
+    FUNCTION_TEST_BEGIN();
+        FUNCTION_LOG_PARAM(EXEC, this);
+        FUNCTION_TEST_PARAM(INT, status);
+        FUNCTION_TEST_PARAM(STRING, output);
+    FUNCTION_TEST_END();
+
+    // Throw the error with as much information as is available
+    THROWP_FMT(
+        errorTypeFromCode(WEXITSTATUS(status)), "%s terminated unexpectedly [%d]%s%s", strZ(this->name),
+        WEXITSTATUS(status), strSize(output) > 0 ? ": " : "", strSize(output) > 0 ? strZ(output) : "");
+
+    FUNCTION_TEST_RETURN_VOID();
+}
+
+// Helper to throw signal error
+static void
+execCheckSignalError(Exec *const this, const int status)
+{
+    THROW_FMT(ExecuteError, "%s terminated unexpectedly on signal %d", strZ(this->name), WTERMSIG(status));
+}
+
 static void
 execCheck(Exec *this)
 {
@@ -174,18 +201,13 @@ execCheck(Exec *this)
         // If the process exited normally
         if (WIFEXITED(processStatus))
         {
-            // Get data from stderr to help diagnose the problem
+            execCheckStatusError(
-            String *errorStr = strTrim(
+                this, processStatus,
-                strNewBuf(ioReadBuf(ioFdReadNewOpen(strNewFmt("%s error", strZ(this->name)), this->fdError, 0))));
+                strTrim(strNewBuf(ioReadBuf(ioFdReadNewOpen(strNewFmt("%s error", strZ(this->name)), this->fdError, 0)))));
-
-            // Throw the error with as much information as is available
-            THROWP_FMT(
-                errorTypeFromCode(WEXITSTATUS(processStatus)), "%s terminated unexpectedly [%d]%s%s", strZ(this->name),
-                WEXITSTATUS(processStatus), strSize(errorStr) > 0 ? ": " : "", strSize(errorStr) > 0 ? strZ(errorStr) : "");
         }
 
         // If the process did not exit normally then it must have been a signal
-        THROW_FMT(ExecuteError, "%s terminated unexpectedly on signal %d", strZ(this->name), WTERMSIG(processStatus));
+        execCheckSignalError(this, processStatus);
     }
 
     FUNCTION_LOG_RETURN_VOID();
@@ -332,11 +354,11 @@ execOpen(Exec *this)
         PIPE_DUP2(pipeError, 1, STDERR_FILENO);
 
         // Execute the binary. This statement will not return if it is successful
-        execvp(strZ(this->command), (char **const)strLstPtr(this->param));
+        execvp(strZ(execCommand(this)), (char **const)strLstPtr(this->param));
 
         // If we got here then there was an error. We can't use a throw as we normally would because we have already shutdown
         // logging and we don't want to execute exit paths that might free parent resources which we still have references to.
-        fprintf(stderr, "unable to execute '%s': [%d] %s\n", strZ(this->command), errno, strerror(errno));
+        fprintf(stderr, "unable to execute '%s': [%d] %s\n", strZ(execCommand(this)), errno, strerror(errno));
         exit(errorTypeCode(&ExecuteError));
     }
 

src/common/exec.h

@@ -29,10 +29,18 @@ Getters/Setters
 ***********************************************************************************************************************************/
 typedef struct ExecPub
 {
+    String *command;                                                // Command to execute
     IoRead *ioReadExec;                                             // Wrapper for file descriptor read interface
     IoWrite *ioWriteExec;                                           // Wrapper for file descriptor write interface
 } ExecPub;
 
+// Exec command
+FN_INLINE_ALWAYS const String *
+execCommand(const Exec *const this)
+{
+    return THIS_PUB(Exec)->command;
+}
+
 // Read interface
 FN_INLINE_ALWAYS IoRead *
 execIoRead(Exec *const this)

test/define.yaml

@@ -406,10 +406,10 @@ unit:
 
       # ----------------------------------------------------------------------------------------------------------------------------
       - name: exec
-        total: 1
+        total: 2
 
         coverage:
-          - common/exec
+          - build/common/exec
 
       # ----------------------------------------------------------------------------------------------------------------------------
       - name: ini
@@ -483,7 +483,7 @@ unit:
           - protocol/server
 
         include:
-          - common/exec
+          - build/common/exec
 
   # ********************************************************************************************************************************
   - name: config
@@ -700,7 +700,7 @@ unit:
     test:
       # ----------------------------------------------------------------------------------------------------------------------------
       - name: test
-        total: 3
+        total: 2
 
         coverage:
           - test/command/test/build

test/src/command/test/test.c

@@ -5,43 +5,13 @@ Test Command
 
 #include <stdlib.h>
 
+#include "build/common/exec.h"
 #include "command/test/build.h"
 #include "common/debug.h"
 #include "common/log.h"
 #include "config/config.h"
 #include "storage/posix/storage.h"
 
-/**********************************************************************************************************************************/
-static const String *cmdTestExecLog;
-
-static void
-cmdTestExec(const String *const command)
-{
-    FUNCTION_LOG_BEGIN(logLevelDebug);
-        FUNCTION_LOG_PARAM(STRING, command);
-    FUNCTION_LOG_END();
-
-    ASSERT(cmdTestExecLog != NULL);
-    ASSERT(command != NULL);
-
-    MEM_CONTEXT_TEMP_BEGIN()
-    {
-        LOG_DETAIL_FMT("exec: %s", strZ(command));
-
-        if (system(zNewFmt("%s > %s 2>&1", strZ(command), strZ(cmdTestExecLog))) != 0)
-        {
-            const Buffer *const buffer = storageGetP(storageNewReadP(storagePosixNewP(FSLASH_STR), cmdTestExecLog));
-
-            THROW_FMT(
-                ExecuteError, "unable to execute: %s > %s 2>&1:\n%s", strZ(command), strZ(cmdTestExecLog),
-                zNewFmt("\n%s", strZ(strTrim(strNewBuf(buffer)))));
-        }
-    }
-    MEM_CONTEXT_TEMP_END();
-
-    FUNCTION_LOG_RETURN_VOID();
-}
-
 /**********************************************************************************************************************************/
 static void
 cmdTestPathCreate(const Storage *const storage, const String *const path)
@@ -55,17 +25,19 @@ cmdTestPathCreate(const Storage *const storage, const String *const path)
 
     ASSERT(storage != NULL);
 
+    const String *const rmCommand = strNewFmt("rm -rf '%s'/*", strZ(storagePathP(storage, path)));
+
     TRY_BEGIN()
     {
-        storagePathRemoveP(storage, path, .recurse = true);
+        execOneP(rmCommand);
     }
     CATCH_ANY()
     {
         // Reset permissions
-        cmdTestExec(strNewFmt("chmod -R 777 %s", strZ(storagePathP(storage, path))));
+        execOneP(strNewFmt("chmod -R 777 '%s'", strZ(storagePathP(storage, path))));
 
         // Try to remove again
-        storagePathRemoveP(storage, path, .recurse = true);
+        execOneP(rmCommand);
     }
     TRY_END();
 
@@ -100,9 +72,6 @@ cmdTest(
 
     MEM_CONTEXT_TEMP_BEGIN()
     {
-        // Log file name
-        cmdTestExecLog = strNewFmt("%s/exec-%u.log", strZ(pathTest), vmId);
-
         // Find test
         ASSERT(moduleName != NULL);
 
@@ -144,9 +113,9 @@ cmdTest(
                 {
                     LOG_DETAIL("meson setup");
 
-                    cmdTestExec(
+                    execOneP(
                         strNewFmt(
-                            "meson setup -Dwerror=true -Dfatal-errors=true %s %s %s", strZ(mesonSetup), strZ(pathUnitBuild),
+                            "meson setup -Dwerror=true -Dfatal-errors=true %s '%s' '%s'", strZ(mesonSetup), strZ(pathUnitBuild),
                             strZ(pathUnit)));
                 }
                 // Else reconfigure
@@ -154,7 +123,7 @@ cmdTest(
                 {
                     LOG_DETAIL("meson configure");
 
-                    cmdTestExec(strNewFmt("meson configure %s %s", strZ(mesonSetup), strZ(pathUnitBuild)));
+                    execOneP(strNewFmt("meson configure %s '%s'", strZ(mesonSetup), strZ(pathUnitBuild)));
                 }
 
                 // Remove old coverage data. Note that coverage can be in different paths depending on the meson version.
@@ -175,7 +144,7 @@ cmdTest(
                 storageRemoveP(storageUnitBuild, STRDEF("gmon.out"));
 
                 // Ninja build
-                cmdTestExec(strNewFmt("ninja -C %s", strZ(pathUnitBuild)));
+                execOneP(strNewFmt("ninja -C '%s'", strZ(pathUnitBuild)));
                 buildRetry = false;
             }
             CATCH_ANY()

test/src/meson.build

@@ -26,6 +26,7 @@ subdir('command/help')
 # test target
 ####################################################################################################################################
 src_test = [
+    '../../src/build/common/exec.c',
     '../../src/build/common/render.c',
     '../../src/build/common/string.c',
     '../../src/build/common/yaml.c',
@@ -34,6 +35,7 @@ src_test = [
     '../../src/command/help/help.c',
 	'../../src/common/compress/bz2/common.c',
 	'../../src/common/compress/bz2/decompress.c',
+    '../../src/common/fork.c',
     '../../src/common/ini.c',
     '../../src/common/io/fd.c',
     '../../src/common/io/fdRead.c',

test/src/module/common/execTest.c

@@ -108,5 +108,43 @@ testRun(void)
         TEST_RESULT_VOID(execFree(exec), "sleep exited as expected");
     }
 
+    // *****************************************************************************************************************************
+    if (testBegin("execOne()"))
+    {
+        Exec *exec = NULL;
+
+        // -------------------------------------------------------------------------------------------------------------------------
+        TEST_TITLE("exec without output");
+
+        TEST_RESULT_STR_Z(execOneP(STRDEF("ls " TEST_PATH)), "", "exec ls");
+
+        // -------------------------------------------------------------------------------------------------------------------------
+        TEST_TITLE("touch file");
+
+        TEST_RESULT_STR_Z(execOneP(STRDEF("touch " TEST_PATH "/file")), "", "exec touch");
+
+        // -------------------------------------------------------------------------------------------------------------------------
+        TEST_TITLE("exec with custom shell and output");
+
+        TEST_RESULT_STR_Z(execOneP(STRDEF("ls " TEST_PATH), .shell = STRDEF("sh -c")), "file\n", "exec ls");
+
+        // -------------------------------------------------------------------------------------------------------------------------
+        TEST_TITLE("exec exits from signal");
+
+        TEST_ASSIGN(exec, execNew(STRDEF("cat"), NULL, STRDEF("cat"), 1000), "new cat exec");
+        TEST_RESULT_VOID(execOpen(exec), "open cat exec");
+        kill(exec->processId, SIGKILL);
+
+        TEST_ERROR(execProcess(exec), ExecuteError, "cat terminated unexpectedly on signal 9");
+        TEST_RESULT_VOID(execFree(exec), "free exec");
+
+        // -------------------------------------------------------------------------------------------------------------------------
+        TEST_TITLE("exec exits with error");
+
+        TEST_ERROR(
+            execOneP(STRDEF("cat missing.txt")), UnknownError,
+            "cat missing.txt terminated unexpectedly [1]: cat: missing.txt: No such file or directory");
+    }
+
     FUNCTION_HARNESS_RETURN_VOID();
 }

test/src/module/protocol/protocolTest.c

@@ -299,7 +299,7 @@ testRun(void)
         OBJ_NEW_BASE_BEGIN(Exec, .childQty = MEM_CONTEXT_QTY_MAX, .callbackQty = 1)
         {
             protocolHelperClient.exec = OBJ_NEW_ALLOC();
-            *protocolHelperClient.exec = (Exec){.name = strNewZ("test"), .command = strNewZ("test"), .processId = INT_MAX};
+            *protocolHelperClient.exec = (Exec){.pub = {.command = strNewZ("test")}, .name = strNewZ("test"), .processId = INT_MAX};
             memContextCallbackSet(memContextCurrent(), execFreeResource, protocolHelperClient.exec);
         }
         OBJ_NEW_END();

test/src/module/test/testTest.c

@@ -66,28 +66,6 @@ testRun(void)
         TEST_RESULT_STR_Z(cmdBldPathRelative(STRDEF("/tmp"), STRDEF("/tmp/sub")), "sub", "base is sub of compare");
     }
 
-    // *****************************************************************************************************************************
-    if (testBegin("cmdTestExec()"))
-    {
-        cmdTestExecLog = STRDEF(TEST_PATH "/error.log");
-        bool error = false;
-
-        // The error will vary by OS so just make sure an error was thrown
-        TRY_BEGIN()
-        {
-            cmdTestExec(STRDEF("/bogus/bogus"));
-        }
-        CATCH(ExecuteError)
-        {
-            error = true;
-        }
-        TRY_END();
-
-        TEST_RESULT_BOOL(error, true, "an error should be thrown");
-
-        cmdTestExecLog = NULL;
-    }
-
     // *****************************************************************************************************************************
     if (testBegin("TestDef and TestBuild"))
     {