[mypyc] Fix missing cross-group header deps in incremental builds (#21490)

## Problem

In `separate=True` mode, each group's `__native_internal_<mod>.h`
reaches into a sibling group's export-table header via an angled
include:

```c
// __native_internal_caller.h
#include <other_group/__native_other.h>
```

The consumer's `.o` file bakes in byte offsets from that struct at C
compile time. If the sibling group's struct layout shifts (e.g. a class
is inserted earlier in the source, shifting all subsequent offsets), the
consumer's `.o` must be recompiled, otherwise it silently resolves
offsets to the wrong object at runtime.

Two bugs in the old `get_header_deps` combined to hide this dep from
setuptools' `Extension.depends`:

1. **No transitive walk.** The cross-group include lives inside
`__native_internal_<mod>.h`, not in the `.c` file itself. The old code
only scanned the `.c` file's direct includes and never opened the
resolved headers.

2. **Single-pass resolution.** Deps were resolved per-group before
sibling groups had written their headers to disk, so the cross-group
header didn't yet exist even if we had looked.

## Fix

- **`_extract_includes` / `_INCLUDE_RE`**: distinguishes `#include
"foo"` (quoted) from `#include <foo>` (angled), matching the
preprocessor's own search rules — quoted form searches the includer's
directory first; angled form searches `-I` paths only.
- **`resolve_cfile_deps`**: transitive walker that opens each resolved
header and follows its `#include` directives, bounded by a visited set.
Headers that don't exist under `target_dir` (lib-rt headers like
`<Python.h>`) are dropped since they never change between builds.
- **Two-pass loop in `mypyc_build`**: first pass writes all groups'
files to disk; second pass resolves deps so every group's headers are
present when cross-group includes are looked up.

## Impact on non-incremental builds

None. `Extension.depends` is only consulted by setuptools when a `.o`
already exists. Cold builds compile everything unconditionally.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: georgesittas

mypyc/build.py

