Resolve struct field types and byte offsets in the parsed IDL

.github/actions/provision-meos/action.yml

@@ -20,7 +20,7 @@ inputs:
 outputs:
   catalog-path:
     description: Absolute path to the generated meos-idl.json.
-    value: ${{ steps.catalog.outputs.catalog-path }}
+    value: ${{ steps.catalog.outputs.catalog-path || steps.catalog_installed.outputs.catalog-path }}
   libmeos-prefix:
     description: Install prefix of libmeos when build-libmeos=true (else empty).
     value: ${{ steps.libmeos.outputs.libmeos-prefix }}
@@ -67,12 +67,15 @@ runs:
         python -m pip install --upgrade pip
         pip install -r "$REPO_ROOT/requirements.txt"
 
-    # Generate the catalog exactly as a downstream consumer does: parse the
-    # pinned MobilityDB MEOS headers with libclang and emit output/meos-idl.json,
-    # pointing the @sqlfn/@ingroup extraction (MDB_SRC_ROOT) at the SAME pinned
-    # checkout so the catalog is reproducibly equivalent to that commit.
+    # Catalog-only path (no libmeos build): parse the pinned MobilityDB MEOS
+    # source headers with libclang and emit output/meos-idl.json, pointing the
+    # @sqlfn/@ingroup extraction (MDB_SRC_ROOT) at the SAME pinned checkout so
+    # the catalog is reproducibly equivalent to that commit. The source-tree
+    # headers wrap PostgreSQL types in stubs conditionally, so struct field
+    # layouts are approximate here; the build path below derives them exactly.
     - name: Generate the catalog
       id: catalog
+      if: inputs.build-libmeos != 'true'
       shell: bash
       run: |
         REPO_ROOT="$(cd "$GITHUB_ACTION_PATH/../../.." && pwd)"
@@ -133,6 +136,38 @@ runs:
           -DPOSTGRESQL_PG_CONFIG=/usr/lib/postgresql/17/bin/pg_config \
           .. 2>&1 | tee "$RUNNER_TEMP/cmake-configure.log"
         make -j "$(nproc)"
+        # Stage the MEOS headers alone into a clean prefix for the catalog parse
+        # BEFORE the shared install: /usr/local/include on the runner carries
+        # unrelated system headers (OpenSSL, PostgreSQL, …) that run.py's header
+        # glob would otherwise pull into the catalog; this prefix holds only MEOS
+        # headers. Runs before `sudo make install` so the build directory (where
+        # cmake writes install_manifest.txt) is still writable by the runner.
+        cmake --install . --prefix "$RUNNER_TEMP/meos-headers"
+        echo "installed MEOS headers under $RUNNER_TEMP/meos-headers/include"
         sudo make install
         echo "libmeos-prefix=/usr/local" >> "$GITHUB_OUTPUT"
         echo "libmeos installed under /usr/local"
+
+    # Build path: derive the catalog from the INSTALLED headers. `make install`
+    # writes the generated, self-contained headers (meos_export.h spliced with
+    # postgres_ext_defs.in.h in place of the source's `#include <postgres.h>`),
+    # so libclang resolves every struct field to its real C type and byte
+    # offset — the FFI-accurate layout the native bindings' `#[repr(C)]` structs
+    # and cgo/cffi field access require. MDB_SRC_ROOT stays the source checkout
+    # for the @sqlfn/@ingroup maps.
+    - name: Generate the catalog from installed headers
+      id: catalog_installed
+      if: inputs.build-libmeos == 'true'
+      shell: bash
+      run: |
+        REPO_ROOT="$(cd "$GITHUB_ACTION_PATH/../../.." && pwd)"
+        cd "$REPO_ROOT"
+        MDB_SRC_ROOT="$GITHUB_WORKSPACE/_mobilitydb_src" \
+          python3 run.py "$RUNNER_TEMP/meos-headers/include"
+        CATALOG="$REPO_ROOT/output/meos-idl.json"
+        if [ ! -s "$CATALOG" ]; then
+          echo "::error::catalog not generated or empty at $CATALOG"
+          exit 1
+        fi
+        echo "catalog-path=$CATALOG" >> "$GITHUB_OUTPUT"
+        echo "Catalog written from installed headers: $CATALOG ($(wc -c < "$CATALOG") bytes)"

.github/workflows/pytest.yml

