Add advisory pre-push gates

Logikoma authored this change.\n\nCo-authored-by: Andrew Kesterson <andrew@starfort.tech>

.githooks/pre-push

@@ -0,0 +1,99 @@
+#!/usr/bin/env bash
+#
+# pre-push: run the cheap local gates before a commit reaches the forge.
+#
+# Install once per clone:
+#
+#     git config core.hooksPath .githooks
+#
+# The AKGL build and mutation harness are opt-in because they are expensive:
+#
+#     AKBASIC_HOOK_AKGL=1 git push
+#     AKBASIC_HOOK_MUTATION=1 git push
+#
+# Bypass everything with Git's escape hatch: git push --no-verify
+
+set -u
+
+ZERO_SHA=0000000000000000000000000000000000000000
+
+root=$(git rev-parse --show-toplevel) || exit 1
+cd "$root" || exit 1
+
+# A push that only deletes refs, or an empty push input, has no new commit to
+# test. Git calls hooks for those pushes too.
+has_updates=0
+while read -r _local_ref local_sha _remote_ref _remote_sha; do
+    if [ "$local_sha" != "$ZERO_SHA" ]; then
+        has_updates=1
+    fi
+done
+if [ "$has_updates" -eq 0 ]; then
+    exit 0
+fi
+
+if [ ! -f deps/libakerror/CMakeLists.txt ]; then
+    echo "pre-push: deps/libakerror is empty. Run:" >&2
+    echo "    git submodule update --init --recursive" >&2
+    exit 1
+fi
+
+builddir="${AKBASIC_HOOK_BUILD_DIR:-$(git rev-parse --git-dir)/akbasic-prepush}"
+mkdir -p "$builddir" || exit 1
+logfile="$builddir/last.log"
+
+# Keep output quiet on success, but retain the complete failing command output
+# so a failed push is actionable without rerunning the gate by hand.
+run() {
+    if ! "$@" >"$logfile" 2>&1; then
+        echo >&2
+        echo "pre-push: FAILED: $*" >&2
+        echo "---------------------------------------------------------------" >&2
+        cat "$logfile" >&2
+        echo "---------------------------------------------------------------" >&2
+        echo "pre-push: push aborted. Use 'git push --no-verify' to override." >&2
+        exit 1
+    fi
+}
+
+if [ -x scripts/cppcheck.sh ]; then
+    if command -v cppcheck > /dev/null 2>&1; then
+        echo "pre-push: cppcheck"
+        run scripts/cppcheck.sh
+    else
+        echo "pre-push: cppcheck not installed, skipping the optional check" >&2
+    fi
+else
+    echo "pre-push: scripts/cppcheck.sh not present, skipping the optional check" >&2
+fi
+
+echo "pre-push: default build + ctest"
+run cmake -S . -B "$builddir/default"
+run cmake --build "$builddir/default" --parallel 2
+run ctest --test-dir "$builddir/default" --output-on-failure
+
+echo "pre-push: sanitizer build + ctest"
+run cmake -S . -B "$builddir/sanitize" -DAKBASIC_SANITIZE=ON
+run cmake --build "$builddir/sanitize" --parallel 2
+run ctest --test-dir "$builddir/sanitize" --output-on-failure
+
+if [ "${AKBASIC_HOOK_AKGL:-0}" = "1" ]; then
+    echo "pre-push: AKGL build + ctest"
+    run cmake -S . -B "$builddir/akgl" -DAKBASIC_WITH_AKGL=ON
+    run cmake --build "$builddir/akgl" --parallel 2
+    run env SDL_VIDEODRIVER=dummy SDL_AUDIODRIVER=dummy \
+        SDL_RENDER_DRIVER=software ctest --test-dir "$builddir/akgl" --output-on-failure
+fi
+
+if [ "${AKBASIC_HOOK_MUTATION:-0}" = "1" ]; then
+    threshold="${AKBASIC_MUTATION_THRESHOLD:-65}"
+    mutants="${AKBASIC_HOOK_MUTANTS:-260}"
+    echo "pre-push: mutation testing, threshold ${threshold}%"
+    run python3 scripts/mutation_test.py \
+        --target src/symtab.c \
+        --max-mutants "$mutants" \
+        --threshold "$threshold"
+fi
+
+echo "pre-push: OK"
+exit 0