@@ -566,22 +566,115 @@ def construct_groups(
     return groups
 
 
-def get_header_deps(cfiles: list[tuple[str, str]]) -> list[str]:
-    """Find all the headers used by a group of cfiles.
+# Single regex that captures both `#include "foo"` and `#include <foo>`. The
+# alternation lets us tell the two forms apart: the quoted-form match populates
+# group 1 and the angle-form match populates group 2. The C preprocessor
+# applies different search rules to each kind (see `_extract_includes`), so we
+# carry the kind through resolution rather than collapsing them up front.
+_INCLUDE_RE = re.compile(r'#\s*include\s+(?:"([^"]+)"|<([^>]+)>)')
+
+
+def _extract_includes(contents: str) -> list[tuple[bool, str]]:
+    """Return each `#include` directive's (is_angled, name) from `contents`.
+
+    is_angled=False for `#include "foo"`, True for `#include <foo>`.
+    """
+    out: list[tuple[bool, str]] = []
+    for quoted, angled in _INCLUDE_RE.findall(contents):
+        if quoted:
+            out.append((False, quoted))
+        else:
+            out.append((True, angled))
+    return out
+
+
+def get_header_deps(cfiles: list[tuple[str, str]]) -> list[tuple[bool, str]]:
+    """Find all the headers directly included by a group of cfiles.
+
+    Returns a sorted, deduplicated list of `(is_angled, header_name)` pairs.
+    Callers that only need the names can ignore the bool, but it is needed by
+    `resolve_cfile_deps` to apply the correct preprocessor search order.
 
     We do this by just regexping the source, which is a bit simpler than
-    properly plumbing the data through.
+    properly plumbing the data through. Transitive header-to-header includes
+    are picked up by `resolve_cfile_deps` in `mypyc_build`, which can read
+    the on-disk headers after every group has written its files.
 
     Arguments:
-        cfiles: A list of (file name, file contents) pairs.
+        cfiles: A list of (file name, file contents) pairs. Contents must be
+            non-empty; callers handling cached groups must re-read the .c
+            from disk before calling, otherwise direct includes are missed
+            and Extension.depends ends up empty.
     """
-    headers: set[str] = set()
+    assert all(
+        contents for _, contents in cfiles
+    ), "get_header_deps requires non-empty file contents"
+    headers: set[tuple[bool, str]] = set()
     for _, contents in cfiles:
-        headers.update(re.findall(r'#include [<"]([^>"]+)[>"]', contents))
+        headers.update(_extract_includes(contents))
 
     return sorted(headers)
 
 
+def resolve_cfile_deps(
+    cfile_dir: str, direct_includes: list[tuple[bool, str]], target_dir: str
+) -> set[str]:
+    """
+    Resolve a .c file's `#include`s to on-disk paths, walking transitively through resolved headers.
+
+    The C preprocessor resolves `#include "foo"` against the includer's directory first, then via
+    -I, while `#include <foo>` only uses -I. We mirror that exactly: quoted includes are searched
+    in (includer_dir, target_dir) order, and angled includes are searched in target_dir only.
+    `target_dir` is the only -I path that holds files we generate; anything we cannot resolve under
+    it (or, for quoted form, the includer's dir) is dropped. Other headers like `<Python.h>` and
+    `<CPy.h>` live elsewhere and do not change between builds, so they are not real dependencies
+    for incremental purposes.
+
+    The walk is transitive: each resolved header is opened and scanned for its own `#include`
+    directives. Without this, cross-group export-table headers reached via `__native_internal_<mod>.h`
+    (which includes `<other_group/__native_other.h>`) would be missed, and edits that shift struct
+    offsets in `other_group` would not trigger a recompile of the consumer's .o file. Its baked-in
+    offsets would then resolve to whatever class/function now occupies that slot => runtime corruption.
+
+    Returns a set of resolved paths suitable for use as an Extension.depends list.
+    """
+    resolved: set[str] = set()
+
+    # Worklist of (search_dir, is_angled, header_name). search_dir is the includer's directory; for the
+    # initial cfile it is the cfile's dir, for a transitively-included header it is that header's dir.
+    # It is only consulted for quoted-form includes.
+    worklist: list[tuple[str, bool, str]] = [
+        (cfile_dir, is_angled, dep) for is_angled, dep in direct_includes
+    ]
+
+    while worklist:
+        search_dir, is_angled, dep = worklist.pop()
+        # Quoted form: includer's dir first, then -I (target_dir).
+        # Angled form: -I only (skips the includer's dir).
+        search_bases = (target_dir,) if is_angled else (search_dir, target_dir)
+        for base in search_bases:
+            candidate = os.path.normpath(os.path.join(base, dep))
+            if not os.path.exists(candidate):
+                continue
+            if candidate in resolved:
+                break
+            resolved.add(candidate)
+            # Recurse only into headers. Some lib-rt sources are pulled in as `#include "init.c"` etc.;
+            # those do not resolve under target_dir so they get filtered out before we would try to scan
+            # them, but the .h guard is a cheap belt-and-braces.
+            if candidate.endswith(".h"):
+                try:
+                    with open(candidate, encoding="utf-8") as f:
+                        header_contents = f.read()
+                except OSError:
+                    header_contents = ""
+                sub_dir = os.path.dirname(candidate)
+                for sub_angled, sub in _extract_includes(header_contents):
+                    worklist.append((sub_dir, sub_angled, sub))
+            break
+    return resolved
+
+
 def mypyc_build(
     paths: list[str],
     compiler_options: CompilerOptions,
@@ -630,29 +723,48 @@ def mypyc_build(
             for (path, dirs, internal) in skip_cgen_input[1]
         ]
 
-    # Write out the generated C and collect the files for each group
+    # Write out the generated C and collect the files for each group.
     # Should this be here??
-    group_cfilenames: list[tuple[list[str], list[str]]] = []
+    #
+    # Header resolution is deferred to a second pass: a header in one group may include a header
+    # generated by another group, so resolving here misses cross-group deps for groups processed first.
+    pending: list[list[tuple[str, list[tuple[bool, str]]]]] = []
     for cfiles in group_cfiles:
-        cfilenames = []
+        per_cfile_deps: list[tuple[str, list[tuple[bool, str]]]] = []
         for cfile, ctext in cfiles:
             cfile = os.path.join(compiler_options.target_dir, cfile)
+
             # Empty contents marks a file the previous run already wrote
             # (fully-cached group): skip the rewrite and just reuse it.
             if ctext and not options.mypyc_skip_c_generation:
                 write_file(cfile, ctext)
-            if os.path.splitext(cfile)[1] == ".c":
-                cfilenames.append(cfile)
-
-        # The header regex matches both quote styles, so the result can
-        # include system headers like `<Python.h>` that don't live under
-        # target_dir. Joining those produces non-existent paths which
-        # would force a full rebuild on every run via Extension.depends.
-        candidate_deps = (
-            os.path.join(compiler_options.target_dir, dep) for dep in get_header_deps(cfiles)
-        )
-        deps = [d for d in candidate_deps if os.path.exists(d)]
-        group_cfilenames.append((cfilenames, deps))
+
+            # For fully-cached groups ctext is empty; read the on-disk .c so the dep resolver
+            # can walk its transitive header chain and populate Extension.depends. Otherwise,
+            # cross-group export-table header changes (e.g. a new class shifting struct offsets)
+            # won't trigger a recompile of this cached consumer's .o.
+            if not ctext and os.path.exists(cfile):
+                try:
+                    with open(cfile, encoding="utf-8") as _f:
+                        ctext = _f.read()
+                except OSError:
+                    pass
+            per_cfile_deps.append((cfile, get_header_deps([(cfile, ctext)])))
+        pending.append(per_cfile_deps)
+
+    # Second pass: assemble each group's .c filenames and resolve transitive deps now that every group's
+    # headers are on disk. See resolve_cfile_deps for the rules.
+    group_cfilenames: list[tuple[list[str], list[str]]] = []
+    for per_cfile in pending:
+        cfilenames = [cf for cf, _ in per_cfile if os.path.splitext(cf)[1] == ".c"]
+        deps_set: set[str] = set()
+        for cfile_full, dep_names in per_cfile:
+            deps_set.update(
+                resolve_cfile_deps(
+                    os.path.dirname(cfile_full), dep_names, compiler_options.target_dir
+                )
+            )
+        group_cfilenames.append((cfilenames, sorted(deps_set)))
 
     return groups, group_cfilenames, source_deps
 

mypyc/test-data/run-multimodule.test

@@ -1391,6 +1391,60 @@ def translate(b: bytes) -> bytes:
 import native
 assert native.translate(b'ABCD') == b'BBCD'
 
+[case testIncrementalCrossGroupExportTableOffsets]
+# Regression: under separate=True, each consumer module's IR is
+# compiled against the positional layout of its deps'
+# `exports_<group>` struct. Reordering the dep's classes keeps the
+# same set of public names, so mypy's interface hash for the dep is
+# unchanged -- the consumer is not invalidated and stays fully
+# cached, which causes `_load_cached_group_files` to return empty
+# cfile content for the consumer's group.
+#
+# Before the fix, `get_header_deps` over empty content returned no
+# includes, so `Extension.depends` for the consumer ended up empty
+# and setuptools never recompiled the consumer's .o when the dep's
+# `__native_<dep>.h` shifted struct offsets. The stale .o kept the
+# old offsets and silently resolved cross-group calls to the wrong
+# class.
+from other_classes import Gamma, Delta
+
+def make_gamma() -> Gamma:
+    return Gamma()
+
+def make_delta() -> Delta:
+    return Delta()
+
+[file other_classes.py]
+class Alpha:
+    a: int = 1
+
+class Beta:
+    b: int = 2
+
+class Gamma:
+    g: int = 3
+
+class Delta:
+    d: int = 4
+
+[file other_classes.py.2]
+class Delta:
+    d: int = 4
+
+class Alpha:
+    a: int = 1
+
+class Beta:
+    b: int = 2
+
+class Gamma:
+    g: int = 3
+
+[file driver.py]
+import native
+assert type(native.make_gamma()).__name__ == "Gamma"
+assert type(native.make_delta()).__name__ == "Delta"
+
 [case testCrossModuleAttrDefaults]
 from other import Parent
 

mypyc/test/test_misc.py

@@ -1,7 +1,10 @@
 from __future__ import annotations
 
+import os
+import tempfile
 import unittest
 
+from mypyc.build import get_header_deps, resolve_cfile_deps
 from mypyc.ir.ops import BasicBlock
 from mypyc.ir.pprint import format_blocks, generate_names_for_ir
 from mypyc.irbuild.ll_builder import LowLevelIRBuilder
@@ -20,3 +23,130 @@ def test_debug_op(self) -> None:
         names = generate_names_for_ir([], [block])
         code = format_blocks([block], names, {})
         assert code[:-1] == ["L0:", "    r0 = 'foo'", "    CPyDebug_PrintObject(r0)"]
+
+
+class TestHeaderDeps(unittest.TestCase):
+    """
+    Tests for the header-dependency tracking used to build `Extension.depends`, which drives
+    setuptools' `newer_group` decision about whether to recompile a .o file on incremental builds.
+    """
+
+    def test_get_header_deps_quoted_includes(self) -> None:
+        # Quoted includes: the historical form. Used by the .c file to reach its own __native_<mod>.h /
+        # __native_internal_<mod>.h. The `False` in each tuple marks the include as non-angled, which
+        # `resolve_cfile_deps` uses to search the includer's directory.
+        cfile = '#include "__native_caller.h"\n#include "__native_internal_caller.h"\n'
+        assert get_header_deps([("caller.c", cfile)]) == [
+            (False, "__native_caller.h"),
+            (False, "__native_internal_caller.h"),
+        ]
+
+    def test_get_header_deps_angle_bracket_includes(self) -> None:
+        # Angle-bracket includes are also matched, and reported with is_angled=True so that the resolver
+        # skips the includer's dir for them (matching the C preprocessor). The cross-group export header
+        # is reached via `#include <other_group/__native_other.h>` in __native_internal_<mod>.h. Before
+        # this was matched the dep was missed entirely and the consumer's .o was never invalidated when
+        # the other group's struct layout shifted.
+        cfile = "#include <Python.h>\n#include <lib/__native_functions.h>\n"
+        assert get_header_deps([("caller.c", cfile)]) == [
+            (True, "Python.h"),
+            (True, "lib/__native_functions.h"),
+        ]
+
+    def test_get_header_deps_mixed_and_whitespace(self) -> None:
+        # The preprocessor tolerates whitespace and the leading-hash form. `get_header_deps` returns sorted
+        # tuples — non-angled (False) sorts before angled (True), then alphabetical within each kind.
+        cfile = '# include "a.h"\n#  include  <b.h>\n#include\t"c.h"\n'
+        assert get_header_deps([("x.c", cfile)]) == [(False, "a.h"), (False, "c.h"), (True, "b.h")]
+
+    def test_resolve_walks_transitively_through_headers(self) -> None:
+        # Reproduces the bug scenario: caller's .c only directly includes caller's own headers, but
+        # caller's __native_internal_caller.h includes the cross-group export header. The resolver
+        # must follow that chain so setuptools sees the cross-group header as a dep.
+        with tempfile.TemporaryDirectory() as tmp:
+            build_dir = tmp
+            os.makedirs(os.path.join(build_dir, "lib"))
+            os.makedirs(os.path.join(build_dir, "other_group"))
+
+            # caller.c's directly-included headers, both live alongside
+            # caller.c under build/ (resolved via target_dir).
+            internal_h = os.path.join(build_dir, "__native_internal_caller.h")
+            caller_h = os.path.join(build_dir, "__native_caller.h")
+            cross_group_h = os.path.join(build_dir, "lib", "__native_functions.h")
+            unrelated_h = os.path.join(build_dir, "other_group", "__native_other.h")
+
+            with open(caller_h, "w") as f:
+                # Headers outside build/ (CPython's <Python.h>, lib-rt's <CPy.h>) don't resolve under
+                # target_dir, so they get dropped during resolution and aren't recursed into.
+                f.write("#include <Python.h>\n#include <CPy.h>\n")
+            with open(internal_h, "w") as f:
+                # This header includes a header in another group via angle brackets. Pre-fix, this dep
+                # was invisible to setuptools.
+                f.write(
+                    "#include <Python.h>\n"
+                    '#include "__native_caller.h"\n'
+                    "#include <lib/__native_functions.h>\n"
+                )
+            with open(cross_group_h, "w") as f:
+                f.write("struct export_table_lib___functions { int x; };\n")
+            with open(unrelated_h, "w") as f:
+                # Sibling group not reached from caller's chain => must NOT appear in the resolved set.
+                f.write("struct unrelated { int x; };\n")
+
+            # caller.c is in build_dir, so its includer-dir is build_dir. Both directly-included headers
+            # are quoted (`False`); the cross-group header that __native_internal_caller.h reaches via
+            # `<lib/__native_functions.h>` is found by the recursive walk re-reading the on-disk header.
+            deps = resolve_cfile_deps(
+                cfile_dir=build_dir,
+                direct_includes=[
+                    (False, "__native_caller.h"),
+                    (False, "__native_internal_caller.h"),
+                ],
+                target_dir=build_dir,
+            )
+
+            assert deps == {caller_h, internal_h, cross_group_h}, (
+                f"expected the cross-group header to be reached transitively; "
+                f"got {sorted(deps)!r}"
+            )
+
+    def test_resolve_drops_unresolvable_includes(self) -> None:
+        # `<Python.h>`, `<CPy.h>`, etc. don't live under target_dir, so they're dropped from depends. They
+        # never change between builds, so this is the right behavior. Crucially, it stops setuptools'
+        # `missing="newer"` from treating them as always-newer and force-rebuilding every translation unit.
+        with tempfile.TemporaryDirectory() as tmp:
+            cfile_dir = tmp
+            deps = resolve_cfile_deps(
+                cfile_dir=cfile_dir,
+                direct_includes=[(True, "Python.h"), (True, "CPy.h"), (False, "init.c")],
+                target_dir=cfile_dir,
+            )
+            assert deps == set()
+
+    def test_resolve_search_order_matches_preprocessor(self) -> None:
+        # When the same header name exists both next to the includer and under target_dir, the C preprocessor
+        # picks the includer-dir copy for `#include "shared.h"` and the target_dir copy for `#include <shared.h>`.
+        # The resolver must record the same path the compiler will actually consume, otherwise mtimes of the
+        # wrong file drive incremental rebuild decisions.
+        with tempfile.TemporaryDirectory() as tmp:
+            includer = os.path.join(tmp, "groupA")
+            target = os.path.join(tmp, "build")
+            os.makedirs(includer)
+            os.makedirs(target)
+
+            local_h = os.path.join(includer, "shared.h")
+            global_h = os.path.join(target, "shared.h")
+            with open(local_h, "w") as f:
+                f.write("/* local */\n")
+            with open(global_h, "w") as f:
+                f.write("/* global */\n")
+
+            # Quoted form: resolves to the includer-dir copy.
+            assert resolve_cfile_deps(
+                cfile_dir=includer, direct_includes=[(False, "shared.h")], target_dir=target
+            ) == {local_h}
+
+            # Angled form: skips the includer-dir copy, resolves under -I.
+            assert resolve_cfile_deps(
+                cfile_dir=includer, direct_includes=[(True, "shared.h")], target_dir=target
+            ) == {global_h}