build: separate lldb_include_dir and lldb_lib_dir

- Separate detection of lldb headers and lldb shared libraries since the headers can be downloaded by us while the libraries cannot at the moment. On some systems we are not able to find the non-versioned liblldb dynamic library (`-llldb`) anyway, it's working fine until now because the lldb will resolve the symbols before loading the plugin, but this will have to be handled differently in the case of addons which need the symbols at build time. Also use ldd to find the correct name of the library. - Use the name `lldb_include_dir` and `lldb_lib_dir` and point them to subdirectories of the lldb installation to match the concepts in llvm-config. - Check the `$lldb_include_dir/lldb/API` for headers since some incomplete installations may not have the API headers installed. - Stop symlinking ./lldb because we will always run scripts/configure.js which can write the path to the headers in the config.gypi anyway. - Put the conditional flags in binding.gyp to target_defaults so they can be shared by the addon target later. - Add more logs in the configuration script about the result of each step. - Detect llvm-config once and use it if it's available later instead of always trying to run it in each configuration step.
nodejs · joyeecheung · Apr 20, 2018 · Apr 9, 2018 · Apr 20, 2018 · Apr 16, 2018
commit a7972a8ce0e70102bd6780f0bc2e0f9576ad3d3d
diff --git a/.gitignore b/.gitignore
@@ -1,11 +1,15 @@
 *.dylib
+*.so
 *.dSYM
 tools/
 out/
 lldb/
+lldb-*/
 build/
 out/
 npm-debug.log
 node_modules/
-options.gypi
+
+# Generated by scripts/configure.js
+config.gypi
 llnode.sh
diff --git a/README.md b/README.md
@@ -251,12 +251,12 @@ node scripts/configure.js
 
 # To configure with the detected lldb installation
 node-gyp configure
-# To configure with a specified path to headers, where `$lldb_header_dir/include`
+# To configure with a specified path to headers, where `$lldb_include_dir`
 # contains the <lldb/*/*.h> headers
-node-gyp configure -- -Dlldb_header_dir=/usr/local/Cellar/llvm/5.0.0
-# To configure with a specified path to the libraries, where `$lldb_lib_dir/lib`
+node-gyp configure -- -Dlldb_include_dir=/usr/local/Cellar/llvm/5.0.0/include
+# To configure with a specified path to the libraries, where `$lldb_lib_dir`
 # contains `liblldb.so` or `liblldb.dylib`
-node-gyp configure -- -Dlldb_lib_dir=/usr/lib/llvm-3.9
+node-gyp configure -- -Dlldb_lib_dir=/usr/lib/llvm-3.9/lib
 
 # Build the plugin
 node-gyp build

diff --git a/binding.gyp b/binding.gyp
@@ -10,21 +10,10 @@
       "lldb_lib_dir%": ""
   },
 
-  "targets": [{
-    "target_name": "plugin",
-    "type": "shared_library",
-    "product_prefix": "",
-
+  "target_defaults": {
     "include_dirs": [
-      ".",
-      "<(lldb_header_dir)/include",
-    ],
-
-    "sources": [
-      "src/llnode.cc",
-      "src/llv8.cc",
-      "src/llv8-constants.cc",
-      "src/llscan.cc",
+      "<(module_root_dir)",
+      "<(lldb_include_dir)",
     ],
 
     "cflags" : [ "-std=c++11" ],
@@ -34,7 +23,7 @@
         # Necessary for node v4.x
         "xcode_settings": {
           "OTHER_CPLUSPLUSFLAGS" : [ "-std=c++11", "-stdlib=libc++" ],
-          "OTHER_LDFLAGS": [ "-stdlib=libc++" ],
+          "OTHER_LDFLAGS": ["-stdlib=libc++"]
         },
 
         "conditions": [
@@ -54,19 +43,25 @@
           {
             "xcode_settings": {
               "OTHER_LDFLAGS": [
-                "-Wl,-rpath,<(lldb_lib_dir)/lib",
-                "-L<(lldb_lib_dir)/lib",
+                "-Wl,-rpath,<(lldb_lib_dir)",
+                "-L<(lldb_lib_dir)",
                 "-l<(lldb_lib)",
               ],
             },
           }],
         ],
-      }],
+      }]
     ]
   },
-  {
-    "target_name": "install",
-    "type":"none",
-    "dependencies" : [ "plugin" ]
+
+  "targets": [{
+    "target_name": "plugin",
+    "type": "shared_library",
+    "sources": [
+      "src/llnode.cc",
+      "src/llv8.cc",
+      "src/llv8-constants.cc",
+      "src/llscan.cc",
+    ]
   }],
 }
diff --git a/llnode.gypi b/llnode.gypi
@@ -9,8 +9,8 @@
     "msvs_multi_core_compile": "0",  # we do enable multicore compiles, but not using the V8 way
     "gcc_version%": "unknown",
     "clang%": 1,