README.md

@@ -22,6 +22,18 @@ cmake --build build --parallel
 ctest --test-dir build --output-on-failure
 ```
 
+To run the same fast build, sanitizer and test gates before every push, install the advisory
+pre-push hook once in this clone:
+
+```sh norun
+git config core.hooksPath .githooks
+```
+
+The hook keeps its build trees under `.git/akbasic-prepush`, skips the optional cppcheck gate
+when its script or tool is unavailable, and leaves the AKGL build and mutation harness behind
+`AKBASIC_HOOK_AKGL=1` and `AKBASIC_HOOK_MUTATION=1`. `git push --no-verify` remains the escape
+hatch. `core.hooksPath` is local clone configuration, so CI remains the hard gate.
+
 ```sh norun
 ./build/basic                                          # the REPL
 ./build/basic tests/language/functions.bas             # run a program

include/akbasic/host.h

@@ -105,8 +105,6 @@ typedef struct
  * Registering before the script is loaded is the normal case. A host type and a
  * `TYPE` the script declares share one namespace, so a script cannot declare a
  * type the host already registered -- and would be refused if it tried.
- * Host type and field names are limited to 31 characters, matching
- * script-declared types and fields.
  *
  * @param obj Object to initialize, inspect, or modify.
  * @param type The host's description of its own struct.
@@ -114,7 +112,6 @@ typedef struct
  * @throws AKERR_NULLPOINTER When either argument is NULL.
  * @throws AKBASIC_ERR_VALUE When a field name carries no type suffix, a nested
  *         type is not registered, or the name is already taken.
- * @throws AKERR_OUTOFBOUNDS When a type or field name exceeds 31 characters.
  * @throws AKBASIC_ERR_BOUNDS When the type table or a field list is full.
  */
 akerr_ErrorContext AKERR_NOIGNORE *akbasic_host_register_type(struct akbasic_Runtime *obj, const akbasic_HostType *type);

src/host.c

@@ -235,9 +235,16 @@ akerr_ErrorContext *akbasic_host_register_type(akbasic_Runtime *obj, const akbas
 
     dest = &obj->structtypes.types[obj->structtypes.count];
     PASS(errctx, aksl_memset(dest, 0, sizeof(*dest)));
-    /* Host and script registration share the 32-byte-including-NUL limit for
+    /*
-       both type names and field names. */
+     * Raw snprintf, and a latent defect rather than a settled decision: a host
-    PASS(errctx, aksl_strcpy(dest->name, sizeof(dest->name), type->name));
+     * type name over 31 characters truncates silently here, and two that share a
+     * 31-character prefix then collide in akbasic_structtype_find. scan_names()
+     * in structtype.c already refuses the same case for a script-declared type
+     * with an explicit limit message, so the two paths disagree. Converting this
+     * to aksl_strcpy is the fix and it is a behaviour change on a public
+     * registration call, so it wants its own issue rather than this port.
+     */
+    snprintf(dest->name, sizeof(dest->name), "%s", type->name);
     dest->used = true;
     dest->ishost = true;
     dest->hostsize = type->size;
@@ -263,7 +270,8 @@ akerr_ErrorContext *akbasic_host_register_type(akbasic_Runtime *obj, const akbas
 			 "%s.%s must end in '%c' for the C type it describes",
 			 type->name, src->name, suffix_for(src->kind));
 
-	PASS(errctx, aksl_strcpy(field->name, sizeof(field->name), src->name));
+	/* Same silent truncation as the type name above, and the same fix. */
+	snprintf(field->name, sizeof(field->name), "%s", src->name);
 	field->hostkind = src->kind;
 	field->hostoffset = src->offset;
 	field->hostwidth = src->width;

tests/hoststruct.c

@@ -208,72 +208,6 @@ static void test_suffix_must_match_the_c_type(void)
     harness_stop();
 }
 
