fix: build precompiled headers after generated inputs

pylib/gyp/generator/make.py

@@ -603,7 +603,7 @@ def CalculateGeneratorInputInfo(params):
 
 # Commands for precompiled header files.
 quiet_cmd_pch_c = CXX($(TOOLSET)) $@
-cmd_pch_c = $(CC.$(TOOLSET)) $(GYP_PCH_CFLAGS) $(DEPFLAGS) $(CXXFLAGS.$(TOOLSET)) -c -o $@ $<
+cmd_pch_c = $(CC.$(TOOLSET)) $(GYP_PCH_CFLAGS) $(DEPFLAGS) $(CFLAGS.$(TOOLSET)) -c -o $@ $<
 quiet_cmd_pch_cc = CXX($(TOOLSET)) $@
 cmd_pch_cc = $(CC.$(TOOLSET)) $(GYP_PCH_CXXFLAGS) $(DEPFLAGS) $(CXXFLAGS.$(TOOLSET)) -c -o $@ $<
 quiet_cmd_pch_m = CXX($(TOOLSET)) $@
@@ -1485,13 +1485,29 @@ def WriteSources(
 # See "Target-specific Variable Values" in the GNU Make manual."""
             )
             self.WriteLn("$(OBJS): TOOLSET := $(TOOLSET)")