-    "lldb_header_dir%": "lldb",
-    "lldb_lib%": "lldb",
+    "lldb_include_dir%": "lldb/include", # location of the lldb headers
+    "lldb_lib%": "lldb",                 # name of the -l library to link
     "conditions": [
       ["GENERATOR == 'ninja'", {
         "OBJ_DIR": "<(PRODUCT_DIR)/obj",

diff --git a/scripts/cleanup.js b/scripts/cleanup.js
@@ -1,31 +1,23 @@
 'use strict';
 
-const os = require('os');
-const child_process = require('child_process');
 const fs = require('fs');
-
 const cwd = process.cwd();
-const osName = os.type();
-var libExt = 'so';
-
-if (osName === 'Darwin') {
-  libExt = 'dylib';
-}
 
+const libExt = require('os').type() === 'Darwin' ? 'dylib' : 'so';
 const llnodeLib = `plugin.${libExt}`;
 const destLib = `llnode.${libExt}`;
 
 let buildPath = `${cwd}/build/Release/${llnodeLib}`;
 
-if (!fs.existsSync(buildPath)) {
+if (!fs.existsSync(buildPath))
   buildPath = `${cwd}/build/Debug/${llnodeLib}`;
-}
 
 const destPath = `${cwd}/${destLib}`;
 
 console.log(`Moving ${buildPath} to ${destPath}`);
 fs.renameSync(buildPath, destPath);
 
-console.log(`${os.EOL}llnode plugin installed, load in lldb with:`);
+console.log(`\nllnode plugin installed, load in lldb with:\n`);
 console.log(`(lldb) plugin load ${destPath}`);
-console.log(`or copy plugin to lldb system plugin directory, see www.npmjs.org/llnode${os.EOL}`);
+console.log('\nor copy plugin to lldb system plugin directory');
+console.log('see https://github.com/nodejs/llnode\n');
diff --git a/scripts/configure.js b/scripts/configure.js
@@ -3,17 +3,15 @@
 const os = require('os');
 const fs = require('fs');
 const path = require('path');
-const child_process = require('child_process');
 
 const lldb = require('./lldb');
 function main() {
   const buildDir = process.cwd();
   console.log('Build dir is: ' + buildDir);
   const osName = os.type();
-  const installation = configureInstallation(osName, buildDir);
-  writeConfig(installation.config);
-  linkHeadersDir(installation.prefix);
-  writeLlnodeScript(buildDir, installation.executable, osName);
+  const result = configureInstallation(osName, buildDir);
+  writeConfig(result.config);
+  writeLlnodeScript(buildDir, result.executable, osName);
   // Exit with success.
   process.exit(0);
 }
@@ -32,61 +30,43 @@ function configureInstallation(osName, buildDir) {
   // - What level of lldb we are running.
   // - If we need to download the headers. (Linux may have them installed)
   let installation;
-  let prefix;  // Similar to what `llvm-config --prefix` returns
-  // On Linux and BSD the config is always empty.
-  const config = {};
+  let includeDir;
+  const config = {
+    variables: {}
+  };
 
   if (osName === 'Darwin') {
-    const darwin = require('./darwin');
-    installation = darwin.getLldbInstallation();
-    prefix = installation.prefix;
-    if (prefix === undefined) {  // Using Xcode installation
-      prefix = lldb.cloneHeaders(installation.version, buildDir);
-    } else {  // Using custom installation
-      // Need to configure with the custom prefix
-      config.variables = { 'lldb_lib_dir%': prefix };
+    installation = require('./darwin').getLldbInstallation();
+    if (installation.libDir) {
+      config.variables['lldb_lib_dir%'] = installation.libDir;
     }
   } else if (osName === 'Linux') {
-    const linux = require('./linux');
-    installation = linux.getLldbInstallation();
-    if (installation.prefix === undefined) {
-      // Could not find the headers, need to download them
-      prefix = lldb.cloneHeaders(installation.version, buildDir);
-    } else {
-      prefix = installation.prefix;
+    installation = require('./linux').getLldbInstallation();
+    if (installation.libDir) {
+      config.variables['lldb_lib_dir%'] = installation.libDir;
     }
+    config.variables['lldb_lib%'] = installation.libName;
   } else if (osName === 'FreeBSD') {
-    const freebsd = require('./freebsd');
-    installation = freebsd.getLldbInstallation();
-    prefix = installation.prefix;
+    installation = require('./freebsd').getLldbInstallation();
+    config.variables['lldb_lib_dir%'] = installation.libDir;
   } else {
     console.log(`Unsupported OS: ${osName}`);
     process.exit(1);
   }
 
+  if (installation.includeDir === undefined) {
+    // Could not find the headers, need to download them
+    includeDir = lldb.cloneHeaders(installation.version, buildDir);
+  } else {
+    includeDir = installation.includeDir;
+  }
+  config.variables['lldb_include_dir%'] = includeDir;
   return {
     executable: installation.executable,
-    version: installation.version,
-    prefix,
     config
   };
 }
 
-/**
- * Link to the headers file so we can run `node-gyp configure` directly
- * and don't need to setup parameters to pass.
- * @param {string} lldbInstallDir The destination of the symlink
- */
-function linkHeadersDir(lldbInstallDir) {
-  console.log(`Linking lldb to installation directory ${lldbInstallDir}`);
-  try {
-    fs.unlinkSync('lldb');
-  } catch (error) {
-    // File does not exist, no need to handle.
-  }
-  fs.symlinkSync(lldbInstallDir, 'lldb');
-}
-
 /**
  * Generate the llnode shortcut script
  * @param {string} buildDir Path of the project directory
@@ -101,7 +81,7 @@ function writeLlnodeScript(buildDir, lldbExe, osName) {
 }
 
 /**
- * Write configuration to options.gypi
+ * Write configuration to config.gypi
  * @param {string} config
  */
 function writeConfig(config) {
@@ -113,8 +93,7 @@ function writeConfig(config) {
 
 function scriptText(osName, lldbExe) {
   let lib = 'llnode.so';
-  if (osName === 'Darwin')
-    lib = 'llnode.dylib';
+  if (osName === 'Darwin') { lib = 'llnode.dylib'; }
 
   return `#!/bin/sh