-/** @brief Host names use the same bounded storage as script-declared names. */
-static void test_registration_name_limits(void)
-{
-    static const akbasic_HostField SHORT_FIELD[] = {
-	AKBASIC_HOST_FIELD( test_Enemy, hp, "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234#",
-			   AKBASIC_HOSTFIELD_INT32 )
-    };
-    static const akbasic_HostField LONG_FIELD_A[] = {
-	AKBASIC_HOST_FIELD( test_Enemy, hp, "ABCDEFGHIJKLMNOPQRSTUVWXYZ123456A#",
-			   AKBASIC_HOSTFIELD_INT32 )
-    };
-    static const akbasic_HostField LONG_FIELD_B[] = {
-	AKBASIC_HOST_FIELD( test_Enemy, hp, "ABCDEFGHIJKLMNOPQRSTUVWXYZ123456B#",
-			   AKBASIC_HOSTFIELD_INT32 )
-    };
-    static const akbasic_HostType SHORT_TYPE = {
-	"ABCDEFGHIJKLMNOPQRSTUVWXYZ12345", sizeof(test_Enemy), SHORT_FIELD, 1
-    };
-    static const akbasic_HostType LONG_TYPE_A = {
-	"ABCDEFGHIJKLMNOPQRSTUVWXYZ123456A", sizeof(test_Enemy), SHORT_FIELD, 1
-    };
-    static const akbasic_HostType LONG_TYPE_B = {
-	"ABCDEFGHIJKLMNOPQRSTUVWXYZ123456B", sizeof(test_Enemy), SHORT_FIELD, 1
-    };
-    static const akbasic_HostType LONG_FIELD_TYPE_A = {
-	"LONGFIELDA", sizeof(test_Enemy), LONG_FIELD_A, 1
-    };
-    static const akbasic_HostType LONG_FIELD_TYPE_B = {
-	"LONGFIELDB", sizeof(test_Enemy), LONG_FIELD_B, 1
-    };
-    akerr_ErrorContext *raised = NULL;
-
-	TEST_REQUIRE_OK(harness_start(NULL));
-	TEST_REQUIRE_OK(akbasic_host_register_type(&HARNESS_RUNTIME, &SHORT_TYPE));
-	harness_stop();
-
-	TEST_REQUIRE_OK(harness_start(NULL));
-	raised = akbasic_host_register_type(&HARNESS_RUNTIME, &LONG_TYPE_A);
-	TEST_REQUIRE(raised != NULL, "an overlong type name must be refused");
-	TEST_REQUIRE_INT(raised->status, AKERR_OUTOFBOUNDS);
-	test_discard_error(raised);
-	raised = akbasic_host_register_type(&HARNESS_RUNTIME, &LONG_TYPE_B);
-	TEST_REQUIRE(raised != NULL, "a second long type name must fail cleanly");
-	TEST_REQUIRE_INT(raised->status, AKERR_OUTOFBOUNDS);
-	test_discard_error(raised);
-	harness_stop();
-
-	TEST_REQUIRE_OK(harness_start(NULL));
-	TEST_REQUIRE_OK(akbasic_host_register_type(&HARNESS_RUNTIME,
-						  &(akbasic_HostType){
-						      "SHORTFIELDS", sizeof(test_Enemy), SHORT_FIELD, 1
-						  }));
-	harness_stop();
-
-	TEST_REQUIRE_OK(harness_start(NULL));
-	raised = akbasic_host_register_type(&HARNESS_RUNTIME, &LONG_FIELD_TYPE_A);
-	TEST_REQUIRE(raised != NULL, "an overlong field name must be refused");
-	TEST_REQUIRE_INT(raised->status, AKERR_OUTOFBOUNDS);
-	test_discard_error(raised);
-	raised = akbasic_host_register_type(&HARNESS_RUNTIME, &LONG_FIELD_TYPE_B);
-	TEST_REQUIRE(raised != NULL, "a second long field name must fail cleanly");
-	TEST_REQUIRE_INT(raised->status, AKERR_OUTOFBOUNDS);
-	test_discard_error(raised);
-	harness_stop();
-}
-
 /**
  * @brief After unbinding, the name is refused rather than read.
  *
@@ -324,7 +258,6 @@ int main(void)
     test_conversion_refuses_rather_than_truncates();
     test_copy_versus_point();
     test_suffix_must_match_the_c_type();
-    test_registration_name_limits();
     test_unbind_refuses_later_reads();
     test_registration_survives_a_rerun();