@@ -2,12 +2,11 @@ name: tests
 
 # Dogfoods the reusable provision-meos composite action, the single step every
 # binding uses to obtain the MEOS artifacts from one pinned MobilityDB commit.
-# Job 1 derives output/meos-idl.json through the action and runs the pytest
-# suite (type recovery + portable alias parity) against it — the same catalog
-# correctness signal master had before, now produced by the shared action.
-# Job 2 exercises the action's optional libmeos path (all families, incl.
-# pgPointCloud via PGDG PG17) so the whole action is validated before any
-# binding depends on it.
+# The action builds and installs all-families libmeos (incl. pgPointCloud via
+# PGDG PG17) and derives output/meos-idl.json from the installed, self-contained
+# headers, then the pytest suite (type recovery + struct layout + portable
+# alias parity) runs against that catalog — the same catalog correctness signal
+# master had before, now produced by the shared action and FFI-accurate.
 
 on:
   pull_request:
@@ -17,19 +16,26 @@ on:
 
 jobs:
   tests:
-    name: Regenerate catalog + pytest
+    name: Build libmeos + regenerate catalog + pytest
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
 
-      # Derive output/meos-idl.json from MobilityDB master via the shared
-      # action — the exact step a downstream binding runs.
-      - name: Provision the MEOS catalog
+      # Build + install all-families libmeos and derive the catalog from the
+      # installed headers — the exact step a native/FFI binding runs.
+      - name: Provision the MEOS catalog and libmeos
         id: provision
         uses: ./.github/actions/provision-meos
         with:
           mobilitydb-ref: master
-          build-libmeos: "false"
+          build-libmeos: "true"
+
+      - name: Assert libmeos.so is installed and print the family summary
+        run: |
+          test -f "${{ steps.provision.outputs.libmeos-prefix }}/lib/libmeos.so"
+          echo "libmeos installed at ${{ steps.provision.outputs.libmeos-prefix }}/lib/libmeos.so"
+          echo "----- cmake family summary -----"
+          grep -E 'Including|Excluding' "$RUNNER_TEMP/cmake-configure.log" || true
 
       - name: Install pytest
         run: pip install pytest
@@ -48,24 +54,3 @@ jobs:
           name: meos-idl
           path: output/meos-idl.json
           if-no-files-found: error
-
-  native:
-    name: Build libmeos (all families)
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v4
-
-      # Same action, optional libmeos path: build + install all families.
-      - name: Provision the MEOS catalog and libmeos
-        id: provision
-        uses: ./.github/actions/provision-meos
-        with:
-          mobilitydb-ref: master
-          build-libmeos: "true"
-
-      - name: Assert libmeos.so is installed and print the family summary
-        run: |
-          test -f "${{ steps.provision.outputs.libmeos-prefix }}/lib/libmeos.so"
-          echo "libmeos installed at ${{ steps.provision.outputs.libmeos-prefix }}/lib/libmeos.so"
-          echo "----- cmake family summary -----"
-          grep -E 'Including|Excluding' "$RUNNER_TEMP/cmake-configure.log" || true

parser/extractors.py

@@ -64,16 +64,21 @@ def find_unlisted_foreign_structs(idl) -> list:
     return sorted(elaborated - bare - set(_EXTERNAL_OPAQUE_STRUCTS))
 
 
+def _bool_norm(spelling: str) -> str:
+    # Normalise clang's ``_Bool`` keyword to the ``bool`` spelling the catalog
+    # uses, token-wise so pointer/const forms (``_Bool *``, ``const _Bool *``)
+    # normalise too — clang spells the pointee's underlying keyword, not the
+    # ``bool`` macro.
+    return re.sub(r"\b_Bool\b", "bool", spelling)
+
+
 def _c_spelling(ty) -> str:
     # Return the declared C spelling, with ``_Bool`` normalised to ``"bool"``.
     # Two bool representations arise depending on which postgres_int_defs.h is
     # in play:
     # - PostgreSQL headers: ``typedef char bool``  -> spelling already ``"bool"``
     # - Stub header:        ``#define bool _Bool`` -> spelling is ``"_Bool"``
-    spelling = ty.spelling
-    if spelling == "_Bool":
-        return "bool"
-    return _demote_external_opaque(spelling)
+    return _demote_external_opaque(_bool_norm(ty.spelling))
 
 
 # Canonical spellings of plain C scalars/builtins.
@@ -131,8 +136,8 @@ def _canonical_c_spelling(ty) -> str:
         return "bool"
     preserved = _preserved_opaque(ty)
     if preserved is not None:
-        return _demote_external_opaque(preserved)
-    return _demote_external_opaque(_canonical_spelling(ty))
+        return _bool_norm(_demote_external_opaque(preserved))
+    return _bool_norm(_demote_external_opaque(_canonical_spelling(ty)))
 
 
 # -----------------------------------------------------------------------------
@@ -217,7 +222,7 @@ def extract_struct(node) -> dict:
         "fields": [
             {
                 "name": f.spelling,
-                "cType": f.type.spelling,
+                "cType": _c_spelling(f.type),
                 "offset_bits": node.type.get_offset(f.spelling),
             }
             for f in node.get_children()

parser/parser.py

@@ -1,13 +1,82 @@
 import re
 import clang.cindex
 import json
+import subprocess
 import tempfile
 import os
 from pathlib import Path
 
 from parser.extractors import extract_function, extract_struct, extract_enum
 from parser.type_resolver import resolve_idl_types
 
+
+def _compiler_system_includes() -> list[str]:
+    """The C compiler's system header search path (builtin resource dir,
+    ``/usr/local/include``, ``/usr/include/<triple>``, ``/usr/include``).
+
+    The pip ``libclang`` wheel ships the shared library but *not* a configured
+    system include path, so the amalgamation fails to find ``<stdbool.h>`` /
+    ``<setjmp.h>`` / ``size_t`` and every ``bool`` / ``uint8`` / ``Datum`` field
+    degrades to ``int`` with ``get_offset`` returning ``-1``. ``provision-meos``
+    installs ``clang``, so ask the installed compiler for its own search list
+    (``clang -E -v``) rather than guessing paths; every entry there is added as
+    ``-isystem`` so a real MEOS compile and this parse see the same headers.
+    """
+    for cc in ("clang", "cc", "gcc"):
+        try:
+            out = subprocess.run(
+                [cc, "-E", "-x", "c", "-v", os.devnull],
+                capture_output=True, text=True, check=True).stderr
+        except Exception:
+            continue
+        dirs, capture = [], False
+        for line in out.splitlines():
+            if line.startswith("#include <...> search starts here:"):
+                capture = True
+                continue
+            if line.startswith("End of search list."):
+                break
+            if capture:
+                d = line.strip()
+                if d and os.path.isdir(d):
+                    dirs.append(d)
+        if dirs:
+            return dirs
+    return []
+
+
+def _clang_extra_args() -> list[str]:
+    """Include paths clang needs to resolve MEOS types to their real C types.
+
+    Without a configured search path the MEOS headers' standard-library
+    includes (``<stdbool.h>`` / ``<stddef.h>`` / ``size_t``) fail, degrading
+    every struct field to ``int`` at offset ``-1`` — unusable for the FFI
+    bindings (``#[repr(C)]`` structs, cgo/cffi field access). Add the
+    compiler's own system search path, and the external family (H3 / GDAL /
+    PROJ) headers when present. ``size_t`` is force-included via
+    ``-include stddef.h`` because ``meos.h`` uses it without an explicit
+    ``#include``.
+
+    The catalog is derived from the *installed* MEOS headers (the generated
+    ``meos_export.h`` written by ``make install``), which splice
+    ``postgres_ext_defs.in.h`` in place of the source tree's
+    ``#include <postgres.h>`` — so they are self-contained and no PostgreSQL
+    server headers are needed here.
+    """
+    args: list[str] = []
+
+    for d in _compiler_system_includes():
+        args.append(f"-isystem{d}")
+    if args:
+        args += ["-include", "stddef.h"]
+
+    for d in ("/usr/include/h3", "/usr/include/gdal", "/usr/include/proj"):
+        if os.path.isdir(d):
+            args.append(f"-isystem{d}")
+
+    return args
+
+
 def merge_meta(idl: dict, meta_path: Path) -> dict:
     with open(meta_path) as f:
         meta = json.load(f)
@@ -36,7 +105,7 @@ def parse_meos(entry: Path, include_dir: Path) -> dict:
         # it, and an undefined ``UNUSED`` makes clang error on the declarator and
         # silently drop the remaining parameters of that prototype.
         "-DUNUSED=__attribute__((unused))",
-    ])
+    ] + _clang_extra_args())
 
     # Collect all .h files belonging to the project
     own_files = {str(p.resolve()) for p in include_dir.glob("**/*.h")}

