fix(gyp): default enable_thin_lto so Node 26 win32 configure works#7
Merged
Conversation
Node 26's bundled common.gypi has a top-level
['enable_thin_lto=="true"', { ... }]
condition, but doesn't always inject a default for the variable
itself. node-gyp 10.x (shipped inside prebuild 13.0.1) doesn't
inject the default either. On Linux/macOS the include chain
happens to define it elsewhere, but on Windows gyp configure
fails before MSBuild even starts:
gyp: name 'enable_thin_lto' is not defined while evaluating
condition 'enable_thin_lto=="true"' in binding.gyp
Providing `enable_thin_lto%='false'` in our own variables block
is a no-op when the variable is already set, so it's safe across
all platforms and node-gyp versions. Verified locally that both
24.15.0 and 26.1.0 still build cleanly on darwin-arm64.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same Node 26 / node-gyp 10 issue as the previous commit, but the error has now moved into the dependency: gyp: name 'enable_thin_lto' is not defined while evaluating condition 'enable_thin_lto=="true"' in deps\libexpat\libexpat.gyp while loading dependencies of binding.gyp gyp loads each .gyp file in its own variable scope, so a `%` default in the parent binding.gyp doesn't propagate down to dependencies. Adding the same default to libexpat.gyp's top-level variables block. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Node 26's bundled common.gypi has a top-level
['enable_thin_lto=="true"', { ... }]
condition, but doesn't always inject a default for the variable itself. node-gyp 10.x (shipped inside prebuild 13.0.1) doesn't inject the default either. On Linux/macOS the include chain happens to define it elsewhere, but on Windows gyp configure fails before MSBuild even starts:
gyp: name 'enable_thin_lto' is not defined while evaluating
condition 'enable_thin_lto=="true"' in binding.gyp
Providing
enable_thin_lto%='false'in our own variables block is a no-op when the variable is already set, so it's safe across all platforms and node-gyp versions. Verified locally that both 24.15.0 and 26.1.0 still build cleanly on darwin-arm64.