Skip to content

Commit

Permalink
Fix build failure in newer node-gyp versions
Browse files Browse the repository at this point in the history
nodejs/node-gyp#1164 interprets additional node-gyp build parameters as
target names, which breaks the current way CL configs are passed to
msbuild in install.js.

Move the detection logic into configuration phase and pass CL configs as
gyp variables.

Fixes nodejs#321, fixes nodejs#377
  • Loading branch information
SuibianP committed Sep 25, 2024
1 parent de1f01d commit b2fbdb9
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 58 deletions.
6 changes: 6 additions & 0 deletions binding.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@
"libraries": [
"<(lldb_lib_dir)/<(lldb_lib)",
],
"msbuild_settings": {
"ClCompile": {
"CLToolPath": "<(cl_tool_path)",
"CLToolExe": "<(cl_tool_exe)",
},
},
}],
[ "coverage == 'true'", {
"cflags": [ "--coverage" ],
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
},
"scripts": {
"preinstall": "node scripts/configure.js",
"install": "node scripts/install.js",
"install": "node-gyp rebuild",
"postinstall": "node scripts/cleanup.js",
"test-plugin": "tape test/plugin/*-test.js",
"test-addon": "tape test/addon/*-test.js",
Expand Down
8 changes: 8 additions & 0 deletions scripts/configure.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ function configureInstallation(osName, buildDir) {
installation = require('./windows').getLldbInstallation();
config.variables['lldb_lib_dir%'] = installation.libDir;
config.variables['lldb_lib%'] = installation.libName;
const clangExe = 'clang-cl.exe';
const clangDir = lldb.findWindowsExeDir(clangExe);
if (!clangDir) {
console.log(`Could not find ${clangExe}`);
process.exit(1);
}
config.variables['cl_tool_path%'] = clangDir;
config.variables['cl_tool_exe%'] = clangExe;
} else {
console.log(`Unsupported OS: ${osName}`);
process.exit(1);
Expand Down
57 changes: 0 additions & 57 deletions scripts/install.js

This file was deleted.

0 comments on commit b2fbdb9

Please sign in to comment.