tests/test_struct_layout.py

@@ -0,0 +1,84 @@
+"""Regression tests for FFI-accurate struct layouts in the IDL.
+
+The amalgamation parse must resolve struct field types to their real C types
+(``uint8`` / ``bool`` / ``Datum`` — not the ``int`` the preprocessor degrades
+them to when ``<stdbool.h>`` is unreachable) and must compute real byte offsets
+(not ``-1``). Both depend on ``parser.parser._clang_extra_args`` putting clang's
+builtin resource dir (``stdbool.h`` / ``stddef.h`` / ``stdint.h``) on the parse
+path. If that regresses, every field collapses to ``int`` with ``offset_bits ==
+-1`` and the FFI bindings (``#[repr(C)]`` structs, cgo/cffi field access) lose
+their layout — so these assertions fail loudly.
+
+Field types are also normalised to the catalog's ``bool`` spelling (clang spells
+the underlying ``_Bool`` keyword) and offsets are in bits.
+
+The IDL is generated, not committed; run ``python run.py`` first.
+"""
+import json
+import unittest
+from pathlib import Path
+
+IDL = Path(__file__).resolve().parents[1] / "output" / "meos-idl.json"
+
+
+class StructLayoutTests(unittest.TestCase):
+    def setUp(self):
+        if not IDL.exists():
+            self.skipTest(f"{IDL} not generated; run `python run.py` first")
+        idl = json.loads(IDL.read_text())
+        self.by_name = {s["name"]: s for s in idl["structs"]}
+
+    def _fields(self, name):
+        self.assertIn(name, self.by_name, f"{name} missing from IDL structs")
+        return {f["name"]: f for f in self.by_name[name]["fields"]}
+
+    def test_span_layout_resolved(self):
+        # Pre-fix every field was ``int`` at offset ``-1``.
+        f = self._fields("Span")
+        self.assertEqual((f["spantype"]["cType"], f["spantype"]["offset_bits"]),
+                         ("uint8", 0))
+        self.assertEqual((f["basetype"]["cType"], f["basetype"]["offset_bits"]),
+                         ("uint8", 8))
+        self.assertEqual(f["lower_inc"]["cType"], "bool")
+        self.assertEqual(f["upper_inc"]["cType"], "bool")
+        self.assertEqual((f["lower"]["cType"], f["lower"]["offset_bits"]),
+                         ("Datum", 64))
+        self.assertEqual((f["upper"]["cType"], f["upper"]["offset_bits"]),
+                         ("Datum", 128))
+
+    def test_stbox_layout_resolved(self):
+        f = self._fields("STBox")
+        self.assertEqual(f["period"]["cType"], "Span")
+        self.assertEqual(f["period"]["offset_bits"], 0)
+        self.assertEqual((f["xmin"]["cType"], f["xmin"]["offset_bits"]),
+                         ("double", 192))
+
+    def test_tinstant_layout_resolved(self):
+        f = self._fields("TInstant")
+        self.assertEqual((f["temptype"]["cType"], f["temptype"]["offset_bits"]),
+                         ("uint8", 32))
+        self.assertEqual((f["t"]["cType"], f["t"]["offset_bits"]),
+                         ("TimestampTz", 64))
+        self.assertEqual((f["value"]["cType"], f["value"]["offset_bits"]),
+                         ("Datum", 128))
+
+    def test_no_bool_keyword_left_as_underscore_Bool(self):
+        # Clang spells the keyword ``_Bool``; the catalog normalises to ``bool``.
+        for s in self.by_name.values():
+            for fld in s["fields"]:
+                self.assertNotIn("_Bool", fld["cType"],
+                                 f"{s['name']}.{fld['name']} not normalised")
+
+    def test_core_structs_have_real_offsets(self):
+        # No CORE-family struct field may keep the degraded ``-1`` offset.
+        for s in self.by_name.values():
+            if s.get("family") != "CORE":
+                continue
+            for fld in s["fields"]:
+                self.assertGreaterEqual(
+                    fld["offset_bits"], 0,
+                    f"{s['name']}.{fld['name']} has unresolved offset")
+
+
+if __name__ == "__main__":
+    unittest.main()