-            self.WriteLn(
-                "$(OBJS): GYP_CFLAGS := "
+            gyp_cflags = (
                 "$(DEFS_$(BUILDTYPE)) "
                 "$(INCS_$(BUILDTYPE)) "
-                "%s " % precompiled_header.GetInclude("c") + "$(CFLAGS_$(BUILDTYPE)) "
+                "%s "
+                "$(CFLAGS_$(BUILDTYPE)) "
                 "$(CFLAGS_C_$(BUILDTYPE))"
             )
+            self.WriteLn(
+                "$(OBJS): GYP_CFLAGS := "
+                + gyp_cflags % precompiled_header.GetInclude("c")
+            )
+            asm_objs = [
+                obj
+                for source, obj in zip(compilable, objs)
+                if source.endswith((".s", ".S"))
+            ]
+            if asm_objs and precompiled_header.compile_headers:
+                # Assembly is compiled with GYP_CFLAGS too, but without a gch
+                # for it, the -include of the C one fails.
+                self.WriteLn(
+                    "%s: GYP_CFLAGS := " % " ".join(asm_objs)
+                    + gyp_cflags % precompiled_header.GetInclude("asm")
+                )
             self.WriteLn(
                 "$(OBJS): GYP_CXXFLAGS := "
                 "$(DEFS_$(BUILDTYPE)) "
@@ -1521,16 +1537,23 @@ def WriteSources(
                     "$(CFLAGS_OBJCC_$(BUILDTYPE))"
                 )
 
-        self.WritePchTargets(precompiled_header.GetPchBuildCommands())
+        self.WritePchTargets(
+            precompiled_header.GetPchBuildCommands(), deps + extra_outputs
+        )
 
         # If there are any object files in our input file list, link them into our
         # output.
         extra_link_deps += [source for source in sources if Linkable(source)]
 
         self.WriteLn()
 
-    def WritePchTargets(self, pch_commands):
-        """Writes make rules to compile prefix headers."""
+    def WritePchTargets(self, pch_commands, order_only_deps):
+        """Writes make rules to compile prefix headers.
+
+        A prefix header may include generated headers, so like the object files,
+        the gch files are built after |order_only_deps|: the target's dependencies
+        and the outputs of its actions and rules.
+        """
         if not pch_commands:
             return
 
@@ -1547,6 +1570,9 @@ def WritePchTargets(self, pch_commands):
                 "m": "GYP_PCH_OBJCFLAGS",
                 "mm": "GYP_PCH_OBJCXXFLAGS",
             }[lang]
+            if order_only_deps:
+                self.WriteMakeRule([gch], order_only_deps, order_only=True)
+            self.WriteLn(f"{gch}: TOOLSET := $(TOOLSET)")
             self.WriteLn(
                 f"{gch}: {var_name} := {lang_flag} " + "$(DEFS_$(BUILDTYPE)) "
                 "$(INCS_$(BUILDTYPE)) "

pylib/gyp/generator/ninja.py

@@ -1253,6 +1253,12 @@ def WriteSourcesForArch(
                     cflags_cc,
                     self.ExpandSpecial,
                 )
+            elif ext in ("s", "S") and precompiled_header.compile_headers:
+                # Assembly gets $cflags_pch_c too, but without a gch for it, the
+                # -include of the C one fails.
+                variables = [
+                    ("cflags_pch_c", precompiled_header.GetInclude("asm", arch))
+                ]
             ninja_file.build(
                 output,
                 command,
@@ -1274,13 +1280,17 @@ def WriteSourcesForArch(
                 ],
             )
 
-        self.WritePchTargets(ninja_file, pch_commands)
+        self.WritePchTargets(ninja_file, pch_commands, predepends)
 
         ninja_file.newline()
         return outputs
 
-    def WritePchTargets(self, ninja_file, pch_commands):
-        """Writes ninja rules to compile prefix headers."""
+    def WritePchTargets(self, ninja_file, pch_commands, predepends):
+        """Writes ninja rules to compile prefix headers.
+
+        A prefix header may include generated headers, so like the object files,
+        the gch files are built after |predepends|.
+        """
         if not pch_commands:
             return
 
@@ -1299,7 +1309,13 @@ def WritePchTargets(self, ninja_file, pch_commands):
                 "mm": "objcxx",
             }
             cmd = map.get(lang)
-            ninja_file.build(gch, cmd, input, variables=[(var_name, lang_flag)])
+            ninja_file.build(
+                gch,
+                cmd,
+                input,
+                order_only=predepends,
+                variables=[(var_name, lang_flag)],
+            )
 
     def WriteLink(self, spec, config_name, config, link_deps, compile_deps):
         """Write out a link step. Fills out target.binary."""

pylib/gyp/xcode_emulation.py

@@ -1429,7 +1429,10 @@ def _CompiledHeader(self, lang, arch):
         return h
 
     def GetInclude(self, lang, arch=None):
-        """Gets the cflags to include the prefix header for language |lang|."""
+        """Gets the cflags to include the prefix header for language |lang|.
+
+        There is no compiled header for assembly ('asm'), which includes the
+        prefix header itself."""
         if self.compile_headers and lang in self.compiled_headers:
             return "-include %s" % self._CompiledHeader(lang, arch)
         elif self.header:

test/fixtures/pch_action.h

@@ -0,0 +1 @@
+#include "action_generated.h"

test/fixtures/pch_asm.S

@@ -0,0 +1,2 @@
+// Assembly gets the prefix header too, but has no precompiled header.
+.text

test/fixtures/pch_dep.h

@@ -0,0 +1 @@
+#include "dep_generated.h"

test/fixtures/pch_prefix.gyp

@@ -0,0 +1,47 @@
+{
+  'targets': [
+    {
+      # The prefix header includes a header generated by the target's action.
+      'target_name': 'pch_action',
+      'type': 'static_library',
+      'sources': [ 'pch.cc', 'pch_asm.S' ],
+      'include_dirs': [ '<(SHARED_INTERMEDIATE_DIR)' ],
+      'xcode_settings': {
+        'GCC_PREFIX_HEADER': 'pch_action.h',
+        'GCC_PRECOMPILE_PREFIX_HEADER': 'YES',
+      },
+      'actions': [
+        {
+          'action_name': 'generate_action_header',
+          'inputs': [],
+          'outputs': [ '<(SHARED_INTERMEDIATE_DIR)/action_generated.h' ],
+          'action': [ 'touch', '<@(_outputs)' ],
+        },
+      ],
+    },
+    {
+      # The prefix header includes a header generated by a dependency.
+      'target_name': 'pch_dep',
+      'type': 'static_library',
+      'dependencies': [ 'generate_dep_header' ],
+      'sources': [ 'pch.cc', 'pch_asm.S' ],
+      'include_dirs': [ '<(SHARED_INTERMEDIATE_DIR)' ],
+      'xcode_settings': {
+        'GCC_PREFIX_HEADER': 'pch_dep.h',
+        'GCC_PRECOMPILE_PREFIX_HEADER': 'YES',
+      },
+    },
+    {
+      'target_name': 'generate_dep_header',
+      'type': 'none',
+      'actions': [
+        {
+          'action_name': 'generate_dep_header',
+          'inputs': [],
+          'outputs': [ '<(SHARED_INTERMEDIATE_DIR)/dep_generated.h' ],
+          'action': [ 'touch', '<@(_outputs)' ],
+        },
+      ],
+    },
+  ],
+}

test/integration_test.py

@@ -5,6 +5,7 @@
 import os
 import re
 import shutil
+import subprocess
 import sys
 import unittest
 
@@ -13,6 +14,7 @@
 fixture_dir = os.path.join(os.path.dirname(__file__), "fixtures")
 gyp_file = os.path.join(fixture_dir, "integration.gyp")
 pch_gyp_file = os.path.join(fixture_dir, "pch.gyp")
+pch_prefix_gyp_file = os.path.join(fixture_dir, "pch_prefix.gyp")
 
 if sys.platform == "win32":
     sysname = sys.platform
@@ -80,6 +82,56 @@ def test_cmake(self) -> None:
         assert_file(self, "out/Default/CMakeLists.txt", "cmake/CMakeLists.txt")
 
 
+class TestGypMac(unittest.TestCase):
+    # The prefix header of each target includes a header generated by its
+    # action (pch_action) or by its dependency (pch_dep). Each output is built
+    # on its own from a clean build, so it fails unless it waits for that header.
+    pch_targets = ("pch_action", "pch_dep")
+    pch_langs = ("c", "cc", "m", "mm")
+
+    def setUp(self) -> None:
+        if sysname != "darwin":
+            self.skipTest("macOS-only test")
+        shutil.rmtree(os.path.join(fixture_dir, "out"), ignore_errors=True)
+
+    def test_make_precompiled_prefix_header(self) -> None:
+        rc = gyp.main(
+            [
+                "-f",
+                "make",
+                "--depth",
+                fixture_dir,
+                "--generator-output",
+                "out",
+                pch_prefix_gyp_file,
+            ]
+        )
+        assert rc == 0
+
+        makefile_dir = os.path.join(fixture_dir, "out")
+        # CXXFLAGS that a C compile rejects check the C gch doesn't get them.
+        env = {**os.environ, "CXXFLAGS": "-std=c++17"}
+        for target in self.pch_targets:
+            obj = f"out/Default/obj.target/{target}"
+            gchs = [f"{obj}/pch-{lang}/{target}.h.gch" for lang in self.pch_langs]
+            for output in [*gchs, f"{obj}/pch_asm.o"]:
+                shutil.rmtree(os.path.join(makefile_dir, "out"), ignore_errors=True)
+                subprocess.run(
+                    ["make", "-C", makefile_dir, output], env=env, check=True
+                )
+
+    def test_ninja_precompiled_prefix_header(self) -> None:
+        rc = gyp.main(["-f", "ninja", "--depth", fixture_dir, pch_prefix_gyp_file])
+        assert rc == 0
+
+        build_dir = os.path.join(fixture_dir, "out", "Default")
+        for target in self.pch_targets:
+            gchs = [f"obj/{target}.{target}.h-{lang}.gch" for lang in self.pch_langs]
+            for output in [*gchs, f"obj/{target}.pch_asm.o"]:
+                subprocess.run(["ninja", "-C", build_dir, "-t", "clean"], check=True)
+                subprocess.run(["ninja", "-C", build_dir, output], check=True)
+
+
 class TestGypWindows(unittest.TestCase):
     def setUp(self) -> None:
         if sys.platform != "win32":