Skip to content

fix: build precompiled headers after generated inputs - #360

Merged
gengjiawen merged 4 commits into
nodejs:mainfrom
kevin-sellers:fix-pch-order-only
Oct 19, 2026
Merged

gengjiawen merged 4 commits into
nodejs:mainfrom
kevin-sellers:fix-pch-order-only

Conversation

@kevin-sellers

@kevin-sellers kevin-sellers commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Object files get a target's dependencies, and its actions and rules outputs, as
order-only prerequisites. The .gch built from a prefix header gets neither, so
a prefix header that reaches a generated header races code generation: make
starts the .gch as soon as the prefix header itself exists, and a clean build
fails on the missing generated header.

This gives the .gch targets the same order-only prerequisites the object files
get.

Motivation

I was attempting to enable precompiled headers for V8's largest targets in a
Node.js build. The prefix header includes isolate-inl.h, which reaches
builtins-definitions.h, which includes the generated
builtins-generated/bytecodes-builtins-list.h. A clean build died with:

../deps/v8/src/builtins/builtins-definitions.h:8:10: fatal error:
    builtins-generated/bytecodes-builtins-list.h: No such file or directory

Any target whose GCC_PREFIX_HEADER transitively includes generated code hits
this today, independently of how the prefix header was configured.

Follow-up commits

Pushed by @gengjiawen after review:

  • 0acf212: the order-only prerequisites are now written in WritePchTargets,
    so all four .gch rules (c, cc, m, mm) get them, not only those of languages
    that have objects, and the .gch rules set TOOLSET as the objects do. The
    ninja .gch builds raced code generation the same way, and now get the
    objects' predepends.
  • 576b074: cmd_pch_c compiled the C prefix header with
    $(CXXFLAGS.$(TOOLSET)); it now uses $(CFLAGS.$(TOOLSET)), as the C objects
    that include it do.
  • f51974c: with GCC_PRECOMPILE_PREFIX_HEADER, .s/.S sources got the
    -include of the C .gch, which nothing built for them, so a target with
    assembly and no C sources failed with pch-c/... file not found. There is no
    compiled header for assembly, so they now include the prefix header itself,
    as without precompilation.

Testing

TestGypMac.test_make_precompiled_prefix_header and
TestGypMac.test_ninja_precompiled_prefix_header generate
test/fixtures/pch_prefix.gyp, then build each .gch (c, cc, m, mm) and a .S
object on its own, each from a clean build. The prefix header of pch_action
includes a header generated by the target's action, and that of pch_dep one
generated by a dependency, so each build fails unless it waits for its
generated header. The make build also sets CXXFLAGS=-std=c++17, which a C
compile rejects.

BEGIN_COMMIT_OVERRIDE
fix: build precompiled headers after generated inputs (#360)
fix(make): compile the C prefix header with CFLAGS (#360)
fix: include the prefix header itself in assembly sources (#360)
END_COMMIT_OVERRIDE

kevin-sellers and others added 4 commits September 10, 2026 19:05
Object files get a target's dependencies, and its actions and rules
outputs, as order-only prerequisites. The .gch built from a prefix
header got neither, so a prefix header that reaches a generated header
races code generation: make starts the .gch as soon as the prefix header
itself exists, and a clean build fails on the missing generated header.
Move the make ordering into WritePchTargets, so all four gch rules get
the target's dependencies and action/rule outputs as order-only
prerequisites, not only those of languages that have objects, and set
TOOLSET on them like on the objects. Give the ninja gch builds the
objects' predepends too; they raced code generation the same way.

The test now builds each gch on its own for a prefix header that
includes a header generated by the target's action, and for one that
includes a header generated by a dependency.
cmd_pch_c passed $(CXXFLAGS.$(TOOLSET)) while the C objects that include
the gch are compiled with $(CFLAGS.$(TOOLSET)), so a C++-only flag such
as -std=c++17 in CXXFLAGS broke the C gch, and other flag differences
made it incompatible with the objects.
With GCC_PRECOMPILE_PREFIX_HEADER, assembly sources got the C objects'
-include of the C gch, but nothing builds that gch for them, so a target
with .S sources and no .c ones failed with "pch-c/... file not found",
and others raced the C gch. There is no compiled header for assembly, so
include the prefix header itself, as without precompilation. GCC also
rejects a C gch for assembly.

The test now builds each output from a clean build, so none passes on a
header or gch that an earlier one built.

@gengjiawen gengjiawen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @kevin-sellers! I pushed follow-up commits for the ninja generator, the C PCH flags, and assembly sources; see the PR description.

@gengjiawen
gengjiawen merged commit d8888e9 into nodejs:main Oct 19, 2026
45 of 46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants